Handle 404 page when getStaticProps.notFound = true or the route is not found #3448
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Ignored Deployments
|
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
alexkirsz
force-pushed
the
alexkirsz/web-448-handle-404-pages
branch
4 times, most recently
from
9d0c5fa
to
2f70ae4
Compare
sokra
requested changes
Jan 24, 2023
alexkirsz
force-pushed
the
alexkirsz/web-452-data-requests-for-dynamic-routes-are
branch
from
984eda7
to
2596995
Compare
alexkirsz
requested review from
chris-olszewski and
NicholasLYang
and removed request for
a team
alexkirsz
force-pushed
the
alexkirsz/web-448-handle-404-pages
branch
from
2f70ae4
to
75853c5
Compare
alexkirsz
force-pushed
the
alexkirsz/web-452-data-requests-for-dynamic-routes-are
branch
from
2596995
to
ebcca97
Compare
alexkirsz
force-pushed
the
alexkirsz/web-448-handle-404-pages
branch
2 times, most recently
from
9cf1ac7
to
f894b2f
Compare
Base automatically changed from
alexkirsz/web-452-data-requests-for-dynamic-routes-are
to
main
January 24, 2023 18:30
alexkirsz
force-pushed
the
alexkirsz/web-448-handle-404-pages
branch
from
f894b2f
to
b06ef20
Compare
alexkirsz
force-pushed
the
alexkirsz/web-448-handle-404-pages
branch
from
b06ef20
to
80ed2b5
Compare
alexkirsz
changed the title
sokra
approved these changes
Jan 26, 2023
| @@ -188,6 +169,19 @@ export default function startHandler({ | |||
| headers: renderData.headers, | |||
| } as any; | |||
| const res: ServerResponse = new ServerResponseShim(req) as any; | |||
|
|
|||
| // Both _error and 404 should receive a 404 status code. | |||
There was a problem hiding this comment.
Could you explain why _error isn't 500?
There was a problem hiding this comment.
When navigating to /_error in Next.js, a 404 status is always returned.
Benchmark for 3ca3704Click to view benchmark
|
|
alexkirsz
force-pushed
the
alexkirsz/web-448-handle-404-pages
branch
from
f685cf8
to
d3b412b
Compare
alexkirsz
added
the
pr: automerge
Benchmark for 40e1f7bClick to view benchmark
|
jridgewell
reviewed
Jan 27, 2023
alexkirsz
force-pushed
the
alexkirsz/web-448-handle-404-pages
branch
from
d3b412b
to
8490a5d
Compare
Benchmark for 6702ba9
Click to view full benchmark
|
|
Merging this as the only CI failure is unrelated to this PR. |
sokra
pushed a commit
that referenced
this pull request
Jan 28, 2023
The new error paths introduced in #3448 both write their outputs to `.next/server/pages/`, which conflicts with: - The main pages source - Each other The infinite loop is very fun: 1. We need a `NodeJsPoolVc` to render pages 2. To get a `NodeJsPoolVc`, you need to write your files onto disk - So a pool is dependent on the contents of those files 3. When we render an error page, we need to write those files to disk 4. The error page shares the same file entrypoint as the main page source So, to render an error, we write the entrypoint, which is shared with main source. This alone is pretty bad, because it will invalidate our page source's node pool (and kill those processes). But, the loop is triggered by a more subtle bug: When we write a file, we read it to see if the contents have changed. Writing creates a dependency on the read! So when the error page is written to disk, it invalidated the read we preformed when we wrote the main page. That invalidated the main page (and it's node pool), and so we rendered again. That wrote the main page's code to disk, invalidating the read of the error page performed when we wrote the error page. ♾️ (I'll be opening more PRs…)
jridgewell
pushed a commit
to vercel/next.js
that referenced
this pull request
Mar 10, 2023
…ot found (vercel/turbo#3448) This specifically supports the case where `getStaticProps.notFound = true` or a matching route is not found, but not the case where `getStaticPaths.fallback = false` and the route is not enumerated; This separate case will be implemented in another PR.
jridgewell
added a commit
to vercel/next.js
that referenced
this pull request
Mar 10, 2023
The new error paths introduced in vercel/turbo#3448 both write their outputs to `.next/server/pages/`, which conflicts with: - The main pages source - Each other The infinite loop is very fun: 1. We need a `NodeJsPoolVc` to render pages 2. To get a `NodeJsPoolVc`, you need to write your files onto disk - So a pool is dependent on the contents of those files 3. When we render an error page, we need to write those files to disk 4. The error page shares the same file entrypoint as the main page source So, to render an error, we write the entrypoint, which is shared with main source. This alone is pretty bad, because it will invalidate our page source's node pool (and kill those processes). But, the loop is triggered by a more subtle bug: When we write a file, we read it to see if the contents have changed. Writing creates a dependency on the read! So when the error page is written to disk, it invalidated the read we preformed when we wrote the main page. That invalidated the main page (and it's node pool), and so we rendered again. That wrote the main page's code to disk, invalidating the read of the error page performed when we wrote the error page. ♾️ (I'll be opening more PRs…)
sokra
pushed a commit
to vercel/next.js
that referenced
this pull request
Mar 13, 2023
…ot found (vercel/turbo#3448) This specifically supports the case where `getStaticProps.notFound = true` or a matching route is not found, but not the case where `getStaticPaths.fallback = false` and the route is not enumerated; This separate case will be implemented in another PR.
sokra
pushed a commit
to vercel/next.js
that referenced
this pull request
Mar 13, 2023
The new error paths introduced in vercel/turbo#3448 both write their outputs to `.next/server/pages/`, which conflicts with: - The main pages source - Each other The infinite loop is very fun: 1. We need a `NodeJsPoolVc` to render pages 2. To get a `NodeJsPoolVc`, you need to write your files onto disk - So a pool is dependent on the contents of those files 3. When we render an error page, we need to write those files to disk 4. The error page shares the same file entrypoint as the main page source So, to render an error, we write the entrypoint, which is shared with main source. This alone is pretty bad, because it will invalidate our page source's node pool (and kill those processes). But, the loop is triggered by a more subtle bug: When we write a file, we read it to see if the contents have changed. Writing creates a dependency on the read! So when the error page is written to disk, it invalidated the read we preformed when we wrote the main page. That invalidated the main page (and it's node pool), and so we rendered again. That wrote the main page's code to disk, invalidating the read of the error page performed when we wrote the error page. ♾️ (I'll be opening more PRs…)
| import Error, { getStaticProps } from "@vercel/turbopack-next/internal/_error"; | ||
|
|
||
| export default Error; | ||
| export { getStaticProps }; |
There was a problem hiding this comment.
this is actually not supported by next.js
https://nextjs.org/docs/advanced-features/custom-error-page#caveats
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This specifically supports the case where
getStaticProps.notFound = trueor a matching route is not found, but not the case wheregetStaticPaths.fallback = falseand the route is not enumerated;This separate case will be implemented in another PR.