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

feat: allow falling back to default error handler from custom error handler #2621

Merged
merged 7 commits into from
Oct 19, 2020

Conversation

salmanm
Copy link
Member

@salmanm salmanm commented Oct 15, 2020

When custom errorHandler is specified in route config, it is responsible for handling all errors. But what if the custom error handler is interested in only some errors and wants to let fastify handle all other errors using it's default error handler.

This PR adds this feature.

Checklist

@salmanm salmanm changed the title Allow falling back to default error handler from custom error handler feat: allow falling back to default error handler from custom error handler Oct 15, 2020
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.

I might prefer to have an explicit:

const { defaultErrorHandler } = require('fastify')

So that developers can reuse our default implementation wherever they want.

lib/context.js Outdated Show resolved Hide resolved
lib/reply.js Outdated Show resolved Hide resolved
lib/fourOhFour.js Outdated Show resolved Hide resolved
fastify.js Outdated Show resolved Hide resolved
@mcollina
Copy link
Member

Can you add docs?

@salmanm salmanm marked this pull request as ready for review October 19, 2020 09:39
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

docs/Server.md Show resolved Hide resolved
Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

LGTM

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

@mcollina mcollina merged commit 2737f43 into fastify:master Oct 19, 2020
@salmanm salmanm deleted the default-handler branch October 19, 2020 13:42
@github-actions
Copy link

github-actions bot commented Feb 6, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 6, 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

Successfully merging this pull request may close these issues.

3 participants