Skip to content

Error Handling Documentation#1130

Merged
mcollina merged 4 commits intofastify:masterfrom
cemremengu:doc-error-handling
Sep 17, 2018
Merged

Error Handling Documentation#1130
mcollina merged 4 commits intofastify:masterfrom
cemremengu:doc-error-handling

Conversation

@cemremengu
Copy link
Copy Markdown
Contributor

@cemremengu cemremengu commented Sep 1, 2018

Initial draft for error handling doc.

Any suggestions on what else we can add/change?

Fixes #1129.

Copy link
Copy Markdown
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.

I'm 👍 for adding a new file

Comment thread docs/Error-Handling.md Outdated

Due to [legacy domain limitations](https://nodejs.org/en/docs/guides/domain-postmortem/) in node, it is not possible to process all uncaught errors in a sensible way. Fastify follows an all-or-nothing approach and aims to be lean and optimal as much as possible. Thus, the developer is responsible for making sure that the errors are handled properly. Most of the errors are usually a result of unexpected input data, so we recommend specifying a [JSON.schema validation](https://github.com/fastify/fastify/blob/master/docs/Validation-and-Serialization.md) for your input data.

Note that, while Fastify doesn't catch uncaught error for you, if your route is declared as async, the error will be caught by the promise and handled by Fastify as a generic `Internal Server Error`. For customizing the behaviour, you should wrap your route logic in a `try/catch` statement. No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you should wrap your route logic in a try/catch statement. => you should wrap your route logic in a try/catch statement or use [setErrorHandler](https://github.com/fastify/fastify/blob/master/docs/Server.md#seterrorhandler)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the preferred way is for errors to bubble up in the error handler.

Comment thread docs/Error-Handling.md Outdated

Due to [legacy domain limitations](https://nodejs.org/en/docs/guides/domain-postmortem/) in node, it is not possible to process all uncaught errors in a sensible way. Fastify follows an all-or-nothing approach and aims to be lean and optimal as much as possible. Thus, the developer is responsible for making sure that the errors are handled properly. Most of the errors are usually a result of unexpected input data, so we recommend specifying a [JSON.schema validation](https://github.com/fastify/fastify/blob/master/docs/Validation-and-Serialization.md) for your input data.

Note that, while Fastify doesn't catch uncaught error for you, if your route is declared as async, the error will be caught by the promise and handled by Fastify as a generic `Internal Server Error`. For customizing the behaviour, you should wrap your route logic in a `try/catch` statement. No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the preferred way is for errors to bubble up in the error handler.

Comment thread docs/Error-Handling.md Outdated

## Error Handling

Due to [legacy domain limitations](https://nodejs.org/en/docs/guides/domain-postmortem/) in node, it is not possible to process all uncaught errors in a sensible way. Fastify follows an all-or-nothing approach and aims to be lean and optimal as much as possible. Thus, the developer is responsible for making sure that the errors are handled properly. Most of the errors are usually a result of unexpected input data, so we recommend specifying a [JSON.schema validation](https://github.com/fastify/fastify/blob/master/docs/Validation-and-Serialization.md) for your input data.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would not start with the domain postmortem, but rather from the fact that uncaught errors are likely to cause memory leaks, file descriptor leaks and other major production issues. Domains were introduced to try fixing this issue, but they did not. The best way to deal with unhandled errors is to crash. Using async/await is safe because fastify would route all errors there.

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Sep 1, 2018

This new file needs to be linked from the README as well.

I would also add a referende to http://npm.im/make-promises-safe and unhandledRejection.

Comment thread docs/Error-Handling.md Outdated

## Error Handling

Uncaught errors are likely to cause memory leaks, file descriptor leaks and other major production issues ([see this])(http://npm.im/make-promises-safe). [Domains](https://nodejs.org/en/docs/guides/domain-postmortem/) were introduced to try fixing this issue, but they did not. The best way to deal with unhandled errors at the moment is to [crash](https://nodejs.org/dist/latest-v8.x/docs/api/deprecations.html#deprecations_dep0018_unhandled_promise_rejections), given the fact that it is not possible to process all uncaught errors in a sensible way
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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 with the nit on the README.

Comment thread README.md Outdated
Copy link
Copy Markdown
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

Comment thread docs/Error-Handling.md Outdated
Copy link
Copy Markdown
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.

LGTM

@delvedor delvedor added the documentation Improvements or additions to documentation label Sep 4, 2018
@mcollina mcollina requested a review from jsumners September 4, 2018 14:36
Comment thread docs/Error-Handling.md Outdated

## Error Handling

Uncaught errors are likely to cause memory leaks, file descriptor leaks and other major production issues. [Domains](https://nodejs.org/en/docs/guides/domain-postmortem/) were introduced to try fixing this issue, but they did not. Given the fact that it is not possible to process all uncaught errors in a sensible way, the best way to deal with them at the moment is to [crash](https://nodejs.org/api/process.html#process_warning_using_uncaughtexception_correctly). In case of promises, make sure to [handle](https://nodejs.org/dist/latest-v8.x/docs/api/deprecations.html#deprecations_dep0018_unhandled_promise_rejections) catch errors [correctly](https://github.com/mcollina/make-promises-safe).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What does "handle catch errors correctly" mean? This needs to be reworded.

Comment thread docs/Error-Handling.md Outdated

Fastify follows an all-or-nothing approach and aims to be lean and optimal as much as possible. Thus, the developer is responsible for making sure that the errors are handled properly. Most of the errors are usually a result of unexpected input data, so we recommend specifying a [JSON.schema validation](https://github.com/fastify/fastify/blob/master/docs/Validation-and-Serialization.md) for your input data.

Note that, while Fastify doesn't catch uncaught error for you, if routes are declared as `async`, the error will safely be caught by the promise and routed to the default error handler of Fastify for a generic `Internal Server Error` response. For customizing this behaviour, you should use [setErrorHandler](https://github.com/fastify/fastify/blob/master/docs/Server.md#seterrorhandler).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"catch uncaught error" => "catch uncaught errors"

@mcollina
Copy link
Copy Markdown
Member

@cemremengu ping

Copy link
Copy Markdown
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. @jsumners can you please check again?

Copy link
Copy Markdown
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

@mcollina mcollina merged commit 4b5f818 into fastify:master Sep 17, 2018
@cemremengu cemremengu deleted the doc-error-handling branch September 17, 2018 12:56
@github-actions
Copy link
Copy Markdown

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

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants