Using custom `SentryExceptionsFilter` is missing features of `BaseExceptionFilter`

[KiwiKilian]

Picking up from our discussion from https://github.com/ericjeker/nestjs-sentry-example/issues/1#issuecomment-1737453215 I've now tried the new Setup with. Thanks for your work!

While I first thought it's great to switch to an ExceptionFilter, I didn't know only one single ExceptionFilter will always be called, I thought one could pass on to the next ExceptionFilter. So with the current implementation, the features of the default BaseExceptionFilter will be lost.

I've also read, an ExceptionFilter should not throw an error. It actually makes sense to me, while it's not explicitly stated in the docs, the example also does not throw as well as the BaseExceptionsFilter.

[KiwiKilian]

This could be a solution, which im happy to contribute, if #8 is resolved.

import { ArgumentsHost, Catch } from '@nestjs/common';
import { BaseExceptionFilter } from '@nestjs/core';
import * as Sentry from '@sentry/node';

/**
 * Catch all exceptions and forward to Sentry, this might not be what you want to do
 * as it's going to catch everything, including 404s, but it's a good starting point.
 *
 * You can further refine or filter out exceptions that you don't want to send to Sentry.
 */
@Catch()
export class SentryExceptionsFilter<T = any> extends BaseExceptionFilter<T> {
  catch(exception: T, host: ArgumentsHost): void {
    // capture the error, you can filter out some errors here
    // const httpStatus = exception instanceof HttpException ? exception.getStatus() : HttpStatus.INTERNAL_SERVER_ERROR;
    Sentry.captureException(exception);

    return super.catch(exception, host);
  }
}

[ericjeker]

Hi @KiwiKilian , I hope this is not too late. I answered in the other issue you opened. I'll check on that ExceptionFilter thing.

Just so that you know, I had a discussion with the Sentry Javascript team a while back and they told me they will change a lot in v8, which was released 3d ago. So this whole repository will probably change accordingly, at least in it's v3.

That's also one of the reason I didn't update it much since then.

[KiwiKilian]

No worries! Awesome to hear things might be getting easier with v8. Happy to give feedback on any new changes but I guess my concerns about the usage of ExceptionFilter still apply. Currently we use the solution commented above.

[ericjeker]

Hi @KiwiKilian,

I created a release/v2.0 branch that will hold the previous version of this example, and also fixed the license problem by adding a MIT-0 license. If you want to contribute you can do a PR on that branch with your proposal.

I reviewed the BaseExceptionFilter and it makes sense to do that change. I actually didn't know about it, I was following the catch all from their documentation.

When using the BaseExceptionFilter I guess it's a matter of ordering the global filters so that more specific filters are declared first as discussed in this answer you mentioned. Currently the filter is declared using APP_FILTER in SentryModule but we can certainly move it to the main.ts file so the declaration order is more explicit.

For example: app.useGlobalFilters(new SentryExceptionFilter(), new NotfoundFilter());

Let me know what you think and I'd be happy to check your PR. And I'll start working on the v3.0 using the new version of Sentry Javascript.

[KiwiKilian]

Thanks once again for your work @ericjeker!

Did you see there is now official docs on NestJS with Sentry? https://docs.sentry.io/platforms/javascript/guides/nestjs/

Did you already do a comparison, does it behave mostly the same as your setup?

[ericjeker]

Hi @KiwiKilian,

Yes, my v3 is based on that doc. I mention it in the README.

But basically this repository is not so useful anymore. I might add more complexities to the setup if necessary.

What do you think would be interesting to show here? Did you migrate to the SDK v8 already?

[KiwiKilian]

Ah sorry, didn't see you already did update to v8 using their docs...

But basically this repository is not so useful anymore.

It was great to have your setup, while there was no official approach. Now I would also say the repo is obsolete. Thanks again!