Error Handling Documentation#1130
Conversation
|
|
||
| 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 |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
I think the preferred way is for errors to bubble up in the error handler.
|
|
||
| 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 |
There was a problem hiding this comment.
I think the preferred way is for errors to bubble up in the error handler.
|
|
||
| ## 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. |
There was a problem hiding this comment.
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.
|
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. |
|
|
||
| ## 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 |
There was a problem hiding this comment.
This is the correct link for the "crash" (https://nodejs.org/api/process.html#process_warning_using_uncaughtexception_correctly). The same is not true for promises, so you'll need to link that deprecation. I would also link to https://github.com/mcollina/make-promises-safe.
mcollina
left a comment
There was a problem hiding this comment.
LGTM with the nit on the README.
9eec044 to
c2b4737
Compare
c2b4737 to
ea1d335
Compare
|
|
||
| ## 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). |
There was a problem hiding this comment.
What does "handle catch errors correctly" mean? This needs to be reworded.
|
|
||
| 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). |
There was a problem hiding this comment.
"catch uncaught error" => "catch uncaught errors"
|
@cemremengu ping |
ea1d335 to
58d933a
Compare
|
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. |
Initial draft for error handling doc.
Any suggestions on what else we can add/change?
Fixes #1129.