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

Preserve API invoked internal server error custom messages. #18

Merged
merged 4 commits into from
Jun 15, 2019
Merged

Preserve API invoked internal server error custom messages. #18

merged 4 commits into from
Jun 15, 2019

Conversation

JGiola
Copy link
Contributor

@JGiola JGiola commented May 2, 2019

This PR is aimed to allow the customization of the 500 errors that are generated via the available APIs and not from thrown errors, even when the errorHandler options is set to true.

The reasoning behind this change is that having the uncaught exceptions not leaked to the users is a valuable things to have but in the current implementation even the errors generated via the APIs will be masked even if the custom error is passed by the developer. To allow custom 500 errors we have to disable the "anti leaking" function.

With this PR I've added an additional flag to the error message if it is generated via the API on the fastify.httpErrors and reply object to allow them to keep their message and not to log an error that is generated always in the same way.
Doing this will keep the API usage the same on every error (can set a custom message and the generated error is not logged by default) and keep the masking on thrown errors.

JGiola added 2 commits May 2, 2019 15:17
Add an additional properties to the internal server error in case
is manually invoked by the user for always allowing the custom message.
Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

Good idea! Can you also add a test to verify what happens if you are not using the internal httpErrors?

README.md Outdated Show resolved Hide resolved
lib/httpErrors.js Outdated Show resolved Hide resolved
@JGiola
Copy link
Contributor Author

JGiola commented May 2, 2019

Good idea! Can you also add a test to verify what happens if you are not using the internal httpErrors?

I don't think I've understand what kind of test you mean. Like importing httpErrors directly and return an error object created from there?

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

Sorry, the test I meant is already present in another file 😅

LGTM

@delvedor delvedor requested a review from mcollina May 2, 2019 16:31
Copy link
Member

@allevo allevo left a comment

Choose a reason for hiding this comment

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

Good!!!

README.md Outdated Show resolved Hide resolved
@mcollina mcollina merged commit 09e2fef into fastify:master Jun 15, 2019
@JGiola JGiola deleted the feature/preserve-api-internal-server-error-message branch June 15, 2019 14:20
lamweili added a commit to lamweili/fastify-sensible that referenced this pull request Jul 31, 2024
…fy#18

Signed-off-by: Lam Wei Li <lam_wei_li@hotmail.com>
mcollina pushed a commit that referenced this pull request Sep 2, 2024
Signed-off-by: Lam Wei Li <lam_wei_li@hotmail.com>
This pull request was closed.
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.

4 participants