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

ENOENT error in renderToHTML should pass err to error page #5625

Closed
vjpr opened this issue Nov 7, 2018 · 3 comments
Closed

ENOENT error in renderToHTML should pass err to error page #5625

vjpr opened this issue Nov 7, 2018 · 3 comments
Assignees

Comments

@vjpr
Copy link

vjpr commented Nov 7, 2018

    try {
      const out = await renderToHTML(req, res, pathname, query, this.renderOpts)
      return out
    } catch (err) {
      if (err.code === 'ENOENT') {
        res.statusCode = 404
        return this.renderErrorToHTML(null, req, res, pathname, query)
      } else {

See also renderScriptError

if (error.code === 'ENOENT' || error.message === 'INVALID_BUILD_ID') {

  1. The ENOENT check should be replaced by a named error. Its not always going to be a 404. Sometimes it might be a bug, and we need the stack trace to work out what is going on - like in After the initial render, any direct url visits to pages result in 404s #5620. If a page doesn't exist, the error would be throw far away in renderToHTML > doRender > requirePage > getPagePath.

  2. There should be a better check for a 404 vs. other ENOENTs.

  3. We should always return the error for 404s.

return this.renderErrorToHTML(null, req, res, pathname, query).

Silencing errors is bad. How to handle a 404 should be taken care of inside renderErrorToHTML

  1. err should be made available outside of server-side renderToHTML.

See #3452.

@vjpr vjpr changed the title ENOENT error in renderToHTML should pass err to error page ENOENT error in renderToHTML should pass err to error page Nov 7, 2018
@timbielawski
Copy link

timbielawski commented Dec 5, 2018

I have added a errorCodeMapper, the fork is here: https://github.com/timbielawski/next.js/tree/add/custom-error-handler

commit:
timbielawski@47e1e27

I want to see what the feeling is and then I will do a PR

Add a errorCodeMapper for the handleRequest and renderToHTML functions.
The purpose is allow the rendered app to throw errors which can be mapped to http error codes, which then results in the correct error html being rendered in Nextjs

Currently Nextjs will render a 500 for any error thrown in the rendered app

In my app I add the error code mapper
const nextLoader = next({ dev, quiet, errorCodeMapper });

Then I can map any error that my app throws to a http code so the correct error is returned from Nextjs

const { CONTENT_404, THEME_404, SERVER_500 } = require("../constants/errors");

const errorCodeMapper = err => { switch (err) { case CONTENT_404: case THEME_404: return 404; default: return 500; } }

module.exports = errorCodeMapper;

@timneutkens
Copy link
Member

I'm going to close this as this has been open for a year and there hasn't been a contribution to solve it. It would require a relatively large refactor of how the server implementation works so I don't feel like it's worth exploring rewriting it just for this specific case.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants