Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic routing doesn't work. #32

Closed
hyunjunian opened this issue Nov 26, 2022 · 31 comments
Closed

Dynamic routing doesn't work. #32

hyunjunian opened this issue Nov 26, 2022 · 31 comments

Comments

@hyunjunian
Copy link

hyunjunian commented Nov 26, 2022

When I set up dynamic routing and deploy it on cloudflare pages, page fails with 500 error.

my code:

image

@hyunjunian hyunjunian mentioned this issue Nov 26, 2022
@jdddog
Copy link

jdddog commented Dec 9, 2022

I'm having issues with this too, except when using dynamic routing with the pages folder rather than app directory.

Did you find a solution?

@hyunjunian
Copy link
Author

I'm having issues with this too, except when using dynamic routing with the pages folder rather than app directory.

Did you find a solution?

Not yet. I think updated period of this repository is slow.

@viantirreau
Copy link

Hey, I'm also using the appDir method and have this problem as well. I think I have narrowed it down to a missing header.
It is called x-matched-path, and when deployed to Vercel using the edge-runtime, the header comes back in the response when browsing a dynamic path, while Cloudflare Pages does not include it. I tinkered with using a _headers file, but I think the 500 error interrupts the chain and ends up without the required header.
Next, I'm going to try using a middleware instead, because I believe the header needs to be set before the next runtime processes the request to then render HTML.

A couple of technical notes I've gathered so far:

  • From the _worker.js minified code, I managed to trace back the error to next.js source code. The error occurs in this method, which deals with HTML rendering. It happens because params is not populated in the renderOpts object, so when setting the pathParams variable in line 958, it is undefined. Finally, when trying to get the dynamic parameter from the segment, the undefined value is treated as an object and indexed by a key, raising an exception at L978.
  • This is one of the few mentions I found in next source code about using the x-matched-path header. Apparently, it is used very early on, just at the beginning of the request handling (it then forwards the matched path to the router, which then ends up calling the render function). Skimming over the source code, it seems likely that for dynamic routes, having that header set is crucial for rendering.
  • I actually confirmed the previous insight by monkeypatching the (compiled + minified) _worker.js right before the check that looks for the header. In my case, I was testing with a dynamic path called /offers/[id], and when hardcoding the header in the code (something along the lines of req.headers["x-matched-path"] = "/offers/[id]"), it worked right away.

