Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve @sentry/nestjs error instrumentation #12768

Closed
Tracked by #12504
nicohrubec opened this issue Jul 4, 2024 · 0 comments · Fixed by #12920
Closed
Tracked by #12504

Improve @sentry/nestjs error instrumentation #12768

nicohrubec opened this issue Jul 4, 2024 · 0 comments · Fixed by #12920
Assignees
Labels
Package: nestjs Issues related to the Sentry Nestjs SDK

Comments

@nicohrubec
Copy link
Contributor

nicohrubec commented Jul 4, 2024

Previous solution:

  • Sentry.setupNestErrorHandler()in main file.
  • Errors are sent to Sentry by proxying the nest base exception filter adding a captureException call and then using the proxy as a global filter.
  • Aside from being not very native to nest this approach had several issues, for example stopping user-defined global exception filters from working.

Attempted updates:

  • Global exception filter extending base exception filter:
    • Call captureException in a global error handler that catches all unhandled exceptions (should work with an empty @Catch()decorator on the filter).
    • According to what I understood from the docs this approach should work just fine but it didn't. NestJS does not allow chaining of exception filters, so according to some rules exactly one exception filter is determined that will be called. The first filter that should be called is the most specific filter. However, this is not the case if introduced in the setupNestErrorHandler method. Rather, the global exception filter always overwrote user-defined global exception filters.
  • Interceptor:
    • Call captureException in a global interceptor.
    • The problem is that in nest interceptors are called before exception filters. So for example even if a user defines an exception filter to catch a specific exception type, the exception will still be logged to Sentry. So this approach kind of works, but is not really what we want, because it would produce a lot of noise for users.

Final solution:

  • Ported the existing code to a nestjs root module.
  • Initially just meant as a setup improvement (much more native to nest), but had the nice side-effect of working properly with user-defined local and global exception filters.
@nicohrubec nicohrubec mentioned this issue Jul 4, 2024
@nicohrubec nicohrubec changed the title Improve SDK error instrumentation. Improve error handler to avoid having to pass anything in. or alternatively, find a way as native to regular Nest.js usage as possible. At the very least investigate current problems and find ways to address them. Improve SDK error instrumentation Jul 4, 2024
@nicohrubec nicohrubec changed the title Improve SDK error instrumentation Improve NestJS SDK error instrumentation Jul 4, 2024
@nicohrubec nicohrubec self-assigned this Jul 4, 2024
@AbhiPrasad AbhiPrasad added the Package: nestjs Issues related to the Sentry Nestjs SDK label Jul 4, 2024
@nicohrubec nicohrubec changed the title Improve NestJS SDK error instrumentation Improve @sentry/nestjs error instrumentation Jul 5, 2024
@nicohrubec nicohrubec linked a pull request Jul 9, 2024 that will close this issue
@nicohrubec nicohrubec linked a pull request Jul 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nestjs Issues related to the Sentry Nestjs SDK
Projects
None yet
2 participants