vercel / next.js

The React Framework
https://nextjs.org
MIT License
124.69k stars 26.61k forks source link

[NEXT-428] AppDir: Static Page generation and build process fails for Dynamic Routes with Fetch (in Version >= 13.1.2) #44998

Closed cachho closed 1 year ago

cachho commented 1 year ago

Verify canary release

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #45~20.04.1-Ubuntu SMP Mon Apr 4 09:38:31 UTC 2022
Binaries:
  Node: 16.15.1
  npm: 8.11.0
  Yarn: 1.22.19
  pnpm: N/A
Relevant packages:
  next: 13.1.3-canary.4
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true), Data fetching (gS(S)P, getInitialProps)

Link to the code that reproduces this issue

https://codesandbox.io/s/build-with-dynamic-routes-fails-in-next-13-1-2-ei0ms2

To Reproduce

Open a new terminal in the sandbox and runnpm run build

Describe the Bug

To generate the static pages, next fetches with [dynamic] as the query param. So instead of https://swapi.dev/api/people/1/?format=json it fetches https://swapi.dev/api/people/[dynamic]/?format=json.

In my production environment, that query can't be handled by the API, no json is returned, or the JSON doesn't match the promise (as you would expect), leading the build process to fails. The api in the code sandbox should return {"detail":"Not found"} for the incorrect call, but it also says: Error: Failed to fetch data.

This worked as intended in 13.1.1 and below.

Even if you catch these errors and the build process is successful, that should leave you with a static page without your desired data.

This will leave you with the following trace:

> build
> next build

warn  - You have enabled experimental feature (appDir) in next.config.js.
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.
info  - Thank you for testing `appDir` please leave your feedback at https://nextjs.link/app-feedback

info  - Creating an optimized production build
info  - Compiled successfully
info  - Linting and checking validity of types
info  - Collecting page data
[=   ] info  - Generating static pages (0/4)https://swapi.dev/api/people/%5Bdynamic%5D/?format=json
[==  ] info  - Generating static pages (3/4)Error: Failed to fetch data
    at getData (/sandbox/.next/server/app/[dynamic]/page.js:146:15)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Page (/sandbox/.next/server/app/[dynamic]/page.js:151:18)
[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 errorinstance which may provide additional details about the nature of the error.] {
  digest: '2682591503'
}

Error occurred prerendering page "/[dynamic]". Read more: https://nextjs.org/docs/messages/prerender-error
Error: Failed to fetch data
    at getData (/sandbox/.next/server/app/[dynamic]/page.js:146:15)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Page (/sandbox/.next/server/app/[dynamic]/page.js:151:18)
info  - Generating static pages (4/4)

> Build error occurred
Error: Export encountered errors on following paths:
        /[dynamic]/page: /[dynamic]
    at /sandbox/node_modules/next/dist/export/index.js:409:19
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Span.traceAsyncFn (/sandbox/node_modules/next/dist/trace/trace.js:79:20)
    at async /sandbox/node_modules/next/dist/build/index.js:1398:21
    at async Span.traceAsyncFn (/sandbox/node_modules/next/dist/trace/trace.js:79:20)
    at async /sandbox/node_modules/next/dist/build/index.js:1258:17
    at async Span.traceAsyncFn (/sandbox/node_modules/next/dist/trace/trace.js:79:20)
    at async Object.build [as default] (/sandbox/node_modules/next/dist/build/index.js:66:29)

Expected Behavior

According to the docs, dynamic routes will cause Next.js to render the whole route dynamically, at request time. Not sure if next just always tries to generatestatic pages as well, but this should not make the build process fail.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-428

remvn commented 1 year ago

Add export const dynamic = 'force-dynamic' or generateStaticParams may solve the problem. According to #45006

jankaifer commented 1 year ago

Might be fixed now https://github.com/vercel/next.js/issues/45006

github-actions[bot] commented 1 year ago

Please verify that your issue can be recreated with next@canary.

