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

Unable to render _error page, renders 404 page instead #435

Closed
2 tasks done
stefee opened this issue Dec 15, 2021 · 2 comments · Fixed by #436
Closed
2 tasks done

Unable to render _error page, renders 404 page instead #435

stefee opened this issue Dec 15, 2021 · 2 comments · Fixed by #436

Comments

@stefee
Copy link
Contributor

stefee commented Dec 15, 2021

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.25.0

Plugin version

7.0.0

Node.js version

16.13.0

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

11.6.1

Description

The plugin uses app.render under the hood, which explicitly disallows rendering _app, _document and _error pages:

https://github.com/vercel/next.js/blob/e75361fd03872b097e817634c049b3185f24cf56/packages/next/server/base-server.ts#L1704

Attempting to render the _error path (as described in the readme) results in a 404 Not Found response.

If you need to render with Next.js from within a custom handler (such as an error handler), use reply.nextRender

app.setErrorHandler((err, req, reply) => {
  reply.status(err.statusCode || 500)
  return reply.nextRender('/_error')
})

Steps to Reproduce

See description.

Expected Behavior

The plugin should either:

  1. Decorate the request with the app instance, so that app.renderError can be called manually
  2. Decorate fastify with a reply.nextRenderError which proxies the app.renderError similar to how reply.nextRender works
@mcollina
Copy link
Member

mcollina commented Dec 15, 2021

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

I'd go with both 1 and 2.

@stefee
Copy link
Contributor Author

stefee commented Dec 15, 2021

Cheers - I think option 2 makes most sense specifically for this problem. Option 1 is a more general feature suggestion.

👉🏻 #436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants