Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

404 Error Page not working #378

Closed
sidartha opened this issue May 2, 2020 · 6 comments · Fixed by #432 or #437
Closed

404 Error Page not working #378

sidartha opened this issue May 2, 2020 · 6 comments · Fixed by #432 or #437
Labels
good first issue Good for newcomers

Comments

@sidartha
Copy link

sidartha commented May 2, 2020

Describe the bug
When using next@9.3.5, navigating to an invalid URL does not show the 404 error page. Instead, you see 200 | An unexpected error has occurred.

To Reproduce
Steps to reproduce the behavior:

  1. Clone this repository: https://github.com/sidarthar/serverless-nextjs-404-error
  2. Run npx serverless to deploy
  3. After deployed, visit the appUrl/pageThatDoesNotExist
  4. See error

Expected behavior
Should see a 404 | Page Not Found error.

Screenshots
image

Desktop (please complete the following information):

  • OS: N/A
  • Version next@9.3.5, serverless-next.js@1.11.0

Additional context
When using next@9.3.4, the 404 page works as expected.

@danielcondemarin
Copy link
Contributor

danielcondemarin commented May 2, 2020

Thanks for reporting! I suspect this started happening since the introduction of https://nextjs.org/docs/advanced-features/custom-error-page#404-page. If that's the case, this would need to be supported in serverless-next.js, probably changing this.

If someone can confirm and is happy to PR I'll happily accept contrib. 👍

@danielcondemarin danielcondemarin added the good first issue Good for newcomers label May 2, 2020
@bhall2001
Copy link

This may be related...
vercel/next.js#12199

@sidartha
Copy link
Author

sidartha commented May 3, 2020

I believe that the issue is because of #11561

On line 283 of next-serverless-loader.ts,

${page === '/_error' ? `res.statusCode = 404` : ''} was deleted and replaced with

${ page === '/_error' ? ` if (!res.statusCode) { res.statusCode = 404 } ` : '' }

If I understand correctly, res.statusCode = 200 is set before rendering the error page, so it doesn't reach the line to set res.statusCode = 404.

@danielcondemarin
Copy link
Contributor

danielcondemarin commented May 3, 2020

I believe that the issue is because of #11561

On line 283 of next-serverless-loader.ts,

${page === '/_error' ? `res.statusCode = 404` : ''} was deleted and replaced with

${ page === '/_error' ? ` if (!res.statusCode) { res.statusCode = 404 } ` : '' }

If I understand correctly, res.statusCode = 200 is set before rendering the error page, so it doesn't reach the line to set res.statusCode = 404.

Good catch.
Maybe setting status and statusDescription as undefined here is the fix. Then it would be up to next to always set a statusCode which I think should be fine, but would need some testing to ensure it doesn't break some use case.

@marcin-piela
Copy link

Is there any workaround (to implement in app) without waiting for new package version?

@lone-cloud
Copy link
Contributor

lone-cloud commented Jun 4, 2020

So I quite literally did what @danielcondemarin suggested in his previous comment and the 404 status is now being returned correctly. I have not noticed any regressions with the change. However, it is being rendered in the _error template instead of the 404 one. I assume this has never worked for this project before and will require a custom implementation.

Commit here: lone-cloud@834886a

You can test it by cloning the set_status_undefined/378 branch, run npm install && npm run packages-install on it and change your component in serverless.yml to point to its local path like component: '../serverless-next.js/packages/serverless-component'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
5 participants