Why was this issue marked with the please verify canary label?

We noticed the provided reproduction was using an older version of Next.js, instead of canary.

The canary version of Next.js ships daily and includes all features and fixes that have not been released to the stable version yet. You can think of canary as a public beta. Some issues may already be fixed in the canary version, so please verify that your issue reproduces by running npm install next@canary and test it in your project, using your reproduction steps.

If the issue does not reproduce with the canary version, then it has already been fixed and this issue can be closed.

How can I quickly verify if my issue has been fixed in canary?

The safest way is to install next@canary in your project and test it, but you can also search through closed Next.js issues for duplicates or check the Next.js releases. You can also use the GitHub template (preferred), or the CodeSandbox or StackBlitz templates to create a reproduction with canary from scratch.

My issue has been open for a long time, why do I need to verify canary now?

Next.js does not backport bug fixes to older versions of Next.js. Instead, we are trying to introduce only a minimal amount of breaking changes between major releases.

What happens if I don't verify against the canary version of Next.js?

An issue with the please verify canary that receives no meaningful activity (e.g. new comments that acknowledge verification against canary) will be automatically closed and locked after 30 days.

If your issue has not been resolved in that time and it has been closed/locked, please open a new issue, with the required reproduction, using next@canary.

I did not open this issue, but it is relevant to me, what can I do to help?

Anyone experiencing the same issue is welcome to provide a minimal reproduction following the above steps. Furthermore, you can upvote the issue using the :+1: reaction on the topmost comment (please do not comment "I have the same issue" without repro steps). Then, we can sort issues by votes to prioritize.

I think my reproduction is good enough, why aren't you looking into it quicker?

We look into every Next.js issue and constantly monitor open issues for new comments.

However, sometimes we might miss one or two due to the popularity/high traffic of the repository. We apologize, and kindly ask you to refrain from tagging core maintainers, as that will usually not result in increased priority.

Upvoting issues to show your interest will help us prioritize and address them as quickly as possible. That said, every issue is important to us, and if an issue gets closed by accident, we encourage you to open a new one linking to the old issue and we will look into it.

Useful Resources

cachho commented 1 year ago

Verified, the issue still exists in 13.1.3-canary.5

kolorfilm commented 1 year ago

I have the exact same issue. The dockerfile is basically doing:

RUN npm ci

...

RUN npm run build

EXPOSE 5000
CMD ["npm", "run", "start"]

When I try to start the container afterwards it says within the log:

