From 20a8d8c9bdcbda057252e657a3281d36f1de979a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 13 Aug 2024 16:30:42 +0200 Subject: [PATCH 01/22] ci: Fail on cache restore where necessary (#13349) This should at least make it clearer why/where stuff is failing. --- .github/workflows/build.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cd5b7a7447ec..a956f585b414 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -977,6 +977,7 @@ jobs: with: path: ${{ github.workspace }}/packages/*/*.tgz key: ${{ env.BUILD_CACHE_TARBALL_KEY }} + fail-on-cache-miss: true - name: Install Playwright uses: ./.github/actions/install-playwright @@ -1076,6 +1077,7 @@ jobs: with: path: ${{ github.workspace }}/packages/*/*.tgz key: ${{ env.BUILD_CACHE_TARBALL_KEY }} + fail-on-cache-miss: true - name: Install Playwright uses: ./.github/actions/install-playwright @@ -1446,6 +1448,7 @@ jobs: path: ${{ env.CACHED_DEPENDENCY_PATHS }} key: ${{ needs.job_build.outputs.dependency_cache_key }} enableCrossOsArchive: true + fail-on-cache-miss: true - name: Restore build cache uses: actions/cache/restore@v4 @@ -1454,6 +1457,7 @@ jobs: path: ${{ env.CACHED_BUILD_PATHS }} key: ${{ needs.job_build.outputs.dependency_cache_key }} enableCrossOsArchive: true + fail-on-cache-miss: true - name: Configure safe directory run: | From 479668b6906abe75fd2f96091d0e070965082ee7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 13 Aug 2024 16:30:58 +0200 Subject: [PATCH 02/22] ci: Automatically clear caches for closed PRs (#13354) Taken from https://github.com/actions/cache/blob/main/tips-and-workarounds.md#force-deletion-of-caches-overriding-default-cache-eviction-policy. Caches are per-branch, so if a PR is closed we do not need to keep any caches for that branch around anymore. This PR adds automation to do just that. --- .github/workflows/cleanup-pr-caches.yml | 38 +++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .github/workflows/cleanup-pr-caches.yml diff --git a/.github/workflows/cleanup-pr-caches.yml b/.github/workflows/cleanup-pr-caches.yml new file mode 100644 index 000000000000..5ebd0b7aad9d --- /dev/null +++ b/.github/workflows/cleanup-pr-caches.yml @@ -0,0 +1,38 @@ +name: "Automation: Cleanup PR caches" +on: + pull_request: + types: + - closed + +jobs: + cleanup: + runs-on: ubuntu-latest + permissions: + # `actions:write` permission is required to delete caches + # See also: https://docs.github.com/en/rest/actions/cache?apiVersion=2022-11-28#delete-a-github-actions-cache-for-a-repository-using-a-cache-id + actions: write + contents: read + steps: + - name: Check out code + uses: actions/checkout@v4 + + - name: Cleanup + run: | + gh extension install actions/gh-actions-cache + + REPO=${{ github.repository }} + BRANCH=refs/pull/${{ github.event.pull_request.number }}/merge + + echo "Fetching list of cache key" + cacheKeysForPR=$(gh actions-cache list -R $REPO -B $BRANCH | cut -f 1 ) + + ## Setting this to not fail the workflow while deleting cache keys. + set +e + echo "Deleting caches..." + for cacheKey in $cacheKeysForPR + do + gh actions-cache delete $cacheKey -R $REPO -B $BRANCH --confirm + done + echo "Done" + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} From 55220cf189b0d252e267578d66a48d953181953f Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 13 Aug 2024 15:59:06 +0100 Subject: [PATCH 03/22] feat: Add support for SENTRY_SPOTLIGHT env var in Node (#13325) Enable sending events to spotlight by setting the `SENTRY_SPOTLIGHT` environment variable --------- Co-authored-by: Burak Yigit Kaya --- .vscode/settings.json | 2 +- packages/node/src/preload.ts | 7 ++- packages/node/src/sdk/index.ts | 13 ++++- packages/node/src/utils/envToBool.ts | 38 +++++++++++++ packages/node/test/utils/envToBool.test.ts | 66 ++++++++++++++++++++++ 5 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 packages/node/src/utils/envToBool.ts create mode 100644 packages/node/test/utils/envToBool.test.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index 615ca5b24472..4926554ffe4b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -42,5 +42,5 @@ "[typescript]": { "editor.defaultFormatter": "biomejs.biome" }, - "cSpell.words": ["arrayify"] + "cSpell.words": ["arrayify", "OTEL"] } diff --git a/packages/node/src/preload.ts b/packages/node/src/preload.ts index 0d62b28d9c91..0e4af146197f 100644 --- a/packages/node/src/preload.ts +++ b/packages/node/src/preload.ts @@ -1,13 +1,14 @@ import { preloadOpenTelemetry } from './sdk/initOtel'; +import { envToBool } from './utils/envToBool'; -const debug = !!process.env.SENTRY_DEBUG; +const debug = envToBool(process.env.SENTRY_DEBUG); const integrationsStr = process.env.SENTRY_PRELOAD_INTEGRATIONS; const integrations = integrationsStr ? integrationsStr.split(',').map(integration => integration.trim()) : undefined; /** - * The @sentry/node/preload export can be used with the node --import and --require args to preload the OTEL instrumentation, - * without initializing the Sentry SDK. + * The @sentry/node/preload export can be used with the node --import and --require args to preload the OTEL + * instrumentation, without initializing the Sentry SDK. * * This is useful if you cannot initialize the SDK immediately, but still want to preload the instrumentation, * e.g. if you have to load the DSN from somewhere else. diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 1a20458802a0..7276e809875a 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -41,6 +41,7 @@ import { getAutoPerformanceIntegrations } from '../integrations/tracing'; import { makeNodeTransport } from '../transports'; import type { NodeClientOptions, NodeOptions } from '../types'; import { isCjs } from '../utils/commonjs'; +import { envToBool } from '../utils/envToBool'; import { defaultStackParser, getSentryRelease } from './api'; import { NodeClient } from './client'; import { initOpenTelemetry, maybeInitializeEsmLoader } from './initOtel'; @@ -221,6 +222,15 @@ function getClientOptions( ? true : options.autoSessionTracking; + if (options.spotlight == null) { + const spotlightEnv = envToBool(process.env.SENTRY_SPOTLIGHT, { strict: true }); + if (spotlightEnv == null) { + options.spotlight = process.env.SENTRY_SPOTLIGHT; + } else { + options.spotlight = spotlightEnv; + } + } + const tracesSampleRate = getTracesSampleRate(options.tracesSampleRate); const baseOptions = dropUndefinedKeys({ @@ -292,8 +302,7 @@ function getTracesSampleRate(tracesSampleRate: NodeOptions['tracesSampleRate']): * for more details. */ function updateScopeFromEnvVariables(): void { - const sentryUseEnvironment = (process.env.SENTRY_USE_ENVIRONMENT || '').toLowerCase(); - if (!['false', 'n', 'no', 'off', '0'].includes(sentryUseEnvironment)) { + if (envToBool(process.env.SENTRY_USE_ENVIRONMENT) !== false) { const sentryTraceEnv = process.env.SENTRY_TRACE; const baggageEnv = process.env.SENTRY_BAGGAGE; const propagationContext = propagationContextFromHeaders(sentryTraceEnv, baggageEnv); diff --git a/packages/node/src/utils/envToBool.ts b/packages/node/src/utils/envToBool.ts new file mode 100644 index 000000000000..4f7fd2201ce8 --- /dev/null +++ b/packages/node/src/utils/envToBool.ts @@ -0,0 +1,38 @@ +export const FALSY_ENV_VALUES = new Set(['false', 'f', 'n', 'no', 'off', '0']); +export const TRUTHY_ENV_VALUES = new Set(['true', 't', 'y', 'yes', 'on', '1']); + +export type StrictBoolCast = { + strict: true; +}; + +export type LooseBoolCast = { + strict?: false; +}; + +export type BoolCastOptions = StrictBoolCast | LooseBoolCast; + +export function envToBool(value: unknown, options?: LooseBoolCast): boolean; +export function envToBool(value: unknown, options: StrictBoolCast): boolean | null; +export function envToBool(value: unknown, options?: BoolCastOptions): boolean | null; +/** + * A helper function which casts an ENV variable value to `true` or `false` using the constants defined above. + * In strict mode, it may return `null` if the value doesn't match any of the predefined values. + * + * @param value The value of the env variable + * @param options -- Only has `strict` key for now, which requires a strict match for `true` in TRUTHY_ENV_VALUES + * @returns true/false if the lowercase value matches the predefined values above. If not, null in strict mode, + * and Boolean(value) in loose mode. + */ +export function envToBool(value: unknown, options?: BoolCastOptions): boolean | null { + const normalized = String(value).toLowerCase(); + + if (FALSY_ENV_VALUES.has(normalized)) { + return false; + } + + if (TRUTHY_ENV_VALUES.has(normalized)) { + return true; + } + + return options && options.strict ? null : Boolean(value); +} diff --git a/packages/node/test/utils/envToBool.test.ts b/packages/node/test/utils/envToBool.test.ts new file mode 100644 index 000000000000..cd06ef132267 --- /dev/null +++ b/packages/node/test/utils/envToBool.test.ts @@ -0,0 +1,66 @@ +import { envToBool } from '../../src/utils/envToBool'; + +describe('envToBool', () => { + it.each([ + ['', true, null], + ['', false, false], + ['t', true, true], + ['T', true, true], + ['t', false, true], + ['T', false, true], + ['y', true, true], + ['Y', true, true], + ['y', false, true], + ['Y', false, true], + ['1', true, true], + ['1', false, true], + ['true', true, true], + ['true', false, true], + ['tRuE', true, true], + ['tRuE', false, true], + ['Yes', true, true], + ['Yes', false, true], + ['yes', true, true], + ['yes', false, true], + ['yEs', true, true], + ['yEs', false, true], + ['On', true, true], + ['On', false, true], + ['on', true, true], + ['on', false, true], + ['oN', true, true], + ['oN', false, true], + ['f', true, false], + ['f', false, false], + ['n', true, false], + ['N', true, false], + ['n', false, false], + ['N', false, false], + ['0', true, false], + ['0', false, false], + ['false', true, false], + ['false', false, false], + ['false', true, false], + ['false', false, false], + ['FaLsE', true, false], + ['FaLsE', false, false], + ['No', true, false], + ['No', false, false], + ['no', true, false], + ['no', false, false], + ['nO', true, false], + ['nO', false, false], + ['Off', true, false], + ['Off', false, false], + ['off', true, false], + ['off', false, false], + ['oFf', true, false], + ['oFf', false, false], + ['xxx', true, null], + ['xxx', false, true], + [undefined, false, false], + [undefined, true, null], + ])('%s becomes (strict: %s): %s', (value, strict, expected) => { + expect(envToBool(value, { strict })).toBe(expected); + }); +}); From 51ef02d432c88e81a5be275c62706a530f835185 Mon Sep 17 00:00:00 2001 From: Nicolas Hrubec Date: Tue, 13 Aug 2024 17:31:22 +0200 Subject: [PATCH 04/22] fix(nestjs): Exception filters in main app module are not being executed (#13278) [ref](https://github.com/getsentry/sentry-javascript/issues/12351) This PR moves the `SentryGlobalFilter` out of the root module, which led to the filter overriding user exception filters in certain scenarios. Now there is two ways to setup sentry error monitoring: - If users have no catch-all exception filter in their application they add the `SentryGlobalFilter` as a provider in their main module. - If users have a catch-all exception filter (annotated with `@Catch()` they can use the newly introduced `SentryCaptureException()` decorator to capture alle exceptions caught by this filter. Testing: Added a new sample application to test the second setup option and expanded the test suite in general. Side note: - Also removed the `@sentry/microservices` dependency again, since it does not come out of the box with every nest application so we cannot rely on it. --- .../nestjs-basic/src/app.controller.ts | 16 +- .../nestjs-basic/src/app.module.ts | 16 +- .../src/example-global-filter.exception.ts | 5 + .../nestjs-basic/src/example-global.filter.ts | 19 ++ .../src/example-local-filter.exception.ts | 5 + .../nestjs-basic/src/example-local.filter.ts | 19 ++ .../nestjs-basic/tests/errors.test.ts | 70 +++++ .../.gitignore | 56 ++++ .../nestjs-with-submodules-decorator/.npmrc | 2 + .../nest-cli.json | 8 + .../package.json | 47 ++++ .../playwright.config.mjs | 7 + .../src/app.controller.ts | 37 +++ .../src/app.module.ts | 32 +++ .../src/app.service.ts | 14 + .../src/example-global.filter.ts | 20 ++ .../src/example-local.exception.ts | 5 + .../src/example-local.filter.ts | 19 ++ .../example.controller.ts | 6 +- .../example.exception.ts | 2 +- .../example.filter.ts | 8 +- .../example.module.ts | 6 +- .../example.controller.ts | 25 ++ .../example.exception.ts | 5 + .../example.filter.ts | 13 + .../example.module.ts | 16 ++ .../example.controller.ts | 19 ++ .../example.exception.ts | 5 + .../example.filter.ts | 13 + .../example.module.ts | 9 + .../src/example-specific.exception.ts | 5 + .../src/example-specific.filter.ts | 19 ++ .../src/instrument.ts | 12 + .../src/main.ts | 15 + .../start-event-proxy.mjs | 6 + .../tests/errors.test.ts | 266 ++++++++++++++++++ .../tests/transactions.test.ts | 240 ++++++++++++++++ .../tsconfig.build.json | 4 + .../tsconfig.json | 21 ++ .../src/app.controller.ts | 16 +- .../nestjs-with-submodules/src/app.module.ts | 20 +- .../src/example-local.exception.ts | 5 + .../src/example-local.filter.ts | 19 ++ .../example.controller.ts | 17 ++ .../example.exception.ts | 5 + .../example.filter.ts | 13 + .../example.module.ts | 16 ++ .../example.controller.ts | 5 + .../src/example-specific.exception.ts | 5 + .../src/example-specific.filter.ts | 19 ++ .../tests/errors.test.ts | 136 +++++++-- .../tests/transactions.test.ts | 47 +++- packages/nestjs/README.md | 45 ++- packages/nestjs/package.json | 6 +- packages/nestjs/src/error-decorator.ts | 24 ++ packages/nestjs/src/helpers.ts | 13 + packages/nestjs/src/index.ts | 1 + packages/nestjs/src/setup.ts | 20 +- .../nest/sentry-nest-instrumentation.ts | 4 +- yarn.lock | 8 - 60 files changed, 1479 insertions(+), 77 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global-filter.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts 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 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-local.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-local.filter.ts rename dev-packages/e2e-tests/test-applications/{nestjs-with-submodules/src/example-module-global-filter-wrong-registration-order => nestjs-with-submodules-decorator/src/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 => nestjs-with-submodules-decorator/src/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 => nestjs-with-submodules-decorator/src/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 => nestjs-with-submodules-decorator/src/example-module-global-filter-registered-first}/example.module.ts (56%) 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/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-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 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 create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.controller.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.exception.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.filter.ts create mode 100644 dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/example-module-global-filter-registered-first/example.module.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 create mode 100644 packages/nestjs/src/error-decorator.ts create mode 100644 packages/nestjs/src/helpers.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..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,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/nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.module.ts index b2aad014c745..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,14 +1,26 @@ 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'; import { ExampleMiddleware } from './example.middleware'; @Module({ imports: [SentryModule.forRoot(), ScheduleModule.forRoot()], controllers: [AppController], - providers: [AppService], + providers: [ + AppService, + { + provide: APP_FILTER, + useClass: SentryGlobalFilter, + }, + { + provide: APP_FILTER, + useClass: ExampleGlobalFilter, + }, + ], }) export class AppModule { configure(consumer: MiddlewareConsumer): void { 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-global.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example-global.filter.ts new file mode 100644 index 000000000000..988696d0e13d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/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/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..505217f5dcbd --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/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/nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/errors.test.ts index 34e626cb8c52..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 @@ -94,3 +94,73 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => { expect(errorEventOccurred).toBe(false); }); + +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(); + + 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!', + }); + + await transactionEventPromise; + + (await fetch(`${baseURL}/flush`)).text(); + + expect(errorEventOccurred).toBe(false); +}); + +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(); + + 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-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..9cc3641d2322 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/package.json @@ -0,0 +1,47 @@ +{ + "name": "nestjs-with-submodules-decorator", + "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..efce824a20c3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.controller.ts @@ -0,0 +1,37 @@ +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) {} + + @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(); + } + + @Get('example-exception-specific-filter') + 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/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts new file mode 100644 index 000000000000..77cf85a4fa9c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/app.module.ts @@ -0,0 +1,32 @@ +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'; +import { ExampleSpecificFilter } from './example-specific.filter'; + +@Module({ + imports: [ + ExampleModuleGlobalFilterRegisteredFirst, + SentryModule.forRoot(), + ExampleModuleGlobalFilter, + ExampleModuleLocalFilter, + ], + controllers: [AppController], + providers: [ + AppService, + { + 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/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..cee50d0d2c7c --- /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 { WithSentry } from '@sentry/nestjs'; +import { Request, Response } from 'express'; + +@Catch() +export class ExampleWrappedGlobalFilter implements ExceptionFilter { + @WithSentry() + 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-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/src/example-module-global-filter-wrong-registration-order/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-decorator/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-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..11a0470ace17 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/src/example-module-local-filter/example.controller.ts @@ -0,0 +1,19 @@ +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(); + } + + @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-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/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/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..3c205b59a2d1 --- /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-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 new file mode 100644 index 000000000000..94c742dd210a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/errors.test.ts @@ -0,0 +1,266 @@ +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-decorator', 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 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, +}) => { + 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-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-decorator', 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-decorator', 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-decorator', 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-decorator', 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-decorator', 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-decorator', 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); +}); + +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); +}); + +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-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..740ab6f5650c --- /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-decorator', 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-decorator', 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-decorator', 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-decorator', 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 + } +} 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..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,8 +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) {} @@ -20,4 +24,14 @@ export class AppController { async flush() { await flush(); } + + @Get('example-exception-specific-filter') + 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/app.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/src/app.module.ts index 212b17a3556b..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 @@ -1,19 +1,31 @@ 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 { 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'; +import { ExampleSpecificFilter } from './example-specific.filter'; @Module({ imports: [ - ExampleModuleGlobalFilterWrongRegistrationOrder, + ExampleModuleGlobalFilterRegisteredFirst, SentryModule.forRoot(), ExampleModuleGlobalFilter, ExampleModuleLocalFilter, ], controllers: [AppController], - providers: [AppService], + providers: [ + AppService, + { + 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-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/src/example-module-global-filter-registered-first/example.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/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/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/src/example-module-global-filter-registered-first/example.exception.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/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/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/src/example-module-global-filter-registered-first/example.filter.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/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/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/src/example-module-global-filter-registered-first/example.module.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/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/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/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/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 6fbc9f2c1f32..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 @@ -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, }) => { @@ -36,7 +64,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 +76,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), @@ -116,33 +144,99 @@ 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!'; + 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 response = await fetch(`${baseURL}/example-module-wrong-order/expected-exception`); - expect(response.status).toBe(500); // should be 400 + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-module-registered-first/expected-exception'; + }); - // should never arrive, but does because the exception is not handled properly - const errorEvent = await errorEventPromise; + const response = await fetch(`${baseURL}/example-module-registered-first/expected-exception`); + expect(response.status).toBe(400); - expect(errorEvent.exception?.values).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.value).toBe('Something went wrong in the example module!'); + await transactionEventPromise; - expect(errorEvent.request).toEqual({ - method: 'GET', - cookies: {}, - headers: expect.any(Object), - url: 'http://localhost:3030/example-module-wrong-order/expected-exception', + (await fetch(`${baseURL}/flush`)).text(); + + 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'; }); - expect(errorEvent.transaction).toEqual('GET /example-module-wrong-order/expected-exception'); + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return transactionEvent?.transaction === 'GET /example-exception-specific-filter'; + }); - expect(errorEvent.contexts?.trace).toEqual({ - trace_id: expect.any(String), - span_id: expect.any(String), + 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); +}); + +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); }); 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..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', 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', 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 +199,42 @@ 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', + }, + ]), + }), + ); +}); diff --git a/packages/nestjs/README.md b/packages/nestjs/README.md index 9e0192551afc..a78a2c45a620 100644 --- a/packages/nestjs/README.md +++ b/packages/nestjs/README.md @@ -55,26 +55,59 @@ async function bootstrap() { bootstrap(); ``` -Then you can add the `SentryModule` as a root module: +Afterwards, add the `SentryModule` as a root module to your main module: ```typescript import { Module } from '@nestjs/common'; import { 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], }) export class AppModule {} ``` -The `SentryModule` needs to be registered before any module that should be instrumented by Sentry. +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: + +```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 { SentryGlobalFilter } from '@sentry/nestjs/setup'; + +@Module({ + providers: [ + { + provide: APP_FILTER, + useClass: SentryGlobalFilter, + }, + // ..other providers + ], +}) +export class AppModule {} +``` ## SentryTraced diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index f4ea1ff1c471..77234da5d263 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/packages/nestjs/src/error-decorator.ts b/packages/nestjs/src/error-decorator.ts new file mode 100644 index 000000000000..bf1fd08d8cee --- /dev/null +++ b/packages/nestjs/src/error-decorator.ts @@ -0,0 +1,24 @@ +import { captureException } from '@sentry/core'; +import { isExpectedError } from './helpers'; + +/** + * A decorator to wrap user-defined exception filters and add Sentry error reporting. + */ +export function WithSentry() { + return function (target: unknown, propertyKey: string, descriptor: PropertyDescriptor) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const originalCatch = descriptor.value as (exception: unknown, host: unknown, ...args: any[]) => void; + + // 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); + 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/index.ts b/packages/nestjs/src/index.ts index 00519cf49b9e..b30fe547103b 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 { WithSentry } from './error-decorator'; diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 5e76f5cbe912..f284c4ed7875 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -6,12 +6,8 @@ import type { NestInterceptor, OnModuleInit, } from '@nestjs/common'; -import { HttpException } from '@nestjs/common'; -import { Catch } from '@nestjs/common'; -import { Injectable } from '@nestjs/common'; -import { Global, Module } from '@nestjs/common'; -import { APP_FILTER, APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; -import { RpcException } from '@nestjs/microservices'; +import { Catch, Global, Injectable, Module } from '@nestjs/common'; +import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -24,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. @@ -70,8 +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 { - // don't report expected errors - if (exception instanceof HttpException || exception instanceof RpcException) { + if (isExpectedError(exception)) { return super.catch(exception, host); } @@ -118,10 +114,6 @@ class SentryModule { module: SentryModule, providers: [ SentryService, - { - provide: APP_FILTER, - useClass: SentryGlobalFilter, - }, { provide: APP_INTERCEPTOR, useClass: SentryTracingInterceptor, @@ -135,10 +127,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/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'; diff --git a/yarn.lock b/yarn.lock index 5603f142ea95..cc4c66703b49 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 7093d92725911aae667dbb8f3118635ce5ddd98d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 14 Aug 2024 01:37:17 +0000 Subject: [PATCH 05/22] feat(deps): bump @prisma/instrumentation from 5.17.0 to 5.18.0 (#13327) --- packages/node/package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index 9ec4c8a7e389..3b5546540ef8 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -88,7 +88,7 @@ "@opentelemetry/resources": "^1.25.1", "@opentelemetry/sdk-trace-base": "^1.25.1", "@opentelemetry/semantic-conventions": "^1.25.1", - "@prisma/instrumentation": "5.17.0", + "@prisma/instrumentation": "5.18.0", "@sentry/core": "8.25.0", "@sentry/opentelemetry": "8.25.0", "@sentry/types": "8.25.0", diff --git a/yarn.lock b/yarn.lock index cc4c66703b49..481175f8db61 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7525,10 +7525,10 @@ resolved "https://registry.yarnpkg.com/@prisma/client/-/client-5.9.1.tgz#d92bd2f7f006e0316cb4fda9d73f235965cf2c64" integrity sha512-caSOnG4kxcSkhqC/2ShV7rEoWwd3XrftokxJqOCMVvia4NYV/TPtJlS9C2os3Igxw/Qyxumj9GBQzcStzECvtQ== -"@prisma/instrumentation@5.17.0": - version "5.17.0" - resolved "https://registry.yarnpkg.com/@prisma/instrumentation/-/instrumentation-5.17.0.tgz#f741ff517f54b1a896fb8605e0d702f29855c6cb" - integrity sha512-c1Sle4ji8aasMcYfBBHFM56We4ljfenVtRmS8aY06BllS7SoU6SmJBwG7vil+GHiR0Yrh+t9iBwt4AY0Jr4KNQ== +"@prisma/instrumentation@5.18.0": + version "5.18.0" + resolved "https://registry.yarnpkg.com/@prisma/instrumentation/-/instrumentation-5.18.0.tgz#8b49e25bf3f8f756eb0c4c199b4cf8b6631db891" + integrity sha512-r074avGkpPXItk+josQPhufZEmGhUCb16PQx4ITPS40vWTpTPET4VsgCBZB2alIN6SS7pRFod2vz2M2HHEEylQ== dependencies: "@opentelemetry/api" "^1.8" "@opentelemetry/instrumentation" "^0.49 || ^0.50 || ^0.51 || ^0.52.0" From ecc299dab7ab9d9d9c6338d70413f1e0fbbb6dfb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 14 Aug 2024 09:57:18 +0200 Subject: [PATCH 06/22] ci: Streamline some caching (#13355) This streamlines some caching stuff for CI: 1. Extract dependency installation & cache out into a composite action for reusability 2. Updated the cache key for dependencies to only include package & dev-package `package.json`, not the E2E test ones. --- .../actions/install-dependencies/action.yml | 29 ++++++++++++++ .github/workflows/build.yml | 40 ++++--------------- 2 files changed, 36 insertions(+), 33 deletions(-) create mode 100644 .github/actions/install-dependencies/action.yml diff --git a/.github/actions/install-dependencies/action.yml b/.github/actions/install-dependencies/action.yml new file mode 100644 index 000000000000..8cb80ac7440e --- /dev/null +++ b/.github/actions/install-dependencies/action.yml @@ -0,0 +1,29 @@ +name: "Install yarn dependencies" +description: "Installs yarn dependencies and caches them." + +outputs: + cache_key: + description: "The dependency cache key" + value: ${{ steps.compute_lockfile_hash.outputs.hash }} + +runs: + using: "composite" + steps: + # we use a hash of yarn.lock as our cache key, because if it hasn't changed, our dependencies haven't changed, + # so no need to reinstall them + - name: Compute dependency cache key + id: compute_lockfile_hash + run: echo "hash=dependencies-${{ hashFiles('yarn.lock', 'packages/*/package.json', 'dev-packages/*/package.json') }}" >> "$GITHUB_OUTPUT" + shell: bash + + - name: Check dependency cache + uses: actions/cache@v4 + id: cache_dependencies + with: + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + key: ${{ steps.compute_lockfile_hash.outputs.hash }} + + - name: Install dependencies + if: steps.cache_dependencies.outputs.cache-hit != 'true' + run: yarn install --ignore-engines --frozen-lockfile + shell: bash diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a956f585b414..d8ccc2c75dec 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -146,22 +146,9 @@ jobs: with: node-version-file: 'package.json' - # we use a hash of yarn.lock as our cache key, because if it hasn't changed, our dependencies haven't changed, - # so no need to reinstall them - - name: Compute dependency cache key - id: compute_lockfile_hash - run: echo "hash=${{ hashFiles('yarn.lock', '**/package.json') }}" >> "$GITHUB_OUTPUT" - - - name: Check dependency cache - uses: actions/cache@v4 - id: cache_dependencies - with: - path: ${{ env.CACHED_DEPENDENCY_PATHS }} - key: ${{ steps.compute_lockfile_hash.outputs.hash }} - - - name: Install dependencies - if: steps.cache_dependencies.outputs.cache-hit != 'true' - run: yarn install --ignore-engines --frozen-lockfile + - name: Install Dependencies + uses: ./.github/actions/install-dependencies + id: install_dependencies - name: Check for Affected Nx Projects uses: dkhunt27/action-nx-affected-list@v5.3 @@ -200,7 +187,7 @@ jobs: run: yarn build outputs: - dependency_cache_key: ${{ steps.compute_lockfile_hash.outputs.hash }} + dependency_cache_key: ${{ steps.install_dependencies.outputs.cache_key }} changed_node_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/node-integration-tests') }} changed_remix: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/remix') }} changed_node: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/node') }} @@ -293,22 +280,9 @@ jobs: with: node-version-file: 'package.json' - # we use a hash of yarn.lock as our cache key, because if it hasn't changed, our dependencies haven't changed, - # so no need to reinstall them - - name: Compute dependency cache key - id: compute_lockfile_hash - run: echo "hash=${{ hashFiles('yarn.lock', '**/package.json') }}" >> "$GITHUB_OUTPUT" - - - name: Check dependency cache - uses: actions/cache@v4 - id: cache_dependencies - with: - path: ${{ env.CACHED_DEPENDENCY_PATHS }} - key: ${{ steps.compute_lockfile_hash.outputs.hash }} - - - name: Install dependencies - if: steps.cache_dependencies.outputs.cache-hit != 'true' - run: yarn install --ignore-engines --frozen-lockfile + - name: Install Dependencies + uses: ./.github/actions/install-dependencies + id: install_dependencies - name: Check file formatting run: yarn lint:prettier && yarn lint:biome From 921e529321096da48b4627e96be11f9c67bcc27b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 14 Aug 2024 10:24:48 +0200 Subject: [PATCH 07/22] ci: Streamline browser & node unit tests (#13307) Instead of having to keep to separate lists of include/excludes, we now keep this in a single list and invert it when necessary. This way, we should no longer have problems where tests are either run multiple times, or not in the correct env - just add the test to the `browser` list in `ci-unit-tests.ts` to make sure it is not run in multiple node versions unnecessarily. I also added this step to the new package checklist. --- docs/new-sdk-release-checklist.md | 2 +- package.json | 8 +- .../{node-unit-tests.ts => ci-unit-tests.ts} | 92 +++++++++++++------ 3 files changed, 70 insertions(+), 32 deletions(-) rename scripts/{node-unit-tests.ts => ci-unit-tests.ts} (54%) diff --git a/docs/new-sdk-release-checklist.md b/docs/new-sdk-release-checklist.md index 1292c5363fb0..7f1811e53d8f 100644 --- a/docs/new-sdk-release-checklist.md +++ b/docs/new-sdk-release-checklist.md @@ -45,8 +45,8 @@ differ slightly for other SDKs depending on how they are structured and how they - [ ] Make sure `build.yml` CI script is correctly set up to cover tests for the new package - - [ ] Ensure dependent packages are correctly set for “Determine changed packages” - [ ] Ensure unit tests run correctly + - [ ] If it is a browser SDK, add it to `BROWSER_TEST_PACKAGES` in `scripts/ci-unit-tests.ts` - [ ] Make sure the file paths in the ["Upload Artifacts" job](https://github.com/getsentry/sentry-javascript/blob/e5c1486eed236b878f2c49d6a04be86093816ac9/.github/workflows/build.yml#L314-L349) diff --git a/package.json b/package.json index ebf4021a7a6a..ae9491de49f2 100644 --- a/package.json +++ b/package.json @@ -35,10 +35,10 @@ "test:unit": "lerna run --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\" test:unit", "test:update-snapshots": "lerna run test:update-snapshots", "test:pr": "nx affected -t test --exclude \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\"", - "test:pr:browser": "yarn test:pr --exclude \"@sentry/{core,utils,opentelemetry,bun,deno,node,profiling-node,aws-serverless,google-cloud-serverless,nextjs,nestjs,astro,cloudflare,solidstart,nuxt,remix,gatsby,sveltekit,vercel-edge}\"", - "test:pr:node": "ts-node ./scripts/node-unit-tests.ts --affected", - "test:ci:browser": "lerna run test --ignore \"@sentry/{core,utils,opentelemetry,bun,deno,node,profiling-node,aws-serverless,google-cloud-serverless,nextjs,nestjs,astro,cloudflare,solidstart,nuxt,remix,gatsby,sveltekit,vercel-edge}\" --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\"", - "test:ci:node": "ts-node ./scripts/node-unit-tests.ts", + "test:pr:browser": "UNIT_TEST_ENV=browser ts-node ./scripts/ci-unit-tests.ts --affected", + "test:pr:node": "UNIT_TEST_ENV=node ts-node ./scripts/ci-unit-tests.ts --affected", + "test:ci:browser": "UNIT_TEST_ENV=browser ts-node ./scripts/ci-unit-tests.ts", + "test:ci:node": "UNIT_TEST_ENV=node ts-node ./scripts/ci-unit-tests.ts", "test:ci:bun": "lerna run test --scope @sentry/bun", "yalc:publish": "lerna run yalc:publish" }, diff --git a/scripts/node-unit-tests.ts b/scripts/ci-unit-tests.ts similarity index 54% rename from scripts/node-unit-tests.ts rename to scripts/ci-unit-tests.ts index e9f22da1309a..ea771b29a957 100644 --- a/scripts/node-unit-tests.ts +++ b/scripts/ci-unit-tests.ts @@ -1,4 +1,6 @@ import * as childProcess from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; type NodeVersion = '14' | '16' | '18' | '20' | '21'; @@ -6,12 +8,17 @@ interface VersionConfig { ignoredPackages: Array<`@${'sentry' | 'sentry-internal'}/${string}`>; } +const UNIT_TEST_ENV = process.env.UNIT_TEST_ENV as 'node' | 'browser' | undefined; + const CURRENT_NODE_VERSION = process.version.replace('v', '').split('.')[0] as NodeVersion; const RUN_AFFECTED = process.argv.includes('--affected'); -const DEFAULT_SKIP_TESTS_PACKAGES = [ - '@sentry-internal/eslint-plugin-sdk', +// These packages are tested separately in CI, so no need to run them here +const DEFAULT_SKIP_PACKAGES = ['@sentry/profiling-node', '@sentry/bun', '@sentry/deno']; + +// All other packages are run for multiple node versions +const BROWSER_TEST_PACKAGES = [ '@sentry/ember', '@sentry/browser', '@sentry/vue', @@ -26,10 +33,9 @@ const DEFAULT_SKIP_TESTS_PACKAGES = [ '@sentry-internal/replay-worker', '@sentry-internal/feedback', '@sentry/wasm', - '@sentry/bun', - '@sentry/deno', ]; +// These are Node-version specific tests that need to be skipped because of support const SKIP_TEST_PACKAGES: Record = { '14': { ignoredPackages: [ @@ -40,6 +46,7 @@ const SKIP_TEST_PACKAGES: Record = { '@sentry/astro', '@sentry/nuxt', '@sentry/nestjs', + '@sentry-internal/eslint-plugin-sdk', ], }, '16': { @@ -56,6 +63,50 @@ const SKIP_TEST_PACKAGES: Record = { }, }; +function getAllPackages(): string[] { + const { workspaces }: { workspaces: string[] } = JSON.parse( + fs.readFileSync(path.join(process.cwd(), 'package.json'), 'utf-8'), + ); + + return workspaces.map(workspacePath => { + const { name }: { name: string } = JSON.parse( + fs.readFileSync(path.join(process.cwd(), workspacePath, 'package.json'), 'utf-8'), + ); + return name; + }); +} + +/** + * Run the tests, accounting for compatibility problems in older versions of Node. + */ +function runTests(): void { + const ignores = new Set(DEFAULT_SKIP_PACKAGES); + + const packages = getAllPackages(); + + if (UNIT_TEST_ENV === 'browser') { + // Since we cannot "include" for affected mode, we instead exclude all other packages + packages.forEach(pkg => { + if (!BROWSER_TEST_PACKAGES.includes(pkg)) { + ignores.add(pkg); + } + }); + } else if (UNIT_TEST_ENV === 'node') { + BROWSER_TEST_PACKAGES.forEach(pkg => ignores.add(pkg)); + } + + const versionConfig = SKIP_TEST_PACKAGES[CURRENT_NODE_VERSION]; + if (versionConfig) { + versionConfig.ignoredPackages.forEach(dep => ignores.add(dep)); + } + + if (RUN_AFFECTED) { + runAffectedTests(ignores); + } else { + runAllTests(ignores); + } +} + /** * Run the given shell command, piping the shell process's `stdin`, `stdout`, and `stderr` to that of the current * process. Returns contents of `stdout`. @@ -67,41 +118,28 @@ function run(cmd: string, options?: childProcess.ExecSyncOptions): void { /** * Run tests, ignoring the given packages */ -function runWithIgnores(skipPackages: string[] = []): void { - const ignoreFlags = skipPackages.map(dep => `--ignore="${dep}"`).join(' '); +function runAllTests(ignorePackages: Set): void { + const ignoreFlags = Array.from(ignorePackages) + .map(dep => `--ignore="${dep}"`) + .join(' '); + run(`yarn test ${ignoreFlags}`); } /** * Run affected tests, ignoring the given packages */ -function runAffectedWithIgnores(skipPackages: string[] = []): void { +function runAffectedTests(ignorePackages: Set): void { const additionalArgs = process.argv .slice(2) .filter(arg => arg !== '--affected') .join(' '); - const ignoreFlags = skipPackages.map(dep => `--exclude="${dep}"`).join(' '); - run(`yarn test:pr ${ignoreFlags} ${additionalArgs}`); -} - -/** - * Run the tests, accounting for compatibility problems in older versions of Node. - */ -function runTests(): void { - const ignores = new Set(); - - DEFAULT_SKIP_TESTS_PACKAGES.forEach(pkg => ignores.add(pkg)); - const versionConfig = SKIP_TEST_PACKAGES[CURRENT_NODE_VERSION]; - if (versionConfig) { - versionConfig.ignoredPackages.forEach(dep => ignores.add(dep)); - } + const excludeFlags = Array.from(ignorePackages) + .map(dep => `--exclude="${dep}"`) + .join(' '); - if (RUN_AFFECTED) { - runAffectedWithIgnores(Array.from(ignores)); - } else { - runWithIgnores(Array.from(ignores)); - } + run(`yarn test:pr ${excludeFlags} ${additionalArgs}`); } runTests(); From e3d73caa86425e04fad476a642b67a88a27c5187 Mon Sep 17 00:00:00 2001 From: Andrei <168741329+andreiborza@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:18:52 +0200 Subject: [PATCH 08/22] fix(solidstart): Make back navigation test less flaky (#13374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤞 Closes: #13345 --- .../solidstart/src/routes/back-navigation.tsx | 9 +++++++++ .../test-applications/solidstart/src/routes/index.tsx | 4 +--- .../solidstart/tests/performance.client.test.ts | 10 ++++++---- 3 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/solidstart/src/routes/back-navigation.tsx diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/back-navigation.tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/back-navigation.tsx new file mode 100644 index 000000000000..ddd970944bf3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/back-navigation.tsx @@ -0,0 +1,9 @@ +import { A } from '@solidjs/router'; + +export default function BackNavigation() { + return ( + + User 6 + + ); +} diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/index.tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/index.tsx index 873d542e4bae..eed722cba4e3 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/index.tsx +++ b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/index.tsx @@ -20,9 +20,7 @@ export default function Home() {
  • - - User 6 - + Test back navigation
    • diff --git a/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.client.test.ts b/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.client.test.ts index 6e5f43e016c8..52d9cb219401 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.client.test.ts @@ -54,8 +54,8 @@ test('updates the transaction when using the back button', async ({ page }) => { return transactionEvent?.transaction === '/users/6' && transactionEvent.contexts?.trace?.op === 'navigation'; }); - await page.goto(`/`); - await page.locator('#navLinkUserBack').click(); + await page.goto(`/back-navigation`); + await page.locator('#navLink').click(); const navigationTxn = await navigationTxnPromise; expect(navigationTxn).toMatchObject({ @@ -72,7 +72,9 @@ test('updates the transaction when using the back button', async ({ page }) => { }); const backNavigationTxnPromise = waitForTransaction('solidstart', async transactionEvent => { - return transactionEvent?.transaction === '/' && transactionEvent.contexts?.trace?.op === 'navigation'; + return ( + transactionEvent?.transaction === '/back-navigation' && transactionEvent.contexts?.trace?.op === 'navigation' + ); }); await page.goBack(); @@ -85,7 +87,7 @@ test('updates the transaction when using the back button', async ({ page }) => { origin: 'auto.navigation.solidstart.solidrouter', }, }, - transaction: '/', + transaction: '/back-navigation', transaction_info: { source: 'url', }, From 140b81d752cc6c13da1374a4459356d72c0c5f23 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 14 Aug 2024 11:45:21 +0200 Subject: [PATCH 09/22] ci: Only store playwright cache on develop (#13358) Should reduce amount of cached data. --- .github/actions/install-playwright/action.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/actions/install-playwright/action.yml b/.github/actions/install-playwright/action.yml index 7f85f5e743ba..9de6e1a2b104 100644 --- a/.github/actions/install-playwright/action.yml +++ b/.github/actions/install-playwright/action.yml @@ -13,14 +13,23 @@ runs: run: echo "version=$(node -p "require('@playwright/test/package.json').version")" >> $GITHUB_OUTPUT shell: bash - - name: Cache playwright binaries - uses: actions/cache@v4 + - name: Restore cached playwright binaries + uses: actions/cache/restore@v4 id: playwright-cache with: path: | ~/.cache/ms-playwright key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} + # Only store cache on develop branch + - name: Store cached playwright binaries + uses: actions/cache/save@v4 + if: github.event_name == 'push' && github.ref == 'refs/heads/develop' + with: + path: | + ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} + # We always install all browsers, if uncached - name: Install Playwright dependencies (uncached) run: npx playwright install chromium webkit firefox --with-deps From 63c4a90975ea6a371166dec9e5264957379702aa Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Wed, 14 Aug 2024 11:55:57 +0200 Subject: [PATCH 10/22] feat: Add options for passing nonces to feedback integration (#13347) --- .../feedback/captureFeedbackCsp/init.js | 19 +++++ .../feedback/captureFeedbackCsp/template.html | 13 +++ .../feedback/captureFeedbackCsp/test.ts | 84 +++++++++++++++++++ .../browser/src/utils/lazyLoadIntegration.ts | 9 +- .../feedback/src/core/components/Actor.css.ts | 6 +- .../feedback/src/core/components/Actor.ts | 5 +- .../feedback/src/core/createMainStyles.ts | 11 ++- packages/feedback/src/core/integration.ts | 12 ++- .../src/modal/components/Dialog.css.ts | 6 +- packages/feedback/src/modal/integration.tsx | 2 +- .../components/ScreenshotEditor.tsx | 4 +- .../components/ScreenshotInput.css.ts | 6 +- packages/types/src/feedback/config.ts | 10 +++ 13 files changed, 175 insertions(+), 12 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/init.js create mode 100644 dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/template.html create mode 100644 dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/init.js b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/init.js new file mode 100644 index 000000000000..067dbec23fd4 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/init.js @@ -0,0 +1,19 @@ +import * as Sentry from '@sentry/browser'; +// Import this separately so that generatePlugin can handle it for CDN scenarios +import { feedbackIntegration } from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + feedbackIntegration({ tags: { from: 'integration init' }, styleNonce: 'foo1234', scriptNonce: 'foo1234' }), + ], +}); + +document.addEventListener('securitypolicyviolation', () => { + const container = document.querySelector('#csp-violation'); + if (container) { + container.innerText = 'CSP Violation'; + } +}); diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/template.html b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/template.html new file mode 100644 index 000000000000..919f372ef468 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/template.html @@ -0,0 +1,13 @@ + + + + + + + +
    + + diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts new file mode 100644 index 000000000000..95a8a2eacee8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts @@ -0,0 +1,84 @@ +import { expect } from '@playwright/test'; + +import { TEST_HOST, sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../utils/helpers'; + +sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => { + if (shouldSkipFeedbackTest()) { + sentryTest.skip(); + } + + const feedbackRequestPromise = page.waitForResponse(res => { + const req = res.request(); + + const postData = req.postData(); + if (!postData) { + return false; + } + + try { + return getEnvelopeType(req) === 'feedback'; + } catch (err) { + return false; + } + }); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await page.getByText('Report a Bug').click(); + expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1); + await page.locator('[name="name"]').fill('Jane Doe'); + await page.locator('[name="email"]').fill('janedoe@example.org'); + await page.locator('[name="message"]').fill('my example feedback'); + await page.locator('[data-sentry-feedback] .btn--primary').click(); + + const feedbackEvent = envelopeRequestParser((await feedbackRequestPromise).request()); + expect(feedbackEvent).toEqual({ + type: 'feedback', + breadcrumbs: expect.any(Array), + contexts: { + feedback: { + contact_email: 'janedoe@example.org', + message: 'my example feedback', + name: 'Jane Doe', + source: 'widget', + url: `${TEST_HOST}/index.html`, + }, + trace: { + trace_id: expect.stringMatching(/\w{32}/), + span_id: expect.stringMatching(/\w{16}/), + }, + }, + level: 'info', + tags: { + from: 'integration init', + }, + timestamp: expect.any(Number), + event_id: expect.stringMatching(/\w{32}/), + environment: 'production', + sdk: { + integrations: expect.arrayContaining(['Feedback']), + version: expect.any(String), + name: 'sentry.javascript.browser', + packages: expect.anything(), + }, + request: { + url: `${TEST_HOST}/index.html`, + headers: { + 'User-Agent': expect.stringContaining(''), + }, + }, + platform: 'javascript', + }); + const cspContainer = await page.locator('#csp-violation'); + expect(cspContainer).not.toContainText('CSP Violation'); +}); diff --git a/packages/browser/src/utils/lazyLoadIntegration.ts b/packages/browser/src/utils/lazyLoadIntegration.ts index b023bc18aa95..168d1fd1013b 100644 --- a/packages/browser/src/utils/lazyLoadIntegration.ts +++ b/packages/browser/src/utils/lazyLoadIntegration.ts @@ -31,7 +31,10 @@ const WindowWithMaybeIntegration = WINDOW as { * Lazy load an integration from the CDN. * Rejects if the integration cannot be loaded. */ -export async function lazyLoadIntegration(name: keyof typeof LazyLoadableIntegrations): Promise { +export async function lazyLoadIntegration( + name: keyof typeof LazyLoadableIntegrations, + scriptNonce?: string, +): Promise { const bundle = LazyLoadableIntegrations[name]; // `window.Sentry` is only set when using a CDN bundle, but this method can also be used via the NPM package @@ -56,6 +59,10 @@ export async function lazyLoadIntegration(name: keyof typeof LazyLoadableIntegra script.crossOrigin = 'anonymous'; script.referrerPolicy = 'origin'; + if (scriptNonce) { + script.setAttribute('nonce', scriptNonce); + } + const waitForLoad = new Promise((resolve, reject) => { script.addEventListener('load', () => resolve()); script.addEventListener('error', reject); diff --git a/packages/feedback/src/core/components/Actor.css.ts b/packages/feedback/src/core/components/Actor.css.ts index 60ae7cebd08e..8a8b2c4efa29 100644 --- a/packages/feedback/src/core/components/Actor.css.ts +++ b/packages/feedback/src/core/components/Actor.css.ts @@ -3,7 +3,7 @@ import { DOCUMENT } from '../../constants'; /** * Creates + diff --git a/dev-packages/e2e-tests/test-applications/astro-4/sentry.client.config.js b/dev-packages/e2e-tests/test-applications/astro-4/sentry.client.config.js new file mode 100644 index 000000000000..2b79ec0ed337 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/sentry.client.config.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/astro'; + +Sentry.init({ + dsn: import.meta.env.PUBLIC_E2E_TEST_DSN, + environment: 'qa', + tracesSampleRate: 1.0, + tunnel: 'http://localhost:3031/', // proxy server +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/sentry.server.config.js b/dev-packages/e2e-tests/test-applications/astro-4/sentry.server.config.js new file mode 100644 index 000000000000..0662d678dc7c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/sentry.server.config.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/astro'; + +Sentry.init({ + dsn: import.meta.env.PUBLIC_E2E_TEST_DSN, + environment: 'qa', + tracesSampleRate: 1.0, + spotlight: true, + tunnel: 'http://localhost:3031/', // proxy server +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/env.d.ts b/dev-packages/e2e-tests/test-applications/astro-4/src/env.d.ts new file mode 100644 index 000000000000..f964fe0cffd8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/env.d.ts @@ -0,0 +1 @@ +/// diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/layouts/Layout.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/layouts/Layout.astro new file mode 100644 index 000000000000..c4e54b834656 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/layouts/Layout.astro @@ -0,0 +1,39 @@ +--- +interface Props { + title: string; +} + +const { title } = Astro.props; +--- + + + + + + + + + + {title} + + + + + + diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/client-error/index.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/client-error/index.astro new file mode 100644 index 000000000000..facd6f077a6e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/client-error/index.astro @@ -0,0 +1,11 @@ +--- +import Layout from "../../layouts/Layout.astro"; +--- + + + + + + diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/endpoint-error/api.ts b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/endpoint-error/api.ts new file mode 100644 index 000000000000..a76accdba010 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/endpoint-error/api.ts @@ -0,0 +1,15 @@ +import type { APIRoute } from 'astro'; + +export const prerender = false; + +export const GET: APIRoute = ({ request, url }) => { + if (url.searchParams.has('error')) { + throw new Error('Endpoint Error'); + } + return new Response( + JSON.stringify({ + search: url.search, + sp: url.searchParams, + }), + ); +}; diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/endpoint-error/index.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/endpoint-error/index.astro new file mode 100644 index 000000000000..f025c76f8365 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/endpoint-error/index.astro @@ -0,0 +1,9 @@ +--- +import Layout from "../../layouts/Layout.astro"; + +export const prerender = false; +--- + + + + diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/index.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/index.astro new file mode 100644 index 000000000000..088205fc4028 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/index.astro @@ -0,0 +1,36 @@ +--- +import Layout from '../layouts/Layout.astro'; +--- + + +
    +

    Astro E2E Test App

    + +
    +     + + diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/ssr-error/index.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/ssr-error/index.astro new file mode 100644 index 000000000000..4ecb7466de70 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/ssr-error/index.astro @@ -0,0 +1,13 @@ +--- +import Layout from "../../layouts/Layout.astro"; + +const a = {} as any; +console.log(a.foo.x); +export const prerender = false; +--- + + + +

    Page with SSR error

    + +     diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/test-ssr/index.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/test-ssr/index.astro new file mode 100644 index 000000000000..58f5d80198d7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/test-ssr/index.astro @@ -0,0 +1,15 @@ +--- +import Layout from "../../layouts/Layout.astro" + +export const prerender = false +--- + + + +

    + This is a server page +

    + + + +     diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/test-static/index.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/test-static/index.astro new file mode 100644 index 000000000000..f71bf00c9adf --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/test-static/index.astro @@ -0,0 +1,15 @@ +--- +import Layout from "../../layouts/Layout.astro"; + +export const prerender = true; +--- + + + +

    + This is a static page +

    + + + +     diff --git a/dev-packages/e2e-tests/test-applications/astro-4/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/astro-4/start-event-proxy.mjs new file mode 100644 index 000000000000..a657dae0f425 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'astro-4', +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.client.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.client.test.ts new file mode 100644 index 000000000000..4cbf4bf36604 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.client.test.ts @@ -0,0 +1,79 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test.describe('client-side errors', () => { + test('captures error thrown on click', async ({ page }) => { + const errorEventPromise = waitForError('astro-4', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'client error'; + }); + + await page.goto('/client-error'); + + await page.getByText('Throw Error').click(); + + const errorEvent = await errorEventPromise; + + const errorEventFrames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + + expect(errorEventFrames?.[errorEventFrames?.length - 1]).toEqual( + expect.objectContaining({ + colno: expect.any(Number), + lineno: expect.any(Number), + filename: expect.stringContaining('/client-error'), + function: 'HTMLButtonElement.onclick', + in_app: true, + }), + ); + + expect(errorEvent).toMatchObject({ + exception: { + values: [ + { + mechanism: { + handled: false, + type: 'onerror', + }, + type: 'Error', + value: 'client error', + stacktrace: expect.any(Object), // detailed check above + }, + ], + }, + level: 'error', + platform: 'javascript', + request: { + url: expect.stringContaining('/client-error'), + headers: { + 'User-Agent': expect.any(String), + }, + }, + event_id: expect.stringMatching(/[a-f0-9]{32}/), + timestamp: expect.any(Number), + sdk: { + integrations: expect.arrayContaining([ + 'InboundFilters', + 'FunctionToString', + 'BrowserApiErrors', + 'Breadcrumbs', + 'GlobalHandlers', + 'LinkedErrors', + 'Dedupe', + 'HttpContext', + 'BrowserTracing', + ]), + name: 'sentry.javascript.astro', + version: expect.any(String), + packages: expect.any(Array), + }, + transaction: '/client-error', + contexts: { + trace: { + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + }, + }, + environment: 'qa', + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.server.test.ts new file mode 100644 index 000000000000..d5f07ebe239a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.server.test.ts @@ -0,0 +1,115 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test.describe('server-side errors', () => { + test('captures SSR error', async ({ page }) => { + const errorEventPromise = waitForError('astro-4', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === "Cannot read properties of undefined (reading 'x')"; + }); + + await page.goto('/ssr-error'); + + const errorEvent = await errorEventPromise; + + expect(errorEvent).toMatchObject({ + contexts: { + app: expect.any(Object), + cloud_resource: expect.any(Object), + culture: expect.any(Object), + device: expect.any(Object), + os: expect.any(Object), + runtime: expect.any(Object), + trace: { + span_id: '', //TODO: This is a bug! We should expect.stringMatching(/[a-f0-9]{16}/) instead of '' + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + environment: 'qa', + event_id: expect.stringMatching(/[a-f0-9]{32}/), + exception: { + values: [ + { + mechanism: { + data: { + function: 'astroMiddleware', + }, + handled: false, + type: 'astro', + }, + stacktrace: expect.any(Object), + type: 'TypeError', + value: "Cannot read properties of undefined (reading 'x')", + }, + ], + }, + platform: 'node', + request: { + cookies: {}, + headers: expect.objectContaining({ + // demonstrates that requestData integration is getting data + host: 'localhost:3030', + 'user-agent': expect.any(String), + }), + method: 'GET', + url: expect.stringContaining('/ssr-error'), + }, + sdk: { + integrations: expect.any(Array), + name: 'sentry.javascript.astro', + packages: expect.any(Array), + version: expect.any(String), + }, + server_name: expect.any(String), + timestamp: expect.any(Number), + transaction: 'GET /ssr-error', + }); + }); + + test('captures endpoint error', async ({ page }) => { + const errorEventPromise = waitForError('astro-4', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Endpoint Error'; + }); + + await page.goto('/endpoint-error'); + await page.getByText('Get Data').click(); + + const errorEvent = await errorEventPromise; + + expect(errorEvent).toMatchObject({ + contexts: { + trace: { + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + exception: { + values: [ + { + mechanism: { + data: { + function: 'astroMiddleware', + }, + handled: false, + type: 'astro', + }, + stacktrace: expect.any(Object), + type: 'Error', + value: 'Endpoint Error', + }, + ], + }, + platform: 'node', + request: { + cookies: {}, + headers: expect.objectContaining({ + accept: expect.any(String), + }), + method: 'GET', + query_string: 'error=1', + url: expect.stringContaining('endpoint-error/api?error=1'), + }, + transaction: 'GET /endpoint-error/api', + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts new file mode 100644 index 000000000000..9a295f677d96 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts @@ -0,0 +1,123 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test.describe('tracing in dynamically rendered (ssr) routes', () => { + test('sends server and client pageload spans with the same trace id', async ({ page }) => { + const clientPageloadTxnPromise = waitForTransaction('astro-4', txnEvent => { + return txnEvent?.transaction === '/test-ssr'; + }); + + const serverPageRequestTxnPromise = waitForTransaction('astro-4', txnEvent => { + return txnEvent?.transaction === 'GET /test-ssr'; + }); + + await page.goto('/test-ssr'); + + const clientPageloadTxn = await clientPageloadTxnPromise; + const serverPageRequestTxn = await serverPageRequestTxnPromise; + + const clientPageloadTraceId = clientPageloadTxn.contexts?.trace?.trace_id; + const clientPageloadParentSpanId = clientPageloadTxn.contexts?.trace?.parent_span_id; + + const serverPageRequestTraceId = serverPageRequestTxn.contexts?.trace?.trace_id; + const serverPageloadSpanId = serverPageRequestTxn.contexts?.trace?.span_id; + + expect(clientPageloadTraceId).toEqual(serverPageRequestTraceId); + expect(clientPageloadParentSpanId).toEqual(serverPageloadSpanId); + + expect(clientPageloadTxn).toMatchObject({ + contexts: { + trace: { + data: expect.objectContaining({ + 'sentry.op': 'pageload', + 'sentry.origin': 'auto.pageload.browser', + 'sentry.sample_rate': 1, + 'sentry.source': 'url', + }), + op: 'pageload', + origin: 'auto.pageload.browser', + span_id: expect.stringMatching(/[a-f0-9]{16}/), + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + environment: 'qa', + event_id: expect.stringMatching(/[a-f0-9]{32}/), + measurements: expect.any(Object), + platform: 'javascript', + request: expect.any(Object), + sdk: { + integrations: expect.any(Array), + name: 'sentry.javascript.astro', + packages: expect.any(Array), + version: expect.any(String), + }, + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: '/test-ssr', + transaction_info: { + source: 'url', + }, + type: 'transaction', + }); + + expect(serverPageRequestTxn).toMatchObject({ + breadcrumbs: expect.any(Array), + contexts: { + app: expect.any(Object), + cloud_resource: expect.any(Object), + culture: expect.any(Object), + device: expect.any(Object), + os: expect.any(Object), + otel: expect.any(Object), + runtime: expect.any(Object), + trace: { + data: { + 'http.response.status_code': 200, + method: 'GET', + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.astro', + 'sentry.sample_rate': 1, + 'sentry.source': 'route', + url: expect.stringContaining('/test-ssr'), + }, + op: 'http.server', + origin: 'auto.http.astro', + status: 'ok', + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + environment: 'qa', + event_id: expect.stringMatching(/[a-f0-9]{32}/), + platform: 'node', + request: { + cookies: {}, + headers: expect.objectContaining({ + // demonstrates that request data integration can extract headers + accept: expect.any(String), + 'accept-encoding': expect.any(String), + 'user-agent': expect.any(String), + }), + method: 'GET', + url: expect.stringContaining('/test-ssr'), + }, + sdk: { + integrations: expect.any(Array), + name: 'sentry.javascript.astro', + packages: expect.any(Array), + version: expect.any(String), + }, + server_name: expect.any(String), + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: 'GET /test-ssr', + transaction_info: { + source: 'route', + }, + type: 'transaction', + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts new file mode 100644 index 000000000000..8817b2b22aa7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts @@ -0,0 +1,62 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test.describe('tracing in static/pre-rendered routes', () => { + test('only sends client pageload span with traceId from pre-rendered tags', async ({ page }) => { + const clientPageloadTxnPromise = waitForTransaction('astro-4', txnEvent => { + return txnEvent?.transaction === '/test-static'; + }); + + waitForTransaction('astro-4', evt => { + if (evt.platform !== 'javascript') { + throw new Error('Server transaction should not be sent'); + } + return false; + }); + + await page.goto('/test-static'); + + const clientPageloadTxn = await clientPageloadTxnPromise; + + const clientPageloadTraceId = clientPageloadTxn.contexts?.trace?.trace_id; + const clientPageloadParentSpanId = clientPageloadTxn.contexts?.trace?.parent_span_id; + + const sentryTraceMetaTagContent = await page.locator('meta[name="sentry-trace"]').getAttribute('content'); + const baggageMetaTagContent = await page.locator('meta[name="baggage"]').getAttribute('content'); + + const [metaTraceId, metaParentSpanId, metaSampled] = sentryTraceMetaTagContent?.split('-') || []; + + expect(clientPageloadTraceId).toMatch(/[a-f0-9]{32}/); + expect(clientPageloadParentSpanId).toMatch(/[a-f0-9]{16}/); + expect(metaSampled).toBe('1'); + + expect(clientPageloadTxn).toMatchObject({ + contexts: { + trace: { + data: expect.objectContaining({ + 'sentry.op': 'pageload', + 'sentry.origin': 'auto.pageload.browser', + 'sentry.sample_rate': 1, + 'sentry.source': 'url', + }), + op: 'pageload', + origin: 'auto.pageload.browser', + parent_span_id: metaParentSpanId, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: metaTraceId, + }, + }, + platform: 'javascript', + transaction: '/test-static', + transaction_info: { + source: 'url', + }, + type: 'transaction', + }); + + expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Ftest-static%2F'); // URL-encoded for 'GET /test-static/' + expect(baggageMetaTagContent).toContain('sentry-sampled=true'); + + await page.waitForTimeout(1000); // wait another sec to ensure no server transaction is sent + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tsconfig.json b/dev-packages/e2e-tests/test-applications/astro-4/tsconfig.json new file mode 100644 index 000000000000..77da9dd00982 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/tsconfig.json @@ -0,0 +1,3 @@ +{ + "extends": "astro/tsconfigs/strict" +} \ No newline at end of file diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 3752bd30d448..95d099ff0526 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -147,7 +147,7 @@ async function instrumentRequest( async span => { const originalResponse = await next(); - if (span && originalResponse.status) { + if (originalResponse.status) { setHttpStatus(span, originalResponse.status); } From d73808aca8a7ffb5d1307bfab5d02c1cefb66268 Mon Sep 17 00:00:00 2001 From: Nicolas Charpentier Date: Fri, 16 Aug 2024 03:24:37 -0400 Subject: [PATCH 17/22] docs: Fix v7 changelog format and link (#13395) Minor changes: I realized that the formatting was broken for `v8` and that the migration guide wasn't linking to the specific section like other changelogs. --- docs/changelog/v7.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/v7.md b/docs/changelog/v7.md index 16072a8f8121..e784702015e0 100644 --- a/docs/changelog/v7.md +++ b/docs/changelog/v7.md @@ -1,7 +1,7 @@ # Changelog for Sentry SDK 7.x Support for Sentry SDK v7 will be dropped soon. We recommend migrating to the latest version of the SDK. You can migrate -from `v7` to `v8 of the SDK by following the [migration guide](../../MIGRATION.md). +from `v7` of the SDK to `v8` by following the [migration guide](../../MIGRATION.md#upgrading-from-7x-to-8x). ## 7.118.0 From a8c0a3bcd0fb44564af0903f8c56861d05ac42a7 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 16 Aug 2024 09:55:55 +0200 Subject: [PATCH 18/22] ci: Fix crypto not being available (#13385) CI is angery. Crypto is only on global from Node 19 onwards. --- dev-packages/browser-integration-tests/utils/fixtures.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-packages/browser-integration-tests/utils/fixtures.ts b/dev-packages/browser-integration-tests/utils/fixtures.ts index 42585a3756f0..e154ddc25988 100644 --- a/dev-packages/browser-integration-tests/utils/fixtures.ts +++ b/dev-packages/browser-integration-tests/utils/fixtures.ts @@ -1,3 +1,4 @@ +import crypto from 'crypto'; import fs from 'fs'; import path from 'path'; /* eslint-disable no-empty-pattern */ From 615c670cfe283e77132339c3d9751060f30d3956 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 16 Aug 2024 10:44:46 +0200 Subject: [PATCH 19/22] ref: Add external contributor to CHANGELOG.md (#13400) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13395 Co-authored-by: mydea <2411343+mydea@users.noreply.github.com> --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index be7298ed213a..7864efac6871 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +Work in this release was contributed by @charpeni. Thank you for your contribution! + ## 8.26.0 ### Important Changes From 89eab5be78f03363091e76f09a62fa17992854de Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 26 Aug 2024 13:58:59 +0200 Subject: [PATCH 20/22] meta: Update Changelog for 8.27.0 --- CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7864efac6871..3742f81fc83d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,25 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 8.27.0 + +### Important Changes + +- **fix(nestjs): Exception filters in main app module are not being executed (#13278)** + + With this release nestjs error monitoring is no longer automatically set up after adding the `SentryModule` to your + application, which led to issues in certain scenarios. You will now have to either add the `SentryGlobalFilter` to your + main module providers or decorate the `catch()` method in your existing global exception filters with the newly + released `@WithSentry()` decorator. See the [docs](https://docs.sentry.io/platforms/javascript/guides/nestjs/) for + more details. + +### Other Changes + +- feat: Add options for passing nonces to feedback integration (#13347) +- feat: Add support for SENTRY_SPOTLIGHT env var in Node (#13325) +- feat(deps): bump @prisma/instrumentation from 5.17.0 to 5.18.0 (#13327) +- fix(deno): Don't rely on `Deno.permissions.querySync` (#13378) + Work in this release was contributed by @charpeni. Thank you for your contribution! ## 8.26.0 From 9d7c483525e7c2f24f770eab856fef68174c6712 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 26 Aug 2024 14:05:03 +0200 Subject: [PATCH 21/22] meta: fix formatting --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3742f81fc83d..6c187aebffe3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,9 +17,9 @@ - **fix(nestjs): Exception filters in main app module are not being executed (#13278)** With this release nestjs error monitoring is no longer automatically set up after adding the `SentryModule` to your - application, which led to issues in certain scenarios. You will now have to either add the `SentryGlobalFilter` to your - main module providers or decorate the `catch()` method in your existing global exception filters with the newly - released `@WithSentry()` decorator. See the [docs](https://docs.sentry.io/platforms/javascript/guides/nestjs/) for + application, which led to issues in certain scenarios. You will now have to either add the `SentryGlobalFilter` to + your main module providers or decorate the `catch()` method in your existing global exception filters with the newly + released `@WithSentry()` decorator. See the [docs](https://docs.sentry.io/platforms/javascript/guides/nestjs/) for more details. ### Other Changes From bb2d34f91644c44ffba5b8fd09b53d030ec9792c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 26 Aug 2024 13:36:24 +0000 Subject: [PATCH 22/22] ci: Pin node 22 for Node.js unit tests --- .github/workflows/build.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 516b1d47b624..c66b6e64c8e8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -454,7 +454,8 @@ jobs: strategy: fail-fast: false matrix: - node: [14, 16, 18, 20, 22] + # TODO(lforst): Unpin Node.js version 22 when https://github.com/protobufjs/protobuf.js/issues/2025 is resolved which broke the nodejs tests + node: [14, 16, 18, 20, '22.6.0'] steps: - name: Check out base commit (${{ github.event.pull_request.base.sha }}) uses: actions/checkout@v4