fix: Error (500) and Not Found (404) routing logic #15584
Conversation
checks for valid path before attempting to create metadata. if invalid, return not-found metadata title. Removes `export const dynamicParams = false` to allow custom 404 and 500 pages
defaults to English; shows not-found page within layout so header and footer remain
github-actions
bot
added
content 🖋️
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
wackerow
changed the title
wackerow
requested review from
corwintines,
pettinarip and
minimalsm
as code owners
There was a problem hiding this comment.
Looks great @wackerow good job! left a few comments to understand the logic with error handlers mainly and see if we can polish them
| @@ -1,23 +1,21 @@ | |||
| "use client" | |||
There was a problem hiding this comment.
why do we need to convert this to a client component? couldn't we keep the server component?
There was a problem hiding this comment.
@pettinarip Without this I was unable to gain any access to the translated versions, so /es/derp was loading English content. Took a few approaches to try and properly load the locale before the page rendered using server-side, but in the end the initial load of the page was always going to English—Switching this to "use client" fixed it.
| return { | ||
| title: t("page-not-found"), | ||
| description: t("page-not-found-description"), | ||
| } |
There was a problem hiding this comment.
Do we need to return this metadata because the 404 page can't handle metadata? or why?
There was a problem hiding this comment.
If the 404 page is a server component, we might be able to handle this in that component, right?
There was a problem hiding this comment.
@pettinarip Throwing errors from the metadata generation step will stop our logic in it's tracks and default to Next.js 500 page. If we want this to handle as a 404, we need to return valid metadata, not throw an error, and then let the page component handle the routing.
| error | ||
| ) | ||
| return [] | ||
| } |
There was a problem hiding this comment.
not sure I follow this logic, why getPostSlugs("/") would fail in Netlify? if it fails means that we don't have any page to build statically and we should break the build, no?
There was a problem hiding this comment.
also, getPostSlugs is now internally handling the same error case, no?
There was a problem hiding this comment.
Jun 4, 01:59:07 PM: 4b95411b ERROR Error: ENOENT: no such file or directory, scandir '/var/task/public/content/'
at async Object.readdir (node:internal/fs/promises:952:18)
at async h (/var/task/.next/server/app/[locale]/[...slug]/page.js:1:826446)
at async D (/var/task/.next/server/app/[locale]/[...slug]/page.js:1:549543)
at async j (/var/task/.next/server/app/[locale]/[...slug]/page.js:1:548863) {
errno: -2,
code: 'ENOENT',
syscall: 'scandir',
path: '/var/task/public/content/',
digest: '930804571'
}
From the Netlify logs it seems that something was triggering this ENOENT error where it couldn't read from this directory in Netlify's serverless environment. This Search suggested that any other hidden system files that end up in there from this environment could be causing this to break when our code attempts to parse...
Wasn't 100% clear, but this step did seem to again prevent us from being able to properly access our internal 404 route, and wrapping in try/catch resolved here, while all desired pages are still loading correctly.
Can play around a bit more to see if perhaps the error handling inside getPostSlugs would be enough for this; that's one thing I hadn't tried yet, but that was the reasoning for this approach.
Description
Fixes logic surrounding 500 error and 404 not-found handing.
dyanamicParams = falsesince that meant encountering an unknown path (a dynamic param) would immediately fallback to Next.js 404. Removing enables our own custom 404 handling with notFound()NotFoundPageinLocaleLayoutto enable the English site layout with header and footer elementsExamples:
Related Issue
Currently all 404's and 500's result in Next.js pages