{"level":"ERROR", "msg":"Error: Could not find a production build in the '/usr/src/app/.next' directory. Try building your app with 'next build' before starting the production server. https://nextjs.org/docs/messages/production-start-no-build-id

Maybe it is important to says that I'm using node:14-alpine within the docker container. I cannot switch to > node 14 for now. The documentation says nextja supports node 14 so it should work I think.

The issue starts with next 13.1.2 and is still there with the latest canary.

abstractvector commented 1 year ago

TL;DR:


I ran into this issue. Specifically, it began to happen between v13.1.1 and v13.1.2.

The error in the console was a little convoluted, but the pertinent line is this:

Error occurred prerendering page "/blog/[slug]". Read more: https://nextjs.org/docs/messages/prerender-error

Between the aforementioned versions, the number of static pages that next build was trying to generated increased by 4 - also the same number of routes I have with dynamic components (e.g. [slug]).

The root cause appears to be that starting with v13.1.2, the build process attempts to generate pages for those routes where it hadn't done before. Since generateStaticParams isn't defined, it sends through dummy data equal to the parameter name, [slug] in my case. My page component (defined in /app/blog/[slug]/page.tsx) wasn't gracefully handling the dummy value and was throwing an uncaught error.

The suggested fix of adding export const dynamic = 'force-dynamic'; to that file did not work in v13.1.2, however it did in v13.1.3. It seems that the fix does begin working in v13.1.3-canary.5, so I suspect #45015 was the solution.

Since it only happened on that one route and not my other dynamic routes (which also don't have generateStaticParams or export const dynamic = 'force-dynamic'; specified), the underlying issue was my uncaught error (accessing a property on undefined).

That said, even in the latest non-canary version (v13.1.5), I still get the same issue without export const dynamic = 'force-dynamic';. Perhaps there's an opportunity to clarify in the documentation what is causing this error (and the potential fix)?

lukasnevosad commented 1 year ago

My problem is probably the same, although the error message is different:

In version >=13.1.2, build process returns errors when generating static pages as it tries to generate a static page for dynamic routes, such as l/[linkId]

All dynamic routes are then marked as "static" in route overview and when deployed to production, all return 404. The only route that works is actually l/[linkId], which is built statically with the contents of not-found.tsx

I confirmed that the problem persist in v13.1.6-canary.1

Route in <13.1.1 as reported after the build:

├ λ /l/[linkId]                               212 B          89.5 kB

Route in >=13.1.2 as reported after the build:

├ ○ /l/[linkId]                               212 B          89.7 kB

Build in >=13.1.2

npm run build          

> web@0.1.0 build
> next build

warn  - You have enabled experimental feature (appDir) in next.config.js.
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.
info  - Thank you for testing `appDir` please leave your feedback at https://nextjs.link/app-feedback

🌼 daisyUI components 2.40.1  https://github.com/saadeghi/daisyui
  ✔︎ Including:  base, components, themes[1], utilities

info  - Creating an optimized production build  
info  - Compiled successfully
info  - Linting and checking validity of types  
info  - Collecting page data  
[    ] info  - Generating static pages (0/10)Error: Invalid linkId "%5BlinkId%5D"
    at Page (/Users/lukasnevosad/Projects/Yomio/scholis/web/.next/server/app/l/[linkId]/page.js:538:114)
    at preloadComponent (/Users/lukasnevosad/Projects/Yomio/scholis/web/node_modules/next/dist/server/app-render.js:85:22)
    at /Users/lukasnevosad/Projects/Yomio/scholis/web/node_modules/next/dist/server/app-render.js:874:24
    at async createComponentTree (/Users/lukasnevosad/Projects/Yomio/scholis/web/node_modules/next/dist/server/app-render.js:873:25)
    at async /Users/lukasnevosad/Projects/Yomio/scholis/web/node_modules/next/dist/server/app-render.js:814:56
    at async Promise.all (index 0)
    at async createComponentTree (/Users/lukasnevosad/Projects/Yomio/scholis/web/node_modules/next/dist/server/app-render.js:779:38)
    at async /Users/lukasnevosad/Projects/Yomio/scholis/web/node_modules/next/dist/server/app-render.js:814:56
    at async Promise.all (index 0)
    at async createComponentTree (/Users/lukasnevosad/Projects/Yomio/scholis/web/node_modules/next/dist/server/app-render.js:779:38)

and 9 others for other dynamic routes...

abstractvector commented 1 year ago

My problem is probably the same, although the error message is different:

@lukasnevosad my guess is that somewhere in your page component, you're throwing an uncaught error for the invalid linkId.

From the error message, it looks like somewhere you're validating the linkId and if it's not valid, throwing new Error('Invalid linkId "%5BlinkId%5D");'. Try wrapping the contents of your function /render()method intry { ... } catch(e) { ...}and returnnotFound()(which you can import fromnext/navigation`) instead of letting the error leave your component uncaught. My guess is that will solve your problem.

If you don't want to leverage the static page generation for that route and you're not using generateStaticParams anyway, you can also add export const dynamic = 'force-dynamic'; but I think handling the error is good practice anyway.

Specifically, if you manually hit /l/[linkId], I suspect you're getting an HTTP 500 error which is a little blunt. Handling the error and returning notFound() would return an HTTP 404 which I feel is a little more appropriate.

lukasnevosad commented 1 year ago

@abstractvector I am not primarily javascript developer, so I might be wrong. But I don't think there's an uncatched error. The code runs with no console errors when run with npm run dev and it also works fine when deployed to production with next 13.1.1. It really is 13.1.2 and later that break things.

Here's the code for head.tsx:

export default async function Head(props: any) {
  try {
    const linkId = props.params.linkId;
    if (!validateId(linkId)) throw Error(`Invalid linkId "${linkId}"`);
    const linkData = await getLinkData(linkId);
    return (
      <LayoutHead title={linkData.title} description={linkData.description}>
        <meta property="og:image" content={linkData.imageUrl} />
      </LayoutHead>
    );
  } catch (error) {
    return (
      <LayoutHead title="Invalid link">
        <meta name="robots" content="noindex, follow" />
      </LayoutHead>
    );
    // Ignore error
  }
}

and page.tsx:

export default async function Page(props: any) {
  try {
    const linkId = props.params.linkId;
    if (!validateId(linkId)) throw Error(`Invalid linkId "${linkId}"`);
    const linkData = await getLinkData(linkId);
    return (
      <LayoutBody>
        <LinkContent linkData={linkData} />
        <DownloadApp />
        {/* @ts-expect-error Server Component */}
        <CollectionsTeaserSection />
      </LayoutBody>
    );
  } catch (error) {
    console.log(error);
    notFound();
  }
}

I simplified the code a bit to keep it readable. The only thing that struck me as a potential problem as I am writing this is the line:

        {/* @ts-expect-error Server Component */}
        <CollectionsTeaserSection />

which is a server async component that causes Typescript type checking issues. But as I said, it works perfectly fine in development with the latest next and in production with next 13.1.1.

abstractvector commented 1 year ago

That certainly looks like you're catching the error! If you runnpm run dev but then manually visit the /l/[linkId] URL in your browser with curl (e.g. curl -i "http://localhost/l/[linkId]" or similar), what HTTP status code do you get, and do you see an error in your console?

lukasnevosad commented 1 year ago
curl -i "http://localhost:3000/l/xxx"     
HTTP/1.1 404 Not Found
Vary: RSC, Next-Router-State-Tree, Next-Router-Prefetch, Accept-Encoding
Cache-Control: no-store, must-revalidate
X-Powered-By: Next.js
Content-Type: text/html; charset=utf-8
Date: Fri, 27 Jan 2023 20:04:19 GMT
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked

I didn't figure out how to make curl accept [] in URL, but http://localhost/l/[linkId] in Chrome console is also 404 Not Found.

hungcrush commented 1 year ago

It's fixed after I added export const dynamic = 'force-dynamic';

feedthejim commented 1 year ago

@jj can you verify this issue?

JJ commented 1 year ago

@JJ can you verify this issue?

I could definitely give it a try, but it's going to be faster trying and find the right person-whose-nick-starts-with-JJ-or-maybe-just-J :rofl:

jankaifer commented 1 year ago

cc @ijjk

ijjk commented 1 year ago

This appears to be a duplicate of https://github.com/vercel/next.js/issues/45515 basically if export const dynamic = '' is not configured for a page and no generateStaticParams are exported then the page is attempted to be rendered during build to detect if it is static or not with the default params.

We are investigating this behavior more although this is the current expected behavior.

lukasnevosad commented 1 year ago

Imo, having to use export const dynamic = 'force-dynamic'; feels like an unnecessary step to me. If the route path contains '[]' it is clearly a dynamic route and should be considered as such by default. Again, this was working in 13.1.1.

If we need to have this for some other reason, may be it would be worth mentioning it in https://beta.nextjs.org/docs/routing/defining-routes#dynamic-segments

ijjk commented 1 year ago

@lukasnevosad dynamic in this context means always SSR instead of statically optimized not that it can handle dynamic params. For most cases you want to take advaantage of statically optimizing as it is much more performant and ideally the pages can tolerate a render pass for accurate detection without the config needing to be present.

github-actions[bot] commented 1 year ago

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.