diagnostics channel for fastify v5 introduced a breaking change by removing custom `shouldHandleError` support

[tobiasdcl]

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 - fastify

SDK Version

9.35.0

Framework Version

fastify 5.4.0

Link to Sentry event

No response

Reproduction Example/SDK Setup

   setupFastifyErrorHandler(app, {
     shouldHandleError(_error, _request, reply) {
       //no error should be reported
       return false;
     },
   });

Steps to Reproduce

The usage of diagnostics channel for Fastify V5 introduced a breaking change by removing the possibility to pass a shouldHandleError option.

Prior to the release of the change in 9.33.0 it was required to explicitly initialize sentry. It was also possible to provide a custom shouldHandleError function - ref.

   setupFastifyErrorHandler(app, {
     shouldHandleError(_error, _request, reply) {
       return reply.statusCode >= 400;
     },
   });

Starting with 9.33.0 there is no need to call setupFastifyErrorHandler when using fastify v5, in fact calling setupFastifyErrorHandler has no effect as stated by the logs emitted by the sentry sdk

Fastify error handler was already registered via diagnostics channel.
You can safely remove `setupFastifyErrorHandler` call.

I think there is currently no way to pass a custom shouldHandleError while using sentry >= 9.33.0 and fastify v5. As a workaround we have downgraded to sentry-sdk 9.32.0 for now.

Thanks!

Expected Result

no errors should be reported by sentry (if thrown inside a fastify handler)

Actual Result

since the provided implementation of shouldHandleError is no longer used it has no effect and all errors that are not filtered by the defaultShouldHandleError are reported anyways

[mydea]

ah, that makes sense of course. Sorry about that, we need to see how we can handle this better, probably passing the handler should overwrite the default handler. cc @onurtemizkan could you take a look at that?

[tobiasdcl]

Hi @mydea,
I saw that the PR is ready for a while now. Could we get this over the finish line? 🚀