customErrorHandler should not obscure original error

[franciscolourenco]

🚀 Feature Proposal

When using the onError hook to log errors to monitoring services, it would be great to have access to the original error.

However this is not currently possible with fastify-sensible customErrorHandler, because it creates a new Error with a new stacktrace for unhandled 500 errors.

Motivation

Being able to log the original error without having to disable fastify-sensible customErrorHandler

Example

fastify.addHook('onError', async (request, reply, error) => {
    Sentry.captureException(error)
}

[mcollina]

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[franciscolourenco]

Preserving the stacktrace seems to be possible without that getting exposed in the response, but how would you go about making the original message available in the hook and at the same time have the reply contain the modified error message?

[Eomm]

The setErrorHandler overrides all the hooks, without calling them.
I'm not 100% sure, but this is done to avoid infinity loops

Here the hook will never be called:

const fastify = require('./fastify')()

fastify.setErrorHandler(function (error, request, reply) {
  console.log('error handler', error.message)
  reply.send('err')
})

fastify.addHook('onError', (request, reply, error, done) => {
  console.log('error hook')
  done()
})

fastify.get('/', async () => {
  throw new Error('ops')
})

fastify.inject('/', (_, res) => {
  console.log(res.statusCode)
})

The use case is clear, but I didn't understand your idea to solve

[franciscolourenco]

@Eomm the 'onError' hook will be called if you reply with an error in the error handled. From the docs:

This hook will be executed only after the customErrorHandler has been executed, and only if the customErrorHandler sends an error back to the user (Note that the default customErrorHandler always sends the error back to the user).

[Eomm]

Ok, I forget about it 👍

but how would you go about making the original message available in the hook and at the same time have the reply contain the modified error message?

in the customErrorHandler there are 2 approaches

const err = new Error('something was wrong')
err.originalError = userError // it could be useful for custom error like "throw { myApp: 'error' }"
Error.captureStackTrace( err, userError ) // useful to manipulate a complete stacktrace in the hook
reply.send(err)

Not sure which one is the best, we should try and check the output

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Fdawgs]

#109 has (kind of) resolved this by removing the customErrorHandler?

[climba03003]

Yes, I would close it since it is no longer valid.