Closed j0nezz closed 2 years ago
We also have multiple builds that are affected by this bug. We have a dynamic route which creates pages based off the slug provided by our CMS. The defect was noticed in production when visiting any route not available in the CMS, which should return a 404, however we are seeing 500 - internal server error. The appearance of the bug seems to coincide with recent version changes, we were able to confirm that by upgrading a previously unaffected site from v3.9.2 to v4.2.3, which introduced the 500 errors. We confirmed that it is being sent to the netlify-odb-handler. We are investigating further now, please let us know if any additional details are needed.
@BenMcGrath1 Function logs would be very useful, thanks
Below is the function log for the odb-handler. Its worth noting we believe the error is happening upstream of the odb-handler function, possibly here: https://github.com/netlify/netlify-plugin-nextjs/blob/6fd7bcc99aacf447559de46f60de6d8cb33e7a59/src/helpers/redirects.ts#L68
Hey @ascorbic,
I'm having some trouble serving a deploy preview locally or to netlify with the suspected problem line commented out in the build plugin to verify this, but I suspect the problem redirects that are being generated in our case relate to our dynamic route template at /pages/[...slug].js
. Here's an example of the generated redirects from the deploy log:
6:20:01 PM: from: '/_next/data/build/:slug/*',
6:20:01 PM: to: '/.netlify/builders/___netlify-odb-handler',
6:20:01 PM: status: 200,
6:20:01 PM: force: false
6:20:01 PM: },
6:20:01 PM: {
6:20:01 PM: from: '/:slug/*',
6:20:01 PM: to: '/.netlify/builders/___netlify-odb-handler',
6:20:01 PM: status: 200,
6:20:01 PM: force: false
6:20:01 PM: },
The template in question uses getStaticProps
without revalidate
and getStaticPaths
with fallback: false
(doesn't take advantage of ISR/SSR). netlify-plugin-nextjs appears to be generating redirects for any prerendered dynamic routes and their related data routes that don't match middleware at line 201 in the redirects.ts file. I must be missing something, however, because that would mean that all prerendered pages, regardless of their fallback status, are being referred to the odb-handler rather than being served from the deploy build cache in cases where the page has already been rendered. A dynamic route is not necessarily an SSR route or an ISR route, but this appears to treat it as such.
Hi @hu0p Pages pre-rendered at build time will take priority over those rewrites (hance force= false), so the ODB handler will only be hit for pages that haven't been rendered. Other pages will hit the handler the first time they are visited, but that response will be then cached until the next deploy. As the comment states though, for reasons that aren't entirely clear to me, Next.js doesn't always pre-render static paths for other locales, and instead serves them dynamically. For this reason we need to hit the origin to be certain it's a 404. I will need to look into possible workarounds for this though, as I understand the problem of not wanting to accumulate these invocations.
Pages pre-rendered at build time will take priority over those rewrites (hance force= false), so the ODB handler will only be hit for pages that haven't been rendered.
Ah, got it. I overlooked that force: false
bit when I was familiarizing myself with the repo and control flow inside redirects.ts. I totally forgot about that option. Thanks for pointing that out! This makes perfect sense.
I will need to look into possible workarounds for this though, as I understand the problem of not wanting to accumulate these invocations.
Thanks for recognizing our concerns. That's a big part of it, but it also leads to negative SEO consequences. 500 errors are logged in GSC as errors that can cause routes to be prematurely dropped from results before they can be caught and redirected by content managers. The current behavior also really doesn't align with the Next.js API as documented. I apologize if it sounds like I'm beating a dead horse. I promise that's not my intention. Rather, I just wanted to share these thoughts so they can be taken into consideration when working towards coming up with the workaround you mentioned.
Perhaps in the meantime a solution would be to manually force a 404 when page data is unavailable? I don't love it because this is not the originally intended use case for this functionality and it otherwise wouldn't be necessary in any other environment, but it could at least address the problem for us (and anyone else who stumbles across this) in the meantime. It might make sense to include a note about this somewhere in the docs as well if you're open to it.
As the comment states though, for reasons that aren't entirely clear to me, Next.js doesn't always pre-render static paths for other locales, and instead serves them dynamically. For this reason we need to hit the origin to be certain it's a 404. I will need to look into possible workarounds for this though, as I understand the problem of not wanting to accumulate these invocations.
We're actually working on a large i18n project at the moment, so this is also relevant to us. Just out of curiosity, when you were testing this, were the locales configured in next.config.js
and being passed as a property for each individual path that is desired to be prerendered in the paths array in getStaticPaths
as shown in the example here? That is a bit of an edge case for the fallback property that sounds like it could apply here.
Thanks for recognizing our concerns. That's a big part of it, but it also leads to negative SEO consequences. 500 errors are logged in GSC as errors that can cause routes to be prematurely dropped from results before they can be caught and redirected by content managers. The current behavior also really doesn't align with the Next.js API as documented. I apologize if it sounds like I'm beating a dead horse. I promise that's not my intention. Rather, I just wanted to share these thoughts so they can be taken into consideration when working towards coming up with the workaround you mentioned.
This is clearly a bug. It should be returning a proper 404 page. Do you have the function log for one of those requests?
Are you sure? The test case here would be visiting a route that doesn't and wouldn't exist, so something like /asdfasdfasdf
. This route isn't listed in the paths array returned from getStaticPaths
, so it isn't prerendered. However, it does match the [...slug]
dynamic route in the manifest, so it gets passed to the ODB handler and hits the origin as you noted, which triggers the following behavior defined in getStaticProps (simplified a bit here):
export async function getStaticProps({ params }) {
const slug = params.slug?.join("/")
const { data } = await getPage(slug)
return {
props: {
key: data?.id ?? null,
content: data?.content ?? null,
},
}
}
You can actually see an example of the function logs in Ben's comment above (we represent the same organization). I'm afraid there's not much interesting going on there. It's just a 404 response from the CMS (Storyblok). Axios or the CMS SDK inside the getPage
call may be throwing an exception or it could be an error during page generation triggering the client-side error. Either way it shouldn't hit the handler at all and should 404, right? There is another site that doesn't use the SDK that we believe is also affected by this, but it has slightly different behavior. Same plugin version and test route (more or less), but it just generates an empty page (200 status) and doesn't make a json request for the page data. My guess is it's just a difference in rendering (null check on the client). These should 404 too.
The logs for that site are pretty interesting though. Those requests come through without error to the ODB handler, but they also include requests for page data that should be pre-rendered e.g. contact, about, careers:
Feb 17, 10:23:00 AM: 11c2ce94 INFO [GET] /_next/data/build/contact.json (ODB)
Feb 17, 10:23:02 AM: 11c2ce94 Duration: 1722.69 ms Memory Usage: 104 MB Init Duration: 218.51 ms
Feb 17, 12:19:23 PM: dd1a1587 INFO [GET] /asdfasdfasdf (ODB)
Feb 17, 12:19:26 PM: dd1a1587 Duration: 1695.81 ms Memory Usage: 104 MB Init Duration: 219.06 ms
Feb 17, 12:19:30 PM: e3073522 INFO [GET] /_next/data/build/about.json (ODB)
Feb 17, 12:19:30 PM: 9deb1acf INFO [GET] /_next/data/build/careers.json (ODB)
Feb 17, 12:19:30 PM: e3073522 Duration: 291.11 ms Memory Usage: 105 MB
Feb 17, 12:19:32 PM: 9deb1acf Duration: 1720.37 ms Memory Usage: 103 MB Init Duration: 213.49 ms
Feb 17, 12:19:32 PM: 7da98e7c Duration: 1873.28 ms Memory Usage: 104 MB Init Duration: 189.15 ms
Feb 17, 12:25:51 PM: dae7e1de INFO [GET] /_next/data/build/contact.json (ODB)
Feb 17, 12:25:53 PM: dae7e1de Duration: 1786.25 ms Memory Usage: 104 MB Init Duration: 205.46 ms
Feb 17, 12:29:07 PM: c63ca70a INFO [GET] /asdfasdfasdf (ODB)
Feb 17, 12:29:08 PM: c63ca70a Duration: 417.19 ms Memory Usage: 106 MB
Feb 17, 12:29:09 PM: 41c5bfad INFO [GET] /_next/data/build/about.json (ODB)
Feb 17, 12:29:09 PM: 6d9b87f5 INFO [GET] /_next/data/build/careers.json (ODB)
Feb 17, 12:29:09 PM: 3485981c INFO [GET] /_next/data/build/our-brands/wkrq.json (ODB)
Feb 17, 12:29:10 PM: 41c5bfad Duration: 481.97 ms Memory Usage: 108 MB
Feb 17, 12:29:11 PM: 6d9b87f5 Duration: 1527.12 ms Memory Usage: 104 MB Init Duration: 165.48 ms
Feb 17, 12:29:12 PM: 3485981c Duration: 1873.55 ms Memory Usage: 105 MB Init Duration: 225.82 ms
Feb 17, 12:29:20 PM: b1d50e4c INFO [GET] /asdfasdfasdfasdfasdfasdfasdfasdfasdf (ODB)
Feb 17, 12:29:20 PM: b1d50e4c Duration: 212.44 ms Memory Usage: 107 MB
Feb 17, 01:26:11 PM: a22eea1b INFO [GET] /404 (ODB)
Feb 17, 01:26:12 PM: a22eea1b Duration: 557.48 ms Memory Usage: 80 MB Init Duration: 206.30 ms
Feb 17, 01:26:59 PM: cbc77314 INFO [GET] /asdfasdfasdfasdfasdfasdfasdf (ODB)
Feb 17, 01:27:00 PM: cbc77314 Duration: 1151.19 ms Memory Usage: 104 MB
I tried manually returning a 404 inside getStaticProps like I mentioned as a workaround before, and I can confirm that works. However, it still triggers an unnecessary invocation, of course.
Something else I noticed: an empty /_next/data/build folder is generated at the top level of the directory when I use netlify-cli
to run ntl dev --live
or ntl build
locally with the plugin (cloned repo, file based configuration in netlify.toml). Do you have a typical workflow for this plugin? I've cloned it into an existing site, configured it in netlify.toml, and run npm run watch
to generate the lib folder, but any time I attempt to do test deploys I experience some odd broken behavior that I don't get in production. Any idea what I could be missing?
Sorry, to be clear I meant it's a bug in the plugin not in your code. I'm going to deal with some other fixes today and then see if I can look into this. Can you confirm whether you're using i18n in this site? If possible, could you share your .next/prerender-manifest.json
file?
For CLI deploys, you need to run netlify deploy --build
, rather than building and deploying separately.
So good news! The pages I noted above that should be pre-rendered were not due to a bug in the plugin. We overlooked a hidden default per_page count/limit in the CMS API. The only issue we're definitely dealing with here is the improper accumulation of invocations for pages that should 404 which match a given dynamicRoute in the .next/prerender-manifest.json
. Sorry for any confusion this may have caused, and let me know if we need to move this to a new issue.
For CLI deploys, you need to run netlify deploy --build, rather than building and deploying separately.
This must be a separate bug. I tried this at one point (and just now) and was able to reproduce the error we're discussing, but I kept getting 500 errors from the _ipx
function which caused all of the images to break.
Can you confirm whether you're using i18n in this site?
Neither of the sites I mentioned are using i18n.
If possible, could you share your .next/prerender-manifest.json file?
Yup, see below for the site that matches the ODB logs I shared above. This is the site that was giving us 200 statuses and empty pages in places where it should 404. The page/pageData being erroneously fetched are handled by
...
"dynamicRoutes": {
...
"/[...slug]": {
"routeRegex": "^/(.+?)(?:/)?$",
"dataRoute": "/_next/data/build/[...slug].json",
"fallback": false,
"dataRouteRegex": "^/_next/data/build/(.+?)\\.json$"
},
...
}
...
Hey was going to make a new issue but I think it's the same.
When visiting a dynamic route that should return a 404, there is an attempt to render server side. https://abtest-middleware.netlify.app/dynamic-routes/this-should-go-to-404
I thought this could be something to do with middleware but also appears to happen on non mw pages https://abtest-middleware.netlify.app/dynamic-routes-no-middleware/this-should-go-to-404
Function log of ___netlify-handler
Mar 7, 04:38:50 PM: 5c6c22dc INFO [GET] /dynamic-routes-no-middleware/this-should-go-to-404 (SSR)
Mar 7, 04:38:50 PM: 5c6c22dc Duration: 58.96 ms Memory Usage: 89 MB
Nothing from ODB v4.2.7
If you need any further info let me know!
Also can we try keep massive logs inside a \
Also can we try keep massive logs inside a \
tagπ
Good idea. Updated!
Sorry for bumping the thread but just wondering if a fix for this bug is in the pipeline? Or maybe I have missed something and this isnt a bug at all π€
I don't like being a +1 dude, but +1.
I found out that this behaviour throws this server error on non-existing pages when using this
filesystem-based method to pass fetched data from getStaticPaths
to getStaticProps
:
https://github.com/vercel/examples/tree/main/solutions/reuse-responses
(However, this does not happen locally, which makes it much harder to find the problem.)
Apr 7, 03:04:54 PM: 7e112390 ERROR Error: ENOENT: no such file or directory, open '/var/task/xyz.db'
I think this happens because the cache file disappears after the build process (at least that's what I thought). But this should be no problem if netlify did not try to run getStaticProps
after the build process as expected when fallback: false
is set.
@pixelino We've had to resort to wrapping our requests in a try/catch that manually returns 404 ({ notFound: true }
) when this occurs while we evaluate other options. It's cumbersome and annoying, but it produces the usual expected behavior in the interim.
hey hu0p! could you share how you achieved this? I dont mind doing it manually as long as it works π
@JohnGemstone Our solution looked something like this:
async function getStaticProps({ params: { slug } }) {
let data
try {
;({ data } = await yourSDKFunction(slug, yourSDKConfig))
} catch (e) {
console.error(e)
return {
notFound: true,
}
}
// do stuff with data and/or return data
}
I hope this helps. This may or may not be directly applicable to your use-case, but we have an SDK that makes requests inside getStaticProps
which chooses to throw when it encounters an error. This is the desired outcome during the build, but not during production which seems to lead to the pain a lot of us are encountering in this thread. To make matters worse, this is not idiomatic Next.js. As I understand it fallback: false
and especially the absence of an arbitrary path in getStaticPaths should mean there is no function hit.
Hey thanks for sharing your solution, I didnt know about that notFound: true
property which looks ideal for this!
I use SanityCMS for most my sites so directly querying the database every page view wouldn't be ideal, what I could probably do is create a prebuild script to store all the relevant slugs then compare them against the slug in getStaticProps like you have in your example. Pretty simple!
Got my fingers crossed to see those 404s π
Anything new about this bug? I still have this problem. Dynamic routes are returning server error 5xx, instead of a regular 404 page.
@ascorbic any news on this? Would be great to know if we have to find a workaround ourselves, or if a fix from Netlify's side is planned. Thanks!
Is there any updates on this @ascorbic ? I am having the same issue. BTW the workaround proposed above works for me.
@orinokai is working on this
Wanted to check in on the status of this. We are running into the same issue and the workaround is and isn't a viable solution without a more expansive refactor.
@inadeqtfuturs Same. I had really hoped this would've been solved by now. The workaround works for small cases like wrapping an SDK but it doesn't scale well for complex logic or some fairly common use-cases. We really began to understand this when we ran into a use-case similar to @pixelino's.
We now have an i18n site that reuses responses from getStaticPaths in getStatpicProps via filesystem-based caching. The ODB handler will attempt to SSR a page that it shouldn't (due to fallback: false
config) and subsequently choke because the file path doesn't exist. This of course builds fine locally but doesn't work in production because the files aren't there. Including the files, in public/ or in the function itself, doesn't seem like a viable solution because it would amount to 132 mb of serialized data (far exceeding Netlify's function size limit).
Couple questions for anyone experiencing this bug:
Heck, react to my comment with a heart if yes for 1, a tada/confetti if yes for 2, and a thumbs down if no to both to make it easy π. It seems like this problem pops up under a couple different conditions, so my hope is that if we can narrow down the problem set we can figure out the origin of the issue.
@orinokai or @ascorbic I'm willing to volunteer some more of my time to investigate this. Hopefully the existing large site we have could serve as a test-case for this.
As the comment states though, for reasons that aren't entirely clear to me, Next.js doesn't always pre-render static paths for other locales, and instead serves them dynamically. For this reason we need to hit the origin to be certain it's a 404.
Has anything changed on this front? It seems like it could be a big part of the problem.
@inadeqtfuturs Have you had the opportunity to investigate if this affects all users or just you? Something else that may be worth sharing is that, at least in this case, these pages do appear to pre-render. It's only in the function logs for the ODB handler that I see these ENOENT errors (file/path doesn't exist). Is this the same for you?
An interestingly development I came across today is a team member experiencing 500 errors on the route in question while I didn't. Yet the page loaded correctly in incognito for her, and a hard refresh didn't resolve the issue. I just can't figure out why it would cache the handler for her and not me.
Thanks @hu0p for the additional information and use cases, much appreciated. We have been looking into this and have a potential way forward that should ensure non-prerendered dynamic routes will bypass the ODB handler when fallback: false
is set. There's a few additional cases in the comments above that I want to make sure we've covered before we push anything, but I will update here with more info soon.
It's only in the function logs for the ODB handler that I see these ENOENT errors (file/path doesn't exist). Is this the same for you?
It's not just me -- it affects all users.
From what I can tell from the build log, the pages we don't want to render are not being built, BUT routes that aren't built seem to fallback to ones that are. So an example would be [param]/page.js
, which would build 6 routes. If you hit something like definitely-not-a-built-param/page
, it will fall back to one of the built param/page
s. I haven't checked the ODB handler though and might get some time to look into it today.
@orinokai Great to hear! When I was briefly messing with this yesterday I threw together a simple demo of the reused responses pattern in demos/default/getStaticProps/cachedResponses/[id].js
using the same TV Maze API as the other examples. Would you be interested in accepting a PR for that? It may be helpful to test against. I'd also be happy to write an integration test for it if you feel it makes sense.
The PR is really useful, thanks @hu0p. We will get it merged in and, as you say, it will be useful for testing. The specific error happens because getStaticProps
runs on those pages and there is no write file access in the function environment. The fix is still in progress as the edge cases are proving tricky - we need need ensure that the handler function is completely bypassed for non-prerendered dynamic-routes with fallback: false
but we also need to ensure custom 404 pages are respected in all locales and provide support for ISR 404 pages with revalidate. Will keep you posted on progress.
Hi, is there anything that needs to be done to deploy this fix? I only wear my nextjs/netlify hat a day or two every couple months, so I'm bad at this, but we get 500 errors when we should get 404s and it's impacting our SEO. The error we get looks a lot like the ones reported here:
Oct 13, 02:39:09 PM: 74406dd1 ERROR Error: ENOENT: no such file or directory, open '/var/task/posts/robots.txt.mdx'
at Object.openSync (node:fs:585:3)
at Object.readFileSync (node:fs:453:35)
at getPostData (/var/task/.next/server/chunks/333.js:1893:68)
at getStaticProps (/var/task/.next/server/pages/[...slug].js:96:57)
at Object.renderToHTML (/var/task/node_modules/next/dist/server/render.js:465:26)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async doRender (/var/task/node_modules/next/dist/server/base-server.js:879:38)
at async cacheEntry.responseCache.get.isManualRevalidate.isManualRevalidate (/var/task/node_modules/next/dist/server/base-server.js:979:28)
at async /var/task/node_modules/next/dist/server/response-cache.js:64:36 {
errno: -2,
syscall: 'open',
path: '/var/task/posts/robots.txt.mdx'
}
This issue is closed and the PR is merged about a month ago. We've deployed since then...is there something else to do?
Here's our site throwing a 500 instead of a 404: https://free.law/robots.txt Our code is here: https://github.com/freelawproject/free.law/
Thank you!
Hm, well, I thought I'd try to fix this while hoping for a response. So far I:
The good news: I have a build with updated deps, so that's nice.
The bad news: I still get 500 errors instead of 404 errors, and I still see errors like the one above when looking in the logs. Bummer.
Darn, and even the fixes suggested here don't work. I switched all my code from this:
export async function getStaticProps({ params }) {
const postData = await getPostData(params.slug);
const allPostsData = await getSortedPostsData();
return {
props: {
postData,
allPostsData,
},
};
}
To this:
export async function getStaticProps({ params }) {
let postData, allPostsData;
try {
postData = await getPostData(params.slug);
allPostsData = await getSortedPostsData();
} catch (e) {
console.error(e);
return {
notFound: true,
};
}
return {
props: {
postData,
allPostsData,
},
};
}
And still no useful 404 pages. Kind of running out of ideas. Would love any help anybody has time to offer.
Summary
Netlify triggers a new ODB function instead of returning the expected 404 when using
fallback: false
.See: https://github.com/netlify/netlify-plugin-nextjs/blob/6fd7bcc99aacf447559de46f60de6d8cb33e7a59/src/helpers/utils.ts#L80-L88
Context: We have a top level dynamic route, so that we can create new landing pages directly from our CMS. Because
fallback: false
was ignored, every request tomyWebsite.com/:uid
resulted in a new ODB function being triggered. However, these requests were for example also triggered by crawlers requestingrobots.txt
(which was missing at that time). Another edge case leading to a huge number of function invocations was the usage of the nextjsLink
component for a route with a configured redirect (in _redirects). Because Nextjs prefetches routes, it tried to prefetch the JSON of the route (which didn't exist because it's a redirect). This would not have been a problem (only a failed request in the network tab of the browser, and it can be fixed by disabling the prefetching)...However, with Netlify ignoringfallback: false
, a new ODB function was spawned for every prefetch request (which could be many per user session).All in all we accumulated 170k+ function invocations in one weekend. I know that the above described scenario is very specific (and could have been avoided) but I wanted to emphasize, that the i18n fix can result in some unpleasent surprises for Netlify customers that use
fallback: false
and expect it to work as described in the nextjs docs.Steps to reproduce
.
A link to a reproduction repository
No response
Plugin version
4.2.1
More information about your build
netlify.toml
)What OS are you using?
No response
Your netlify.toml file
`netlify.toml`
```toml # Paste content of your `netlify.toml` file here ```Your public/_redirects file
`_redirects`
```toml # Paste content of your `_redirects` file here ```Your
next.config.js
file`next.config.js`
```toml # Paste content of your `next.config.js` file here. Check there is no private info in there. ```Builds logs (or link to your logs)
Build logs
``` # Paste logs here ```Function logs
Function logs
``` # Paste logs here ```.next JSON files
generated .next JSON files
``` # Paste file contents here. Please check there isn't any private info in them # You can either build locally, or download the deploy from Netlify by clicking the arrow next to the deploy time. ```