cloudflare / next-on-pages

CLI to build and develop Next.js apps for Cloudflare Pages
https://www.npmjs.com/package/@cloudflare/next-on-pages
MIT License
1.29k stars 126 forks source link

Dynamic routing doesn't work. #32

Closed hyunjunian closed 1 year ago

hyunjunian commented 1 year ago

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

my code:

image
jdddog commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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:

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Should this be closed?

tannakartikey commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

james-elicx commented 1 year ago

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

Not really unless you want to change it to be SSR

lenryk commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/cloudflare/next-on-pages/issues/428#issuecomment-1676365680

sanchitha-pof commented 1 year ago

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 commented 1 year ago

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

icanq commented 1 year ago

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

james-elicx commented 1 year ago

@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 commented 8 months ago

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 commented 8 months ago

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 commented 6 months ago

Stuck here too chief

Harsh2509 commented 1 month ago

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 commented 1 month ago

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.