Hapi errors are reported before onPreResponse

[camsteffen]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.9.2

Framework Version

Hapi 21.3.10

Link to Sentry event

No response

SDK Setup/Reproduction Example

I did some debugging and found that the issue can be fixed by specifying the 'error' channel here, like this:

server.events.on({name:'request', channels: ['error']}, (request, event) => {

Otherwise, an internal channel event occurs and that event is apparently too soon in the request lifecycle. I figured this out by comparing with hapi-sentry (code).

I believe this is enough for a minimal reproducible example:

Sentry.init();
Sentry.setupHapiErrorHandler(server);
server.ext('onPreResponse', (response, h) => {
  throw Boom.notFound();
});

Steps to Reproduce

1. Send a request where the route handler throws an Error
2. The onPreResponse hook replaces the error with a Boom error

Expected Result

No error is reported to Sentry since Boom error responses do not generate Hapi response error events. And the error thrown from the handler should not be considered uncaught.

Actual Result

The Error thrown from the handler is reported to Sentry.

[s1gr1d]

Thanks for reporting this! I will look into this

[s1gr1d]

I tested this and could not reproduce it. The boom errors do not land in Sentry. In which environment did you test your application? If it was in browser, could you try again with the following snippet:

  server.route({
    method: 'GET',
    path: '/favicon.ico',
    handler: (request, h) => {
      return h.response('MOCK FAVICON').type('image/x-icon').code(200);
    }
  });

It could be that the "Not Found" errors come from the browser trying to request the favicon.

However, boom errors are always reported if they happen in a route. What would be the desired outcome? I would propose having the same behavior for error reporting regardless of the error thrown inside onPreRequest or in a route. Boom 4xx should not be reported but Boom 5xx should. What do you think of this?

[camsteffen]

Sorry, I wasn't very clear. The bug is that errors thrown from the handler are reported to Sentry, even if the onPreResponse hook handles the error. For example, the handler could throw a custom DbEntityNotFoundError, the hook can convert that to a Boom 404 response, and nothing should be reported.

[s1gr1d]

Ah, I understand this makes sense! I created a PR to fix this.