Skip to content

Commit 5bbba8a

Browse files
authored
fix(nestjs): Improve control flow exception filtering (#19524)
Our conditions for dropping control flow errors were a bit broad, leading to silently dropping errors from being sent to Sentry including `axios` errors. Closes #19519
1 parent 60969fc commit 5bbba8a

File tree

6 files changed

+87
-5
lines changed

6 files changed

+87
-5
lines changed

dev-packages/e2e-tests/test-applications/nestjs-basic/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"@nestjs/platform-express": "^10.0.0",
2222
"@sentry/nestjs": "latest || *",
2323
"reflect-metadata": "^0.2.0",
24+
"axios": "1.13.5",
2425
"rxjs": "^7.8.1"
2526
},
2627
"devDependencies": {

dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ export class AppController {
6767
return this.appService.testExpectedRpcException(id);
6868
}
6969

70+
@Get('test-axios-error/:id')
71+
async testAxiosError(@Param('id') id: string) {
72+
return this.appService.testAxiosError(id);
73+
}
74+
7075
@Get('test-span-decorator-async')
7176
async testSpanDecoratorAsync() {
7277
return { result: await this.appService.testSpanDecoratorAsync() };

dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { RpcException } from '@nestjs/microservices';
33
import { Cron, SchedulerRegistry } from '@nestjs/schedule';
44
import * as Sentry from '@sentry/nestjs';
55
import { SentryCron, SentryTraced } from '@sentry/nestjs';
6+
import { AxiosError } from 'axios';
67

78
const monitorConfig = {
89
schedule: {
@@ -42,6 +43,17 @@ export class AppService {
4243
throw new RpcException(`This is an expected RPC exception with id ${id}`);
4344
}
4445

46+
testAxiosError(id: string) {
47+
throw new AxiosError(
48+
`This is an axios error with id ${id}`,
49+
'ERR_BAD_RESPONSE',
50+
undefined,
51+
undefined,
52+
// Simulating an upstream API 502 response
53+
{ status: 502, statusText: 'Bad Gateway', headers: {}, config: { headers: {} }, data: {} } as any,
54+
);
55+
}
56+
4557
@SentryTraced('wait and return a string')
4658
async wait() {
4759
await new Promise(resolve => setTimeout(resolve, 500));

dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,24 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
3434
});
3535
});
3636

37+
test('Sends AxiosError to Sentry', async ({ baseURL }) => {
38+
const errorEventPromise = waitForError('nestjs-basic', event => {
39+
return !event.type && event.exception?.values?.[0]?.value === 'This is an axios error with id 123';
40+
});
41+
42+
const response = await fetch(`${baseURL}/test-axios-error/123`);
43+
expect(response.status).toBe(500);
44+
45+
const errorEvent = await errorEventPromise;
46+
47+
expect(errorEvent.exception?.values).toHaveLength(1);
48+
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an axios error with id 123');
49+
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
50+
handled: false,
51+
type: 'auto.http.nestjs.global_filter',
52+
});
53+
});
54+
3755
test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
3856
let errorEventOccurred = false;
3957

packages/nestjs/src/helpers.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,34 @@
11
/**
2-
* Determines if the exception is an expected control flow error.
3-
* - HttpException errors will have a status property
4-
* - RpcException errors will have an error property
2+
* Determines if the exception is an expected NestJS control flow error.
3+
* - HttpException have getStatus and getResponse methods: https://github.com/nestjs/nest/blob/master/packages/microservices/exceptions/rpc-exception.ts
4+
* - RpcException have getError and initMessage methods: https://github.com/nestjs/nest/blob/master/packages/common/exceptions/http.exception.ts
5+
*
6+
* We cannot use `instanceof HttpException` here because this file is imported
7+
* from the main entry point (via decorators.ts), and importing @nestjs/common at that
8+
* point would load it before OpenTelemetry instrumentation can patch it, breaking instrumentations.
59
*
610
* @returns `true` if the exception is expected and should not be reported to Sentry, otherwise `false`.
711
*/
812
export function isExpectedError(exception: unknown): boolean {
9-
if (typeof exception === 'object' && exception !== null) {
10-
return 'status' in exception || 'error' in exception;
13+
if (typeof exception !== 'object' || exception === null) {
14+
return false;
15+
}
16+
17+
const ex = exception as Record<string, unknown>;
18+
19+
// HttpException
20+
if (
21+
typeof ex.getStatus === 'function' &&
22+
typeof ex.getResponse === 'function' &&
23+
typeof ex.initMessage === 'function'
24+
) {
25+
return true;
26+
}
27+
28+
// RpcException
29+
if (typeof ex.getError === 'function' && typeof ex.initMessage === 'function') {
30+
return true;
1131
}
32+
1233
return false;
1334
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { HttpException, HttpStatus } from '@nestjs/common';
2+
import { describe, expect, it } from 'vitest';
3+
import { isExpectedError } from '../src/helpers';
4+
5+
describe('isExpectedError', () => {
6+
it('should return true for HttpException', () => {
7+
expect(isExpectedError(new HttpException('Bad Request', HttpStatus.BAD_REQUEST))).toBe(true);
8+
});
9+
10+
it('should return true for RpcException-like objects', () => {
11+
const rpcLike = {
12+
getError: () => 'some error',
13+
initMessage: () => {},
14+
};
15+
expect(isExpectedError(rpcLike)).toBe(true);
16+
});
17+
18+
it('should return false for plain Error', () => {
19+
expect(isExpectedError(new Error('test'))).toBe(false);
20+
});
21+
22+
it('should return false for object with status property', () => {
23+
expect(isExpectedError({ status: 502, message: 'Bad Gateway' })).toBe(false);
24+
});
25+
});

0 commit comments

Comments
 (0)