Skip to content

Conversation

@stefee
Copy link
Contributor

@stefee stefee commented Dec 15, 2021

Closes #435

Proxy to Next.js app.renderError.

The source code for app.renderError at time of writing can be found here:

This method will render an appropriate error page based on the response status code - the pathname does not affect the page rendering.

The page that will be rendered by this method can include:

  • /pages/_error.js
  • /pages/404.js
  • /pages/500.js

Checklist

Proxy to Next.js app.renderError - this method will render an
appropriate error page based on the response status code.
Comment on lines +107 to +118
const reply = this
const { request } = reply

// set custom headers as next will finish the request
for (const [headerName, headerValue] of Object.entries(reply.getHeaders())) {
// Fastify sets content-length to `undefined` for error handlers, which is an invalid value
if (headerName === 'content-length' && headerValue === undefined) {
continue
}

reply.raw.setHeader(headerName, headerValue)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from render above

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a unit test?

@stefee
Copy link
Contributor Author

stefee commented Dec 15, 2021

@mcollina
I updated the existing "error page" test to be more accurate - it was not testing anything before other than the ability to call reply.nextRender within fastify.setErrorHandler - would you prefer I add the existing test back and create a new test?

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@stefee
Copy link
Contributor Author

stefee commented Dec 15, 2021

I am unsure if the CI failures are a result of the changes.

@mcollina
Copy link
Member

It might be, it seems code coverage got reduced.

@stefee
Copy link
Contributor Author

stefee commented Dec 15, 2021

Ah yes, I will look into that.

@stefee
Copy link
Contributor Author

stefee commented Dec 16, 2021

This should be ready for landing :-)

@mcollina mcollina merged commit ff58f06 into fastify:master Dec 27, 2021
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 this pull request may close these issues.

Unable to render _error page, renders 404 page instead

2 participants