Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@nestjs/platform-express": "^10.0.0",
"@sentry/nestjs": "latest || *",
"reflect-metadata": "^0.2.0",
"axios": "1.13.5",
"rxjs": "^7.8.1"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ export class AppController {
return this.appService.testExpectedRpcException(id);
}

@Get('test-axios-error/:id')
async testAxiosError(@Param('id') id: string) {
return this.appService.testAxiosError(id);
}

@Get('test-span-decorator-async')
async testSpanDecoratorAsync() {
return { result: await this.appService.testSpanDecoratorAsync() };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { RpcException } from '@nestjs/microservices';
import { Cron, SchedulerRegistry } from '@nestjs/schedule';
import * as Sentry from '@sentry/nestjs';
import { SentryCron, SentryTraced } from '@sentry/nestjs';
import { AxiosError } from 'axios';

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

testAxiosError(id: string) {
throw new AxiosError(
`This is an axios error with id ${id}`,
'ERR_BAD_RESPONSE',
undefined,
undefined,
// Simulating an upstream API 502 response
{ status: 502, statusText: 'Bad Gateway', headers: {}, config: { headers: {} }, data: {} } as any,
);
}

@SentryTraced('wait and return a string')
async wait() {
await new Promise(resolve => setTimeout(resolve, 500));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,24 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
});
});

test('Sends AxiosError to Sentry', async ({ baseURL }) => {
const errorEventPromise = waitForError('nestjs-basic', event => {
return !event.type && event.exception?.values?.[0]?.value === 'This is an axios error with id 123';
});

const response = await fetch(`${baseURL}/test-axios-error/123`);
expect(response.status).toBe(500);

const errorEvent = await errorEventPromise;

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an axios error with id 123');
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});
});

test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
let errorEventOccurred = false;

Expand Down
31 changes: 26 additions & 5 deletions packages/nestjs/src/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,34 @@
/**
* Determines if the exception is an expected control flow error.
* - HttpException errors will have a status property
* - RpcException errors will have an error property
* Determines if the exception is an expected NestJS control flow error.
* - HttpException have getStatus and getResponse methods: https://github.com/nestjs/nest/blob/master/packages/microservices/exceptions/rpc-exception.ts
* - RpcException have getError and initMessage methods: https://github.com/nestjs/nest/blob/master/packages/common/exceptions/http.exception.ts
*
* We cannot use `instanceof HttpException` here because this file is imported
* from the main entry point (via decorators.ts), and importing @nestjs/common at that
* point would load it before OpenTelemetry instrumentation can patch it, breaking instrumentations.
*
* @returns `true` if the exception is expected and should not be reported to Sentry, otherwise `false`.
*/
export function isExpectedError(exception: unknown): boolean {
if (typeof exception === 'object' && exception !== null) {
return 'status' in exception || 'error' in exception;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Are we dropping the status check on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could add it as an additional condition but don't think it's necessary

if (typeof exception !== 'object' || exception === null) {
return false;
}

const ex = exception as Record<string, unknown>;

// HttpException
if (
typeof ex.getStatus === 'function' &&
typeof ex.getResponse === 'function' &&
typeof ex.initMessage === 'function'
) {
return true;
}

// RpcException
if (typeof ex.getError === 'function' && typeof ex.initMessage === 'function') {
return true;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duck-typing check may break for older NestJS versions

Medium Severity

The isExpectedError duck-typing relies on initMessage being a function on both HttpException and RpcException. The package's peerDependencies support @nestjs/common ^8.0.0 || ^9.0.0, but initMessage appears to have been added as a public method in NestJS v10 (older documentation only lists getStatus and getResponse). If initMessage doesn't exist in v8/v9, isExpectedError would always return false, causing HttpException and RpcException to be incorrectly reported to Sentry. The existing nestjs-8 e2e test app has tests verifying these exceptions are NOT sent to Sentry, which would break.

Fix in Cursor Fix in Web

}

return false;
}
25 changes: 25 additions & 0 deletions packages/nestjs/test/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { HttpException, HttpStatus } from '@nestjs/common';
import { describe, expect, it } from 'vitest';
import { isExpectedError } from '../src/helpers';

describe('isExpectedError', () => {
it('should return true for HttpException', () => {
expect(isExpectedError(new HttpException('Bad Request', HttpStatus.BAD_REQUEST))).toBe(true);
});

it('should return true for RpcException-like objects', () => {
const rpcLike = {
getError: () => 'some error',
initMessage: () => {},
};
expect(isExpectedError(rpcLike)).toBe(true);
});

it('should return false for plain Error', () => {
expect(isExpectedError(new Error('test'))).toBe(false);
});

it('should return false for object with status property', () => {
expect(isExpectedError({ status: 502, message: 'Bad Gateway' })).toBe(false);
});
});
Loading