From 6e6b32efdf10c826829fba80b41d28d229974d78 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Thu, 8 Aug 2024 14:40:04 +0200 Subject: [PATCH 01/36] Add test that demonstrates wrong behavior --- .../nestjs-basic/src/app.controller.ts | 7 +++++- .../nestjs-basic/src/app.module.ts | 10 ++++++++- .../nestjs-basic/src/bad-request.filter.ts | 22 +++++++++++++++++++ .../nestjs-basic/tests/errors.test.ts | 16 ++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts index ec0a921da2c4..65dd0ff76ba6 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts @@ -1,4 +1,4 @@ -import { Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors } from '@nestjs/common'; +import {BadRequestException, Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors} from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; import { ExampleGuard } from './example.guard'; @@ -74,4 +74,9 @@ export class AppController { async flush() { await flush(); } + + @Get('bad-request') + async badRequest() { + throw new BadRequestException('Plain bad request!'); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts index b2aad014c745..739a293eb582 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts @@ -4,11 +4,19 @@ import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; import { ExampleMiddleware } from './example.middleware'; +import { APP_FILTER } from "@nestjs/core"; +import { BadRequestExceptionFilter } from "./bad-request.filter"; @Module({ imports: [SentryModule.forRoot(), ScheduleModule.forRoot()], controllers: [AppController], - providers: [AppService], + providers: [ + AppService, + { + provide: APP_FILTER, + useClass: BadRequestExceptionFilter + } + ], }) export class AppModule { configure(consumer: MiddlewareConsumer): void { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts new file mode 100644 index 000000000000..4499d603c86c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts @@ -0,0 +1,22 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from "@nestjs/common"; +import { Request, Response } from 'express'; + +@Catch(BadRequestException) +export class BadRequestExceptionFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + const status = exception.getStatus(); + console.log('Bad request exception filter!'); + + response + .status(status) + .json({ + statusCode: status, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Bad request exception filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts index 34e626cb8c52..59850fd3a082 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts @@ -94,3 +94,19 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { expect(errorEventOccurred).toBe(false); }); + +test('Global exception filter registered in root module is applied', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/bad-request`); + const responseBody = await response.json(); + + console.log(responseBody); + + expect(response.status).toBe(400); + + // this should fail but doesn't because the bad request exception filter is not being properly applied + expect(responseBody).toEqual({ + message: 'Plain bad request!', + error: 'Bad Request', + statusCode: 400 + }); +}) From fd3a30a5b30d611aa4d7742b2f298390d26b1b6e Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Thu, 8 Aug 2024 15:01:23 +0200 Subject: [PATCH 02/36] Use example exception instead of bad request --- .../nestjs-basic/src/app.controller.ts | 9 ++++---- .../nestjs-basic/src/app.module.ts | 14 ++++++------ .../nestjs-basic/src/bad-request.filter.ts | 22 ------------------- .../src/example-with-filter.exception.ts | 5 +++++ .../nestjs-basic/src/example.filter.ts | 21 ++++++++++++++++++ .../nestjs-basic/tests/errors.test.ts | 12 +++++----- 6 files changed, 43 insertions(+), 40 deletions(-) delete mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.filter.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts index 65dd0ff76ba6..32bf352c9ae9 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts @@ -1,6 +1,7 @@ -import {BadRequestException, Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors} from '@nestjs/common'; +import { Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; +import { ExampleExceptionWithFilter } from './example-with-filter.exception'; import { ExampleGuard } from './example.guard'; import { ExampleInterceptor } from './example.interceptor'; @@ -75,8 +76,8 @@ export class AppController { await flush(); } - @Get('bad-request') - async badRequest() { - throw new BadRequestException('Plain bad request!'); + @Get('example-exception-with-filter') + async exampleExceptionWithFilter() { + throw new ExampleExceptionWithFilter(); } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts index 739a293eb582..eaa6508ff0f4 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts @@ -1,22 +1,22 @@ import { MiddlewareConsumer, Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; import { ScheduleModule } from '@nestjs/schedule'; import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { ExampleExceptionFilter } from './example.filter'; import { ExampleMiddleware } from './example.middleware'; -import { APP_FILTER } from "@nestjs/core"; -import { BadRequestExceptionFilter } from "./bad-request.filter"; @Module({ imports: [SentryModule.forRoot(), ScheduleModule.forRoot()], controllers: [AppController], providers: [ AppService, - { - provide: APP_FILTER, - useClass: BadRequestExceptionFilter - } - ], + { + provide: APP_FILTER, + useClass: ExampleExceptionFilter, + }, + ], }) export class AppModule { configure(consumer: MiddlewareConsumer): void { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts deleted file mode 100644 index 4499d603c86c..000000000000 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/bad-request.filter.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from "@nestjs/common"; -import { Request, Response } from 'express'; - -@Catch(BadRequestException) -export class BadRequestExceptionFilter implements ExceptionFilter { - catch(exception: BadRequestException, host: ArgumentsHost): void { - const ctx = host.switchToHttp(); - const response = ctx.getResponse(); - const request = ctx.getRequest(); - const status = exception.getStatus(); - console.log('Bad request exception filter!'); - - response - .status(status) - .json({ - statusCode: status, - timestamp: new Date().toISOString(), - path: request.url, - message: 'Bad request exception filter!', - }); - } -} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts new file mode 100644 index 000000000000..9be1977c48d4 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionWithFilter extends Error { + constructor() { + super('Original example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.filter.ts new file mode 100644 index 000000000000..52de1a0a550d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.filter.ts @@ -0,0 +1,21 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionWithFilter } from './example-with-filter.exception'; + +@Catch(ExampleExceptionWithFilter) +export class ExampleExceptionFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + const status = exception.getStatus(); + console.log('Example exception filter!'); + + response.status(status).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts index 59850fd3a082..85a9ec4d0496 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts @@ -96,17 +96,15 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { }); test('Global exception filter registered in root module is applied', async ({ baseURL }) => { - const response = await fetch(`${baseURL}/bad-request`); + const response = await fetch(`${baseURL}/example-exception-with-filter`); const responseBody = await response.json(); console.log(responseBody); - expect(response.status).toBe(400); - // this should fail but doesn't because the bad request exception filter is not being properly applied + expect(response.status).toBe(500); expect(responseBody).toEqual({ - message: 'Plain bad request!', - error: 'Bad Request', - statusCode: 400 + message: 'Internal server error', + statusCode: 500, }); -}) +}); From 1bcaf37258044b5f2c1f169b412767d3be728efc Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Thu, 8 Aug 2024 15:36:11 +0200 Subject: [PATCH 03/36] Rename --- .../nestjs-basic/src/app.controller.ts | 8 ++++---- .../test-applications/nestjs-basic/src/app.module.ts | 4 ++-- .../src/example-global-filter.exception.ts | 5 +++++ .../{example.filter.ts => example-global.filter.ts} | 10 +++++----- .../nestjs-basic/src/example-with-filter.exception.ts | 5 ----- .../nestjs-basic/tests/errors.test.ts | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global-filter.exception.ts rename dev-packages/e2e-tests/test-applications/nestjs-basic/src/{example.filter.ts => example-global.filter.ts} (63%) delete mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts index 32bf352c9ae9..c40f06986ed5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts @@ -1,9 +1,9 @@ import { Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; -import { ExampleExceptionWithFilter } from './example-with-filter.exception'; import { ExampleGuard } from './example.guard'; import { ExampleInterceptor } from './example.interceptor'; +import { ExampleExceptionGlobalFilter } from "./example-global-filter.exception"; @Controller() export class AppController { @@ -76,8 +76,8 @@ export class AppController { await flush(); } - @Get('example-exception-with-filter') - async exampleExceptionWithFilter() { - throw new ExampleExceptionWithFilter(); + @Get('example-exception-global-filter') + async exampleExceptionGlobalFilter() { + throw new ExampleExceptionGlobalFilter(); } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts index eaa6508ff0f4..ec5947fe27ab 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts @@ -4,8 +4,8 @@ import { ScheduleModule } from '@nestjs/schedule'; import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; -import { ExampleExceptionFilter } from './example.filter'; import { ExampleMiddleware } from './example.middleware'; +import {ExampleGlobalFilter} from "./example-global.filter"; @Module({ imports: [SentryModule.forRoot(), ScheduleModule.forRoot()], @@ -14,7 +14,7 @@ import { ExampleMiddleware } from './example.middleware'; AppService, { provide: APP_FILTER, - useClass: ExampleExceptionFilter, + useClass: ExampleGlobalFilter, }, ], }) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global-filter.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global-filter.exception.ts new file mode 100644 index 000000000000..41981ba748fe --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global-filter.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionGlobalFilter extends Error { + constructor() { + super('Original global example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts similarity index 63% rename from dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.filter.ts rename to dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts index 52de1a0a550d..bcfc9a5fca20 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts @@ -1,21 +1,21 @@ import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; import { Request, Response } from 'express'; -import { ExampleExceptionWithFilter } from './example-with-filter.exception'; +import { ExampleExceptionGlobalFilter } from "./example-global-filter.exception"; -@Catch(ExampleExceptionWithFilter) -export class ExampleExceptionFilter implements ExceptionFilter { +@Catch(ExampleExceptionGlobalFilter) +export class ExampleGlobalFilter implements ExceptionFilter { catch(exception: BadRequestException, host: ArgumentsHost): void { const ctx = host.switchToHttp(); const response = ctx.getResponse(); const request = ctx.getRequest(); const status = exception.getStatus(); - console.log('Example exception filter!'); + console.log('Example global exception filter!'); response.status(status).json({ statusCode: 400, timestamp: new Date().toISOString(), path: request.url, - message: 'Example exception was handled by filter!', + message: 'Example exception was handled by global filter!', }); } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts deleted file mode 100644 index 9be1977c48d4..000000000000 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-with-filter.exception.ts +++ /dev/null @@ -1,5 +0,0 @@ -export class ExampleExceptionWithFilter extends Error { - constructor() { - super('Original example exception!'); - } -} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts index 85a9ec4d0496..4cf272c5b7aa 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts @@ -96,7 +96,7 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { }); test('Global exception filter registered in root module is applied', async ({ baseURL }) => { - const response = await fetch(`${baseURL}/example-exception-with-filter`); + const response = await fetch(`${baseURL}/example-exception-global-filter`); const responseBody = await response.json(); console.log(responseBody); From 22eadbec683d18a77c435cf07042e41aee640fa4 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Thu, 8 Aug 2024 15:42:48 +0200 Subject: [PATCH 04/36] Add local filter test --- .../nestjs-basic/src/app.controller.ts | 12 +++++++++-- .../nestjs-basic/src/app.module.ts | 2 +- .../nestjs-basic/src/example-global.filter.ts | 2 +- .../src/example-local-filter.exception.ts | 5 +++++ .../nestjs-basic/src/example-local.filter.ts | 21 +++++++++++++++++++ .../nestjs-basic/tests/errors.test.ts | 18 ++++++++++++++-- 6 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local-filter.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts index c40f06986ed5..9cda3c96f9a6 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts @@ -1,11 +1,14 @@ -import { Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors } from '@nestjs/common'; +import { Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; +import { ExampleExceptionGlobalFilter } from './example-global-filter.exception'; +import { ExampleExceptionLocalFilter } from './example-local-filter.exception'; +import { ExampleLocalFilter } from './example-local.filter'; import { ExampleGuard } from './example.guard'; import { ExampleInterceptor } from './example.interceptor'; -import { ExampleExceptionGlobalFilter } from "./example-global-filter.exception"; @Controller() +@UseFilters(ExampleLocalFilter) export class AppController { constructor(private readonly appService: AppService) {} @@ -80,4 +83,9 @@ export class AppController { async exampleExceptionGlobalFilter() { throw new ExampleExceptionGlobalFilter(); } + + @Get('example-exception-local-filter') + async exampleExceptionLocalFilter() { + throw new ExampleExceptionLocalFilter(); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts index ec5947fe27ab..881982a5ca17 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts @@ -4,8 +4,8 @@ import { ScheduleModule } from '@nestjs/schedule'; import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { ExampleGlobalFilter } from './example-global.filter'; import { ExampleMiddleware } from './example.middleware'; -import {ExampleGlobalFilter} from "./example-global.filter"; @Module({ imports: [SentryModule.forRoot(), ScheduleModule.forRoot()], diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts index bcfc9a5fca20..6e8cda808d56 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts @@ -1,6 +1,6 @@ import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; import { Request, Response } from 'express'; -import { ExampleExceptionGlobalFilter } from "./example-global-filter.exception"; +import { ExampleExceptionGlobalFilter } from './example-global-filter.exception'; @Catch(ExampleExceptionGlobalFilter) export class ExampleGlobalFilter implements ExceptionFilter { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local-filter.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local-filter.exception.ts new file mode 100644 index 000000000000..8f76520a3b94 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local-filter.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionLocalFilter extends Error { + constructor() { + super('Original local example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts new file mode 100644 index 000000000000..918f8e1c50fc --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts @@ -0,0 +1,21 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionLocalFilter } from './example-local-filter.exception'; + +@Catch(ExampleExceptionLocalFilter) +export class ExampleLocalFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + const status = exception.getStatus(); + console.log('Example local exception filter!'); + + response.status(status).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by local filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts index 4cf272c5b7aa..74e9b9b9d4be 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts @@ -95,13 +95,27 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { expect(errorEventOccurred).toBe(false); }); -test('Global exception filter registered in root module is applied', async ({ baseURL }) => { +test('Global exception filter registered in main module is applied', async ({ baseURL }) => { const response = await fetch(`${baseURL}/example-exception-global-filter`); const responseBody = await response.json(); console.log(responseBody); - // this should fail but doesn't because the bad request exception filter is not being properly applied + // this should fail but doesn't because the exception filter is not being properly applied + expect(response.status).toBe(500); + expect(responseBody).toEqual({ + message: 'Internal server error', + statusCode: 500, + }); +}); + +test('Local exception filter registered in main module is applied', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/example-exception-global-filter`); + const responseBody = await response.json(); + + console.log(responseBody); + + // this should fail but doesn't because the exception filter is not being properly applied expect(response.status).toBe(500); expect(responseBody).toEqual({ message: 'Internal server error', From 61d5fe6fa784dbf870703db21cb479c6abc05bdb Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Thu, 8 Aug 2024 15:49:47 +0200 Subject: [PATCH 05/36] Fix tests --- .../nestjs-basic/src/example-global.filter.ts | 3 +-- .../test-applications/nestjs-basic/src/example-local.filter.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts index 6e8cda808d56..cf280876d605 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts @@ -8,10 +8,9 @@ export class ExampleGlobalFilter implements ExceptionFilter { const ctx = host.switchToHttp(); const response = ctx.getResponse(); const request = ctx.getRequest(); - const status = exception.getStatus(); console.log('Example global exception filter!'); - response.status(status).json({ + response.status(400).json({ statusCode: 400, timestamp: new Date().toISOString(), path: request.url, diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts index 918f8e1c50fc..07ee8c5463a3 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts @@ -8,10 +8,9 @@ export class ExampleLocalFilter implements ExceptionFilter { const ctx = host.switchToHttp(); const response = ctx.getResponse(); const request = ctx.getRequest(); - const status = exception.getStatus(); console.log('Example local exception filter!'); - response.status(status).json({ + response.status(400).json({ statusCode: 400, timestamp: new Date().toISOString(), path: request.url, From 885fb6bd7af0a2664daeda5fc42d845c6fa1f871 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Fri, 9 Aug 2024 10:21:06 +0200 Subject: [PATCH 06/36] Start on base exception filter patch --- .../src/integrations/tracing/nest/nest.ts | 6 ++ .../nest/sentry-nest-error-instrumentation.ts | 58 +++++++++++++++++++ .../src/integrations/tracing/nest/types.ts | 8 ++- 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index 4f8d88fa8f86..73a1b6899650 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -14,6 +14,7 @@ import { logger } from '@sentry/utils'; import { generateInstrumentOnce } from '../../../otel/instrument'; import { SentryNestInstrumentation } from './sentry-nest-instrumentation'; import type { MinimalNestJsApp, NestJsErrorFilter } from './types'; +import {SentryNestErrorInstrumentation} from "./sentry-nest-error-instrumentation"; const INTEGRATION_NAME = 'Nest'; @@ -25,10 +26,15 @@ const instrumentNestCommon = generateInstrumentOnce('Nest-Common', () => { return new SentryNestInstrumentation(); }); +const instrumentNestCoreErrors = generateInstrumentOnce('Nest-Core-Errors', () => { + return new SentryNestErrorInstrumentation(); +}) + export const instrumentNest = Object.assign( (): void => { instrumentNestCore(); instrumentNestCommon(); + instrumentNestCoreErrors(); }, { id: INTEGRATION_NAME }, ); diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts new file mode 100644 index 000000000000..842c5ee7a67e --- /dev/null +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts @@ -0,0 +1,58 @@ +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { + InstrumentationBase, + InstrumentationNodeModuleDefinition, + InstrumentationNodeModuleFile, +} from '@opentelemetry/instrumentation'; +import { SDK_VERSION } from '@sentry/utils'; +import type {BaseExceptionFilter} from './types'; + +const supportedVersions = ['>=8.0.0 <11']; + +/** + * + */ +export class SentryNestErrorInstrumentation extends InstrumentationBase { + public static readonly COMPONENT = '@nestjs/core'; + public static readonly COMMON_ATTRIBUTES = { + component: SentryNestErrorInstrumentation.COMPONENT, + }; + + public constructor(config: InstrumentationConfig = {}) { + super('sentry-nestjs-error', SDK_VERSION, config); + } + + /** + * + */ + public init(): InstrumentationNodeModuleDefinition { + const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestErrorInstrumentation.COMPONENT, supportedVersions); + + moduleDef.files.push(this._getBaseExceptionFilterFileInstrumentation(supportedVersions)); + + return moduleDef; + } + + /** + * + */ + private _getBaseExceptionFilterFileInstrumentation(versions: string[]): InstrumentationNodeModuleFile { + return new InstrumentationNodeModuleFile( + '@nestjs/core/exceptions/base-exception-filter.js', + versions, + (moduleExports: { BaseExceptionFilter: BaseExceptionFilter }) => { + console.log('exports:'); + console.log(moduleExports); + console.log('prototype: '); + console.log(moduleExports.BaseExceptionFilter.prototype); + console.log('catch: '); + console.log(moduleExports.BaseExceptionFilter.prototype.catch); + + + }, + (BaseExceptionFilterClass: any) => { + console.log('unpatch!'); + } + ); + } +} diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index 42aa0b003315..4a38f437b81c 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -64,6 +64,12 @@ export interface CatchTarget { sentryPatched?: boolean; __SENTRY_INTERNAL__?: boolean; prototype: { - catch?: (...args: any[]) => any; + catch?: (exception: unknown, host: unknown, ...args: any[]) => any; }; } + +export interface BaseExceptionFilter { + prototype: { + catch(exception: unknown, host: unknown): void; + } +} From d4ab597ba504968745310946f2445abc0902a903 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Fri, 9 Aug 2024 14:42:55 +0200 Subject: [PATCH 07/36] Basic patch for base exception filter seems to work (we can log something) --- .../nest/sentry-nest-error-instrumentation.ts | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts index 842c5ee7a67e..8e92c1317f2a 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts @@ -6,6 +6,7 @@ import { } from '@opentelemetry/instrumentation'; import { SDK_VERSION } from '@sentry/utils'; import type {BaseExceptionFilter} from './types'; +import {isWrapped} from "@opentelemetry/core"; const supportedVersions = ['>=8.0.0 <11']; @@ -48,11 +49,29 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { console.log('catch: '); console.log(moduleExports.BaseExceptionFilter.prototype.catch); - + if (isWrapped(moduleExports.BaseExceptionFilter.prototype)) { + this._unwrap(moduleExports.BaseExceptionFilter.prototype, 'catch'); + } + console.log('wrap'); + this._wrap(moduleExports.BaseExceptionFilter.prototype, 'catch', this._createWrapCatch()); + return moduleExports; }, - (BaseExceptionFilterClass: any) => { - console.log('unpatch!'); + (moduleExports: { BaseExceptionFilter: BaseExceptionFilter }) => { + this._unwrap(moduleExports.BaseExceptionFilter.prototype, 'catch'); } ); } + + /** + * + */ + private _createWrapCatch() { + console.log('in wrap'); + return function wrapCatch(originalCatch: Function) { + return function wrappedCatch(exception: unknown, host: unknown) { + console.log('patching the base exception filter!'); + return originalCatch.apply(exception, host); + } + } + } } From c9b690fa56a54f7c9aea709a7399321cc9fbacff Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Fri, 9 Aug 2024 15:11:42 +0200 Subject: [PATCH 08/36] This works! (I think) --- packages/nestjs/src/setup.ts | 30 ------------------- .../src/integrations/tracing/nest/nest.ts | 29 ------------------ .../nest/sentry-nest-error-instrumentation.ts | 17 +++++++++-- 3 files changed, 14 insertions(+), 62 deletions(-) diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 5e76f5cbe912..4a701b6a1ff2 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -60,28 +60,6 @@ class SentryTracingInterceptor implements NestInterceptor { Injectable()(SentryTracingInterceptor); export { SentryTracingInterceptor }; -/** - * Global filter to handle exceptions and report them to Sentry. - */ -class SentryGlobalFilter extends BaseExceptionFilter { - public static readonly __SENTRY_INTERNAL__ = true; - - /** - * Catches exceptions and reports them to Sentry unless they are expected errors. - */ - public catch(exception: unknown, host: ArgumentsHost): void { - // don't report expected errors - if (exception instanceof HttpException || exception instanceof RpcException) { - return super.catch(exception, host); - } - - captureException(exception); - return super.catch(exception, host); - } -} -Catch()(SentryGlobalFilter); -export { SentryGlobalFilter }; - /** * Service to set up Sentry performance tracing for Nest.js applications. */ @@ -118,10 +96,6 @@ class SentryModule { module: SentryModule, providers: [ SentryService, - { - provide: APP_FILTER, - useClass: SentryGlobalFilter, - }, { provide: APP_INTERCEPTOR, useClass: SentryTracingInterceptor, @@ -135,10 +109,6 @@ Global()(SentryModule); Module({ providers: [ SentryService, - { - provide: APP_FILTER, - useClass: SentryGlobalFilter, - }, { provide: APP_INTERCEPTOR, useClass: SentryTracingInterceptor, diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index 73a1b6899650..348773915e33 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -86,35 +86,6 @@ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsE return next.handle(); }, }); - - const wrappedFilter = new Proxy(baseFilter, { - get(target, prop, receiver) { - if (prop === 'catch') { - const originalCatch = Reflect.get(target, prop, receiver); - - return (exception: unknown, host: unknown) => { - const exceptionIsObject = typeof exception === 'object' && exception !== null; - const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; - const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; - - /* - Don't report expected NestJS control flow errors - - `HttpException` errors will have a `status` property - - `RpcException` errors will have an `error` property - */ - if (exceptionStatusCode !== null || exceptionErrorProperty !== null) { - return originalCatch.apply(target, [exception, host]); - } - - captureException(exception); - return originalCatch.apply(target, [exception, host]); - }; - } - return Reflect.get(target, prop, receiver); - }, - }); - - app.useGlobalFilters(wrappedFilter); } function addNestSpanAttributes(span: Span): void { diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts index 8e92c1317f2a..27dfb3656939 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts @@ -67,10 +67,21 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { */ private _createWrapCatch() { console.log('in wrap'); - return function wrapCatch(originalCatch: Function) { - return function wrappedCatch(exception: unknown, host: unknown) { + return function wrapCatch(originalCatch: (exception: unknown, host: unknown) => void) { + return function wrappedCatch(this: any, exception: unknown, host: unknown) { console.log('patching the base exception filter!'); - return originalCatch.apply(exception, host); + + console.log(exception); + return originalCatch.apply(this, [exception, host]); + /* + return new Proxy(originalCatch, { + apply: (originalCatch, thisArgCatch, argsCatch) => { + console.log('patching the base exception filter!'); + return originalCatch.apply(thisArgCatch, argsCatch); + } + }) + + */ } } } From e73698ab4feb45aef350810c3c2ed139fe1342f4 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Fri, 9 Aug 2024 15:23:06 +0200 Subject: [PATCH 09/36] Adding correct logic to patch --- .../nest/sentry-nest-error-instrumentation.ts | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts index 27dfb3656939..2937fa293987 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts @@ -6,7 +6,8 @@ import { } from '@opentelemetry/instrumentation'; import { SDK_VERSION } from '@sentry/utils'; import type {BaseExceptionFilter} from './types'; -import {isWrapped} from "@opentelemetry/core"; +import {isWrapped} from '@opentelemetry/core'; +import {captureException} from '@sentry/core'; const supportedVersions = ['>=8.0.0 <11']; @@ -42,17 +43,9 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { '@nestjs/core/exceptions/base-exception-filter.js', versions, (moduleExports: { BaseExceptionFilter: BaseExceptionFilter }) => { - console.log('exports:'); - console.log(moduleExports); - console.log('prototype: '); - console.log(moduleExports.BaseExceptionFilter.prototype); - console.log('catch: '); - console.log(moduleExports.BaseExceptionFilter.prototype.catch); - if (isWrapped(moduleExports.BaseExceptionFilter.prototype)) { this._unwrap(moduleExports.BaseExceptionFilter.prototype, 'catch'); } - console.log('wrap'); this._wrap(moduleExports.BaseExceptionFilter.prototype, 'catch', this._createWrapCatch()); return moduleExports; }, @@ -66,22 +59,27 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { * */ private _createWrapCatch() { - console.log('in wrap'); return function wrapCatch(originalCatch: (exception: unknown, host: unknown) => void) { - return function wrappedCatch(this: any, exception: unknown, host: unknown) { + return function wrappedCatch(this: BaseExceptionFilter, exception: unknown, host: unknown) { console.log('patching the base exception filter!'); console.log(exception); - return originalCatch.apply(this, [exception, host]); - /* - return new Proxy(originalCatch, { - apply: (originalCatch, thisArgCatch, argsCatch) => { - console.log('patching the base exception filter!'); - return originalCatch.apply(thisArgCatch, argsCatch); - } - }) + const exceptionIsObject = typeof exception === 'object' && exception !== null; + const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; + const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; + /* + Don't report expected NestJS control flow errors + - `HttpException` errors will have a `status` property + - `RpcException` errors will have an `error` property */ + if (exceptionStatusCode !== null || exceptionErrorProperty !== null) { + return originalCatch.apply(this, [exception, host]); + } + + captureException(exception); + + return originalCatch.apply(this, [exception, host]); } } } From f71297e3ff8fce78af0f99bdfc7a639374af4db0 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Fri, 9 Aug 2024 15:32:26 +0200 Subject: [PATCH 10/36] Cleanup --- packages/nestjs/src/setup.ts | 15 ++------------ .../src/integrations/tracing/nest/nest.ts | 9 ++++++--- .../nest/sentry-nest-error-instrumentation.ts | 20 +++++++++---------- .../src/integrations/tracing/nest/types.ts | 2 +- 4 files changed, 19 insertions(+), 27 deletions(-) diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 4a701b6a1ff2..e539820f6aac 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -1,21 +1,10 @@ -import type { - ArgumentsHost, - CallHandler, - DynamicModule, - ExecutionContext, - NestInterceptor, - OnModuleInit, -} from '@nestjs/common'; -import { HttpException } from '@nestjs/common'; -import { Catch } from '@nestjs/common'; +import type { CallHandler, DynamicModule, ExecutionContext, NestInterceptor, OnModuleInit } from '@nestjs/common'; import { Injectable } from '@nestjs/common'; import { Global, Module } from '@nestjs/common'; -import { APP_FILTER, APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; -import { RpcException } from '@nestjs/microservices'; +import { APP_INTERCEPTOR } from '@nestjs/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - captureException, getClient, getDefaultIsolationScope, getIsolationScope, diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index 348773915e33..b1cbf1a50236 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -2,7 +2,6 @@ import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core' import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - captureException, defineIntegration, getClient, getDefaultIsolationScope, @@ -12,9 +11,9 @@ import { import type { IntegrationFn, Span } from '@sentry/types'; import { logger } from '@sentry/utils'; import { generateInstrumentOnce } from '../../../otel/instrument'; +import { SentryNestErrorInstrumentation } from './sentry-nest-error-instrumentation'; import { SentryNestInstrumentation } from './sentry-nest-instrumentation'; import type { MinimalNestJsApp, NestJsErrorFilter } from './types'; -import {SentryNestErrorInstrumentation} from "./sentry-nest-error-instrumentation"; const INTEGRATION_NAME = 'Nest'; @@ -28,7 +27,7 @@ const instrumentNestCommon = generateInstrumentOnce('Nest-Common', () => { const instrumentNestCoreErrors = generateInstrumentOnce('Nest-Core-Errors', () => { return new SentryNestErrorInstrumentation(); -}) +}); export const instrumentNest = Object.assign( (): void => { @@ -59,6 +58,10 @@ export const nestIntegration = defineIntegration(_nestIntegration); * Setup an error handler for Nest. */ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsErrorFilter): void { + if (baseFilter) { + logger.warn('The base filter does not do anything anymore!'); // TODO: better message + } + // Sadly, NestInstrumentation has no requestHook, so we need to add the attributes here // We register this hook in this method, because if we register it in the integration `setup`, // it would always run even for users that are not even using Nest.js diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts index 2937fa293987..93021af0921f 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts @@ -1,13 +1,13 @@ +import { isWrapped } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition, InstrumentationNodeModuleFile, } from '@opentelemetry/instrumentation'; +import { captureException } from '@sentry/core'; import { SDK_VERSION } from '@sentry/utils'; -import type {BaseExceptionFilter} from './types'; -import {isWrapped} from '@opentelemetry/core'; -import {captureException} from '@sentry/core'; +import type { BaseExceptionFilter } from './types'; const supportedVersions = ['>=8.0.0 <11']; @@ -28,7 +28,10 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { * */ public init(): InstrumentationNodeModuleDefinition { - const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestErrorInstrumentation.COMPONENT, supportedVersions); + const moduleDef = new InstrumentationNodeModuleDefinition( + SentryNestErrorInstrumentation.COMPONENT, + supportedVersions, + ); moduleDef.files.push(this._getBaseExceptionFilterFileInstrumentation(supportedVersions)); @@ -51,7 +54,7 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { }, (moduleExports: { BaseExceptionFilter: BaseExceptionFilter }) => { this._unwrap(moduleExports.BaseExceptionFilter.prototype, 'catch'); - } + }, ); } @@ -61,9 +64,6 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { private _createWrapCatch() { return function wrapCatch(originalCatch: (exception: unknown, host: unknown) => void) { return function wrappedCatch(this: BaseExceptionFilter, exception: unknown, host: unknown) { - console.log('patching the base exception filter!'); - - console.log(exception); const exceptionIsObject = typeof exception === 'object' && exception !== null; const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; @@ -80,7 +80,7 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { captureException(exception); return originalCatch.apply(this, [exception, host]); - } - } + }; + }; } } diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index 4a38f437b81c..26d6598ce373 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -71,5 +71,5 @@ export interface CatchTarget { export interface BaseExceptionFilter { prototype: { catch(exception: unknown, host: unknown): void; - } + }; } From 1c87a97e66f4793123ffb42611d9577b1616c0e6 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 09:44:35 +0200 Subject: [PATCH 11/36] Fix tests to reflect new working state --- .../nestjs-basic/src/example-global.filter.ts | 1 - .../nestjs-basic/src/example-local.filter.ts | 1 - .../nestjs-basic/tests/errors.test.ts | 24 ++++++------ .../tests/errors.test.ts | 38 +++++++++---------- 4 files changed, 28 insertions(+), 36 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts index cf280876d605..988696d0e13d 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts @@ -8,7 +8,6 @@ export class ExampleGlobalFilter implements ExceptionFilter { const ctx = host.switchToHttp(); const response = ctx.getResponse(); const request = ctx.getRequest(); - console.log('Example global exception filter!'); response.status(400).json({ statusCode: 400, diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts index 07ee8c5463a3..505217f5dcbd 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-local.filter.ts @@ -8,7 +8,6 @@ export class ExampleLocalFilter implements ExceptionFilter { const ctx = host.switchToHttp(); const response = ctx.getResponse(); const request = ctx.getRequest(); - console.log('Example local exception filter!'); response.status(400).json({ statusCode: 400, diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts index 74e9b9b9d4be..5a3d107faf70 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts @@ -99,26 +99,24 @@ test('Global exception filter registered in main module is applied', async ({ ba const response = await fetch(`${baseURL}/example-exception-global-filter`); const responseBody = await response.json(); - console.log(responseBody); - - // this should fail but doesn't because the exception filter is not being properly applied - expect(response.status).toBe(500); + expect(response.status).toBe(400); expect(responseBody).toEqual({ - message: 'Internal server error', - statusCode: 500, + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-global-filter', + message: 'Example exception was handled by global filter!', }); }); test('Local exception filter registered in main module is applied', async ({ baseURL }) => { - const response = await fetch(`${baseURL}/example-exception-global-filter`); + const response = await fetch(`${baseURL}/example-exception-local-filter`); const responseBody = await response.json(); - console.log(responseBody); - - // this should fail but doesn't because the exception filter is not being properly applied - expect(response.status).toBe(500); + expect(response.status).toBe(400); expect(responseBody).toEqual({ - message: 'Internal server error', - statusCode: 500, + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-local-filter', + message: 'Example exception was handled by local filter!', }); }); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts index 6fbc9f2c1f32..f406014b69be 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts @@ -116,33 +116,29 @@ test('Does not send exception to Sentry if user-defined local exception filter a expect(errorEventOccurred).toBe(false); }); -test('Does not handle expected exception if exception is thrown in module registered before Sentry', async ({ +test('Does not send expected exception to Sentry if exception is thrown in module registered before Sentry', async ({ baseURL, }) => { - const errorEventPromise = waitForError('nestjs-with-submodules', event => { - return !event.type && event.exception?.values?.[0]?.value === 'Something went wrong in the example module!'; - }); - - const response = await fetch(`${baseURL}/example-module-wrong-order/expected-exception`); - expect(response.status).toBe(500); // should be 400 + let errorEventOccurred = false; - // should never arrive, but does because the exception is not handled properly - const errorEvent = await errorEventPromise; + waitForError('nestjs-with-submodules', event => { + if (!event.type && event.exception?.values?.[0].value === 'Something went wrong in the example module!') { + errorEventOccurred = true; + } - expect(errorEvent.exception?.values).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.value).toBe('Something went wrong in the example module!'); + return event?.transaction === 'GET /example-module-wrong-order/expected-exception'; + }); - expect(errorEvent.request).toEqual({ - method: 'GET', - cookies: {}, - headers: expect.any(Object), - url: 'http://localhost:3030/example-module-wrong-order/expected-exception', + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-module-wrong-order/expected-exception'; }); - expect(errorEvent.transaction).toEqual('GET /example-module-wrong-order/expected-exception'); + const response = await fetch(`${baseURL}/example-module-wrong-order/expected-exception`); + expect(response.status).toBe(400); - expect(errorEvent.contexts?.trace).toEqual({ - trace_id: expect.any(String), - span_id: expect.any(String), - }); + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); }); From 83998d2e031120cbbd9e1a5264893bc375f844fe Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 09:50:59 +0200 Subject: [PATCH 12/36] Remove nest microservice dependency --- packages/nestjs/package.json | 6 ++---- yarn.lock | 8 -------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index f84533498e8e..b336101c5530 100644 --- a/packages/nestjs/package.json +++ b/packages/nestjs/package.json @@ -51,13 +51,11 @@ }, "devDependencies": { "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0", - "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0", - "@nestjs/microservices": "^8.0.0 || ^9.0.0 || ^10.0.0" + "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0" }, "peerDependencies": { "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0", - "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0", - "@nestjs/microservices": "^8.0.0 || ^9.0.0 || ^10.0.0" + "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0" }, "scripts": { "build": "run-p build:transpile build:types", diff --git a/yarn.lock b/yarn.lock index 660c53cce594..953f0eee5f3a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6135,14 +6135,6 @@ path-to-regexp "3.2.0" tslib "2.6.3" -"@nestjs/microservices@^8.0.0 || ^9.0.0 || ^10.0.0": - version "10.3.10" - resolved "https://registry.yarnpkg.com/@nestjs/microservices/-/microservices-10.3.10.tgz#e00957e0c22b0cc8b041242a40538e2d862255fb" - integrity sha512-zZrilhZmXU2Ik5Usrcy4qEX262Uhvz0/9XlIdX6SRn8I39ns1EE9tAhEBmmkMwh7lsEikRFa4aaa05loi8Gsow== - dependencies: - iterare "1.2.1" - tslib "2.6.3" - "@nestjs/platform-express@^10.3.3": version "10.3.3" resolved "https://registry.yarnpkg.com/@nestjs/platform-express/-/platform-express-10.3.3.tgz#c1484d30d1e7666c4c8d0d7cde31cfc0b9d166d7" From 5be44402606ab86cd2436f8b26c5d8d9ab92a500 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 10:51:13 +0200 Subject: [PATCH 13/36] Add docstrings and improve deprecation message --- packages/node/src/integrations/tracing/nest/nest.ts | 2 +- .../tracing/nest/sentry-nest-error-instrumentation.ts | 9 +++++++-- .../tracing/nest/sentry-nest-instrumentation.ts | 4 +++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index b1cbf1a50236..b9a83aa2a82c 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -59,7 +59,7 @@ export const nestIntegration = defineIntegration(_nestIntegration); */ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsErrorFilter): void { if (baseFilter) { - logger.warn('The base filter does not do anything anymore!'); // TODO: better message + logger.warn('baseFilter is deprecated and no longer needed.'); } // Sadly, NestInstrumentation has no requestHook, so we need to add the attributes here diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts index 93021af0921f..3406705d7038 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts @@ -12,7 +12,9 @@ import type { BaseExceptionFilter } from './types'; const supportedVersions = ['>=8.0.0 <11']; /** + * Custom error instrumentation for nestjs. * + * This patches the `BaseExceptionFilter` to report unexpected exceptions to Sentry. */ export class SentryNestErrorInstrumentation extends InstrumentationBase { public static readonly COMPONENT = '@nestjs/core'; @@ -25,7 +27,7 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { } /** - * + * Initializes the instrumentation by defining the modules to be patched. */ public init(): InstrumentationNodeModuleDefinition { const moduleDef = new InstrumentationNodeModuleDefinition( @@ -39,7 +41,7 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { } /** - * + * Wraps the `BaseExceptionFilter`. */ private _getBaseExceptionFilterFileInstrumentation(versions: string[]): InstrumentationNodeModuleFile { return new InstrumentationNodeModuleFile( @@ -59,7 +61,10 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { } /** + * Creates a wrapper function for the `BaseExceptionFilter`. * + * Reports unexpected exceptions to Sentry and then calls the original `BaseExceptionFilter`. + * Expected NestJS control flow errors get filtered. */ private _createWrapCatch() { return function wrapCatch(originalCatch: (exception: unknown, host: unknown) => void) { diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts index 28d5a74ef63d..a8d02e5cbe69 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -16,7 +16,9 @@ const supportedVersions = ['>=8.0.0 <11']; /** * Custom instrumentation for nestjs. * - * This hooks into the @Injectable decorator, which is applied on class middleware, interceptors and guards. + * This hooks into + * 1. @Injectable decorator, which is applied on class middleware, interceptors and guards. + * 2. @Catch decorator, which is applied on exception filters. */ export class SentryNestInstrumentation extends InstrumentationBase { public static readonly COMPONENT = '@nestjs/common'; From daf1156fb15f11fb25492774992bc8720aa12084 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 10:55:52 +0200 Subject: [PATCH 14/36] Pass all arguments --- .../tracing/nest/sentry-nest-error-instrumentation.ts | 10 ++++++---- packages/node/src/integrations/tracing/nest/types.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts index 3406705d7038..415e7497ee09 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts @@ -67,8 +67,10 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { * Expected NestJS control flow errors get filtered. */ private _createWrapCatch() { - return function wrapCatch(originalCatch: (exception: unknown, host: unknown) => void) { - return function wrappedCatch(this: BaseExceptionFilter, exception: unknown, host: unknown) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function wrapCatch(originalCatch: (exception: unknown, host: unknown, ...args: any[]) => void) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function wrappedCatch(this: BaseExceptionFilter, exception: unknown, host: unknown, ...args: any[]) { const exceptionIsObject = typeof exception === 'object' && exception !== null; const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; @@ -79,12 +81,12 @@ export class SentryNestErrorInstrumentation extends InstrumentationBase { - `RpcException` errors will have an `error` property */ if (exceptionStatusCode !== null || exceptionErrorProperty !== null) { - return originalCatch.apply(this, [exception, host]); + return originalCatch.apply(this, [exception, host, ...args]); } captureException(exception); - return originalCatch.apply(this, [exception, host]); + return originalCatch.apply(this, [exception, host, ...args]); }; }; } diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index 26d6598ce373..fecd5d50b989 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -70,6 +70,6 @@ export interface CatchTarget { export interface BaseExceptionFilter { prototype: { - catch(exception: unknown, host: unknown): void; + catch(exception: unknown, host: unknown, ...args: any[]): void; }; } From 468fb3013b6080c0bf0bd16a252bffc1dacb9802 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 11:22:49 +0200 Subject: [PATCH 15/36] Add test for instrumentation in module registered before Sentry --- .../nestjs-with-submodules/src/app.module.ts | 4 +- .../example.controller.ts | 6 +-- .../example.exception.ts | 2 +- .../example.filter.ts | 8 ++-- .../example.module.ts | 6 +-- .../tests/errors.test.ts | 12 +++--- .../tests/transactions.test.ts | 41 ++++++++++++++++++- 7 files changed, 58 insertions(+), 21 deletions(-) rename dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/{example-module-global-filter-wrong-registration-order => example-module-global-filter-registered-first}/example.controller.ts (63%) rename dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/{example-module-global-filter-wrong-registration-order => example-module-global-filter-registered-first}/example.exception.ts (54%) rename dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/{example-module-global-filter-wrong-registration-order => example-module-global-filter-registered-first}/example.filter.ts (52%) rename dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/{example-module-global-filter-wrong-registration-order => example-module-global-filter-registered-first}/example.module.ts (56%) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts index 212b17a3556b..2a4d38d3ca73 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts @@ -2,13 +2,13 @@ import { Module } from '@nestjs/common'; import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; -import { ExampleModuleGlobalFilterWrongRegistrationOrder } from './example-module-global-filter-wrong-registration-order/example.module'; +import { ExampleModuleGlobalFilterRegisteredFirst } from './example-module-global-filter-registered-first/example.module'; import { ExampleModuleGlobalFilter } from './example-module-global-filter/example.module'; import { ExampleModuleLocalFilter } from './example-module-local-filter/example.module'; @Module({ imports: [ - ExampleModuleGlobalFilterWrongRegistrationOrder, + ExampleModuleGlobalFilterRegisteredFirst, SentryModule.forRoot(), ExampleModuleGlobalFilter, ExampleModuleLocalFilter, diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.controller.ts similarity index 63% rename from dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.controller.ts rename to dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.controller.ts index 028af4a43f87..967886948a25 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.controller.ts @@ -1,13 +1,13 @@ import { Controller, Get } from '@nestjs/common'; -import { ExampleExceptionWrongRegistrationOrder } from './example.exception'; +import { ExampleExceptionRegisteredFirst } from './example.exception'; -@Controller('example-module-wrong-order') +@Controller('example-module-registered-first') export class ExampleController { constructor() {} @Get('/expected-exception') getCaughtException(): string { - throw new ExampleExceptionWrongRegistrationOrder(); + throw new ExampleExceptionRegisteredFirst(); } @Get('/unexpected-exception') diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.exception.ts similarity index 54% rename from dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.exception.ts rename to dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.exception.ts index 0e4f58314fa2..9bb3b5948343 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.exception.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.exception.ts @@ -1,4 +1,4 @@ -export class ExampleExceptionWrongRegistrationOrder extends Error { +export class ExampleExceptionRegisteredFirst extends Error { constructor() { super('Something went wrong in the example module!'); } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.filter.ts similarity index 52% rename from dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.filter.ts rename to dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.filter.ts index 6ecdf88937aa..6c3b81d395b5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.filter.ts @@ -1,11 +1,11 @@ import { ArgumentsHost, BadRequestException, Catch } from '@nestjs/common'; import { BaseExceptionFilter } from '@nestjs/core'; -import { ExampleExceptionWrongRegistrationOrder } from './example.exception'; +import { ExampleExceptionRegisteredFirst } from './example.exception'; -@Catch(ExampleExceptionWrongRegistrationOrder) -export class ExampleExceptionFilterWrongRegistrationOrder extends BaseExceptionFilter { +@Catch(ExampleExceptionRegisteredFirst) +export class ExampleExceptionFilterRegisteredFirst extends BaseExceptionFilter { catch(exception: unknown, host: ArgumentsHost) { - if (exception instanceof ExampleExceptionWrongRegistrationOrder) { + if (exception instanceof ExampleExceptionRegisteredFirst) { return super.catch(new BadRequestException(exception.message), host); } return super.catch(exception, host); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.module.ts similarity index 56% rename from dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.module.ts rename to dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.module.ts index c98a5757b01c..8751856f99cd 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order/example.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.module.ts @@ -1,7 +1,7 @@ import { Module } from '@nestjs/common'; import { APP_FILTER } from '@nestjs/core'; import { ExampleController } from './example.controller'; -import { ExampleExceptionFilterWrongRegistrationOrder } from './example.filter'; +import { ExampleExceptionFilterRegisteredFirst } from './example.filter'; @Module({ imports: [], @@ -9,8 +9,8 @@ import { ExampleExceptionFilterWrongRegistrationOrder } from './example.filter'; providers: [ { provide: APP_FILTER, - useClass: ExampleExceptionFilterWrongRegistrationOrder, + useClass: ExampleExceptionFilterRegisteredFirst, }, ], }) -export class ExampleModuleGlobalFilterWrongRegistrationOrder {} +export class ExampleModuleGlobalFilterRegisteredFirst {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts index f406014b69be..8a5235d2735b 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts @@ -36,7 +36,7 @@ test('Sends unexpected exception to Sentry if thrown in module that was register return !event.type && event.exception?.values?.[0]?.value === 'This is an uncaught exception!'; }); - const response = await fetch(`${baseURL}/example-module-wrong-order/unexpected-exception`); + const response = await fetch(`${baseURL}/example-module-registered-first/unexpected-exception`); expect(response.status).toBe(500); const errorEvent = await errorEventPromise; @@ -48,10 +48,10 @@ test('Sends unexpected exception to Sentry if thrown in module that was register method: 'GET', cookies: {}, headers: expect.any(Object), - url: 'http://localhost:3030/example-module-wrong-order/unexpected-exception', + url: 'http://localhost:3030/example-module-registered-first/unexpected-exception', }); - expect(errorEvent.transaction).toEqual('GET /example-module-wrong-order/unexpected-exception'); + expect(errorEvent.transaction).toEqual('GET /example-module-registered-first/unexpected-exception'); expect(errorEvent.contexts?.trace).toEqual({ trace_id: expect.any(String), @@ -126,14 +126,14 @@ test('Does not send expected exception to Sentry if exception is thrown in modul errorEventOccurred = true; } - return event?.transaction === 'GET /example-module-wrong-order/expected-exception'; + return event?.transaction === 'GET /example-module-registered-first/expected-exception'; }); const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { - return transactionEvent?.transaction === 'GET /example-module-wrong-order/expected-exception'; + return transactionEvent?.transaction === 'GET /example-module-registered-first/expected-exception'; }); - const response = await fetch(`${baseURL}/example-module-wrong-order/expected-exception`); + const response = await fetch(`${baseURL}/example-module-registered-first/expected-exception`); expect(response.status).toBe(400); await transactionEventPromise; diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts index 9217249faad0..101d13646466 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts @@ -122,7 +122,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { ); }); -test('API route transaction includes exception filter span for global filter', async ({ baseURL }) => { +test('API route transaction includes exception filter span for global filter in module registered after Sentry', async ({ baseURL }) => { const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && @@ -159,7 +159,7 @@ test('API route transaction includes exception filter span for global filter', a ); }); -test('API route transaction includes exception filter span for local filter', async ({ baseURL }) => { +test('API route transaction includes exception filter span for local filter in module registered after Sentry', async ({ baseURL }) => { const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && @@ -195,3 +195,40 @@ test('API route transaction includes exception filter span for local filter', as }), ); }); + +test('API route transaction includes exception filter span for global filter in module registered before Sentry', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module-registered-first/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module-registered-first/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module-registered-first/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'ExampleExceptionFilterRegisteredFirst', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); +}); From 1f47da1d9a3d121af1bc0753281f0c5cd2e69af6 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 11:28:22 +0200 Subject: [PATCH 16/36] Lint2 --- .../tests/transactions.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts index 101d13646466..a2ea1db9c506 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts @@ -122,7 +122,9 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { ); }); -test('API route transaction includes exception filter span for global filter in module registered after Sentry', async ({ baseURL }) => { +test('API route transaction includes exception filter span for global filter in module registered after Sentry', async ({ + baseURL, +}) => { const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && @@ -159,7 +161,9 @@ test('API route transaction includes exception filter span for global filter in ); }); -test('API route transaction includes exception filter span for local filter in module registered after Sentry', async ({ baseURL }) => { +test('API route transaction includes exception filter span for local filter in module registered after Sentry', async ({ + baseURL, +}) => { const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && @@ -196,7 +200,9 @@ test('API route transaction includes exception filter span for local filter in m ); }); -test('API route transaction includes exception filter span for global filter in module registered before Sentry', async ({ baseURL }) => { +test('API route transaction includes exception filter span for global filter in module registered before Sentry', async ({ + baseURL, +}) => { const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && From 75058b40e6badacedc5ea9424d05815f93d9911d Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 11:32:41 +0200 Subject: [PATCH 17/36] Remove note about registration order from readme --- packages/nestjs/README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/nestjs/README.md b/packages/nestjs/README.md index 9e0192551afc..17f3926789ee 100644 --- a/packages/nestjs/README.md +++ b/packages/nestjs/README.md @@ -74,8 +74,6 @@ import { AppService } from './app.service'; export class AppModule {} ``` -The `SentryModule` needs to be registered before any module that should be instrumented by Sentry. - ## SentryTraced Use the `@SentryTraced()` decorator to gain additional performance insights for any function within your NestJS From 20891b39c4c8d93a332d3091f5483f6578e9074b Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 12:45:17 +0200 Subject: [PATCH 18/36] Add tests to node sdk --- .../node-nestjs-basic/src/app.controller.ts | 16 +++++++++++- .../node-nestjs-basic/src/app.module.ts | 10 ++++++- .../src/example-global-filter.exception.ts | 5 ++++ .../src/example-global.filter.ts | 19 ++++++++++++++ .../src/example-local-filter.exception.ts | 5 ++++ .../src/example-local.filter.ts | 19 ++++++++++++++ .../node-nestjs-basic/tests/errors.test.ts | 26 +++++++++++++++++++ 7 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts index ec0a921da2c4..9cda3c96f9a6 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts @@ -1,10 +1,14 @@ -import { Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors } from '@nestjs/common'; +import { Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; +import { ExampleExceptionGlobalFilter } from './example-global-filter.exception'; +import { ExampleExceptionLocalFilter } from './example-local-filter.exception'; +import { ExampleLocalFilter } from './example-local.filter'; import { ExampleGuard } from './example.guard'; import { ExampleInterceptor } from './example.interceptor'; @Controller() +@UseFilters(ExampleLocalFilter) export class AppController { constructor(private readonly appService: AppService) {} @@ -74,4 +78,14 @@ export class AppController { async flush() { await flush(); } + + @Get('example-exception-global-filter') + async exampleExceptionGlobalFilter() { + throw new ExampleExceptionGlobalFilter(); + } + + @Get('example-exception-local-filter') + async exampleExceptionLocalFilter() { + throw new ExampleExceptionLocalFilter(); + } } diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts index 567dbefeadb7..8b2586416e83 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts @@ -1,13 +1,21 @@ import { MiddlewareConsumer, Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; import { ScheduleModule } from '@nestjs/schedule'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { ExampleGlobalFilter } from './example-global.filter'; import { ExampleMiddleware } from './example.middleware'; @Module({ imports: [ScheduleModule.forRoot()], controllers: [AppController], - providers: [AppService], + providers: [ + AppService, + { + provide: APP_FILTER, + useClass: ExampleGlobalFilter, + }, + ], }) export class AppModule { configure(consumer: MiddlewareConsumer): void { diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts new file mode 100644 index 000000000000..41981ba748fe --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionGlobalFilter extends Error { + constructor() { + super('Original global example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts new file mode 100644 index 000000000000..988696d0e13d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts @@ -0,0 +1,19 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionGlobalFilter } from './example-global-filter.exception'; + +@Catch(ExampleExceptionGlobalFilter) +export class ExampleGlobalFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + + response.status(400).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by global filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts new file mode 100644 index 000000000000..8f76520a3b94 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionLocalFilter extends Error { + constructor() { + super('Original local example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts new file mode 100644 index 000000000000..505217f5dcbd --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts @@ -0,0 +1,19 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionLocalFilter } from './example-local-filter.exception'; + +@Catch(ExampleExceptionLocalFilter) +export class ExampleLocalFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + + response.status(400).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by local filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts index 0155c3887805..4dd06a344136 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts @@ -94,3 +94,29 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { expect(errorEventOccurred).toBe(false); }); + +test('Global exception filter registered in main module is applied', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/example-exception-global-filter`); + const responseBody = await response.json(); + + expect(response.status).toBe(400); + expect(responseBody).toEqual({ + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-global-filter', + message: 'Example exception was handled by global filter!', + }); +}); + +test('Local exception filter registered in main module is applied', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/example-exception-local-filter`); + const responseBody = await response.json(); + + expect(response.status).toBe(400); + expect(responseBody).toEqual({ + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-local-filter', + message: 'Example exception was handled by local filter!', + }); +}); From 6d1a150a0c6864c630a76c8245b784420ac868e4 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 12:51:37 +0200 Subject: [PATCH 19/36] Rename error isntrumentation --- packages/node/src/integrations/tracing/nest/nest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index b9a83aa2a82c..002a8bb939ed 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -25,7 +25,7 @@ const instrumentNestCommon = generateInstrumentOnce('Nest-Common', () => { return new SentryNestInstrumentation(); }); -const instrumentNestCoreErrors = generateInstrumentOnce('Nest-Core-Errors', () => { +const instrumentNestErrors = generateInstrumentOnce('Nest-Errors', () => { return new SentryNestErrorInstrumentation(); }); @@ -33,7 +33,7 @@ export const instrumentNest = Object.assign( (): void => { instrumentNestCore(); instrumentNestCommon(); - instrumentNestCoreErrors(); + instrumentNestErrors(); }, { id: INTEGRATION_NAME }, ); From ddf23b288439392b9ca1debd92895c6774288876 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 13:01:13 +0200 Subject: [PATCH 20/36] Default base filter to undefined --- packages/node/src/integrations/tracing/nest/nest.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index 002a8bb939ed..593745bd179f 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -57,8 +57,12 @@ export const nestIntegration = defineIntegration(_nestIntegration); /** * Setup an error handler for Nest. */ -export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsErrorFilter): void { +export function setupNestErrorHandler( + app: MinimalNestJsApp, + baseFilter: NestJsErrorFilter | undefined = undefined, +): void { if (baseFilter) { + // No need to pass a base filter to setup the error handler anymore, since we always patch the `BaseExceptionFilter`. logger.warn('baseFilter is deprecated and no longer needed.'); } From 2b5d119ae12f1c160760d162f89259f639c8faa4 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 13:06:27 +0200 Subject: [PATCH 21/36] Add comment --- packages/node/src/integrations/tracing/nest/nest.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index 593745bd179f..9a8d000e0a13 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -55,7 +55,10 @@ const _nestIntegration = (() => { export const nestIntegration = defineIntegration(_nestIntegration); /** - * Setup an error handler for Nest. + * Set span attributes for the nest instrumentation. + * + * Previously this method was used to setup an error handler reporting to Sentry, but this is no longer the case. + * This method should be renamed in the next major. */ export function setupNestErrorHandler( app: MinimalNestJsApp, From 8fa920aae409dc1a2de5cdfba347a9953f4bd2c1 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 12 Aug 2024 17:47:05 +0200 Subject: [PATCH 22/36] Revert some stuff --- packages/nestjs/src/setup.ts | 37 +++++++- .../src/integrations/tracing/nest/nest.ts | 51 ++++++---- .../nest/sentry-nest-error-instrumentation.ts | 93 ------------------- .../src/integrations/tracing/nest/types.ts | 8 +- 4 files changed, 66 insertions(+), 123 deletions(-) delete mode 100644 packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index e539820f6aac..11a279521c3f 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -1,10 +1,17 @@ -import type { CallHandler, DynamicModule, ExecutionContext, NestInterceptor, OnModuleInit } from '@nestjs/common'; -import { Injectable } from '@nestjs/common'; -import { Global, Module } from '@nestjs/common'; -import { APP_INTERCEPTOR } from '@nestjs/core'; +import type { + ArgumentsHost, + CallHandler, + DynamicModule, + ExecutionContext, + NestInterceptor, + OnModuleInit, +} from '@nestjs/common'; +import { Catch, Global, HttpException, Injectable, Module } from '@nestjs/common'; +import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + captureException, getClient, getDefaultIsolationScope, getIsolationScope, @@ -49,6 +56,28 @@ class SentryTracingInterceptor implements NestInterceptor { Injectable()(SentryTracingInterceptor); export { SentryTracingInterceptor }; +/** + * Global filter to handle exceptions and report them to Sentry. + */ +class SentryGlobalFilter extends BaseExceptionFilter { + public static readonly __SENTRY_INTERNAL__ = true; + + /** + * Catches exceptions and reports them to Sentry unless they are expected errors. + */ + public catch(exception: unknown, host: ArgumentsHost): void { + // don't report expected errors + if (exception instanceof HttpException) { + return super.catch(exception, host); + } + + captureException(exception); + return super.catch(exception, host); + } +} +Catch()(SentryGlobalFilter); +export { SentryGlobalFilter }; + /** * Service to set up Sentry performance tracing for Nest.js applications. */ diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts index 9a8d000e0a13..4f8d88fa8f86 100644 --- a/packages/node/src/integrations/tracing/nest/nest.ts +++ b/packages/node/src/integrations/tracing/nest/nest.ts @@ -2,6 +2,7 @@ import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core' import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + captureException, defineIntegration, getClient, getDefaultIsolationScope, @@ -11,7 +12,6 @@ import { import type { IntegrationFn, Span } from '@sentry/types'; import { logger } from '@sentry/utils'; import { generateInstrumentOnce } from '../../../otel/instrument'; -import { SentryNestErrorInstrumentation } from './sentry-nest-error-instrumentation'; import { SentryNestInstrumentation } from './sentry-nest-instrumentation'; import type { MinimalNestJsApp, NestJsErrorFilter } from './types'; @@ -25,15 +25,10 @@ const instrumentNestCommon = generateInstrumentOnce('Nest-Common', () => { return new SentryNestInstrumentation(); }); -const instrumentNestErrors = generateInstrumentOnce('Nest-Errors', () => { - return new SentryNestErrorInstrumentation(); -}); - export const instrumentNest = Object.assign( (): void => { instrumentNestCore(); instrumentNestCommon(); - instrumentNestErrors(); }, { id: INTEGRATION_NAME }, ); @@ -55,20 +50,9 @@ const _nestIntegration = (() => { export const nestIntegration = defineIntegration(_nestIntegration); /** - * Set span attributes for the nest instrumentation. - * - * Previously this method was used to setup an error handler reporting to Sentry, but this is no longer the case. - * This method should be renamed in the next major. + * Setup an error handler for Nest. */ -export function setupNestErrorHandler( - app: MinimalNestJsApp, - baseFilter: NestJsErrorFilter | undefined = undefined, -): void { - if (baseFilter) { - // No need to pass a base filter to setup the error handler anymore, since we always patch the `BaseExceptionFilter`. - logger.warn('baseFilter is deprecated and no longer needed.'); - } - +export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsErrorFilter): void { // Sadly, NestInstrumentation has no requestHook, so we need to add the attributes here // We register this hook in this method, because if we register it in the integration `setup`, // it would always run even for users that are not even using Nest.js @@ -96,6 +80,35 @@ export function setupNestErrorHandler( return next.handle(); }, }); + + const wrappedFilter = new Proxy(baseFilter, { + get(target, prop, receiver) { + if (prop === 'catch') { + const originalCatch = Reflect.get(target, prop, receiver); + + return (exception: unknown, host: unknown) => { + const exceptionIsObject = typeof exception === 'object' && exception !== null; + const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; + const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; + + /* + Don't report expected NestJS control flow errors + - `HttpException` errors will have a `status` property + - `RpcException` errors will have an `error` property + */ + if (exceptionStatusCode !== null || exceptionErrorProperty !== null) { + return originalCatch.apply(target, [exception, host]); + } + + captureException(exception); + return originalCatch.apply(target, [exception, host]); + }; + } + return Reflect.get(target, prop, receiver); + }, + }); + + app.useGlobalFilters(wrappedFilter); } function addNestSpanAttributes(span: Span): void { diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts deleted file mode 100644 index 415e7497ee09..000000000000 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-error-instrumentation.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { isWrapped } from '@opentelemetry/core'; -import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; -import { - InstrumentationBase, - InstrumentationNodeModuleDefinition, - InstrumentationNodeModuleFile, -} from '@opentelemetry/instrumentation'; -import { captureException } from '@sentry/core'; -import { SDK_VERSION } from '@sentry/utils'; -import type { BaseExceptionFilter } from './types'; - -const supportedVersions = ['>=8.0.0 <11']; - -/** - * Custom error instrumentation for nestjs. - * - * This patches the `BaseExceptionFilter` to report unexpected exceptions to Sentry. - */ -export class SentryNestErrorInstrumentation extends InstrumentationBase { - public static readonly COMPONENT = '@nestjs/core'; - public static readonly COMMON_ATTRIBUTES = { - component: SentryNestErrorInstrumentation.COMPONENT, - }; - - public constructor(config: InstrumentationConfig = {}) { - super('sentry-nestjs-error', SDK_VERSION, config); - } - - /** - * Initializes the instrumentation by defining the modules to be patched. - */ - public init(): InstrumentationNodeModuleDefinition { - const moduleDef = new InstrumentationNodeModuleDefinition( - SentryNestErrorInstrumentation.COMPONENT, - supportedVersions, - ); - - moduleDef.files.push(this._getBaseExceptionFilterFileInstrumentation(supportedVersions)); - - return moduleDef; - } - - /** - * Wraps the `BaseExceptionFilter`. - */ - private _getBaseExceptionFilterFileInstrumentation(versions: string[]): InstrumentationNodeModuleFile { - return new InstrumentationNodeModuleFile( - '@nestjs/core/exceptions/base-exception-filter.js', - versions, - (moduleExports: { BaseExceptionFilter: BaseExceptionFilter }) => { - if (isWrapped(moduleExports.BaseExceptionFilter.prototype)) { - this._unwrap(moduleExports.BaseExceptionFilter.prototype, 'catch'); - } - this._wrap(moduleExports.BaseExceptionFilter.prototype, 'catch', this._createWrapCatch()); - return moduleExports; - }, - (moduleExports: { BaseExceptionFilter: BaseExceptionFilter }) => { - this._unwrap(moduleExports.BaseExceptionFilter.prototype, 'catch'); - }, - ); - } - - /** - * Creates a wrapper function for the `BaseExceptionFilter`. - * - * Reports unexpected exceptions to Sentry and then calls the original `BaseExceptionFilter`. - * Expected NestJS control flow errors get filtered. - */ - private _createWrapCatch() { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return function wrapCatch(originalCatch: (exception: unknown, host: unknown, ...args: any[]) => void) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return function wrappedCatch(this: BaseExceptionFilter, exception: unknown, host: unknown, ...args: any[]) { - const exceptionIsObject = typeof exception === 'object' && exception !== null; - const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; - const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; - - /* - Don't report expected NestJS control flow errors - - `HttpException` errors will have a `status` property - - `RpcException` errors will have an `error` property - */ - if (exceptionStatusCode !== null || exceptionErrorProperty !== null) { - return originalCatch.apply(this, [exception, host, ...args]); - } - - captureException(exception); - - return originalCatch.apply(this, [exception, host, ...args]); - }; - }; - } -} diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index fecd5d50b989..42aa0b003315 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -64,12 +64,6 @@ export interface CatchTarget { sentryPatched?: boolean; __SENTRY_INTERNAL__?: boolean; prototype: { - catch?: (exception: unknown, host: unknown, ...args: any[]) => any; - }; -} - -export interface BaseExceptionFilter { - prototype: { - catch(exception: unknown, host: unknown, ...args: any[]): void; + catch?: (...args: any[]) => any; }; } From d7a62631398b996f621693794c30418d0b555dc7 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 10:03:12 +0200 Subject: [PATCH 23/36] Update to decorator and explicit sentry global filter provider --- .../nestjs-basic/src/app.module.ts | 6 +++- .../nestjs-with-submodules/src/app.module.ts | 11 +++++-- packages/nestjs/src/error-decorator.ts | 30 +++++++++++++++++++ packages/nestjs/src/index.ts | 1 + packages/nestjs/src/setup.ts | 12 ++++++-- 5 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 packages/nestjs/src/error-decorator.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts index 881982a5ca17..3de3c82dc925 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts @@ -1,7 +1,7 @@ import { MiddlewareConsumer, Module } from '@nestjs/common'; import { APP_FILTER } from '@nestjs/core'; import { ScheduleModule } from '@nestjs/schedule'; -import { SentryModule } from '@sentry/nestjs/setup'; +import { SentryGlobalFilter, SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; import { ExampleGlobalFilter } from './example-global.filter'; @@ -12,6 +12,10 @@ import { ExampleMiddleware } from './example.middleware'; controllers: [AppController], providers: [ AppService, + { + provide: APP_FILTER, + useClass: SentryGlobalFilter, + }, { provide: APP_FILTER, useClass: ExampleGlobalFilter, diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts index 2a4d38d3ca73..24a616c83a37 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts @@ -1,5 +1,6 @@ import { Module } from '@nestjs/common'; -import { SentryModule } from '@sentry/nestjs/setup'; +import { APP_FILTER } from '@nestjs/core'; +import { SentryGlobalFilter, SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; import { ExampleModuleGlobalFilterRegisteredFirst } from './example-module-global-filter-registered-first/example.module'; @@ -14,6 +15,12 @@ import { ExampleModuleLocalFilter } from './example-module-local-filter/example. ExampleModuleLocalFilter, ], controllers: [AppController], - providers: [AppService], + providers: [ + AppService, + { + provide: APP_FILTER, + useClass: SentryGlobalFilter, + }, + ], }) export class AppModule {} diff --git a/packages/nestjs/src/error-decorator.ts b/packages/nestjs/src/error-decorator.ts new file mode 100644 index 000000000000..67ca45383330 --- /dev/null +++ b/packages/nestjs/src/error-decorator.ts @@ -0,0 +1,30 @@ +import { HttpException } from '@nestjs/common'; +import { captureException } from '@sentry/core'; + +/** + * A decorator usable to wrap user-defined exception filters to add sentry error reporting. + */ +export function SentryCaptureException() { + return function (target: unknown, propertyKey: string, descriptor: PropertyDescriptor) { + descriptor.value = new Proxy(descriptor.value, { + apply: (originalCatch, thisArgCatch, argsCatch) => { + const exception = argsCatch[0]; + const exceptionIsObject = typeof exception === 'object' && exception !== null; + const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; + + /* + Don't report expected NestJS control flow errors + - `HttpException` + - `RpcException` errors will have an `error` property and we cannot rely directly on the `RpcException` class + because it is part of `@nestjs/microservices`, which is not a dependency for all nest applications + */ + if (exception instanceof HttpException || exceptionErrorProperty !== null) { + return originalCatch.apply(thisArgCatch, argsCatch); + } + + captureException(exception); + return originalCatch.apply(thisArgCatch, argsCatch); + }, + }); + }; +} diff --git a/packages/nestjs/src/index.ts b/packages/nestjs/src/index.ts index 00519cf49b9e..1336b8c19d72 100644 --- a/packages/nestjs/src/index.ts +++ b/packages/nestjs/src/index.ts @@ -4,3 +4,4 @@ export { init } from './sdk'; export { SentryTraced } from './span-decorator'; export { SentryCron } from './cron-decorator'; +export { SentryCaptureException } from './error-decorator'; diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 11a279521c3f..0076d6fb6329 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -66,8 +66,16 @@ class SentryGlobalFilter extends BaseExceptionFilter { * Catches exceptions and reports them to Sentry unless they are expected errors. */ public catch(exception: unknown, host: ArgumentsHost): void { - // don't report expected errors - if (exception instanceof HttpException) { + const exceptionIsObject = typeof exception === 'object' && exception !== null; + const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; + + /* + Don't report expected NestJS control flow errors + - `HttpException` + - `RpcException` errors will have an `error` property and we cannot rely directly on the `RpcException` class + because it is part of `@nestjs/microservices`, which is not a dependency for all nest applications + */ + if (exception instanceof HttpException || exceptionErrorProperty !== null) { return super.catch(exception, host); } From 47481fa292dd2b49ef702307ab4d8c713950a357 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 11:14:23 +0200 Subject: [PATCH 24/36] make stuff not broken --- packages/nestjs/src/error-decorator.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/nestjs/src/error-decorator.ts b/packages/nestjs/src/error-decorator.ts index 67ca45383330..b0676b21942c 100644 --- a/packages/nestjs/src/error-decorator.ts +++ b/packages/nestjs/src/error-decorator.ts @@ -1,4 +1,3 @@ -import { HttpException } from '@nestjs/common'; import { captureException } from '@sentry/core'; /** @@ -10,19 +9,23 @@ export function SentryCaptureException() { apply: (originalCatch, thisArgCatch, argsCatch) => { const exception = argsCatch[0]; const exceptionIsObject = typeof exception === 'object' && exception !== null; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; /* Don't report expected NestJS control flow errors - - `HttpException` - - `RpcException` errors will have an `error` property and we cannot rely directly on the `RpcException` class - because it is part of `@nestjs/microservices`, which is not a dependency for all nest applications + - `HttpException` errors will have a `status` property + - `RpcException` errors will have an `error` property */ - if (exception instanceof HttpException || exceptionErrorProperty !== null) { + if (exceptionStatusCode !== null || exceptionErrorProperty !== null) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access return originalCatch.apply(thisArgCatch, argsCatch); } captureException(exception); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access return originalCatch.apply(thisArgCatch, argsCatch); }, }); From 2d4dc358a2341ef33c236d211ca879b6e3c7498a Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 11:31:35 +0200 Subject: [PATCH 25/36] Revert node tests --- .../node-nestjs-basic/src/app.controller.ts | 16 +----------- .../node-nestjs-basic/src/app.module.ts | 10 +------ .../node-nestjs-basic/tests/errors.test.ts | 26 ------------------- 3 files changed, 2 insertions(+), 50 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts index 9cda3c96f9a6..ec0a921da2c4 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts @@ -1,14 +1,10 @@ -import { Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common'; +import { Controller, Get, Param, ParseIntPipe, UseGuards, UseInterceptors } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; -import { ExampleExceptionGlobalFilter } from './example-global-filter.exception'; -import { ExampleExceptionLocalFilter } from './example-local-filter.exception'; -import { ExampleLocalFilter } from './example-local.filter'; import { ExampleGuard } from './example.guard'; import { ExampleInterceptor } from './example.interceptor'; @Controller() -@UseFilters(ExampleLocalFilter) export class AppController { constructor(private readonly appService: AppService) {} @@ -78,14 +74,4 @@ export class AppController { async flush() { await flush(); } - - @Get('example-exception-global-filter') - async exampleExceptionGlobalFilter() { - throw new ExampleExceptionGlobalFilter(); - } - - @Get('example-exception-local-filter') - async exampleExceptionLocalFilter() { - throw new ExampleExceptionLocalFilter(); - } } diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts index 8b2586416e83..567dbefeadb7 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts @@ -1,21 +1,13 @@ import { MiddlewareConsumer, Module } from '@nestjs/common'; -import { APP_FILTER } from '@nestjs/core'; import { ScheduleModule } from '@nestjs/schedule'; import { AppController } from './app.controller'; import { AppService } from './app.service'; -import { ExampleGlobalFilter } from './example-global.filter'; import { ExampleMiddleware } from './example.middleware'; @Module({ imports: [ScheduleModule.forRoot()], controllers: [AppController], - providers: [ - AppService, - { - provide: APP_FILTER, - useClass: ExampleGlobalFilter, - }, - ], + providers: [AppService], }) export class AppModule { configure(consumer: MiddlewareConsumer): void { diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts index 4dd06a344136..0155c3887805 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts @@ -94,29 +94,3 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { expect(errorEventOccurred).toBe(false); }); - -test('Global exception filter registered in main module is applied', async ({ baseURL }) => { - const response = await fetch(`${baseURL}/example-exception-global-filter`); - const responseBody = await response.json(); - - expect(response.status).toBe(400); - expect(responseBody).toEqual({ - statusCode: 400, - timestamp: expect.any(String), - path: '/example-exception-global-filter', - message: 'Example exception was handled by global filter!', - }); -}); - -test('Local exception filter registered in main module is applied', async ({ baseURL }) => { - const response = await fetch(`${baseURL}/example-exception-local-filter`); - const responseBody = await response.json(); - - expect(response.status).toBe(400); - expect(responseBody).toEqual({ - statusCode: 400, - timestamp: expect.any(String), - path: '/example-exception-local-filter', - message: 'Example exception was handled by local filter!', - }); -}); From eb72826f2d02aed4907422dbbd6f63faa099bb61 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 11:48:16 +0200 Subject: [PATCH 26/36] Update tests --- .../nestjs-basic/tests/errors.test.ts | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts index 5a3d107faf70..ee7d12dd22ca 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts @@ -95,7 +95,23 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { expect(errorEventOccurred).toBe(false); }); -test('Global exception filter registered in main module is applied', async ({ baseURL }) => { +test('Global exception filter registered in main module is applied and exception is not sent to Sentry', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-basic', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by global filter!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-exception-global-filter'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-basic', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-exception-global-filter'; + }); + const response = await fetch(`${baseURL}/example-exception-global-filter`); const responseBody = await response.json(); @@ -106,9 +122,31 @@ test('Global exception filter registered in main module is applied', async ({ ba path: '/example-exception-global-filter', message: 'Example exception was handled by global filter!', }); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); }); -test('Local exception filter registered in main module is applied', async ({ baseURL }) => { +test('Local exception filter registered in main module is applied and exception is not sent to Sentry', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-basic', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by local filter!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-exception-local-filter'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-basic', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-exception-local-filter'; + }); + const response = await fetch(`${baseURL}/example-exception-local-filter`); const responseBody = await response.json(); @@ -119,4 +157,10 @@ test('Local exception filter registered in main module is applied', async ({ bas path: '/example-exception-local-filter', message: 'Example exception was handled by local filter!', }); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); }); From 66fa8bbdddce85547e35f6e8d1f27d258f8c9c65 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 13:14:13 +0200 Subject: [PATCH 27/36] Add sample application with new decorator setup --- .../.gitignore | 56 ++++ .../nestjs-with-submodules-decorator/.npmrc | 2 + .../nest-cli.json | 8 + .../package.json | 47 ++++ .../playwright.config.mjs | 7 + .../src/app.controller.ts | 23 ++ .../src/app.module.ts | 27 ++ .../src/app.service.ts | 14 + .../src/example-global.filter.ts | 20 ++ .../example.controller.ts | 17 ++ .../example.exception.ts | 5 + .../example.filter.ts | 13 + .../example.module.ts | 16 ++ .../example.controller.ts | 25 ++ .../example.exception.ts | 5 + .../example.filter.ts | 13 + .../example.module.ts | 16 ++ .../example.controller.ts | 14 + .../example.exception.ts | 5 + .../example.filter.ts | 13 + .../example.module.ts | 9 + .../src/instrument.ts | 12 + .../src/main.ts | 15 ++ .../start-event-proxy.mjs | 6 + .../tests/errors.test.ts | 160 ++++++++++++ .../tests/transactions.test.ts | 240 ++++++++++++++++++ .../tsconfig.build.json | 4 + .../tsconfig.json | 21 ++ 28 files changed, 813 insertions(+) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/nest-cli.json create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/playwright.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.service.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.controller.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.module.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.controller.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.module.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.module.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/instrument.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/main.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.build.json create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.json diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.gitignore b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.gitignore new file mode 100644 index 000000000000..4b56acfbebf4 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.gitignore @@ -0,0 +1,56 @@ +# compiled output +/dist +/node_modules +/build + +# Logs +logs +*.log +npm-debug.log* +pnpm-debug.log* +yarn-debug.log* +yarn-error.log* +lerna-debug.log* + +# OS +.DS_Store + +# Tests +/coverage +/.nyc_output + +# IDEs and editors +/.idea +.project +.classpath +.c9/ +*.launch +.settings/ +*.sublime-workspace + +# IDE - VSCode +.vscode/* +!.vscode/settings.json +!.vscode/tasks.json +!.vscode/launch.json +!.vscode/extensions.json + +# dotenv environment variable files +.env +.env.development.local +.env.test.local +.env.production.local +.env.local + +# temp directory +.temp +.tmp + +# Runtime data +pids +*.pid +*.seed +*.pid.lock + +# Diagnostic reports (https://nodejs.org/api/report.html) +report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.npmrc b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/nest-cli.json b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/nest-cli.json new file mode 100644 index 000000000000..f9aa683b1ad5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/nest-cli.json @@ -0,0 +1,8 @@ +{ + "$schema": "https://json.schemastore.org/nest-cli", + "collection": "@nestjs/schematics", + "sourceRoot": "src", + "compilerOptions": { + "deleteOutDir": true + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json new file mode 100644 index 000000000000..dfbe5e83e640 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json @@ -0,0 +1,47 @@ +{ + "name": "nestjs-with-submodules", + "version": "0.0.1", + "private": true, + "scripts": { + "build": "nest build", + "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"", + "start": "nest start", + "start:dev": "nest start --watch", + "start:debug": "nest start --debug --watch", + "start:prod": "node dist/main", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "test": "playwright test", + "test:build": "pnpm install", + "test:assert": "pnpm test" + }, + "dependencies": { + "@nestjs/common": "^10.0.0", + "@nestjs/core": "^10.0.0", + "@nestjs/platform-express": "^10.0.0", + "@sentry/nestjs": "latest || *", + "@sentry/types": "latest || *", + "reflect-metadata": "^0.2.0", + "rxjs": "^7.8.1" + }, + "devDependencies": { + "@playwright/test": "^1.44.1", + "@sentry-internal/test-utils": "link:../../../test-utils", + "@nestjs/cli": "^10.0.0", + "@nestjs/schematics": "^10.0.0", + "@nestjs/testing": "^10.0.0", + "@types/express": "^4.17.17", + "@types/node": "18.15.1", + "@types/supertest": "^6.0.0", + "@typescript-eslint/eslint-plugin": "^6.0.0", + "@typescript-eslint/parser": "^6.0.0", + "eslint": "^8.42.0", + "eslint-config-prettier": "^9.0.0", + "eslint-plugin-prettier": "^5.0.0", + "prettier": "^3.0.0", + "source-map-support": "^0.5.21", + "supertest": "^6.3.3", + "ts-loader": "^9.4.3", + "tsconfig-paths": "^4.2.0", + "typescript": "^4.9.5" + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/playwright.config.mjs new file mode 100644 index 000000000000..31f2b913b58b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/playwright.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts new file mode 100644 index 000000000000..0d2c46e90da2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts @@ -0,0 +1,23 @@ +import { Controller, Get, Param } from '@nestjs/common'; +import { flush } from '@sentry/nestjs'; +import { AppService } from './app.service'; + +@Controller() +export class AppController { + constructor(private readonly appService: AppService) {} + + @Get('test-exception/:id') + async testException(@Param('id') id: string) { + return this.appService.testException(id); + } + + @Get('test-expected-exception/:id') + async testExpectedException(@Param('id') id: string) { + return this.appService.testExpectedException(id); + } + + @Get('flush') + async flush() { + await flush(); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts new file mode 100644 index 000000000000..64360e9387f2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts @@ -0,0 +1,27 @@ +import { Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; +import { SentryModule } from '@sentry/nestjs/setup'; +import { AppController } from './app.controller'; +import { AppService } from './app.service'; +import { ExampleWrappedGlobalFilter } from './example-global.filter'; +import { ExampleModuleGlobalFilterRegisteredFirst } from './example-module-global-filter-registered-first/example.module'; +import { ExampleModuleGlobalFilter } from './example-module-global-filter/example.module'; +import { ExampleModuleLocalFilter } from './example-module-local-filter/example.module'; + +@Module({ + imports: [ + ExampleModuleGlobalFilterRegisteredFirst, + SentryModule.forRoot(), + ExampleModuleGlobalFilter, + ExampleModuleLocalFilter, + ], + controllers: [AppController], + providers: [ + AppService, + { + provide: APP_FILTER, + useClass: ExampleWrappedGlobalFilter, + }, + ], +}) +export class AppModule {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.service.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.service.ts new file mode 100644 index 000000000000..242408023586 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.service.ts @@ -0,0 +1,14 @@ +import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; + +@Injectable() +export class AppService { + constructor() {} + + testException(id: string) { + 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); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts new file mode 100644 index 000000000000..ab30d7557d23 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts @@ -0,0 +1,20 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { SentryCaptureException } from '@sentry/nestjs'; +import { Request, Response } from 'express'; + +@Catch() +export class ExampleWrappedGlobalFilter implements ExceptionFilter { + @SentryCaptureException() + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + + response.status(501).json({ + statusCode: 501, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by global filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.controller.ts new file mode 100644 index 000000000000..967886948a25 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.controller.ts @@ -0,0 +1,17 @@ +import { Controller, Get } from '@nestjs/common'; +import { ExampleExceptionRegisteredFirst } from './example.exception'; + +@Controller('example-module-registered-first') +export class ExampleController { + constructor() {} + + @Get('/expected-exception') + getCaughtException(): string { + throw new ExampleExceptionRegisteredFirst(); + } + + @Get('/unexpected-exception') + getUncaughtException(): string { + throw new Error(`This is an uncaught exception!`); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.exception.ts new file mode 100644 index 000000000000..9bb3b5948343 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionRegisteredFirst extends Error { + constructor() { + super('Something went wrong in the example module!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.filter.ts new file mode 100644 index 000000000000..6c3b81d395b5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.filter.ts @@ -0,0 +1,13 @@ +import { ArgumentsHost, BadRequestException, Catch } from '@nestjs/common'; +import { BaseExceptionFilter } from '@nestjs/core'; +import { ExampleExceptionRegisteredFirst } from './example.exception'; + +@Catch(ExampleExceptionRegisteredFirst) +export class ExampleExceptionFilterRegisteredFirst extends BaseExceptionFilter { + catch(exception: unknown, host: ArgumentsHost) { + if (exception instanceof ExampleExceptionRegisteredFirst) { + return super.catch(new BadRequestException(exception.message), host); + } + return super.catch(exception, host); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.module.ts new file mode 100644 index 000000000000..8751856f99cd --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first/example.module.ts @@ -0,0 +1,16 @@ +import { Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; +import { ExampleController } from './example.controller'; +import { ExampleExceptionFilterRegisteredFirst } from './example.filter'; + +@Module({ + imports: [], + controllers: [ExampleController], + providers: [ + { + provide: APP_FILTER, + useClass: ExampleExceptionFilterRegisteredFirst, + }, + ], +}) +export class ExampleModuleGlobalFilterRegisteredFirst {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.controller.ts new file mode 100644 index 000000000000..53356e906130 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.controller.ts @@ -0,0 +1,25 @@ +import { Controller, Get } from '@nestjs/common'; +import * as Sentry from '@sentry/nestjs'; +import { ExampleException } from './example.exception'; + +@Controller('example-module') +export class ExampleController { + constructor() {} + + @Get('/expected-exception') + getCaughtException(): string { + throw new ExampleException(); + } + + @Get('/unexpected-exception') + getUncaughtException(): string { + throw new Error(`This is an uncaught exception!`); + } + + @Get('/transaction') + testTransaction() { + Sentry.startSpan({ name: 'test-span' }, () => { + Sentry.startSpan({ name: 'child-span' }, () => {}); + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.exception.ts new file mode 100644 index 000000000000..ac43dddfa8dc --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.exception.ts @@ -0,0 +1,5 @@ +export class ExampleException extends Error { + constructor() { + super('Something went wrong in the example module!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.filter.ts new file mode 100644 index 000000000000..848441caf855 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.filter.ts @@ -0,0 +1,13 @@ +import { ArgumentsHost, BadRequestException, Catch } from '@nestjs/common'; +import { BaseExceptionFilter } from '@nestjs/core'; +import { ExampleException } from './example.exception'; + +@Catch(ExampleException) +export class ExampleExceptionFilter extends BaseExceptionFilter { + catch(exception: unknown, host: ArgumentsHost) { + if (exception instanceof ExampleException) { + return super.catch(new BadRequestException(exception.message), host); + } + return super.catch(exception, host); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.module.ts new file mode 100644 index 000000000000..8052cb137aac --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-global-filter/example.module.ts @@ -0,0 +1,16 @@ +import { Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; +import { ExampleController } from './example.controller'; +import { ExampleExceptionFilter } from './example.filter'; + +@Module({ + imports: [], + controllers: [ExampleController], + providers: [ + { + provide: APP_FILTER, + useClass: ExampleExceptionFilter, + }, + ], +}) +export class ExampleModuleGlobalFilter {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts new file mode 100644 index 000000000000..41d75d6eaf89 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts @@ -0,0 +1,14 @@ +import { Controller, Get, UseFilters } from '@nestjs/common'; +import { LocalExampleException } from './example.exception'; +import { LocalExampleExceptionFilter } from './example.filter'; + +@Controller('example-module-local-filter') +@UseFilters(LocalExampleExceptionFilter) +export class ExampleControllerLocalFilter { + constructor() {} + + @Get('/expected-exception') + getCaughtException() { + throw new LocalExampleException(); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.exception.ts new file mode 100644 index 000000000000..85a64d26d75e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.exception.ts @@ -0,0 +1,5 @@ +export class LocalExampleException extends Error { + constructor() { + super('Something went wrong in the example module with local filter!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.filter.ts new file mode 100644 index 000000000000..9b6797c95e44 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.filter.ts @@ -0,0 +1,13 @@ +import { ArgumentsHost, BadRequestException, Catch } from '@nestjs/common'; +import { BaseExceptionFilter } from '@nestjs/core'; +import { LocalExampleException } from './example.exception'; + +@Catch(LocalExampleException) +export class LocalExampleExceptionFilter extends BaseExceptionFilter { + catch(exception: unknown, host: ArgumentsHost) { + if (exception instanceof LocalExampleException) { + return super.catch(new BadRequestException(exception.message), host); + } + return super.catch(exception, host); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.module.ts new file mode 100644 index 000000000000..91d229a553c1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.module.ts @@ -0,0 +1,9 @@ +import { Module } from '@nestjs/common'; +import { ExampleControllerLocalFilter } from './example.controller'; + +@Module({ + imports: [], + controllers: [ExampleControllerLocalFilter], + providers: [], +}) +export class ExampleModuleLocalFilter {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/instrument.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/instrument.ts new file mode 100644 index 000000000000..4f16ebb36d11 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/instrument.ts @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/nestjs'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1, + transportOptions: { + // We expect the app to send a lot of events in a short time + bufferSize: 1000, + }, +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/main.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/main.ts new file mode 100644 index 000000000000..71ce685f4d61 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/main.ts @@ -0,0 +1,15 @@ +// Import this first +import './instrument'; + +// Import other modules +import { NestFactory } from '@nestjs/core'; +import { AppModule } from './app.module'; + +const PORT = 3030; + +async function bootstrap() { + const app = await NestFactory.create(AppModule); + await app.listen(PORT); +} + +bootstrap(); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs new file mode 100644 index 000000000000..6ec54bc59e4f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'nestjs-with-submodules', +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts new file mode 100644 index 000000000000..ae78fc27aa2e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts @@ -0,0 +1,160 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; + +test('Sends unexpected exception to Sentry if thrown in module with global filter', async ({ baseURL }) => { + const errorEventPromise = waitForError('nestjs-with-submodules', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an uncaught exception!'; + }); + + const response = await fetch(`${baseURL}/example-module/unexpected-exception`); + const responseBody = await response.json(); + + expect(response.status).toBe(501); + expect(responseBody).toEqual({ + statusCode: 501, + timestamp: expect.any(String), + path: '/example-module/unexpected-exception', + message: 'Example exception was handled by global filter!', + }); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!'); + + expect(errorEvent.request).toEqual({ + method: 'GET', + cookies: {}, + headers: expect.any(Object), + url: 'http://localhost:3030/example-module/unexpected-exception', + }); + + expect(errorEvent.transaction).toEqual('GET /example-module/unexpected-exception'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); + +test('Sends unexpected exception to Sentry if thrown in module that was registered before Sentry', async ({ + baseURL, +}) => { + const errorEventPromise = waitForError('nestjs-with-submodules', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an uncaught exception!'; + }); + + const response = await fetch(`${baseURL}/example-module-registered-first/unexpected-exception`); + const responseBody = await response.json(); + + expect(response.status).toBe(501); + expect(responseBody).toEqual({ + statusCode: 501, + timestamp: expect.any(String), + path: '/example-module-registered-first/unexpected-exception', + message: 'Example exception was handled by global filter!', + }); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!'); + + expect(errorEvent.request).toEqual({ + method: 'GET', + cookies: {}, + headers: expect.any(Object), + url: 'http://localhost:3030/example-module-registered-first/unexpected-exception', + }); + + expect(errorEvent.transaction).toEqual('GET /example-module-registered-first/unexpected-exception'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); + +test('Does not send exception to Sentry if user-defined global exception filter already catches the exception', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-with-submodules', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'Something went wrong in the example module!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-module/expected-exception'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-module/expected-exception'; + }); + + const response = await fetch(`${baseURL}/example-module/expected-exception`); + expect(response.status).toBe(400); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); + +test('Does not send exception to Sentry if user-defined local exception filter already catches the exception', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-with-submodules', event => { + if ( + !event.type && + event.exception?.values?.[0]?.value === 'Something went wrong in the example module with local filter!' + ) { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-module-local-filter/expected-exception'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-module-local-filter/expected-exception'; + }); + + const response = await fetch(`${baseURL}/example-module-local-filter/expected-exception`); + expect(response.status).toBe(400); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); + +test('Does not send expected exception to Sentry if exception is thrown in module registered before Sentry', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-with-submodules', event => { + if (!event.type && event.exception?.values?.[0].value === 'Something went wrong in the example module!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-module-registered-first/expected-exception'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-module-registered-first/expected-exception'; + }); + + const response = await fetch(`${baseURL}/example-module-registered-first/expected-exception`); + expect(response.status).toBe(400); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts new file mode 100644 index 000000000000..a2ea1db9c506 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts @@ -0,0 +1,240 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('Sends an API route transaction from module', async ({ baseURL }) => { + const pageloadTransactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module/transaction' + ); + }); + + await fetch(`${baseURL}/example-module/transaction`); + + const transactionEvent = await pageloadTransactionEventPromise; + + expect(transactionEvent.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/example-module/transaction', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/example-module/transaction', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/example-module/transaction', + 'http.user_agent': 'node', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': expect.any(String), + 'net.host.port': expect.any(Number), + 'net.peer.ip': expect.any(String), + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/example-module/transaction', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + trace_id: expect.any(String), + origin: 'auto.http.otel.http', + }); + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + data: { + 'express.name': '/example-module/transaction', + 'express.type': 'request_handler', + 'http.route': '/example-module/transaction', + 'sentry.origin': 'auto.http.otel.express', + 'sentry.op': 'request_handler.express', + }, + op: 'request_handler.express', + description: '/example-module/transaction', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'auto.http.otel.express', + }, + { + data: { + 'sentry.origin': 'manual', + }, + description: 'test-span', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'manual', + }, + { + data: { + 'sentry.origin': 'manual', + }, + description: 'child-span', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'manual', + }, + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.http.otel.nestjs', + 'sentry.op': 'handler.nestjs', + component: '@nestjs/core', + 'nestjs.version': expect.any(String), + 'nestjs.type': 'handler', + 'nestjs.callback': 'testTransaction', + }, + description: 'testTransaction', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'auto.http.otel.nestjs', + op: 'handler.nestjs', + }, + ]), + transaction: 'GET /example-module/transaction', + type: 'transaction', + transaction_info: { + source: 'route', + }, + }), + ); +}); + +test('API route transaction includes exception filter span for global filter in module registered after Sentry', async ({ + baseURL, +}) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'ExampleExceptionFilter', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); +}); + +test('API route transaction includes exception filter span for local filter in module registered after Sentry', async ({ + baseURL, +}) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module-local-filter/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module-local-filter/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module-local-filter/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'LocalExampleExceptionFilter', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); +}); + +test('API route transaction includes exception filter span for global filter in module registered before Sentry', async ({ + baseURL, +}) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module-registered-first/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module-registered-first/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module-registered-first/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'ExampleExceptionFilterRegisteredFirst', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.build.json b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.build.json new file mode 100644 index 000000000000..26c30d4eddf2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.build.json @@ -0,0 +1,4 @@ +{ + "extends": "./tsconfig.json", + "exclude": ["node_modules", "test", "dist"] +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.json b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.json new file mode 100644 index 000000000000..95f5641cf7f3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tsconfig.json @@ -0,0 +1,21 @@ +{ + "compilerOptions": { + "module": "commonjs", + "declaration": true, + "removeComments": true, + "emitDecoratorMetadata": true, + "experimentalDecorators": true, + "allowSyntheticDefaultImports": true, + "target": "ES2021", + "sourceMap": true, + "outDir": "./dist", + "baseUrl": "./", + "incremental": true, + "skipLibCheck": true, + "strictNullChecks": false, + "noImplicitAny": false, + "strictBindCallApply": false, + "forceConsistentCasingInFileNames": false, + "noFallthroughCasesInSwitch": false + } +} From 38d0093140484f633cd64adc80dfda1537bbf4c4 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 13:42:46 +0200 Subject: [PATCH 28/36] Fix tests? --- .../package.json | 2 +- .../start-event-proxy.mjs | 2 +- .../tests/errors.test.ts | 16 ++++++++-------- .../tests/transactions.test.ts | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json index dfbe5e83e640..9cc3641d2322 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json @@ -1,5 +1,5 @@ { - "name": "nestjs-with-submodules", + "name": "nestjs-with-submodules-decorator", "version": "0.0.1", "private": true, "scripts": { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs index 6ec54bc59e4f..3c205b59a2d1 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/start-event-proxy.mjs @@ -2,5 +2,5 @@ import { startEventProxyServer } from '@sentry-internal/test-utils'; startEventProxyServer({ port: 3031, - proxyServerName: 'nestjs-with-submodules', + proxyServerName: 'nestjs-with-submodules-decorator', }); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts index ae78fc27aa2e..cb748450faca 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts @@ -2,7 +2,7 @@ import { expect, test } from '@playwright/test'; import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; test('Sends unexpected exception to Sentry if thrown in module with global filter', async ({ baseURL }) => { - const errorEventPromise = waitForError('nestjs-with-submodules', event => { + const errorEventPromise = waitForError('nestjs-with-submodules-decorator', event => { return !event.type && event.exception?.values?.[0]?.value === 'This is an uncaught exception!'; }); @@ -40,7 +40,7 @@ test('Sends unexpected exception to Sentry if thrown in module with global filte test('Sends unexpected exception to Sentry if thrown in module that was registered before Sentry', async ({ baseURL, }) => { - const errorEventPromise = waitForError('nestjs-with-submodules', event => { + const errorEventPromise = waitForError('nestjs-with-submodules-decorator', event => { return !event.type && event.exception?.values?.[0]?.value === 'This is an uncaught exception!'; }); @@ -80,7 +80,7 @@ test('Does not send exception to Sentry if user-defined global exception filter }) => { let errorEventOccurred = false; - waitForError('nestjs-with-submodules', event => { + waitForError('nestjs-with-submodules-decorator', event => { if (!event.type && event.exception?.values?.[0]?.value === 'Something went wrong in the example module!') { errorEventOccurred = true; } @@ -88,7 +88,7 @@ test('Does not send exception to Sentry if user-defined global exception filter return event?.transaction === 'GET /example-module/expected-exception'; }); - const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { return transactionEvent?.transaction === 'GET /example-module/expected-exception'; }); @@ -107,7 +107,7 @@ test('Does not send exception to Sentry if user-defined local exception filter a }) => { let errorEventOccurred = false; - waitForError('nestjs-with-submodules', event => { + waitForError('nestjs-with-submodules-decorator', event => { if ( !event.type && event.exception?.values?.[0]?.value === 'Something went wrong in the example module with local filter!' @@ -118,7 +118,7 @@ test('Does not send exception to Sentry if user-defined local exception filter a return event?.transaction === 'GET /example-module-local-filter/expected-exception'; }); - const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { return transactionEvent?.transaction === 'GET /example-module-local-filter/expected-exception'; }); @@ -137,7 +137,7 @@ test('Does not send expected exception to Sentry if exception is thrown in modul }) => { let errorEventOccurred = false; - waitForError('nestjs-with-submodules', event => { + waitForError('nestjs-with-submodules-decorator', event => { if (!event.type && event.exception?.values?.[0].value === 'Something went wrong in the example module!') { errorEventOccurred = true; } @@ -145,7 +145,7 @@ test('Does not send expected exception to Sentry if exception is thrown in modul return event?.transaction === 'GET /example-module-registered-first/expected-exception'; }); - const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { return transactionEvent?.transaction === 'GET /example-module-registered-first/expected-exception'; }); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts index a2ea1db9c506..740ab6f5650c 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts @@ -2,7 +2,7 @@ import { expect, test } from '@playwright/test'; import { waitForTransaction } from '@sentry-internal/test-utils'; test('Sends an API route transaction from module', async ({ baseURL }) => { - const pageloadTransactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + const pageloadTransactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /example-module/transaction' @@ -125,7 +125,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { test('API route transaction includes exception filter span for global filter in module registered after Sentry', async ({ baseURL, }) => { - const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /example-module/expected-exception' && @@ -164,7 +164,7 @@ test('API route transaction includes exception filter span for global filter in test('API route transaction includes exception filter span for local filter in module registered after Sentry', async ({ baseURL, }) => { - const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /example-module-local-filter/expected-exception' && @@ -203,7 +203,7 @@ test('API route transaction includes exception filter span for local filter in m test('API route transaction includes exception filter span for global filter in module registered before Sentry', async ({ baseURL, }) => { - const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { return ( transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /example-module-registered-first/expected-exception' && From fbc5a7a448b6afa40ae35ce64b18212b3508467c Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 13:56:05 +0200 Subject: [PATCH 29/36] Add test: Unexpected exception not caught by local filter is reported --- .../example.controller.ts | 5 +++ .../tests/errors.test.ts | 36 +++++++++++++++++++ .../example.controller.ts | 5 +++ .../tests/errors.test.ts | 28 +++++++++++++++ 4 files changed, 74 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts index 41d75d6eaf89..11a0470ace17 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts @@ -11,4 +11,9 @@ export class ExampleControllerLocalFilter { getCaughtException() { throw new LocalExampleException(); } + + @Get('/unexpected-exception') + getUncaughtException(): string { + throw new Error(`This is an uncaught exception!`); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts index cb748450faca..c0cad562a31d 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts @@ -37,6 +37,42 @@ test('Sends unexpected exception to Sentry if thrown in module with global filte }); }); +test('Sends unexpected exception to Sentry if thrown in module with local filter', async ({ baseURL }) => { + const errorEventPromise = waitForError('nestjs-with-submodules-decorator', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an uncaught exception!'; + }); + + const response = await fetch(`${baseURL}/example-module-local-filter/unexpected-exception`); + const responseBody = await response.json(); + + expect(response.status).toBe(501); + expect(responseBody).toEqual({ + statusCode: 501, + timestamp: expect.any(String), + path: '/example-module-local-filter/unexpected-exception', + message: 'Example exception was handled by global filter!', + }); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!'); + + expect(errorEvent.request).toEqual({ + method: 'GET', + cookies: {}, + headers: expect.any(Object), + url: 'http://localhost:3030/example-module-local-filter/unexpected-exception', + }); + + expect(errorEvent.transaction).toEqual('GET /example-module-local-filter/unexpected-exception'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); + test('Sends unexpected exception to Sentry if thrown in module that was registered before Sentry', async ({ baseURL, }) => { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-local-filter/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-local-filter/example.controller.ts index 41d75d6eaf89..11a0470ace17 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-local-filter/example.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-local-filter/example.controller.ts @@ -11,4 +11,9 @@ export class ExampleControllerLocalFilter { getCaughtException() { throw new LocalExampleException(); } + + @Get('/unexpected-exception') + getUncaughtException(): string { + throw new Error(`This is an uncaught exception!`); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts index 8a5235d2735b..ef99d2877e96 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts @@ -29,6 +29,34 @@ test('Sends unexpected exception to Sentry if thrown in module with global filte }); }); +test('Sends unexpected exception to Sentry if thrown in module with local filter', async ({ baseURL }) => { + const errorEventPromise = waitForError('nestjs-with-submodules', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an uncaught exception!'; + }); + + const response = await fetch(`${baseURL}/example-module-local-filter/unexpected-exception`); + 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 uncaught exception!'); + + expect(errorEvent.request).toEqual({ + method: 'GET', + cookies: {}, + headers: expect.any(Object), + url: 'http://localhost:3030/example-module-local-filter/unexpected-exception', + }); + + expect(errorEvent.transaction).toEqual('GET /example-module-local-filter/unexpected-exception'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); + test('Sends unexpected exception to Sentry if thrown in module that was registered before Sentry', async ({ baseURL, }) => { From b09bb16b6d9d10fe82efc2a80a00dcbf7f9cdb77 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 14:21:55 +0200 Subject: [PATCH 30/36] Add specific filter test --- .../src/app.controller.ts | 6 ++++ .../src/app.module.ts | 5 +++ .../src/example-specific.exception.ts | 5 +++ .../src/example-specific.filter.ts | 19 ++++++++++ .../tests/errors.test.ts | 35 +++++++++++++++++++ .../src/app.controller.ts | 6 ++++ .../nestjs-with-submodules/src/app.module.ts | 5 +++ .../src/example-specific.exception.ts | 5 +++ .../src/example-specific.filter.ts | 19 ++++++++++ .../tests/errors.test.ts | 35 +++++++++++++++++++ 10 files changed, 140 insertions(+) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.filter.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts index 0d2c46e90da2..5996b2831289 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts @@ -1,6 +1,7 @@ import { Controller, Get, Param } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; +import { ExampleExceptionSpecificFilter } from './example-specific.exception'; @Controller() export class AppController { @@ -20,4 +21,9 @@ export class AppController { async flush() { await flush(); } + + @Get('example-exception-specific-filter') + async exampleExceptionGlobalFilter() { + throw new ExampleExceptionSpecificFilter(); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts index 64360e9387f2..77cf85a4fa9c 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts @@ -7,6 +7,7 @@ import { ExampleWrappedGlobalFilter } from './example-global.filter'; import { ExampleModuleGlobalFilterRegisteredFirst } from './example-module-global-filter-registered-first/example.module'; import { ExampleModuleGlobalFilter } from './example-module-global-filter/example.module'; import { ExampleModuleLocalFilter } from './example-module-local-filter/example.module'; +import { ExampleSpecificFilter } from './example-specific.filter'; @Module({ imports: [ @@ -22,6 +23,10 @@ import { ExampleModuleLocalFilter } from './example-module-local-filter/example. provide: APP_FILTER, useClass: ExampleWrappedGlobalFilter, }, + { + provide: APP_FILTER, + useClass: ExampleSpecificFilter, + }, ], }) export class AppModule {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.exception.ts new file mode 100644 index 000000000000..a26cb1a26d52 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionSpecificFilter extends Error { + constructor() { + super('Original specific example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.filter.ts new file mode 100644 index 000000000000..bf85385a9a86 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-specific.filter.ts @@ -0,0 +1,19 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionSpecificFilter } from './example-specific.exception'; + +@Catch(ExampleExceptionSpecificFilter) +export class ExampleSpecificFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + + response.status(400).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by specific filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts index c0cad562a31d..2137c1299ec4 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts @@ -194,3 +194,38 @@ test('Does not send expected exception to Sentry if exception is thrown in modul expect(errorEventOccurred).toBe(false); }); + +test('Global specific exception filter registered in main module is applied and exception is not sent to Sentry', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-with-submodules-decorator', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by specific filter!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-exception-specific-filter'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-exception-specific-filter'; + }); + + const response = await fetch(`${baseURL}/example-exception-specific-filter`); + const responseBody = await response.json(); + + expect(response.status).toBe(400); + expect(responseBody).toEqual({ + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-specific-filter', + message: 'Example exception was handled by specific filter!', + }); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts index 0d2c46e90da2..5996b2831289 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts @@ -1,6 +1,7 @@ import { Controller, Get, Param } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; +import { ExampleExceptionSpecificFilter } from './example-specific.exception'; @Controller() export class AppController { @@ -20,4 +21,9 @@ export class AppController { async flush() { await flush(); } + + @Get('example-exception-specific-filter') + async exampleExceptionGlobalFilter() { + throw new ExampleExceptionSpecificFilter(); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts index 24a616c83a37..2a93747ac075 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts @@ -6,6 +6,7 @@ import { AppService } from './app.service'; import { ExampleModuleGlobalFilterRegisteredFirst } from './example-module-global-filter-registered-first/example.module'; import { ExampleModuleGlobalFilter } from './example-module-global-filter/example.module'; import { ExampleModuleLocalFilter } from './example-module-local-filter/example.module'; +import { ExampleSpecificFilter } from './example-specific.filter'; @Module({ imports: [ @@ -21,6 +22,10 @@ import { ExampleModuleLocalFilter } from './example-module-local-filter/example. provide: APP_FILTER, useClass: SentryGlobalFilter, }, + { + provide: APP_FILTER, + useClass: ExampleSpecificFilter, + }, ], }) export class AppModule {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.exception.ts new file mode 100644 index 000000000000..a26cb1a26d52 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionSpecificFilter extends Error { + constructor() { + super('Original specific example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.filter.ts new file mode 100644 index 000000000000..bf85385a9a86 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-specific.filter.ts @@ -0,0 +1,19 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionSpecificFilter } from './example-specific.exception'; + +@Catch(ExampleExceptionSpecificFilter) +export class ExampleSpecificFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + + response.status(400).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by specific filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts index ef99d2877e96..4f43ad47c3d1 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts @@ -170,3 +170,38 @@ test('Does not send expected exception to Sentry if exception is thrown in modul expect(errorEventOccurred).toBe(false); }); + +test('Global specific exception filter registered in main module is applied and exception is not sent to Sentry', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-with-submodules', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by specific filter!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-exception-specific-filter'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-exception-specific-filter'; + }); + + const response = await fetch(`${baseURL}/example-exception-specific-filter`); + const responseBody = await response.json(); + + expect(response.status).toBe(400); + expect(responseBody).toEqual({ + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-specific-filter', + message: 'Example exception was handled by specific filter!', + }); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); From 878ee0baf54afd6c030c52007a7220485377f314 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 14:46:02 +0200 Subject: [PATCH 31/36] Add test local filter --- .../src/app.controller.ts | 10 +++++- .../src/example-local.exception.ts | 5 +++ .../src/example-local.filter.ts | 19 ++++++++++ .../tests/errors.test.ts | 35 +++++++++++++++++++ .../src/app.controller.ts | 10 +++++- .../src/example-local.exception.ts | 5 +++ .../src/example-local.filter.ts | 19 ++++++++++ .../tests/errors.test.ts | 35 +++++++++++++++++++ 8 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.filter.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts index 5996b2831289..efce824a20c3 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts @@ -1,9 +1,12 @@ -import { Controller, Get, Param } from '@nestjs/common'; +import { Controller, Get, Param, UseFilters } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; +import { ExampleExceptionLocalFilter } from './example-local.exception'; +import { ExampleLocalFilter } from './example-local.filter'; import { ExampleExceptionSpecificFilter } from './example-specific.exception'; @Controller() +@UseFilters(ExampleLocalFilter) export class AppController { constructor(private readonly appService: AppService) {} @@ -26,4 +29,9 @@ export class AppController { async exampleExceptionGlobalFilter() { throw new ExampleExceptionSpecificFilter(); } + + @Get('example-exception-local-filter') + async exampleExceptionLocalFilter() { + throw new ExampleExceptionLocalFilter(); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.exception.ts new file mode 100644 index 000000000000..8f76520a3b94 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionLocalFilter extends Error { + constructor() { + super('Original local example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.filter.ts new file mode 100644 index 000000000000..0e93e5f7fac2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.filter.ts @@ -0,0 +1,19 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionLocalFilter } from './example-local.exception'; + +@Catch(ExampleExceptionLocalFilter) +export class ExampleLocalFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + + response.status(400).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by local filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts index 2137c1299ec4..94c742dd210a 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts @@ -229,3 +229,38 @@ test('Global specific exception filter registered in main module is applied and expect(errorEventOccurred).toBe(false); }); + +test('Local specific exception filter registered in main module is applied and exception is not sent to Sentry', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-with-submodules-decorator', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by local filter!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-exception-local-filter'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-with-submodules-decorator', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-exception-local-filter'; + }); + + const response = await fetch(`${baseURL}/example-exception-local-filter`); + const responseBody = await response.json(); + + expect(response.status).toBe(400); + expect(responseBody).toEqual({ + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-local-filter', + message: 'Example exception was handled by local filter!', + }); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts index 5996b2831289..efce824a20c3 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.controller.ts @@ -1,9 +1,12 @@ -import { Controller, Get, Param } from '@nestjs/common'; +import { Controller, Get, Param, UseFilters } from '@nestjs/common'; import { flush } from '@sentry/nestjs'; import { AppService } from './app.service'; +import { ExampleExceptionLocalFilter } from './example-local.exception'; +import { ExampleLocalFilter } from './example-local.filter'; import { ExampleExceptionSpecificFilter } from './example-specific.exception'; @Controller() +@UseFilters(ExampleLocalFilter) export class AppController { constructor(private readonly appService: AppService) {} @@ -26,4 +29,9 @@ export class AppController { async exampleExceptionGlobalFilter() { throw new ExampleExceptionSpecificFilter(); } + + @Get('example-exception-local-filter') + async exampleExceptionLocalFilter() { + throw new ExampleExceptionLocalFilter(); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.exception.ts new file mode 100644 index 000000000000..8f76520a3b94 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.exception.ts @@ -0,0 +1,5 @@ +export class ExampleExceptionLocalFilter extends Error { + constructor() { + super('Original local example exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.filter.ts new file mode 100644 index 000000000000..0e93e5f7fac2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-local.filter.ts @@ -0,0 +1,19 @@ +import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; +import { Request, Response } from 'express'; +import { ExampleExceptionLocalFilter } from './example-local.exception'; + +@Catch(ExampleExceptionLocalFilter) +export class ExampleLocalFilter implements ExceptionFilter { + catch(exception: BadRequestException, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + const request = ctx.getRequest(); + + response.status(400).json({ + statusCode: 400, + timestamp: new Date().toISOString(), + path: request.url, + message: 'Example exception was handled by local filter!', + }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts index 4f43ad47c3d1..03d4679fa3c0 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/errors.test.ts @@ -205,3 +205,38 @@ test('Global specific exception filter registered in main module is applied and expect(errorEventOccurred).toBe(false); }); + +test('Local specific exception filter registered in main module is applied and exception is not sent to Sentry', async ({ + baseURL, +}) => { + let errorEventOccurred = false; + + waitForError('nestjs-with-submodules', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by local filter!') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /example-exception-local-filter'; + }); + + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-exception-local-filter'; + }); + + const response = await fetch(`${baseURL}/example-exception-local-filter`); + const responseBody = await response.json(); + + expect(response.status).toBe(400); + expect(responseBody).toEqual({ + statusCode: 400, + timestamp: expect.any(String), + path: '/example-exception-local-filter', + message: 'Example exception was handled by local filter!', + }); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); From 56a823c03065ce02ee1cc8e28ff5f0f639b23083 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 14:59:32 +0200 Subject: [PATCH 32/36] Rename decorator --- .../src/example-global.filter.ts | 4 ++-- packages/nestjs/src/error-decorator.ts | 2 +- packages/nestjs/src/index.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts index ab30d7557d23..cee50d0d2c7c 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-global.filter.ts @@ -1,10 +1,10 @@ import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; -import { SentryCaptureException } from '@sentry/nestjs'; +import { WithSentry } from '@sentry/nestjs'; import { Request, Response } from 'express'; @Catch() export class ExampleWrappedGlobalFilter implements ExceptionFilter { - @SentryCaptureException() + @WithSentry() catch(exception: BadRequestException, host: ArgumentsHost): void { const ctx = host.switchToHttp(); const response = ctx.getResponse(); diff --git a/packages/nestjs/src/error-decorator.ts b/packages/nestjs/src/error-decorator.ts index b0676b21942c..688085433e05 100644 --- a/packages/nestjs/src/error-decorator.ts +++ b/packages/nestjs/src/error-decorator.ts @@ -3,7 +3,7 @@ import { captureException } from '@sentry/core'; /** * A decorator usable to wrap user-defined exception filters to add sentry error reporting. */ -export function SentryCaptureException() { +export function WithSentry() { return function (target: unknown, propertyKey: string, descriptor: PropertyDescriptor) { descriptor.value = new Proxy(descriptor.value, { apply: (originalCatch, thisArgCatch, argsCatch) => { diff --git a/packages/nestjs/src/index.ts b/packages/nestjs/src/index.ts index 1336b8c19d72..b30fe547103b 100644 --- a/packages/nestjs/src/index.ts +++ b/packages/nestjs/src/index.ts @@ -4,4 +4,4 @@ export { init } from './sdk'; export { SentryTraced } from './span-decorator'; export { SentryCron } from './cron-decorator'; -export { SentryCaptureException } from './error-decorator'; +export { WithSentry } from './error-decorator'; From 45de32e2ef13da3699fb51f44b1ca8eccd8f839d Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 15:12:34 +0200 Subject: [PATCH 33/36] Make decorator nicer --- packages/nestjs/src/error-decorator.ts | 37 ++++++++++---------------- packages/nestjs/src/helpers.ts | 13 +++++++++ packages/nestjs/src/setup.ts | 14 +++------- 3 files changed, 30 insertions(+), 34 deletions(-) create mode 100644 packages/nestjs/src/helpers.ts diff --git a/packages/nestjs/src/error-decorator.ts b/packages/nestjs/src/error-decorator.ts index 688085433e05..bf1fd08d8cee 100644 --- a/packages/nestjs/src/error-decorator.ts +++ b/packages/nestjs/src/error-decorator.ts @@ -1,33 +1,24 @@ import { captureException } from '@sentry/core'; +import { isExpectedError } from './helpers'; /** - * A decorator usable to wrap user-defined exception filters to add sentry error reporting. + * A decorator to wrap user-defined exception filters and add Sentry error reporting. */ export function WithSentry() { return function (target: unknown, propertyKey: string, descriptor: PropertyDescriptor) { - descriptor.value = new Proxy(descriptor.value, { - apply: (originalCatch, thisArgCatch, argsCatch) => { - const exception = argsCatch[0]; - const exceptionIsObject = typeof exception === 'object' && exception !== null; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const originalCatch = descriptor.value as (exception: unknown, host: unknown, ...args: any[]) => void; - /* - Don't report expected NestJS control flow errors - - `HttpException` errors will have a `status` property - - `RpcException` errors will have an `error` property - */ - if (exceptionStatusCode !== null || exceptionErrorProperty !== null) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return originalCatch.apply(thisArgCatch, argsCatch); - } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + descriptor.value = function (exception: unknown, host: unknown, ...args: any[]) { + if (isExpectedError(exception)) { + return originalCatch.apply(this, args); + } - captureException(exception); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return originalCatch.apply(thisArgCatch, argsCatch); - }, - }); + captureException(exception); + return originalCatch.apply(this, args); + }; + + return descriptor; }; } diff --git a/packages/nestjs/src/helpers.ts b/packages/nestjs/src/helpers.ts new file mode 100644 index 000000000000..34450aa3ac75 --- /dev/null +++ b/packages/nestjs/src/helpers.ts @@ -0,0 +1,13 @@ +/** + * Determines if the exception is an expected control flow error. + * - HttpException errors will have a status property + * - RpcException errors will have an error property + * + * @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; + } + return false; +} diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 0076d6fb6329..f284c4ed7875 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -6,7 +6,7 @@ import type { NestInterceptor, OnModuleInit, } from '@nestjs/common'; -import { Catch, Global, HttpException, Injectable, Module } from '@nestjs/common'; +import { Catch, Global, Injectable, Module } from '@nestjs/common'; import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, @@ -20,6 +20,7 @@ import { import type { Span } from '@sentry/types'; import { logger } from '@sentry/utils'; import type { Observable } from 'rxjs'; +import { isExpectedError } from './helpers'; /** * Note: We cannot use @ syntax to add the decorators, so we add them directly below the classes as function wrappers. @@ -66,16 +67,7 @@ class SentryGlobalFilter extends BaseExceptionFilter { * Catches exceptions and reports them to Sentry unless they are expected errors. */ public catch(exception: unknown, host: ArgumentsHost): void { - const exceptionIsObject = typeof exception === 'object' && exception !== null; - const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null; - - /* - Don't report expected NestJS control flow errors - - `HttpException` - - `RpcException` errors will have an `error` property and we cannot rely directly on the `RpcException` class - because it is part of `@nestjs/microservices`, which is not a dependency for all nest applications - */ - if (exception instanceof HttpException || exceptionErrorProperty !== null) { + if (isExpectedError(exception)) { return super.catch(exception, host); } From 0b49d0dff7eb9a4f70db69adb63296d443f0df24 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 15:41:26 +0200 Subject: [PATCH 34/36] Update readme --- packages/nestjs/README.md | 58 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/nestjs/README.md b/packages/nestjs/README.md index 17f3926789ee..7c71897fb163 100644 --- a/packages/nestjs/README.md +++ b/packages/nestjs/README.md @@ -55,13 +55,47 @@ async function bootstrap() { bootstrap(); ``` -Then you can add the `SentryModule` as a root module: +Then you can add the `SentryModule` as a root module. Also add the `SentryGlobalFilter` if you are not already using any +global catch-all exception filters (annotated with `@Catch()` and registered in your app module providers or with +`app.useGlobalFilters()`): ```typescript import { Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; +import { SentryGlobalFilter, SentryModule } from '@sentry/nestjs/setup'; +import { AppController } from './app.controller'; +import { AppService } from './app.service'; + +@Module({ + imports: [ + SentryModule.forRoot(), + // ...other modules + ], + controllers: [AppController], + providers: [ + AppService, + { + provide: APP_FILTER, + useClass: SentryGlobalFilter, + }, + // ..other providers + ], +}) +export class AppModule {} +``` + +The `SentryGlobalFilter` needs to be registered before any other exception filters. + +If you are already using custom catch-all exception filters, do not add `SentryGlobalFilter` as a provider. Instead, +annotate the catch method in your catch-all exception filter with `WithSentry()`: + +```typescript +import { Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { CatchAllExceptionFilter } from './catch-all.filter'; @Module({ imports: [ @@ -69,11 +103,31 @@ import { AppService } from './app.service'; // ...other modules ], controllers: [AppController], - providers: [AppService], + providers: [ + AppService, + { + provide: APP_FILTER, + useClass: CatchAllExceptionFilter, + }, + // ..other providers + ], }) export class AppModule {} ``` +```typescript +import { ArgumentsHost, Catch, ExceptionFilter } from '@nestjs/common'; +import { WithSentry } from '@sentry/nestjs'; + +@Catch() +export class CatchAllExceptionFilter implements ExceptionFilter { + @WithSentry() + catch(exception: BadRequestException, host: ArgumentsHost): void { + // your implementation here + } +} +``` + ## SentryTraced Use the `@SentryTraced()` decorator to gain additional performance insights for any function within your NestJS From 570de0e261a678d3e58136207d65e8776d714863 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 13 Aug 2024 14:28:42 +0000 Subject: [PATCH 35/36] Update readme --- packages/nestjs/README.md | 67 ++++++++++++++------------------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/packages/nestjs/README.md b/packages/nestjs/README.md index 7c71897fb163..a78a2c45a620 100644 --- a/packages/nestjs/README.md +++ b/packages/nestjs/README.md @@ -55,59 +55,53 @@ async function bootstrap() { bootstrap(); ``` -Then you can add the `SentryModule` as a root module. Also add the `SentryGlobalFilter` if you are not already using any -global catch-all exception filters (annotated with `@Catch()` and registered in your app module providers or with -`app.useGlobalFilters()`): +Afterwards, add the `SentryModule` as a root module to your main module: ```typescript import { Module } from '@nestjs/common'; -import { APP_FILTER } from '@nestjs/core'; -import { SentryGlobalFilter, SentryModule } from '@sentry/nestjs/setup'; -import { AppController } from './app.controller'; -import { AppService } from './app.service'; +import { SentryModule } from '@sentry/nestjs/setup'; @Module({ imports: [ SentryModule.forRoot(), // ...other modules ], - controllers: [AppController], - providers: [ - AppService, - { - provide: APP_FILTER, - useClass: SentryGlobalFilter, - }, - // ..other providers - ], }) export class AppModule {} ``` -The `SentryGlobalFilter` needs to be registered before any other exception filters. +In case you are using a global catch-all exception filter (which is either a filter registered with +`app.useGlobalFilters()` or a filter registered in your app module providers annotated with an empty `@Catch()` +decorator), add a `@WithSentry()` decorator to the `catch()` method of this global error filter. This decorator will +report all unexpected errors that are received by your global error filter to Sentry: -If you are already using custom catch-all exception filters, do not add `SentryGlobalFilter` as a provider. Instead, -annotate the catch method in your catch-all exception filter with `WithSentry()`: +```typescript +import { Catch, ExceptionFilter } from '@nestjs/common'; +import { WithSentry } from '@sentry/nestjs'; + +@Catch() +export class YourCatchAllExceptionFilter implements ExceptionFilter { + @WithSentry() + catch(exception, host): void { + // your implementation here + } +} +``` + +In case you do not have a global catch-all exception filter, add the `SentryGlobalFilter` to the providers of your main +module. This filter will report all unhandled errors that are not caught by any other error filter to Sentry. +**Important:** The `SentryGlobalFilter` needs to be registered before any other exception filters. ```typescript import { Module } from '@nestjs/common'; import { APP_FILTER } from '@nestjs/core'; -import { SentryModule } from '@sentry/nestjs/setup'; -import { AppController } from './app.controller'; -import { AppService } from './app.service'; -import { CatchAllExceptionFilter } from './catch-all.filter'; +import { SentryGlobalFilter } from '@sentry/nestjs/setup'; @Module({ - imports: [ - SentryModule.forRoot(), - // ...other modules - ], - controllers: [AppController], providers: [ - AppService, { provide: APP_FILTER, - useClass: CatchAllExceptionFilter, + useClass: SentryGlobalFilter, }, // ..other providers ], @@ -115,19 +109,6 @@ import { CatchAllExceptionFilter } from './catch-all.filter'; export class AppModule {} ``` -```typescript -import { ArgumentsHost, Catch, ExceptionFilter } from '@nestjs/common'; -import { WithSentry } from '@sentry/nestjs'; - -@Catch() -export class CatchAllExceptionFilter implements ExceptionFilter { - @WithSentry() - catch(exception: BadRequestException, host: ArgumentsHost): void { - // your implementation here - } -} -``` - ## SentryTraced Use the `@SentryTraced()` decorator to gain additional performance insights for any function within your NestJS From 242e62eb9e03cb23f26899b8632083b000e382da Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Tue, 13 Aug 2024 16:33:17 +0200 Subject: [PATCH 36/36] Remove useless files --- .../src/example-global-filter.exception.ts | 5 ----- .../src/example-global.filter.ts | 19 ------------------- .../src/example-local-filter.exception.ts | 5 ----- .../src/example-local.filter.ts | 19 ------------------- 4 files changed, 48 deletions(-) delete mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts delete mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts delete mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts delete mode 100644 dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts deleted file mode 100644 index 41981ba748fe..000000000000 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global-filter.exception.ts +++ /dev/null @@ -1,5 +0,0 @@ -export class ExampleExceptionGlobalFilter extends Error { - constructor() { - super('Original global example exception!'); - } -} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts deleted file mode 100644 index 988696d0e13d..000000000000 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-global.filter.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; -import { Request, Response } from 'express'; -import { ExampleExceptionGlobalFilter } from './example-global-filter.exception'; - -@Catch(ExampleExceptionGlobalFilter) -export class ExampleGlobalFilter implements ExceptionFilter { - catch(exception: BadRequestException, host: ArgumentsHost): void { - const ctx = host.switchToHttp(); - const response = ctx.getResponse(); - const request = ctx.getRequest(); - - response.status(400).json({ - statusCode: 400, - timestamp: new Date().toISOString(), - path: request.url, - message: 'Example exception was handled by global filter!', - }); - } -} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts deleted file mode 100644 index 8f76520a3b94..000000000000 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local-filter.exception.ts +++ /dev/null @@ -1,5 +0,0 @@ -export class ExampleExceptionLocalFilter extends Error { - constructor() { - super('Original local example exception!'); - } -} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts deleted file mode 100644 index 505217f5dcbd..000000000000 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example-local.filter.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { ArgumentsHost, BadRequestException, Catch, ExceptionFilter } from '@nestjs/common'; -import { Request, Response } from 'express'; -import { ExampleExceptionLocalFilter } from './example-local-filter.exception'; - -@Catch(ExampleExceptionLocalFilter) -export class ExampleLocalFilter implements ExceptionFilter { - catch(exception: BadRequestException, host: ArgumentsHost): void { - const ctx = host.switchToHttp(); - const response = ctx.getResponse(); - const request = ctx.getRequest(); - - response.status(400).json({ - statusCode: 400, - timestamp: new Date().toISOString(), - path: request.url, - message: 'Example exception was handled by local filter!', - }); - } -}