TL;DR: My hypothesis is that the problem does not lie in the edge runtime code itself. Instead, Vercel sets this header before hitting the handleRequest function (second note's link), and Cloudflare Pages should follow suit.

@GregBrimble do you think this could be a plausible explanation?

@viantirreau
Copy link

Update on the middleware experiment.

At first, I tried using CF Pages-like middlewares by creating a middleware.ts inside the functions path, both inside the .vercel/output and in the source (root /functions) directory, but they were ignored. Then, I read about proper next middlewares and added a middleware.ts file at the root of my project with this content:

import { NextResponse } from "next/server";
import type { NextRequest } from "next/server";

export const config = {
  matcher: "/offers/:path*",
};

export function middleware(req: NextRequest) {
  console.log("[middleware.ts] headers before:", req.headers);
  req.headers.set("x-matched-path", "/offers/[id]");
  console.log("[middleware.ts] headers after:", req.headers);
  return NextResponse.next();

  /**
   * I also tried this:
   * 
  
  const url = req.nextUrl.clone();
  url.pathname = "/offers/[id]";
  return NextResponse.rewrite(url);
  
  */
}

Although the middleware runs (the headers are printed in stdout), the underlying dynamic server component does not ¹. Normally, without the middleware, requesting that route returns a 500 status with the plaintext Internal server error, but when it's enabled, it returns status 200, an empty response body, and no x-matched-path header (only x-middleware-next: 1). So now the middleware is not working as expected, or maybe I'm doing something wrong.

In contrast, with next dev the middleware is not needed: the route works properly, returns status 200 and still works when the middleware is active (confirmed via the console.logs). Interestingly, the response does not have an x-matched-path header, while a Vercel deployment does. This is somehow aligned with the hypothesis that Vercel is doing something behind the scenes that we need to mimic in CF, i.e. it might not be encoded in the edge-runtime.


¹ Note that for this part of the experiment, I was using npx wrangler pages dev and npx @cloudflare/next-on-pages --watch side by side.

@viantirreau
Copy link

Another update: manually setting the x-matched-path: /offers/[id] header in Firefox, when performing a GET request to the dynamic path, solves the problem.

I see the id parameter being picked up correctly by the server component, although I have more errors down the line (seemingly related to a fetch call).

@frusanov
Copy link

frusanov commented Jan 4, 2023

I was able to create a workaround for this problem. Not perfect, but it works:

import { NextPageContext } from "next";
import { URLPattern } from "next/server";

export function parsePath(pathname: string, ctx: NextPageContext): any {
  const pattern = new URLPattern({ pathname });
  const result = pattern.exec(`http://host${ctx.req?.url}`);

  return result?.pathname.groups || {};
}

Then you can use this function in getServerSideProps:

export async function getServerSideProps(ctx: NextPageContext) {
  const { id } = parsePath('/post/:id', ctx);

  const post = await fetch(`https://jsonplaceholder.typicode.com/posts/${id}`)
    .then((response) => response.json());

  return {
    props: {
      post,
    }
  }
}

@viantirreau
Copy link

That's a great idea!
I think this would also work in Next 13 with app directory, when getting data within React server components (similar to getServerSideProps with pages directory)

@hanford
Copy link
Contributor

hanford commented Jan 4, 2023

Hello, I've been running into this as well and have come up with two solutions, 1 being the better option:

  1. Setting this header in next-on-pages before invoking any Next.js code, in my testing fixes Dynamic routing as well as it "hydrates" params (dynamic segements) in Next.js#handleRequest. I've opened a PR for this over here. I've also created a patch with my suggested PR so that my PR can be used now without waiting for a new @cloudflare/next-on-pages release. If you drop the patch file in your application, you can apply it in a postinstall hook using a tool like patch-package

  2. The not as great approach that I've been using patches Next.js itself. I've found that iterating over the dynamic routes, running the regex, and populating parsedURL.query also fixes this. That patch is available over here, but would warn that your milage may vary, and 1 is definitely the preferred solution IMO.


I've created a reproduction of this bug, along with the patched version of next-on-pages, proving 1. solution is viable. It's visible over here

@hanford
Copy link
Contributor

hanford commented Jan 5, 2023

After doing some additional testing it appears my PR does not work 😭 ... I thought it was working as I was still using a patched version of next.js ...

My 2nd approach still works, but I'd really like to not have to patch Next.js if it's avoidable.

--

I've updated my reproduction case with data fetching (appDir / async function), along with the x-matched-path header and see the same fetch errors that @viantirreau mentioned.

https://github.com/hanford/next-on-pages-32/tree/with-fetch

@balthazar
Copy link

Should this be closed?

@tannakartikey
Copy link

I am also having the same issue. On the development and on Firebase Hosting things are working fine. But on Cloudflare pages, I am getting 500 on the local environment and on production.

I have made the root path dynamic. I have two files src/pages/index.jsx and src/pages/[record].jsx

it seems that all the requests are going to the root path. context.params also does not seem to be populating.

@mule-stand
Copy link

That's a great idea! I think this would also work in Next 13 with app directory, when getting data within React server components (similar to getServerSideProps with pages directory)

How can I do this using the experimental app directory?

@madaskig
Copy link

madaskig commented Mar 3, 2023

Hello, I've been running into this as well and have come up with two solutions, 1 being the better option:

1. Setting this header in `next-on-pages` before invoking any Next.js code, in my testing fixes Dynamic routing as well as it "hydrates" params (dynamic segements) in `Next.js#handleRequest`. I've opened a PR for this over [here](https://github.com/cloudflare/next-on-pages/pull/51). I've also created a [patch](https://gist.github.com/hanford/ede056b57f7716f42c8a7e7d70bae166) with my suggested PR so that my PR can be used now without waiting for a new `@cloudflare/next-on-pages` release. If you drop the `patch` file in your application, you can apply it in a `postinstall` hook using a tool like [patch-package](https://www.npmjs.com/package/patch-package)

2. The not as great approach that I've been using patches Next.js itself. I've found that iterating over the dynamic routes, running the regex, and populating `parsedURL.query` also fixes this. That patch is available over [here](https://gist.github.com/hanford/5ce01eb69b710c7258f9454d262d960b), but would warn that your milage may vary, and 1 is definitely the preferred solution IMO.

I've created a reproduction of this bug, along with the patched version of next-on-pages, proving 1. solution is viable. It's visible over here

Hi! I've been able to make your first solution work. It seems like the Request's headers are "immutable" so we need to copy the request before setting the x-matched-path header.

I made a patch file (based on @hanford 's) for those who need a quick fix.

I'm not sure why this issue is closed? It's still present in v0.3.0

@surjithctly
Copy link

surjithctly commented Apr 21, 2023

I don't know why this is closed. I still have this issue:

I have a dead simple setup but it's not working for dynamic pages: blog/[slug].js

It returns 404 error

export async function getStaticPaths() {
  return {
    paths: [],
    fallback: true,
  };
}

export async function getStaticProps(context) {
  return {
    props: {},
  };
}

@james-elicx
Copy link
Contributor

james-elicx commented Apr 21, 2023

I don't know why this is closed. I still have this issue:

I have a dead simple setup but it's not working for dynamic pages: blog/[slug].js

It returns 404 error

export async function getStaticPaths() {
  return {
    paths: [],
    fallback: true,
  };
}

export async function getStaticProps(context) {
  return {
    props: {},
  };
}

Hello, It sounds like your page might be getting statically generated to a HTML file at build time. That is actually a separate problem.

At the moment, static HTML files are not handled properly - we need to rewrite URLs during the routing process, accounting for the rules in the config given to us in the Vercel build output. This would allow us to point the path towards the HTML file that the config tells us to use for that route, instead of relying on the outputted functions like we currently do.

I believe this is why you are experiencing 404s at the moment. It is one of the things that #129 aims to resolve.

If you are able to share a reproduction, that may be useful to confirm that this is the case.

@surjithctly
Copy link

@james-elicx oh.. okay. Any workaround for the timebeing?

@james-elicx
Copy link
Contributor

@james-elicx oh.. okay. Any workaround for the timebeing?

Not really unless you want to change it to be SSR

@lenryk
Copy link

lenryk commented Apr 23, 2023

Not sure why this issue was closed? I don't see a reliable fix from the replies yet.

Still getting this issue with NextJS 13.2.4 and dynamic routes in both the pages and app dirs.

@james-elicx
Copy link
Contributor

Not sure why this issue was closed? I don't see a reliable fix from the replies yet.

Still getting this issue with NextJS 13.2.4 and dynamic routes in both the pages and app dirs.

Hello @lenryk, this issue was to do with next-on-pages not handling dynamic parameters at all. While closed prematurely, the problem that this issue referred to was resolved.

Your issue is most likely due to your project statically generating HTML files for the page at build time. You can see my explanation about that problem further above.

This theory can be confirmed through your build logs if you would like to share them, or a reproduction.

@sanchitha-pof
Copy link

i am also facing same issues on my project app directory on local build it was loading dynamic page but on production it was throwing 404 not found how to fix this issue on other environment

@james-elicx
Copy link
Contributor

i am also facing same issues on my project app directory on local build it was loading dynamic page but on production it was throwing 404 not found how to fix this issue on other environment

Hello @sanchitha-pof, can you please confirm that you are using the latest version of next-on-pages by checking the build log for your production deployment? Additionally, could you please share your deployment method; the Pages Git integration, or publishing with Wrangler?

You'll see a little message at the start of the next-on-pages CLI that tells you the version it is currently using.

⚡️ @cloudflare/next-on-pages CLI v.1.1.0

If you are using the latest version and this issue is still occuring, would you be able to please provide more information about your dynamic routes and, if possible, a reproduction?

@emrecoban
Copy link

emrecoban commented Aug 12, 2023

i am also facing same issues on my project app directory on local build it was loading dynamic page but on production it was throwing 404 not found how to fix this issue on other environment

Also me. It throws 404 in the [slug] segments. By the way, doesn't display images.

As far as I see, no route works properly. Because the page refreshing when navigating pages. Actually I didn't change the app, just deployed. What causes this issue? @james-elicx

20:05:14.692 | devDependencies:
20:05:14.693 | + @cloudflare/next-on-pages 1.5.0
20:05:14.693 | + vercel 30.0.0

Solved: #428 (comment)

@sanchitha-pof
Copy link

i am also facing same issues on my project app directory on local build it was loading dynamic page but on production it was throwing 404 not found how to fix this issue on other environment

Hello @sanchitha-pof, can you please confirm that you are using the latest version of next-on-pages by checking the build log for your production deployment? Additionally, could you please share your deployment method; the Pages Git integration, or publishing with Wrangler?

You'll see a little message at the start of the next-on-pages CLI that tells you the version it is currently using.

⚡️ @cloudflare/next-on-pages CLI v.1.1.0

If you are using the latest version and this issue is still occuring, would you be able to please provide more information about your dynamic routes and, if possible, a reproduction?

just update new nextjs version

@9oelM
Copy link

9oelM commented Sep 20, 2023

why is this closed? experiencing the same issue today with the latest latest version.

@icanq
Copy link

icanq commented Sep 29, 2023

I'm still facing the same problem, is there any updates to solve this?

@james-elicx
Copy link
Contributor

@9oelM it's closed because this specific issue and its cause was resolved.

@9oelM @icanq If you are running into problems, please open a new issue with information about what exactly you're doing and is going wrong, as well as including some sort of reproduction so your problem can be properly investigated.

@dennysq
Copy link

dennysq commented Feb 25, 2024

I have used the below solution, it applies for static generation approach:
create a file _redirects inside public folder, for instance:
/course/:slug /course/[courseId] 200
/course/:id/participants /course/[courseId]/participants 200
/lesson/:slug /lesson/[lessonId] 200
/tag/:slug /tag/[tagId] 200
/profile/:slug /profile/[profileId] 200
/video/:slug /video/[videoId] 200
/category/:slug /category/[categoryId] 200
ref:
https://medium.com/@devakrishna33/eliminating-404-errors-on-dynamic-urls-with-cloudflares-next-on-pages-887c73ca649f
https://developers.cloudflare.com/pages/configuration/redirects/

@leye195
Copy link

leye195 commented Mar 22, 2024

my dynamic route page not rendering anything if i enter the page directly or refresh on page.
and if i run it on local i get this error messages:

Error: The 'credentials' field on 'RequestInitializerDict' is not implemented.
...
...
[Error: An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.] {

    digest: '367096776'
  }


[wrangler:inf] GET /category/strategy 500 Internal Server Error (11ms)
image

is there any way to fix it???? and any update??

@AshGw
Copy link

AshGw commented May 20, 2024

Stuck here too chief

@Harsh2509
Copy link

When I set up dynamic routing and deploy it on cloudflare pages, page fails with 500 error.

my code:

image

This issue is still not fixed? I am also getting 500 (Internal Server Error) while everything is working fine on my local system.. Is there any alternative to this?

@Barre
Copy link

Barre commented Oct 4, 2024

I got this today after disabling zaraz. Enabling it again would fix the issue. I am not sure why.

Uptime robot was returning:

Ongoing incident on www.merklemap.com/

HTTP(S) monitor for https://www.merklemap.com/

Root cause
500 Internal Server Error

With

{
  "date": "Fri, 04 Oct 2024 12:53:40 GMT",
  "content-type": "text/html; charset=utf-8",
  "connection": "close",
  "access-control-allow-origin": "*",
  "cache-control": "public, max-age=0, must-revalidate",
  "referrer-policy": "strict-origin-when-cross-origin",
  "x-content-type-options": "nosniff",
  "x-matched-path": "/500",
  "report-to": "{\"endpoints\":[{\"url\":\"https:\\/\\/a.nel.cloudflare.com\\/report\\/v4?s=qObGhk%2FFvXarG3aE43QSyHvuVSnPGo2AAZZAWnNUXAuwjKf4vVRXWkPhfMFC9l0ygL4ohdXSUBNQu8ETuV3R%2BEsv8%2F2raWWiQllbWKB4zIjOd9%2BxNiFAwWx1L%2FE2B0wSzlxYdg%3D%3D\"}],\"group\":\"cf-nel\",\"max_age\":604800}",
  "nel": "{\"success_fraction\":0,\"report_to\":\"cf-nel\",\"max_age\":604800}",
  "vary": "Accept-Encoding",
  "speculation-rules": "\"/cdn-cgi/speculation\"",
  "cf-cache-status": "DYNAMIC",
  "server": "cloudflare",
  "cf-ray": "8cd550f1d97b2cd4-DFW"
}

I was not able to reproduce using curl and the same request data. I ended up rollbacking my use of pages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.