Skip to content

docs: add a warning about setErrorHandler overriding a previously defined error handler on an encapsulated context#6097

Merged
gurgunday merged 8 commits intofastify:mainfrom
jean-michelet:docs/add-warning-seterrorhandler
May 8, 2025
Merged

docs: add a warning about setErrorHandler overriding a previously defined error handler on an encapsulated context#6097
gurgunday merged 8 commits intofastify:mainfrom
jean-michelet:docs/add-warning-seterrorhandler

Conversation

@jean-michelet
Copy link
Copy Markdown
Member

@jean-michelet jean-michelet commented May 1, 2025

Regarding #6096

@github-actions github-actions bot added the documentation Improvements or additions to documentation label May 1, 2025
@jean-michelet
Copy link
Copy Markdown
Member Author

jean-michelet commented May 1, 2025

I am gonna try to offer a fix rather than just documentation, as a lot of people don't read it or forget.
This is not good DX.

@jean-michelet jean-michelet changed the title docs: calling setErrorHandler more than once in the same scope will s… setErrorHandler should not silently overrides previously defined handler on an encapsulated context May 1, 2025
@jean-michelet jean-michelet changed the title setErrorHandler should not silently overrides previously defined handler on an encapsulated context fix: setErrorHandler should not silently overrides previously defined handler on an encapsulated context May 1, 2025
@jean-michelet jean-michelet removed the documentation Improvements or additions to documentation label May 1, 2025
@jean-michelet jean-michelet requested review from a team and Eomm and removed request for Eomm May 1, 2025 14:39
@github-actions github-actions bot added the documentation Improvements or additions to documentation label May 2, 2025
metcoder95
metcoder95 reviewed May 2, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

I'm not convinced of changing the behavior (that will also impose a breaking change), but rather extend documentation about it.

by allowing overrides, we allow to customize different levels of error handler and also enable plugins to do so; this might be better if is extended in the documentation rather than changed in this way.

@jean-michelet
Copy link
Copy Markdown
Member Author

jean-michelet commented May 2, 2025 •
edited
Loading

We can make a documentation PR and keep this one for next major (EDIT: did the opposite).
I can provide a server option allowSetErrorHandlerOverride to default: false and a clear error message that indicates the existence of this configuration.

@jean-michelet jean-michelet force-pushed the docs/add-warning-seterrorhandler branch from 9dcf3a4 to 367a535 Compare May 3, 2025 07:43
@jean-michelet jean-michelet changed the title fix: setErrorHandler should not silently overrides previously defined handler on an encapsulated context docs: add a warning about setErrorHandler overriding a previously defined error handler on an encapsulated context May 3, 2025
@jean-michelet jean-michelet mentioned this pull request May 3, 2025
Merged
@jean-michelet
Copy link
Copy Markdown
Member Author

jean-michelet commented May 3, 2025

@metcoder95
Reverted to doc PR

@jean-michelet jean-michelet requested a review from metcoder95 May 3, 2025 07:50
Fdawgs
Fdawgs reviewed May 3, 2025
View reviewed changes
gurgunday
gurgunday approved these changes May 5, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

lgtm

@gurgunday gurgunday merged commit 59f21ed into fastify:main May 8, 2025
5 checks passed
@jean-michelet jean-michelet deleted the docs/add-warning-seterrorhandler branch May 8, 2025 07:43
jean-michelet added a commit to jean-michelet/fastify that referenced this pull request May 9, 2025
@jean-michelet
docs: add a warning about setErrorHandler overriding a previously d…
72e42f7 
…efined error handler on an encapsulated context (fastify#6097)

* docs: calling setErrorHandler more than once in the same scope will silently override the previous handler

* fix: lint markdown

* docs: nit

* docs: apply convention

* docs: apply convention
jean-michelet added a commit to jean-michelet/fastify that referenced this pull request May 13, 2025
@jean-michelet
docs: add a warning about setErrorHandler overriding a previously d…
3c32545 
…efined error handler on an encapsulated context (fastify#6097)

* docs: calling setErrorHandler more than once in the same scope will silently override the previous handler

* fix: lint markdown

* docs: nit

* docs: apply convention

* docs: apply convention
@jean-michelet
Copy link
Copy Markdown
Member Author

jean-michelet commented Jun 12, 2025

If you can share:
https://www.linkedin.com/posts/jean-michelet-32801a262_fastify-users-plz-set-allowerrorhandleroverride-activity-7338933264121835520-ddS8/?utm_source=share&utm_medium=member_desktop&rcm=ACoAAEBuNlAB5kRlC3C9yyxCkvBM7UxtvmepOSs

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 23, 2026

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 Mar 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@metcoder95 metcoder95 metcoder95 approved these changes

@Fdawgs Fdawgs Fdawgs approved these changes

@gurgunday gurgunday gurgunday approved these changes

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jean-michelet @metcoder95 @Fdawgs @gurgunday