From 7c7443241cc682bdfa5e92170771af52a01defa3 Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:10:14 +0200 Subject: [PATCH 1/9] feat(nestjs): add generic global filter and allow specifying application ref in global filter --- .../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 | 31 +++++ .../src/app.resolver.ts | 14 +++ .../src/app.service.ts | 97 ++++++++++++++ .../src/instrument.ts | 8 ++ .../nestjs-basic-with-graphql/src/main.ts | 15 +++ .../start-event-proxy.mjs | 6 + .../tests/errors.test.ts | 118 ++++++++++++++++++ .../tsconfig.build.json | 4 + .../nestjs-basic-with-graphql/tsconfig.json | 22 ++++ package.json | 3 +- packages/nestjs/src/setup.ts | 35 +++++- 17 files changed, 495 insertions(+), 3 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/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..d5ecc381f965 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.module.ts @@ -0,0 +1,31 @@ +import { ApolloDriver } from '@nestjs/apollo'; +import { Logger, Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; +import { GraphQLModule } from '@nestjs/graphql'; +import { SentryGlobalGraphQLFilter, SentryModule } from '@sentry/nestjs/setup'; +import { AppResolver } from './app.resolver'; +import { AppController } from './app.controller'; + +@Module({ + imports: [ + SentryModule.forRoot(), + GraphQLModule.forRoot({ + driver: ApolloDriver, + autoSchemaFile: true, + playground: true, // sets up a playground on https://localhost:3000/graphql + }), + ], + controllers: [AppController], + providers: [ + AppResolver, + { + provide: APP_FILTER, + useClass: SentryGlobalGraphQLFilter, + }, + { + 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..3e4639040a7e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/src/app.service.ts @@ -0,0 +1,97 @@ +import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; +import { RpcException } from '@nestjs/microservices'; +import { Cron, SchedulerRegistry } from '@nestjs/schedule'; +import * as Sentry from '@sentry/nestjs'; +import { SentryCron, SentryTraced } from '@sentry/nestjs'; +import type { MonitorConfig } from '@sentry/types'; + +const monitorConfig: MonitorConfig = { + schedule: { + type: 'crontab', + value: '* * * * *', + }, +}; + +@Injectable() +export class AppService { + constructor(private schedulerRegistry: SchedulerRegistry) {} + + testTransaction() { + Sentry.startSpan({ name: 'test-span' }, () => { + Sentry.startSpan({ name: 'child-span' }, () => {}); + }); + } + + testSpan() { + // span that should not be a child span of the middleware span + Sentry.startSpan({ name: 'test-controller-span' }, () => {}); + } + + 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); + } + + testExpectedRpcException(id: string) { + throw new RpcException(`This is an expected RPC exception with id ${id}`); + } + + @SentryTraced('wait and return a string') + async wait() { + await new Promise(resolve => setTimeout(resolve, 500)); + return 'test'; + } + + async testSpanDecoratorAsync() { + return await this.wait(); + } + + @SentryTraced('return a string') + getString(): { result: string } { + return { result: 'test' }; + } + + async testSpanDecoratorSync() { + const returned = this.getString(); + // Will fail if getString() is async, because returned will be a Promise<> + return returned.result; + } + + /* + Actual cron schedule differs from schedule defined in config because Sentry + only supports minute granularity, but we don't want to wait (worst case) a + full minute for the tests to finish. + */ + @Cron('*/5 * * * * *', { name: 'test-cron-job' }) + @SentryCron('test-cron-slug', monitorConfig) + async testCron() { + console.log('Test cron!'); + } + + async killTestCron() { + this.schedulerRegistry.deleteCronJob('test-cron-job'); + } + + use() { + console.log('Test use!'); + } + + transform() { + console.log('Test transform!'); + } + + intercept() { + console.log('Test intercept!'); + } + + canActivate() { + console.log('Test canActivate!'); + } +} 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..71ce685f4d61 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic-with-graphql/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-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..3a3d817bb1a0 --- /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', 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', 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', 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', transactionEvent => { + return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id'; + }); + + const transactionEventPromise500 = waitForTransaction('nestjs-basic', 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-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/package.json b/package.json index 4b9ad0383c02..75bd2e767901 100644 --- a/package.json +++ b/package.json @@ -155,5 +155,6 @@ "proseWrap": "always", "singleQuote": true, "trailingComma": "all" - } + }, + "packageManager": "yarn@1.22.19+sha1.4ba7fc5c6e704fce2066ecbfb0b0d8976fe62447" } diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 88d58ffea22f..3ff79af0b5ed 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -8,6 +8,7 @@ import type { } from '@nestjs/common'; import { Catch, Global, HttpException, Injectable, Logger, Module } from '@nestjs/common'; import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; +import type { HttpServer } from '@nestjs/common'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -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,36 @@ 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 { + public readonly __SENTRY_INTERNAL__: boolean; + private readonly _httpFilter: SentryGlobalFilter; + private readonly _graphqlFilter: SentryGlobalGraphQLFilter; + + public constructor(applicationRef?: HttpServer) { + this.__SENTRY_INTERNAL__ = true; + this._httpFilter = new SentryGlobalFilter(applicationRef); + this._graphqlFilter = new SentryGlobalGraphQLFilter(); + } + + /** + * Catches exceptions and reports them to Sentry unless they are HttpExceptions. + */ + public catch(exception: unknown, host: ArgumentsHost): void { + if (host.getType<'graphql'>() === 'graphql') { + return this._graphqlFilter.catch(exception, host); + } + + this._httpFilter.catch(exception, host); + } +} +Catch()(SentryGlobalGenericFilter); +export { SentryGlobalGenericFilter }; + /** * Service to set up Sentry performance tracing for Nest.js applications. */ From dff1b71faea3f335ce66542b613ea0160dc7f973 Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:16:42 +0200 Subject: [PATCH 2/9] chore(workflow): add nestjs basic with graphql to build workflow --- .../nestjs-basic-with-graphql/src/app.module.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index d5ecc381f965..9c0ae518dbfe 100644 --- 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 @@ -2,7 +2,7 @@ import { ApolloDriver } from '@nestjs/apollo'; import { Logger, Module } from '@nestjs/common'; import { APP_FILTER } from '@nestjs/core'; import { GraphQLModule } from '@nestjs/graphql'; -import { SentryGlobalGraphQLFilter, SentryModule } from '@sentry/nestjs/setup'; +import { SentryGlobalGenericFilter, SentryModule } from '@sentry/nestjs/setup'; import { AppResolver } from './app.resolver'; import { AppController } from './app.controller'; @@ -20,7 +20,7 @@ import { AppController } from './app.controller'; AppResolver, { provide: APP_FILTER, - useClass: SentryGlobalGraphQLFilter, + useClass: SentryGlobalGenericFilter, }, { provide: Logger, From 948d1519703c8f801c42b6abfb19dbf8170fcc10 Mon Sep 17 00:00:00 2001 From: Sjoert <63722509+Sjoertjuh@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:23:58 +0200 Subject: [PATCH 3/9] Discard changes to package.json --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 75bd2e767901..4b9ad0383c02 100644 --- a/package.json +++ b/package.json @@ -155,6 +155,5 @@ "proseWrap": "always", "singleQuote": true, "trailingComma": "all" - }, - "packageManager": "yarn@1.22.19+sha1.4ba7fc5c6e704fce2066ecbfb0b0d8976fe62447" + } } From 7367fb38ea4464bc073b415b9127380be06341e5 Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:26:23 +0200 Subject: [PATCH 4/9] chore(workflow): add nestjs basic with graphql to build workflow --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 38102ff204c7..e2e1ff826072 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -909,6 +909,7 @@ jobs: 'nestjs-distributed-tracing', 'nestjs-with-submodules', 'nestjs-with-submodules-decorator', + 'nestjs-basic-with-graphql', 'nestjs-graphql', 'node-exports-test-app', 'node-koa', From 1cba3779223959c8196e2e55d2ee1ea00ddecaa9 Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Fri, 13 Sep 2024 11:19:24 +0200 Subject: [PATCH 5/9] chore(nestjs): fix tests & apply linting --- .../src/app.module.ts | 2 +- .../src/app.service.ts | 81 ------------------- packages/nestjs/src/setup.ts | 2 +- 3 files changed, 2 insertions(+), 83 deletions(-) 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 index 9c0ae518dbfe..b458a7f82043 100644 --- 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 @@ -3,8 +3,8 @@ import { Logger, Module } from '@nestjs/common'; import { APP_FILTER } from '@nestjs/core'; import { GraphQLModule } from '@nestjs/graphql'; import { SentryGlobalGenericFilter, SentryModule } from '@sentry/nestjs/setup'; -import { AppResolver } from './app.resolver'; import { AppController } from './app.controller'; +import { AppResolver } from './app.resolver'; @Module({ imports: [ 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 index 3e4639040a7e..79118f937ce2 100644 --- 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 @@ -1,32 +1,7 @@ import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; -import { RpcException } from '@nestjs/microservices'; -import { Cron, SchedulerRegistry } from '@nestjs/schedule'; -import * as Sentry from '@sentry/nestjs'; -import { SentryCron, SentryTraced } from '@sentry/nestjs'; -import type { MonitorConfig } from '@sentry/types'; - -const monitorConfig: MonitorConfig = { - schedule: { - type: 'crontab', - value: '* * * * *', - }, -}; @Injectable() export class AppService { - constructor(private schedulerRegistry: SchedulerRegistry) {} - - testTransaction() { - Sentry.startSpan({ name: 'test-span' }, () => { - Sentry.startSpan({ name: 'child-span' }, () => {}); - }); - } - - testSpan() { - // span that should not be a child span of the middleware span - Sentry.startSpan({ name: 'test-controller-span' }, () => {}); - } - testException(id: string) { throw new Error(`This is an exception with id ${id}`); } @@ -38,60 +13,4 @@ export class AppService { testExpected500Exception(id: string) { throw new HttpException(`This is an expected 500 exception with id ${id}`, HttpStatus.INTERNAL_SERVER_ERROR); } - - testExpectedRpcException(id: string) { - throw new RpcException(`This is an expected RPC exception with id ${id}`); - } - - @SentryTraced('wait and return a string') - async wait() { - await new Promise(resolve => setTimeout(resolve, 500)); - return 'test'; - } - - async testSpanDecoratorAsync() { - return await this.wait(); - } - - @SentryTraced('return a string') - getString(): { result: string } { - return { result: 'test' }; - } - - async testSpanDecoratorSync() { - const returned = this.getString(); - // Will fail if getString() is async, because returned will be a Promise<> - return returned.result; - } - - /* - Actual cron schedule differs from schedule defined in config because Sentry - only supports minute granularity, but we don't want to wait (worst case) a - full minute for the tests to finish. - */ - @Cron('*/5 * * * * *', { name: 'test-cron-job' }) - @SentryCron('test-cron-slug', monitorConfig) - async testCron() { - console.log('Test cron!'); - } - - async killTestCron() { - this.schedulerRegistry.deleteCronJob('test-cron-job'); - } - - use() { - console.log('Test use!'); - } - - transform() { - console.log('Test transform!'); - } - - intercept() { - console.log('Test intercept!'); - } - - canActivate() { - console.log('Test canActivate!'); - } } diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 3ff79af0b5ed..6b642c8d35cc 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -7,8 +7,8 @@ import type { OnModuleInit, } from '@nestjs/common'; import { Catch, Global, HttpException, Injectable, Logger, Module } from '@nestjs/common'; -import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; import type { HttpServer } from '@nestjs/common'; +import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, From 7e1277055969fc14f416b30ea432dce7d1b8ed8d Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Thu, 19 Sep 2024 10:36:07 +0200 Subject: [PATCH 6/9] chore(nestjs): add app service to test app module --- .../nestjs-basic-with-graphql/src/app.module.ts | 2 ++ 1 file changed, 2 insertions(+) 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 index b458a7f82043..e5b25da62cb8 100644 --- 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 @@ -5,6 +5,7 @@ import { GraphQLModule } from '@nestjs/graphql'; import { SentryGlobalGenericFilter, SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppResolver } from './app.resolver'; +import { AppService } from './app.service'; @Module({ imports: [ @@ -17,6 +18,7 @@ import { AppResolver } from './app.resolver'; ], controllers: [AppController], providers: [ + AppService, AppResolver, { provide: APP_FILTER, From 82de516f555cea4608cb1382155366b3bf46e92d Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Fri, 20 Sep 2024 10:16:15 +0200 Subject: [PATCH 7/9] chore(nestjs): rename generic filter and fix tests --- .../nestjs-basic-with-graphql/src/app.module.ts | 7 +------ .../nestjs-basic-with-graphql/src/main.ts | 7 ++++++- .../nestjs-basic-with-graphql/tests/errors.test.ts | 12 ++++++------ packages/nestjs/src/setup.ts | 8 ++++---- 4 files changed, 17 insertions(+), 17 deletions(-) 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 index e5b25da62cb8..7e76a0e0980f 100644 --- 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 @@ -1,8 +1,7 @@ import { ApolloDriver } from '@nestjs/apollo'; import { Logger, Module } from '@nestjs/common'; -import { APP_FILTER } from '@nestjs/core'; import { GraphQLModule } from '@nestjs/graphql'; -import { SentryGlobalGenericFilter, SentryModule } from '@sentry/nestjs/setup'; +import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppResolver } from './app.resolver'; import { AppService } from './app.service'; @@ -20,10 +19,6 @@ import { AppService } from './app.service'; providers: [ AppService, AppResolver, - { - provide: APP_FILTER, - useClass: SentryGlobalGenericFilter, - }, { provide: Logger, useClass: Logger, 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 index 71ce685f4d61..708ba72b11cb 100644 --- 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 @@ -2,13 +2,18 @@ import './instrument'; // Import other modules -import { NestFactory } from '@nestjs/core'; +import { HttpAdapterHost, NestFactory } from '@nestjs/core'; import { AppModule } from './app.module'; +import { SentryGlobalGenericFilter } from '@sentry/nestjs/setup'; 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); } 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 index 3a3d817bb1a0..2071e436e133 100644 --- 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 @@ -2,7 +2,7 @@ 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', event => { + const errorEventPromise = waitForError('nestjs-basic-with-graphql', event => { return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123'; }); @@ -32,7 +32,7 @@ test('Sends exception to Sentry', async ({ baseURL }) => { test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => { let errorEventOccurred = false; - waitForError('nestjs-basic', event => { + 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; } @@ -40,7 +40,7 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => { return event?.transaction === 'GET /test-expected-400-exception/:id'; }); - waitForError('nestjs-basic', event => { + 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; } @@ -48,11 +48,11 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => { return event?.transaction === 'GET /test-expected-500-exception/:id'; }); - const transactionEventPromise400 = waitForTransaction('nestjs-basic', transactionEvent => { + const transactionEventPromise400 = waitForTransaction('nestjs-basic-with-graphql', transactionEvent => { return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id'; }); - const transactionEventPromise500 = waitForTransaction('nestjs-basic', transactionEvent => { + const transactionEventPromise500 = waitForTransaction('nestjs-basic-with-graphql', transactionEvent => { return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id'; }); @@ -71,7 +71,7 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => { }); test('Sends graphql exception to Sentry', async ({ baseURL }) => { - const errorEventPromise = waitForError('nestjs-graphql', event => { + const errorEventPromise = waitForError('nestjs-basic-with-graphql', event => { return !event.type && event.exception?.values?.[0]?.value === 'This is an exception!'; }); diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 6b642c8d35cc..44cae0dc8f53 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -131,24 +131,24 @@ export { SentryGlobalGraphQLFilter }; */ class SentryGlobalGenericFilter { public readonly __SENTRY_INTERNAL__: boolean; - private readonly _httpFilter: SentryGlobalFilter; + private readonly _globalFilter : SentryGlobalFilter; private readonly _graphqlFilter: SentryGlobalGraphQLFilter; public constructor(applicationRef?: HttpServer) { this.__SENTRY_INTERNAL__ = true; - this._httpFilter = new SentryGlobalFilter(applicationRef); + this._globalFilter = new SentryGlobalFilter(applicationRef); this._graphqlFilter = new SentryGlobalGraphQLFilter(); } /** - * Catches exceptions and reports them to Sentry unless they are HttpExceptions. + * 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); } - this._httpFilter.catch(exception, host); + this._globalFilter .catch(exception, host); } } Catch()(SentryGlobalGenericFilter); From 73a1b3b38fcf59f49da240467a59abaa4b77b797 Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Fri, 20 Sep 2024 11:23:17 +0200 Subject: [PATCH 8/9] chore(nestjs): extend global filter for simplicity --- packages/nestjs/src/setup.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 44cae0dc8f53..a18f95417f11 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -129,14 +129,13 @@ export { SentryGlobalGraphQLFilter }; * * This filter is a generic filter that can handle both HTTP and GraphQL exceptions. */ -class SentryGlobalGenericFilter { +class SentryGlobalGenericFilter extends SentryGlobalFilter { public readonly __SENTRY_INTERNAL__: boolean; - private readonly _globalFilter : SentryGlobalFilter; private readonly _graphqlFilter: SentryGlobalGraphQLFilter; public constructor(applicationRef?: HttpServer) { + super(applicationRef); this.__SENTRY_INTERNAL__ = true; - this._globalFilter = new SentryGlobalFilter(applicationRef); this._graphqlFilter = new SentryGlobalGraphQLFilter(); } @@ -148,7 +147,7 @@ class SentryGlobalGenericFilter { return this._graphqlFilter.catch(exception, host); } - this._globalFilter .catch(exception, host); + super.catch(exception, host); } } Catch()(SentryGlobalGenericFilter); From 62998e63484b767061019ab7ff04012a8498781c Mon Sep 17 00:00:00 2001 From: Sjoertjuh <63722509+Sjoertjuh@users.noreply.github.com> Date: Tue, 24 Sep 2024 09:19:26 +0200 Subject: [PATCH 9/9] chore(nestjs): format files --- .../test-applications/nestjs-basic-with-graphql/src/main.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 708ba72b11cb..947539414ddf 100644 --- 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 @@ -3,8 +3,8 @@ import './instrument'; // Import other modules import { HttpAdapterHost, NestFactory } from '@nestjs/core'; -import { AppModule } from './app.module'; import { SentryGlobalGenericFilter } from '@sentry/nestjs/setup'; +import { AppModule } from './app.module'; const PORT = 3030; @@ -12,7 +12,7 @@ async function bootstrap() { const app = await NestFactory.create(AppModule); const { httpAdapter } = app.get(HttpAdapterHost); - app.useGlobalFilters(new SentryGlobalGenericFilter(httpAdapter as any)) + app.useGlobalFilters(new SentryGlobalGenericFilter(httpAdapter as any)); await app.listen(PORT); }