Added logging to the default error formatter #251
Conversation
|
Tested, and it works. But should the logging be part of the error formatter? Seems like a side effect that may not be evident to users, and thus they'd fail to reimplement it when they use a custom formatter. |
lib/errors.js
Outdated
| @@ -74,13 +83,15 @@ function defaultErrorFormatter (err) { | |||
| }).reduce((acc, val) => acc.concat(val), []) | |||
| } | |||
|
|
|||
| return { | |||
| const res = { | |||
There was a problem hiding this comment.
can't we just return here?
This come from experience. There is a significant amount of devs that asked for a similar feature to be removed from Fastify. If I move it outside of the errorFormatter, I would need to add a new option. By putting it in the errorFormatter, I leave it to the user by default. |
Yeah, but then you force devs that don't want the logs to rewrite the error formatter. To me the two features should just not be linked. |
|
In terms of logging in the error formatter, we re-implemented the error formatter with logging & error capturing (i.e. Sentry) only for specific errors which are unrecognised. I'd argue that the default error formatter is intended for development or basic requirements, but anyone who's running a production env will quite likely want to capture & report errors. Fastify itself also logs requests out. Possibly the I do get the argument that it could possibly be clearer, or there could be more options, but I'd also argue that it's still a net positive change this way, and further updates can be made. |
| } | ||
|
|
||
| function maybeFormatErrors (execution, reply) { | ||
| function maybeFormatErrors (execution, context) { |
There was a problem hiding this comment.
@mcollina could you update the index.d.ts definition of errorFormatter? Very coincidental timing on your PR but I'm just now trying out/setting up fastify-gql and wanted to access the context from my error logging. :-) Thanks!
There was a problem hiding this comment.
If you have time to send a PR, it'd be quicker.
As titled