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.

Sign up for GitHub

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

Jump to bottom

feat(nestjs): Filter all HttpExceptions #13120

Merged
merged 2 commits into from
Jul 31, 2024
Merged

feat(nestjs): Filter all HttpExceptions #13120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ade4fe5
Filter all http exceptions
nicohrubec Jul 30, 2024
7f5dc73
Update tests
nicohrubec Jul 30, 2024
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
11 changes: 8 additions & 3 deletions dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,14 @@ export class AppController {
return this.appService.testException(id);
}

@Get('test-expected-exception/:id')
async testExpectedException(@Param('id') id: string) {
return this.appService.testExpectedException(id);
@Get('test-expected-400-exception/:id')
async testExpected400Exception(@Param('id') id: string) {
return this.appService.testExpected400Exception(id);
}

@Get('test-expected-500-exception/:id')
async testExpected500Exception(@Param('id') id: string) {
return this.appService.testExpected500Exception(id);
}

@Get('test-span-decorator-async')
Expand Down
8 changes: 6 additions & 2 deletions dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ export class AppService {
throw new Error(`This is an exception with id ${id}`);
}

testExpectedException(id: string) {
throw new HttpException(`This is an expected exception with id ${id}`, HttpStatus.FORBIDDEN);
testExpected400Exception(id: string) {
throw new HttpException(`This is an expected 400 exception with id ${id}`, HttpStatus.BAD_REQUEST);
}

testExpected500Exception(id: string) {
throw new HttpException(`This is an expected 500 exception with id ${id}`, HttpStatus.INTERNAL_SERVER_ERROR);
}

@SentryTraced('wait and return a string')
Expand Down
32 changes: 24 additions & 8 deletions dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,41 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
});
});

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

waitForError('nestjs', event => {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected exception with id 123') {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 400 exception with id 123') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /test-expected-exception/:id';
return event?.transaction === 'GET /test-expected-400-exception/:id';
});

const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-exception/:id';
waitForError('nestjs', event => {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 500 exception with id 123') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /test-expected-500-exception/:id';
});

const response = await fetch(`${baseURL}/test-expected-exception/123`);
expect(response.status).toBe(403);
const transactionEventPromise400 = waitForTransaction('nestjs', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id';
});

const transactionEventPromise500 = waitForTransaction('nestjs', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id';
});

const response400 = await fetch(`${baseURL}/test-expected-400-exception/123`);
expect(response400.status).toBe(400);

const response500 = await fetch(`${baseURL}/test-expected-500-exception/123`);
expect(response500.status).toBe(500);

await transactionEventPromise;
await transactionEventPromise400;
await transactionEventPromise500;

await new Promise(resolve => setTimeout(resolve, 10000));

Expand Down
11 changes: 8 additions & 3 deletions dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,14 @@ export class AppController {
return this.appService.testException(id);
}

@Get('test-expected-exception/:id')
async testExpectedException(@Param('id') id: string) {
return this.appService.testExpectedException(id);
@Get('test-expected-400-exception/:id')
async testExpected400Exception(@Param('id') id: string) {
return this.appService.testExpected400Exception(id);
}

@Get('test-expected-500-exception/:id')
async testExpected500Exception(@Param('id') id: string) {
return this.appService.testExpected500Exception(id);
}

@Get('test-span-decorator-async')
Expand Down
8 changes: 6 additions & 2 deletions dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ export class AppService {
throw new Error(`This is an exception with id ${id}`);
}

testExpectedException(id: string) {
throw new HttpException(`This is an expected exception with id ${id}`, HttpStatus.FORBIDDEN);
testExpected400Exception(id: string) {
throw new HttpException(`This is an expected 400 exception with id ${id}`, HttpStatus.BAD_REQUEST);
}

testExpected500Exception(id: string) {
throw new HttpException(`This is an expected 500 exception with id ${id}`, HttpStatus.INTERNAL_SERVER_ERROR);
}

@SentryTraced('wait and return a string')
Expand Down
32 changes: 24 additions & 8 deletions dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,41 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
});
});

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

waitForError('nestjs', event => {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected exception with id 123') {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 400 exception with id 123') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /test-expected-exception/:id';
return event?.transaction === 'GET /test-expected-400-exception/:id';
});

const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-exception/:id';
waitForError('nestjs', event => {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 500 exception with id 123') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /test-expected-500-exception/:id';
});

const response = await fetch(`${baseURL}/test-expected-exception/123`);
expect(response.status).toBe(403);
const transactionEventPromise400 = waitForTransaction('nestjs', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id';
});

const transactionEventPromise500 = waitForTransaction('nestjs', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id';
});

const response400 = await fetch(`${baseURL}/test-expected-400-exception/123`);
expect(response400.status).toBe(400);

const response500 = await fetch(`${baseURL}/test-expected-500-exception/123`);
expect(response500.status).toBe(500);

await transactionEventPromise;
await transactionEventPromise400;
await transactionEventPromise500;

await new Promise(resolve => setTimeout(resolve, 10000));

Expand Down
5 changes: 2 additions & 3 deletions packages/nestjs/src/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
NestInterceptor,
OnModuleInit,
} from '@nestjs/common';
import { HttpException } from '@nestjs/common';
import { Catch } from '@nestjs/common';
import { Injectable } from '@nestjs/common';
import { Global, Module } from '@nestjs/common';
Expand Down Expand Up @@ -63,10 +64,8 @@ class SentryGlobalFilter extends BaseExceptionFilter {
* Catches exceptions and reports them to Sentry unless they are expected errors.
*/
public catch(exception: unknown, host: ArgumentsHost): void {
const status_code = (exception as { status?: number }).status;

// don't report expected errors
if (status_code !== undefined && status_code >= 400 && status_code < 500) {
if (exception instanceof HttpException) {
return super.catch(exception, host);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/tracing/nest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsE
const status_code = (exception as { status?: number }).status;

// don't report expected errors
if (status_code !== undefined && status_code >= 400 && status_code < 500) {
if (status_code !== undefined) {
return originalCatch.apply(target, [exception, host]);
}

Expand Down
Loading