-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Clarify NestJS error filters #12054
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
Clarify NestJS error filters #12054
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,8 @@ export class YourCatchAllExceptionFilter implements ExceptionFilter { | |
| } | ||
| ``` | ||
|
|
||
| By default, only unhandled exceptions that are not caught by an error filter are reported to Sentry. | ||
| `HttpException`s (including [derivatives](https://docs.nestjs.com/exception-filters#built-in-http-exceptions)) are also not captured by default because they mostly act as control flow vehicles. | ||
|
|
||
| {/* TODO(v9): Remove this note */} | ||
| _Note that `@SentryExceptionCaptured()` was called `@WithSentry` in SDK versions `8.38.0` and prior._ | ||
|
|
@@ -73,12 +75,13 @@ import { SentryGlobalFilter } from "@sentry/nestjs/setup"; | |
| }) | ||
| export class AppModule {} | ||
| ``` | ||
|
|
||
| {/* TODO(v9): Remove this note. */} | ||
|
|
||
| **Note:** If you have a NestJS + GraphQL application and you are using the `@sentry/nestjs` SDK version `8.38.0` or earlier, replace the `SentryGlobalFilter` with the `SentryGlobalGenericFilter`. | ||
| In SDK versions `8.39.0` and above, the `SentryGlobalGenericFilter` is deprecated because the `SentryGlobalFilter` will handle GraphQL contexts automatically. | ||
|
|
||
| By default, exceptions with status code 4xx are not sent to Sentry. If you still want to capture these exceptions, you can do so manually with `Sentry.captureException()`: | ||
| If you have error filters for specific types of exceptions (for example `@Catch(HttpException)`, or any other `@Catch(...)` with arguments) and you want to capture errors caught by these filters, capture the errors in the `catch()` handler with `Sentry.captureException()`: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case they would also need to remove our There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a tricky one to think through. In general, our dedupe integration will drop the second capture call, as long as the two capture calls happen consecutively (no capture call on another error in between). I am not sure if we should tell people to remove their |
||
|
|
||
| ```javascript {9} | ||
| import { ArgumentsHost, BadRequestException, Catch } from '@nestjs/common'; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: Can we also mention the last point, about HttpExceptions not being caught, below the code block just above this line? I think it's valuable information since users might expect otherwise in a catch-all filter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you move it in general or duplicate it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as you have it now