From 5fffd1b1beb2b08de17150296c772d5e593dfde9 Mon Sep 17 00:00:00 2001 From: Sjoert <63722509+Sjoertjuh@users.noreply.github.com> Date: Tue, 24 Sep 2024 11:13:10 +0200 Subject: [PATCH] feat(nestjs): Add `SentryGlobalGenericFilter` and allow specifying application ref in global filter (#13673) --- .github/workflows/build.yml | 1 + .../nestjs-basic-with-graphql/.gitignore | 56 +++++++++ .../nestjs-basic-with-graphql/.npmrc | 2 + .../nestjs-basic-with-graphql/nest-cli.json | 8 ++ .../nestjs-basic-with-graphql/package.json | 50 ++++++++ .../playwright.config.mjs | 7 ++ .../src/app.controller.ts | 22 ++++ .../src/app.module.ts | 28 +++++ .../src/app.resolver.ts | 14 +++ .../src/app.service.ts | 16 +++ .../src/instrument.ts | 8 ++ .../nestjs-basic-with-graphql/src/main.ts | 20 +++ .../start-event-proxy.mjs | 6 + .../tests/errors.test.ts | 118 ++++++++++++++++++ .../tsconfig.build.json | 4 + .../nestjs-basic-with-graphql/tsconfig.json | 22 ++++ packages/nestjs/src/setup.ts | 34 ++++- 17 files changed, 414 insertions(+), 2 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/nest-cli.json create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/package.json create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/playwright.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.controller.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.module.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.resolver.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.service.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/instrument.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/main.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/start-event-proxy.mjs create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tests/errors.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tsconfig.build.json create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tsconfig.json diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9b36572032b1..ebb03dfe89ac 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -923,6 +923,7 @@ jobs: 'nestjs-distributed-tracing', 'nestjs-with-submodules', 'nestjs-with-submodules-decorator', + 'nestjs-basic-with-graphql', 'nestjs-graphql', 'node-exports-test-app', 'node-koa', diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/.gitignore b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/.gitignore new file mode 100644 index 000000000000..4b56acfbebf4 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/.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-basic-with-graphql/.npmrc b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/.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-basic-with-graphql/nest-cli.json b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/nest-cli.json new file mode 100644 index 000000000000..f9aa683b1ad5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/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-basic-with-graphql/package.json b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/package.json new file mode 100644 index 000000000000..45eccd244d9b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/package.json @@ -0,0 +1,50 @@ +{ + "name": "nestjs-basic-with-graphql", + "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": { + "@apollo/server": "^4.10.4", + "@nestjs/apollo": "^12.2.0", + "@nestjs/common": "^10.3.10", + "@nestjs/core": "^10.3.10", + "@nestjs/graphql": "^12.2.0", + "@nestjs/platform-express": "^10.3.10", + "@sentry/nestjs": "^8.21.0", + "graphql": "^16.9.0", + "reflect-metadata": "^0.1.13", + "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-basic-with-graphql/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/playwright.config.mjs new file mode 100644 index 000000000000..31f2b913b58b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/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-basic-with-graphql/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.controller.ts new file mode 100644 index 000000000000..50f9bc266c2d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.controller.ts @@ -0,0 +1,22 @@ +import { Controller, Get, Param } from '@nestjs/common'; +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-400-exception/:id') + async testExpected400Exception(@Param('id') id: string) { + return this.appService.testExpected400Exception(id); + } + + @Get('test-expected-500-exception/:id') + async testExpected500Exception(@Param('id') id: string) { + return this.appService.testExpected500Exception(id); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.module.ts new file mode 100644 index 000000000000..7e76a0e0980f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.module.ts @@ -0,0 +1,28 @@ +import { ApolloDriver } from '@nestjs/apollo'; +import { Logger, Module } from '@nestjs/common'; +import { GraphQLModule } from '@nestjs/graphql'; +import { SentryModule } from '@sentry/nestjs/setup'; +import { AppController } from './app.controller'; +import { AppResolver } from './app.resolver'; +import { AppService } from './app.service'; + +@Module({ + imports: [ + SentryModule.forRoot(), + GraphQLModule.forRoot({ + driver: ApolloDriver, + autoSchemaFile: true, + playground: true, // sets up a playground on https://localhost:3000/graphql + }), + ], + controllers: [AppController], + providers: [ + AppService, + AppResolver, + { + provide: Logger, + useClass: Logger, + }, + ], +}) +export class AppModule {} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.resolver.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.resolver.ts new file mode 100644 index 000000000000..0e4dfc643918 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.resolver.ts @@ -0,0 +1,14 @@ +import { Query, Resolver } from '@nestjs/graphql'; + +@Resolver() +export class AppResolver { + @Query(() => String) + test(): string { + return 'Test endpoint!'; + } + + @Query(() => String) + error(): string { + throw new Error('This is an exception!'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.service.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.service.ts new file mode 100644 index 000000000000..79118f937ce2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.service.ts @@ -0,0 +1,16 @@ +import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; + +@Injectable() +export class AppService { + testException(id: string) { + throw new Error(`This is an exception with id ${id}`); + } + + testExpected400Exception(id: string) { + throw new HttpException(`This is an expected 400 exception with id ${id}`, HttpStatus.BAD_REQUEST); + } + + testExpected500Exception(id: string) { + throw new HttpException(`This is an expected 500 exception with id ${id}`, HttpStatus.INTERNAL_SERVER_ERROR); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/instrument.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/instrument.ts new file mode 100644 index 000000000000..f1f4de865435 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/instrument.ts @@ -0,0 +1,8 @@ +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, +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/main.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/main.ts new file mode 100644 index 000000000000..947539414ddf --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/main.ts @@ -0,0 +1,20 @@ +// Import this first +import './instrument'; + +// Import other modules +import { HttpAdapterHost, NestFactory } from '@nestjs/core'; +import { SentryGlobalGenericFilter } from '@sentry/nestjs/setup'; +import { AppModule } from './app.module'; + +const PORT = 3030; + +async function bootstrap() { + const app = await NestFactory.create(AppModule); + + const { httpAdapter } = app.get(HttpAdapterHost); + app.useGlobalFilters(new SentryGlobalGenericFilter(httpAdapter as any)); + + await app.listen(PORT); +} + +bootstrap(); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/start-event-proxy.mjs new file mode 100644 index 000000000000..7914cd10a146 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'nestjs-basic-with-graphql', +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tests/errors.test.ts new file mode 100644 index 000000000000..2071e436e133 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tests/errors.test.ts @@ -0,0 +1,118 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; + +test('Sends exception to Sentry', async ({ baseURL }) => { + const errorEventPromise = waitForError('nestjs-basic-with-graphql', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123'; + }); + + const response = await fetch(`${baseURL}/test-exception/123`); + expect(response.status).toBe(500); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123'); + + expect(errorEvent.request).toEqual({ + method: 'GET', + cookies: {}, + headers: expect.any(Object), + url: 'http://localhost:3030/test-exception/123', + }); + + expect(errorEvent.transaction).toEqual('GET /test-exception/:id'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); + +test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => { + let errorEventOccurred = false; + + waitForError('nestjs-basic-with-graphql', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 400 exception with id 123') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /test-expected-400-exception/:id'; + }); + + waitForError('nestjs-basic-with-graphql', event => { + if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 500 exception with id 123') { + errorEventOccurred = true; + } + + return event?.transaction === 'GET /test-expected-500-exception/:id'; + }); + + const transactionEventPromise400 = waitForTransaction('nestjs-basic-with-graphql', transactionEvent => { + return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id'; + }); + + const transactionEventPromise500 = waitForTransaction('nestjs-basic-with-graphql', transactionEvent => { + return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id'; + }); + + const response400 = await fetch(`${baseURL}/test-expected-400-exception/123`); + expect(response400.status).toBe(400); + + const response500 = await fetch(`${baseURL}/test-expected-500-exception/123`); + expect(response500.status).toBe(500); + + await transactionEventPromise400; + await transactionEventPromise500; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); + +test('Sends graphql exception to Sentry', async ({ baseURL }) => { + const errorEventPromise = waitForError('nestjs-basic-with-graphql', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an exception!'; + }); + + const response = await fetch(`${baseURL}/graphql`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + query: `query { error }`, + }), + }); + + const json_response = await response.json(); + const errorEvent = await errorEventPromise; + + expect(json_response?.errors[0]).toEqual({ + message: 'This is an exception!', + locations: expect.any(Array), + path: ['error'], + extensions: { + code: 'INTERNAL_SERVER_ERROR', + stacktrace: expect.any(Array), + }, + }); + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception!'); + + expect(errorEvent.request).toEqual({ + method: 'POST', + cookies: {}, + data: '{"query":"query { error }"}', + headers: expect.any(Object), + url: 'http://localhost:3030/graphql', + }); + + expect(errorEvent.transaction).toEqual('POST /graphql'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tsconfig.build.json b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tsconfig.build.json new file mode 100644 index 000000000000..26c30d4eddf2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/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-basic-with-graphql/tsconfig.json b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tsconfig.json new file mode 100644 index 000000000000..cf79f029c781 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/tsconfig.json @@ -0,0 +1,22 @@ +{ + "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, + "moduleResolution": "Node16" + } +} diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 88d58ffea22f..a18f95417f11 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -7,6 +7,7 @@ import type { OnModuleInit, } from '@nestjs/common'; import { Catch, Global, HttpException, Injectable, Logger, Module } from '@nestjs/common'; +import type { HttpServer } from '@nestjs/common'; import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, @@ -67,8 +68,8 @@ export { SentryTracingInterceptor }; class SentryGlobalFilter extends BaseExceptionFilter { public readonly __SENTRY_INTERNAL__: boolean; - public constructor() { - super(); + public constructor(applicationRef?: HttpServer) { + super(applicationRef); this.__SENTRY_INTERNAL__ = true; } @@ -123,6 +124,35 @@ class SentryGlobalGraphQLFilter { Catch()(SentryGlobalGraphQLFilter); export { SentryGlobalGraphQLFilter }; +/** + * Global filter to handle exceptions and report them to Sentry. + * + * This filter is a generic filter that can handle both HTTP and GraphQL exceptions. + */ +class SentryGlobalGenericFilter extends SentryGlobalFilter { + public readonly __SENTRY_INTERNAL__: boolean; + private readonly _graphqlFilter: SentryGlobalGraphQLFilter; + + public constructor(applicationRef?: HttpServer) { + super(applicationRef); + this.__SENTRY_INTERNAL__ = true; + this._graphqlFilter = new SentryGlobalGraphQLFilter(); + } + + /** + * Catches exceptions and forwards them to the according error filter. + */ + public catch(exception: unknown, host: ArgumentsHost): void { + if (host.getType<'graphql'>() === 'graphql') { + return this._graphqlFilter.catch(exception, host); + } + + super.catch(exception, host); + } +} +Catch()(SentryGlobalGenericFilter); +export { SentryGlobalGenericFilter }; + /** * Service to set up Sentry performance tracing for Nest.js applications. */