NestJS SentryExeceptionCaptured doesn't catch custom filters

[adamghowiba]

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/nestjs

SDK Version

8.40.0

Framework Version

10.3.9

Link to Sentry event

No response

Reproduction Example/SDK Setup

Maybe I'm confused, but when using @SentryExeceptionCaptured() on an exception filter with a specific Catch condition, it doesn't report the error to sentry. It does work with a Catch-all all exception filter. e.g an exception filter an empty @Catch() decorator

If custom filters require manually catching the exception, maybe we can make this more apparent in the docs. The only thing it mentioned about custom exception filters is that if you want to catch 4.x.x errors you have to do it manually.

Custom Filter (Doesn't work)

@Catch(HttpException)
export class HttpExceptionFilter implements ExceptionFilter {

  @SentryExceptionCaptured()
  catch(
    exception: HttpException & BaseApiErrorResponse,
    host: ArgumentsHost
  ): void {
    const ctx = host.switchToHttp();
    const response = ctx.getResponse<Response>();
    const request = ctx.getRequest<Request>();

    response.status(500).json({error: "Some error"});
  }
}

Catch All Filter (Works)

@Catch()
export class AllExceptionFilter implements ExceptionFilter {

  @SentryExceptionCaptured()
  catch(
    exception: unknown & { getStatus?: () => number; message?: string },
    host: ArgumentsHost
  ): void {
    const ctx = host.switchToHttp();
    const response = ctx.getResponse<Response>();

    response.status(500).json({error: "testing error"});
  }
}

app.module.ts

@Module({
  imports: [
    SentryModule.forRoot(),
  ],
  controllers: [AppController],
  providers: [
    {
      provide: APP_FILTER,
      useClass: AllExceptionFilter,
    },
    {
      provide: APP_FILTER,
      useClass: HttpExceptionFilter,
    },
    {
      provide: APP_FILTER,
      useClass: AxiosExceptionFilter,
    },
    {
      provide: APP_GUARD,
      useClass: RoleGuard,
    },
  ],
})
export class AppModule {}

Steps to Reproduce

1.) Create a global exception filter using @SentryExceptionCaptured()
2.) Create a targeted exception filter e.g @Catch(TestingError)
3.) Global filter catches exceptions, custom filter does not

Expected Result

I would imagine that @SentryExceptionCaptured() should catch the exception when specified on any catch function of an exception filter

Actual Result

Exceptions caught by specific exception filters are not reported

[Lms24]

Hi @adamghowiba thanks for writing in! We'll take a look

[Lms24]

So, I was able to reproduce this and I think it is a problem specifically with HttpException errors and all classes inheriting from HttpException.

In our SDK we filter out "expected" errors, meaning anything this check deems expected won't be reported to Sentry. I'm not yet sure if this is by design or we have a bug but I asked team members with more context on NestJS what they think.

If I throw a normal Error somewhere in a route and create a filter that catches it, the error is sent to Sentry.

I created a minimal reproduction app to demonstrate both behaviours: https://github.com/Lms24/gh-sentry-javascript-14580-nestjs-custom-exception-filter

[Lms24]

@adamghowiba I discussed this with @chargome and we think this is expected behaviour. Here's why: Usually you would throw HttpException or its derivatives manually in the app. If you also specifically catch this error in your filter, chances are high that you actually handle the error and you don't want to send it to Sentry. Therefore, we consider the error "expected" and don't catch it.

As you already pointed out, you can of course manually call Sentry.captureException() and still decide to send them.

You're right that our docs are incorrect here and need some updates. I'll open a PR for this in a minute. Thanks for reporting!

[Lms24]

Looks like @lforst was faster with the docs update 😅 Anyway, I think this should be clarified and ready to close once the docs PR is merged.

Let us know if you have other questions!