diff --git a/CHANGELOG.md b/CHANGELOG.md index 9566c79ebf66..208ec7eb2a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,11 +31,15 @@ upcoming release. - feat(feedback): Make cropped screenshot area draggable (#13071) - feat(core): Adapt spans for client-side fetch to streaming responses (#12723) - feat(core): Capture # of dropped spans through `beforeSendTransaction` (#13022) -- feat(deps): bump @opentelemetry/instrumentation-aws-sdk from 0.43.0 to 0.43.1 (#13089) -- feat(deps): bump @opentelemetry/instrumentation-express from 0.41.0 to 0.41.1 (#13090) +- feat(deps): bump `@opentelemetry/instrumentation-aws-sdk` from 0.43.0 to 0.43.1 (#13089) +- feat(deps): bump `@opentelemetry/instrumentation-express` from 0.41.0 to 0.41.1 (#13090) +- feat(nestjs): Automatic instrumentation of nestjs middleware (#13065) +- feat(node): Upgrade `import-in-the-middle` to 1.11.0 (#13107) - feat(nuxt): Add connected tracing meta tags (#13098) - feat(nuxt): Add vue-router instrumentation (#13054) +- feat(solidstart): Add server action instrumentation helper (#13035) - fix(feedback): Ensure pluggable feedback CDN bundle is correctly built (#13081) +- fix(nextjs): Only delete clientside bundle source maps with `sourcemaps.deleteFilesAfterUpload` (#13102) - fix(node): Improve OTEL validation logic (#13079) ## 8.20.0 diff --git a/dev-packages/browser-integration-tests/utils/fixtures.ts b/dev-packages/browser-integration-tests/utils/fixtures.ts index f6c989a53778..cf34c9b7e693 100644 --- a/dev-packages/browser-integration-tests/utils/fixtures.ts +++ b/dev-packages/browser-integration-tests/utils/fixtures.ts @@ -68,12 +68,18 @@ const sentryTest = base.extend({ // Ensure feedback can be lazy loaded await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback-modal.min.js`, route => { const filePath = path.resolve(testDir, './dist/feedback-modal.bundle.js'); - return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue(); + if (!fs.existsSync(filePath)) { + throw new Error(`Feedback modal bundle (${filePath}) not found`); + } + return route.fulfill({ path: filePath }); }); await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback-screenshot.min.js`, route => { const filePath = path.resolve(testDir, './dist/feedback-screenshot.bundle.js'); - return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue(); + if (!fs.existsSync(filePath)) { + throw new Error(`Feedback screenshot bundle (${filePath}) not found`); + } + return route.fulfill({ path: filePath }); }); } diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index 69e8f946fc89..30939c40c955 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -272,6 +272,19 @@ class SentryScenarioGenerationPlugin { fileName, ); + if (integration === 'feedback') { + addStaticAssetSymlink( + this.localOutPath, + path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-modal.js'), + 'feedback-modal.bundle.js', + ); + addStaticAssetSymlink( + this.localOutPath, + path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-screenshot.js'), + 'feedback-screenshot.bundle.js', + ); + } + const integrationObject = createHtmlTagObject('script', { src: fileName, }); 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 b54604d999cb..2a4f14cae541 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 @@ -10,6 +10,11 @@ export class AppController { return this.appService.testTransaction(); } + @Get('test-middleware-instrumentation') + testMiddlewareInstrumentation() { + return this.appService.testMiddleware(); + } + @Get('test-exception/:id') async testException(@Param('id') id: string) { return this.appService.testException(id); 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 f4c5ceb0cc5a..b2aad014c745 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,12 +1,17 @@ -import { Module } from '@nestjs/common'; +import { MiddlewareConsumer, Module } from '@nestjs/common'; import { ScheduleModule } from '@nestjs/schedule'; import { SentryModule } from '@sentry/nestjs/setup'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { ExampleMiddleware } from './example.middleware'; @Module({ imports: [SentryModule.forRoot(), ScheduleModule.forRoot()], controllers: [AppController], providers: [AppService], }) -export class AppModule {} +export class AppModule { + configure(consumer: MiddlewareConsumer): void { + consumer.apply(ExampleMiddleware).forRoutes('test-middleware-instrumentation'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts index 3afb7b5147bd..9a47f0e08e7a 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts @@ -21,6 +21,11 @@ export class AppService { }); } + testMiddleware() { + // span that should not be a child span of the middleware span + Sentry.startSpan({ name: 'test-controller-span' }, () => {}); + } + testException(id: string) { throw new Error(`This is an exception with id ${id}`); } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.middleware.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.middleware.ts new file mode 100644 index 000000000000..31d15c9372ea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/example.middleware.ts @@ -0,0 +1,12 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; +import * as Sentry from '@sentry/nestjs'; +import { NextFunction, Request, Response } from 'express'; + +@Injectable() +export class ExampleMiddleware implements NestMiddleware { + use(req: Request, res: Response, next: NextFunction) { + // span that should be a child span of the middleware span + Sentry.startSpan({ name: 'test-middleware-span' }, () => {}); + next(); + } +} diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts index ae5d8b63b16d..b7017b72dbf5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts @@ -121,3 +121,82 @@ test('Sends an API route transaction', async ({ baseURL }) => { }), ); }); + +test('API route transaction includes nest middleware span. Spans created in and after middleware are nested correctly', async ({ + baseURL, +}) => { + const pageloadTransactionEventPromise = waitForTransaction('nestjs', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /test-middleware-instrumentation' + ); + }); + + await fetch(`${baseURL}/test-middleware-instrumentation`); + + const transactionEvent = await pageloadTransactionEventPromise; + + 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: 'ExampleMiddleware', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); + + const exampleMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'ExampleMiddleware'); + const exampleMiddlewareSpanId = exampleMiddlewareSpan?.span_id; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-controller-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-middleware-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + ]), + }), + ); + + // verify correct span parent-child relationships + const testMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'test-middleware-span'); + const testControllerSpan = transactionEvent.spans.find(span => span.description === 'test-controller-span'); + + // 'ExampleMiddleware' is the parent of 'test-middleware-span' + expect(testMiddlewareSpan.parent_span_id).toBe(exampleMiddlewareSpanId); + + // 'ExampleMiddleware' is NOT the parent of 'test-controller-span' + expect(testControllerSpan.parent_span_id).not.toBe(exampleMiddlewareSpanId); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts index 58453223a4cb..955988101724 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts @@ -22,10 +22,10 @@ assert.match(buildStdout, /(λ|ƒ) \/server-component\/parameter\/\[\.\.\.parame assert.match(buildStdout, /(λ|ƒ) \/server-component\/parameter\/\[parameter\]/); // Read the contents of the directory -const files = fs.readdirSync(path.join(process.cwd(), '.next', 'server')); +const files = fs.readdirSync(path.join(process.cwd(), '.next', 'static')); const mapFiles = files.filter(file => path.extname(file) === '.map'); if (mapFiles.length > 0) { - throw new Error('.map files found even though `sourcemaps.deleteSourcemapsAfterUpload` option is set!'); + throw new Error('Client bundle .map files found even though `sourcemaps.deleteSourcemapsAfterUpload` option is set!'); } export {}; diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts index b54604d999cb..2a4f14cae541 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.controller.ts @@ -10,6 +10,11 @@ export class AppController { return this.appService.testTransaction(); } + @Get('test-middleware-instrumentation') + testMiddlewareInstrumentation() { + return this.appService.testMiddleware(); + } + @Get('test-exception/:id') async testException(@Param('id') id: string) { return this.appService.testException(id); diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts index ceb7199a99cf..567dbefeadb7 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.module.ts @@ -1,11 +1,16 @@ -import { Module } from '@nestjs/common'; +import { MiddlewareConsumer, Module } from '@nestjs/common'; import { ScheduleModule } from '@nestjs/schedule'; import { AppController } from './app.controller'; import { AppService } from './app.service'; +import { ExampleMiddleware } from './example.middleware'; @Module({ imports: [ScheduleModule.forRoot()], controllers: [AppController], providers: [AppService], }) -export class AppModule {} +export class AppModule { + configure(consumer: MiddlewareConsumer): void { + consumer.apply(ExampleMiddleware).forRoutes('test-middleware-instrumentation'); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts index 3afb7b5147bd..9a47f0e08e7a 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/app.service.ts @@ -21,6 +21,11 @@ export class AppService { }); } + testMiddleware() { + // span that should not be a child span of the middleware span + Sentry.startSpan({ name: 'test-controller-span' }, () => {}); + } + testException(id: string) { throw new Error(`This is an exception with id ${id}`); } diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example.middleware.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example.middleware.ts new file mode 100644 index 000000000000..31d15c9372ea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/src/example.middleware.ts @@ -0,0 +1,12 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; +import * as Sentry from '@sentry/nestjs'; +import { NextFunction, Request, Response } from 'express'; + +@Injectable() +export class ExampleMiddleware implements NestMiddleware { + use(req: Request, res: Response, next: NextFunction) { + // span that should be a child span of the middleware span + Sentry.startSpan({ name: 'test-middleware-span' }, () => {}); + next(); + } +} diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts index ae5d8b63b16d..b7017b72dbf5 100644 --- a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts @@ -121,3 +121,82 @@ test('Sends an API route transaction', async ({ baseURL }) => { }), ); }); + +test('API route transaction includes nest middleware span. Spans created in and after middleware are nested correctly', async ({ + baseURL, +}) => { + const pageloadTransactionEventPromise = waitForTransaction('nestjs', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /test-middleware-instrumentation' + ); + }); + + await fetch(`${baseURL}/test-middleware-instrumentation`); + + const transactionEvent = await pageloadTransactionEventPromise; + + 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: 'ExampleMiddleware', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); + + const exampleMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'ExampleMiddleware'); + const exampleMiddlewareSpanId = exampleMiddlewareSpan?.span_id; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-controller-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: expect.any(Object), + description: 'test-middleware-span', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + origin: 'manual', + }, + ]), + }), + ); + + // verify correct span parent-child relationships + const testMiddlewareSpan = transactionEvent.spans.find(span => span.description === 'test-middleware-span'); + const testControllerSpan = transactionEvent.spans.find(span => span.description === 'test-controller-span'); + + // 'ExampleMiddleware' is the parent of 'test-middleware-span' + expect(testMiddlewareSpan.parent_span_id).toBe(exampleMiddlewareSpanId); + + // 'ExampleMiddleware' is NOT the parent of 'test-controller-span' + expect(testControllerSpan.parent_span_id).not.toBe(exampleMiddlewareSpanId); +}); diff --git a/dev-packages/e2e-tests/test-applications/solidstart/package.json b/dev-packages/e2e-tests/test-applications/solidstart/package.json index 6409d191de5b..dfcf8a47402a 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/package.json +++ b/dev-packages/e2e-tests/test-applications/solidstart/package.json @@ -11,6 +11,7 @@ "This is currently not an issue outside of our repo. See: https://github.com/nksaraf/vinxi/issues/177" ], "preview": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi dev", + "start": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi start", "test:prod": "TEST_ENV=production playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", "test:assert": "pnpm test:prod" @@ -31,7 +32,7 @@ "jsdom": "^24.0.0", "solid-js": "1.8.17", "typescript": "^5.4.5", - "vinxi": "^0.3.12", + "vinxi": "^0.4.0", "vite": "^5.2.8", "vite-plugin-solid": "^2.10.2", "vitest": "^1.5.0" diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/entry-client.tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/entry-client.tsx index 9391faa9652d..11087fbb5918 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/src/entry-client.tsx +++ b/dev-packages/e2e-tests/test-applications/solidstart/src/entry-client.tsx @@ -12,6 +12,7 @@ Sentry.init({ tunnel: 'http://localhost:3031/', // proxy server // Performance Monitoring tracesSampleRate: 1.0, // Capture 100% of the transactions + debug: !!import.meta.env.DEBUG, }); mount(() => , document.getElementById('app')!); diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.mjs b/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.mjs index 4146470295e1..3dd5d8933b7b 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.mjs +++ b/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.mjs @@ -5,4 +5,5 @@ Sentry.init({ environment: 'qa', // dynamic sampling bias to keep transactions tracesSampleRate: 1.0, // Capture 100% of the transactions tunnel: 'http://localhost:3031/', // proxy server + debug: !!process.env.DEBUG, }); 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 f1635dee3b63..873d542e4bae 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 @@ -11,6 +11,9 @@ export default function Home() {
  • Client error
    • +
  • + Server error +
  • User 5 diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/server-error.tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/server-error.tsx new file mode 100644 index 000000000000..05dce5e10a56 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/server-error.tsx @@ -0,0 +1,17 @@ +import { withServerActionInstrumentation } from '@sentry/solidstart'; +import { createAsync } from '@solidjs/router'; + +const getPrefecture = async () => { + 'use server'; + return await withServerActionInstrumentation('getPrefecture', () => { + throw new Error('Error thrown from Solid Start E2E test app server route'); + + return { prefecture: 'Kanagawa' }; + }); +}; + +export default function ServerErrorPage() { + const data = createAsync(() => getPrefecture()); + + return
    Prefecture: {data()?.prefecture}
    ; +} diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/users/[id].tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/users/[id].tsx index 639ab0be8118..22abd3ba8803 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/users/[id].tsx +++ b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/users/[id].tsx @@ -1,6 +1,21 @@ -import { useParams } from '@solidjs/router'; +import { withServerActionInstrumentation } from '@sentry/solidstart'; +import { createAsync, useParams } from '@solidjs/router'; +const getPrefecture = async () => { + 'use server'; + return await withServerActionInstrumentation('getPrefecture', () => { + return { prefecture: 'Ehime' }; + }); +}; export default function User() { const params = useParams(); - return
    User ID: {params.id}
    ; + const userData = createAsync(() => getPrefecture()); + + return ( +
    + User ID: {params.id} +
    + Prefecture: {userData()?.prefecture} +
    + ); } diff --git a/dev-packages/e2e-tests/test-applications/solidstart/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/solidstart/tests/errors.server.test.ts new file mode 100644 index 000000000000..0ccea7d3767e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/solidstart/tests/errors.server.test.ts @@ -0,0 +1,31 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test.describe('server-side errors', () => { + test('captures server action error', async ({ page }) => { + const errorEventPromise = waitForError('solidstart', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Error thrown from Solid Start E2E test app server route'; + }); + + await page.goto(`/server-error`); + + const error = await errorEventPromise; + + expect(error.tags).toMatchObject({ runtime: 'node' }); + expect(error).toMatchObject({ + exception: { + values: [ + { + type: 'Error', + value: 'Error thrown from Solid Start E2E test app server route', + mechanism: { + type: 'solidstart', + handled: false, + }, + }, + ], + }, + transaction: 'GET /server-error', + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.server.test.ts b/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.server.test.ts new file mode 100644 index 000000000000..bfd53bbb6bfa --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.server.test.ts @@ -0,0 +1,55 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; + +test('sends a server action transaction on pageload', async ({ page }) => { + const transactionPromise = waitForTransaction('solidstart', transactionEvent => { + return transactionEvent?.transaction === 'GET /users/6'; + }); + + await page.goto('/users/6'); + + const transaction = await transactionPromise; + + expect(transaction.spans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + description: 'getPrefecture', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.server_action', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.solidstart', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + }, + }), + ]), + ); +}); + +test('sends a server action transaction on client navigation', async ({ page }) => { + const transactionPromise = waitForTransaction('solidstart', transactionEvent => { + return transactionEvent?.transaction === 'POST getPrefecture'; + }); + + await page.goto('/'); + await page.locator('#navLink').click(); + await page.waitForURL('/users/5'); + + const transaction = await transactionPromise; + + expect(transaction.spans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + description: 'getPrefecture', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.server_action', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.solidstart', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + }, + }), + ]), + ); +}); diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index 4e7882b33e32..55381f52be17 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -1,3 +1,5 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + import type { BrowserClient } from '@sentry/browser'; import { browserTracingIntegration, @@ -8,9 +10,8 @@ import { } from '@sentry/browser'; import * as SentryBrowser from '@sentry/browser'; import { SDK_VERSION, getClient } from '@sentry/browser'; -import { vi } from 'vitest'; -import { init } from '../../../astro/src/client/sdk'; +import { init } from '../../src/client/sdk'; const browserInit = vi.spyOn(SentryBrowser, 'init'); @@ -66,7 +67,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations; + const integrationsToInit = browserInit.mock.calls[0]![0]?.defaultIntegrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); @@ -82,7 +83,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations || []; + const integrationsToInit = browserInit.mock.calls[0]![0]?.defaultIntegrations || []; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); @@ -90,20 +91,20 @@ describe('Sentry client SDK', () => { }); it("doesn't add browserTracingIntegration if `__SENTRY_TRACING__` is set to false", () => { - globalThis.__SENTRY_TRACING__ = false; + (globalThis as any).__SENTRY_TRACING__ = false; init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', enableTracing: true, }); - const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations || []; + const integrationsToInit = browserInit.mock.calls[0]![0]?.defaultIntegrations || []; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeUndefined(); - delete globalThis.__SENTRY_TRACING__; + delete (globalThis as any).__SENTRY_TRACING__; }); it('Overrides the automatically default browserTracingIntegration instance with a a user-provided browserTracingIntegration instance', () => { diff --git a/packages/astro/test/integration/index.files.test.ts b/packages/astro/test/integration/index.files.test.ts index f0b15f6a48c2..0b28b2a0550d 100644 --- a/packages/astro/test/integration/index.files.test.ts +++ b/packages/astro/test/integration/index.files.test.ts @@ -1,12 +1,10 @@ -import { vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { sentryAstro } from '../../src/integration'; -vi.mock('fs', async () => { - const actual = await vi.importActual('fs'); +vi.mock('fs', async requireActual => { return { - // @ts-expect-error - just mocking around - ...actual, + ...(await requireActual()), existsSync: vi.fn(p => p.endsWith('js')), }; }); diff --git a/packages/astro/test/integration/index.test.ts b/packages/astro/test/integration/index.test.ts index 886e77ada2dd..008132264602 100644 --- a/packages/astro/test/integration/index.test.ts +++ b/packages/astro/test/integration/index.test.ts @@ -1,4 +1,4 @@ -import { vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { sentryAstro } from '../../src/integration'; @@ -294,7 +294,7 @@ describe('sentryAstro integration', () => { it.each([{ output: 'static' }, { output: undefined }])( "doesn't add middleware if in static mode (config %s)", - async config => { + async (config: any) => { const integration = sentryAstro({}); const addMiddleware = vi.fn(); const updateConfig = vi.fn(); diff --git a/packages/astro/test/integration/middleware/index.test.ts b/packages/astro/test/integration/middleware/index.test.ts index b9d1273261de..3b12508feaa7 100644 --- a/packages/astro/test/integration/middleware/index.test.ts +++ b/packages/astro/test/integration/middleware/index.test.ts @@ -1,4 +1,4 @@ -import { vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { onRequest } from '../../../src/integration/middleware'; diff --git a/packages/astro/test/integration/snippets.test.ts b/packages/astro/test/integration/snippets.test.ts index 5911372c6647..04aaa866aee9 100644 --- a/packages/astro/test/integration/snippets.test.ts +++ b/packages/astro/test/integration/snippets.test.ts @@ -1,3 +1,5 @@ +import { describe, expect, it } from 'vitest'; + import { buildClientSnippet, buildSdkInitFileImportSnippet, buildServerSnippet } from '../../src/integration/snippets'; const allSdkOptions = { diff --git a/packages/astro/tsconfig.test.json b/packages/astro/tsconfig.test.json index 3fbe012384ee..c41efeacd92f 100644 --- a/packages/astro/tsconfig.test.json +++ b/packages/astro/tsconfig.test.json @@ -5,6 +5,6 @@ "compilerOptions": { // should include all types from `./tsconfig.json` plus types for all test frameworks used - "types": ["node", "vitest/globals"] + "types": ["node"] } } diff --git a/packages/astro/vite.config.ts b/packages/astro/vite.config.ts index 1094fe0d79da..f18ec92095bc 100644 --- a/packages/astro/vite.config.ts +++ b/packages/astro/vite.config.ts @@ -4,6 +4,5 @@ export default { ...baseConfig, test: { ...baseConfig.test, - environment: 'jsdom', }, }; diff --git a/packages/nextjs/src/config/webpackPluginOptions.ts b/packages/nextjs/src/config/webpackPluginOptions.ts index 6b755cf2c839..f70862bfe484 100644 --- a/packages/nextjs/src/config/webpackPluginOptions.ts +++ b/packages/nextjs/src/config/webpackPluginOptions.ts @@ -76,9 +76,12 @@ export function getWebpackPluginOptions( ignore: sentryBuildOptions.sourcemaps?.ignore ?? sourcemapUploadIgnore, filesToDeleteAfterUpload: sentryBuildOptions.sourcemaps?.deleteSourcemapsAfterUpload ? [ - path.join(distDirAbsPath, '**', '*.js.map'), - path.join(distDirAbsPath, '**', '*.mjs.map'), - path.join(distDirAbsPath, '**', '*.cjs.map'), + // We only care to delete client bundle source maps because they would be the ones being served. + // Removing the server source maps crashes Vercel builds for (thus far) unknown reasons: + // https://github.com/getsentry/sentry-javascript/issues/13099 + path.join(distDirAbsPath, 'static', '**', '*.js.map'), + path.join(distDirAbsPath, 'static', '**', '*.mjs.map'), + path.join(distDirAbsPath, 'static', '**', '*.cjs.map'), ] : undefined, ...sentryBuildOptions.unstable_sentryWebpackPluginOptions?.sourcemaps, diff --git a/packages/node/package.json b/packages/node/package.json index 41e321c66f53..5390c57bd0a3 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -92,7 +92,7 @@ "@sentry/opentelemetry": "8.20.0", "@sentry/types": "8.20.0", "@sentry/utils": "8.20.0", - "import-in-the-middle": "^1.10.0" + "import-in-the-middle": "^1.11.0" }, "devDependencies": { "@types/node": "^14.18.0" diff --git a/packages/node/src/integrations/tracing/nest.ts b/packages/node/src/integrations/tracing/nest.ts index ab6a66fdb895..2ec5fa840387 100644 --- a/packages/node/src/integrations/tracing/nest.ts +++ b/packages/node/src/integrations/tracing/nest.ts @@ -1,16 +1,27 @@ +import { isWrapped } from '@opentelemetry/core'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { + InstrumentationBase, + InstrumentationNodeModuleDefinition, + InstrumentationNodeModuleFile, +} from '@opentelemetry/instrumentation'; import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core'; import { + SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, captureException, defineIntegration, + getActiveSpan, getClient, getDefaultIsolationScope, getIsolationScope, spanToJSON, + startSpanManual, + withActiveSpan, } from '@sentry/core'; import type { IntegrationFn, Span } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { addNonEnumerableProperty, logger } from '@sentry/utils'; import { generateInstrumentOnce } from '../../otel/instrument'; interface MinimalNestJsExecutionContext { @@ -44,7 +55,155 @@ interface MinimalNestJsApp { const INTEGRATION_NAME = 'Nest'; -export const instrumentNest = generateInstrumentOnce(INTEGRATION_NAME, () => new NestInstrumentation()); +const supportedVersions = ['>=8.0.0 <11']; + +const sentryPatched = 'sentryPatched'; + +/** + * Represents an injectable target class in NestJS. + */ +export interface InjectableTarget { + name: string; + sentryPatched?: boolean; + prototype: { + use?: (req: unknown, res: unknown, next: () => void) => void; + }; +} + +/** + * Helper checking if a concrete target class is already patched. + * + * We already guard duplicate patching with isWrapped. However, isWrapped checks whether a file has been patched, whereas we use this check for concrete target classes. + * This check might not be necessary, but better to play it safe. + */ +export function isPatched(target: InjectableTarget): boolean { + if (target.sentryPatched) { + return true; + } + + addNonEnumerableProperty(target, sentryPatched, true); + return false; +} + +/** + * Custom instrumentation for nestjs. + * + * This hooks into the @Injectable decorator, which is applied on class middleware, interceptors and guards. + */ +export class SentryNestInstrumentation extends InstrumentationBase { + public static readonly COMPONENT = '@nestjs/common'; + public static readonly COMMON_ATTRIBUTES = { + component: SentryNestInstrumentation.COMPONENT, + }; + + public constructor(config: InstrumentationConfig = {}) { + super('sentry-nestjs', SDK_VERSION, config); + } + + /** + * Initializes the instrumentation by defining the modules to be patched. + */ + public init(): InstrumentationNodeModuleDefinition { + const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestInstrumentation.COMPONENT, supportedVersions); + + moduleDef.files.push(this._getInjectableFileInstrumentation(supportedVersions)); + return moduleDef; + } + + /** + * Wraps the @Injectable decorator. + */ + private _getInjectableFileInstrumentation(versions: string[]): InstrumentationNodeModuleFile { + return new InstrumentationNodeModuleFile( + '@nestjs/common/decorators/core/injectable.decorator.js', + versions, + (moduleExports: { Injectable: InjectableTarget }) => { + if (isWrapped(moduleExports.Injectable)) { + this._unwrap(moduleExports, 'Injectable'); + } + this._wrap(moduleExports, 'Injectable', this._createWrapInjectable()); + return moduleExports; + }, + (moduleExports: { Injectable: InjectableTarget }) => { + this._unwrap(moduleExports, 'Injectable'); + }, + ); + } + + /** + * Creates a wrapper function for the @Injectable decorator. + * + * Wraps the use method to instrument nest class middleware. + */ + private _createWrapInjectable() { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function wrapInjectable(original: any) { + return function wrappedInjectable(options?: unknown) { + return function (target: InjectableTarget) { + // patch middleware + if (typeof target.prototype.use === 'function') { + // patch only once + if (isPatched(target)) { + return original(options)(target); + } + + target.prototype.use = new Proxy(target.prototype.use, { + apply: (originalUse, thisArgUse, argsUse) => { + const [req, res, next, ...args] = argsUse; + const prevSpan = getActiveSpan(); + + startSpanManual( + { + name: target.name, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'middleware.nestjs', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.middleware.nestjs', + }, + }, + (span: Span) => { + const nextProxy = new Proxy(next, { + apply: (originalNext, thisArgNext, argsNext) => { + span.end(); + + if (prevSpan) { + withActiveSpan(prevSpan, () => { + Reflect.apply(originalNext, thisArgNext, argsNext); + }); + } else { + Reflect.apply(originalNext, thisArgNext, argsNext); + } + }, + }); + + originalUse.apply(thisArgUse, [req, res, nextProxy, args]); + }, + ); + }, + }); + } + + return original(options)(target); + }; + }; + }; + } +} + +const instrumentNestCore = generateInstrumentOnce('Nest-Core', () => { + return new NestInstrumentation(); +}); + +const instrumentNestCommon = generateInstrumentOnce('Nest-Common', () => { + return new SentryNestInstrumentation(); +}); + +export const instrumentNest = Object.assign( + (): void => { + instrumentNestCore(); + instrumentNestCommon(); + }, + { id: INTEGRATION_NAME }, +); const _nestIntegration = (() => { return { diff --git a/packages/node/test/integrations/tracing/nest.test.ts b/packages/node/test/integrations/tracing/nest.test.ts new file mode 100644 index 000000000000..3dc321f28008 --- /dev/null +++ b/packages/node/test/integrations/tracing/nest.test.ts @@ -0,0 +1,17 @@ +import type { InjectableTarget } from '../../../src/integrations/tracing/nest'; +import { isPatched } from '../../../src/integrations/tracing/nest'; + +describe('Nest', () => { + describe('isPatched', () => { + it('should return true if target is already patched', () => { + const target = { name: 'TestTarget', sentryPatched: true, prototype: {} }; + expect(isPatched(target)).toBe(true); + }); + + it('should add the sentryPatched property and return false if target is not patched', () => { + const target: InjectableTarget = { name: 'TestTarget', prototype: {} }; + expect(isPatched(target)).toBe(false); + expect(target.sentryPatched).toBe(true); + }); + }); +}); diff --git a/packages/nuxt/README.md b/packages/nuxt/README.md index a2f9d9d0d22e..1227aa5e2c82 100644 --- a/packages/nuxt/README.md +++ b/packages/nuxt/README.md @@ -126,18 +126,6 @@ other imports: } ``` -If you are getting an `import-in-the-middle` error message, add the package with a minimum version of `1.10.0` as a -dependency to your `package.json` -([issue reference](https://github.com/getsentry/sentry-javascript-examples/pull/38#issuecomment-2245259327)): - -```json -{ - "dependencies": { - "import-in-the-middle": "1.10.0" - } -} -``` - ## Uploading Source Maps To upload source maps, you can use the `sourceMapsUploadOptions` option inside the `sentry` options of your diff --git a/packages/solidstart/package.json b/packages/solidstart/package.json index 4dd4d345f5e7..a22c235576f9 100644 --- a/packages/solidstart/package.json +++ b/packages/solidstart/package.json @@ -72,7 +72,8 @@ "@sentry/solid": "8.20.0", "@sentry/types": "8.20.0", "@sentry/utils": "8.20.0", - "@sentry/vite-plugin": "2.19.0" + "@sentry/vite-plugin": "2.19.0", + "@opentelemetry/instrumentation": "^0.52.1" }, "devDependencies": { "@solidjs/router": "^0.13.4", diff --git a/packages/solidstart/rollup.npm.config.mjs b/packages/solidstart/rollup.npm.config.mjs index 5a36889c70f0..b0087a93c6fe 100644 --- a/packages/solidstart/rollup.npm.config.mjs +++ b/packages/solidstart/rollup.npm.config.mjs @@ -16,7 +16,7 @@ export default makeNPMConfigVariants( // prevent this internal code from ending up in our built package (this doesn't happen automatially because // the name doesn't match an SDK dependency) packageSpecificConfig: { - external: ['solid-js', '@sentry/solid', '@sentry/solid/solidrouter'], + external: ['solid-js/web', 'solid-js', '@sentry/solid', '@sentry/solid/solidrouter'], output: { dynamicImportInCjs: true, }, diff --git a/packages/solidstart/src/index.types.ts b/packages/solidstart/src/index.types.ts index a2c4d1ab7c06..89eaa14662e3 100644 --- a/packages/solidstart/src/index.types.ts +++ b/packages/solidstart/src/index.types.ts @@ -1,5 +1,5 @@ // We export everything from both the client part of the SDK and from the server part. -// Some of the exports collide, which is not allowed, unless we redifine the colliding +// Some of the exports collide, which is not allowed, unless we redefine the colliding // exports in this file - which we do below. export * from './client'; export * from './server'; diff --git a/packages/solidstart/src/server/index.ts b/packages/solidstart/src/server/index.ts index 8b6ab6148fb8..75a67d3bb847 100644 --- a/packages/solidstart/src/server/index.ts +++ b/packages/solidstart/src/server/index.ts @@ -126,3 +126,5 @@ export { withSentryErrorBoundary } from '@sentry/solid'; // ------------------------- // Solid Start SDK exports: export { init } from './sdk'; + +export * from './withServerActionInstrumentation'; diff --git a/packages/solidstart/src/server/utils.ts b/packages/solidstart/src/server/utils.ts new file mode 100644 index 000000000000..f3d26e5d3a26 --- /dev/null +++ b/packages/solidstart/src/server/utils.ts @@ -0,0 +1,33 @@ +import { flush } from '@sentry/node'; +import { logger } from '@sentry/utils'; +import { DEBUG_BUILD } from '../common/debug-build'; + +/** Flush the event queue to ensure that events get sent to Sentry before the response is finished and the lambda ends */ +export async function flushIfServerless(): Promise { + const isServerless = !!process.env.LAMBDA_TASK_ROOT || !!process.env.VERCEL; + + if (isServerless) { + try { + DEBUG_BUILD && logger.log('Flushing events...'); + await flush(2000); + DEBUG_BUILD && logger.log('Done flushing events'); + } catch (e) { + DEBUG_BUILD && logger.log('Error while flushing events:\n', e); + } + } +} + +/** + * Determines if a thrown "error" is a redirect Response which Solid Start users can throw to redirect to another route. + * see: https://docs.solidjs.com/solid-router/reference/data-apis/response-helpers#redirect + * @param error the potential redirect error + */ +export function isRedirect(error: unknown): boolean { + if (error == null || !(error instanceof Response)) { + return false; + } + + const hasValidLocation = typeof error.headers.get('location') === 'string'; + const hasValidStatus = error.status >= 300 && error.status <= 308; + return hasValidLocation && hasValidStatus; +} diff --git a/packages/solidstart/src/server/withServerActionInstrumentation.ts b/packages/solidstart/src/server/withServerActionInstrumentation.ts new file mode 100644 index 000000000000..4e976322c6e4 --- /dev/null +++ b/packages/solidstart/src/server/withServerActionInstrumentation.ts @@ -0,0 +1,58 @@ +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, handleCallbackErrors } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, captureException, getActiveSpan, spanToJSON, startSpan } from '@sentry/node'; +import { flushIfServerless, isRedirect } from './utils'; + +/** + * Wraps a server action (functions that use the 'use server' directive) + * function body with Sentry Error and Performance instrumentation. + */ +export async function withServerActionInstrumentation     unknown>( + serverActionName: string, + callback: A, +): Promise> { + const activeSpan = getActiveSpan(); + + if (activeSpan) { + const spanData = spanToJSON(activeSpan).data; + + // In solid start, server function calls are made to `/_server` which doesn't tell us + // a lot. We rewrite the span's route to be that of the sever action name but only + // if the target is `/_server`, otherwise we'd overwrite pageloads on routes that use + // server actions (which are more meaningful, e.g. a request to `GET /users/5` is more + // meaningful than overwriting it with `GET doSomeFunctionCall`). + if (spanData && !spanData['http.route'] && spanData['http.target'] === '/_server') { + activeSpan.setAttribute('http.route', serverActionName); + activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'component'); + } + } + + try { + return await startSpan( + { + op: 'function.server_action', + name: serverActionName, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.solidstart', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + }, + }, + async span => { + const result = await handleCallbackErrors(callback, error => { + if (!isRedirect(error)) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(error, { + mechanism: { + handled: false, + type: 'solidstart', + }, + }); + } + }); + + return result; + }, + ); + } finally { + await flushIfServerless(); + } +} diff --git a/packages/solidstart/test/server/withServerActionInstrumentation.test.ts b/packages/solidstart/test/server/withServerActionInstrumentation.test.ts new file mode 100644 index 000000000000..9a5b1e0c2b51 --- /dev/null +++ b/packages/solidstart/test/server/withServerActionInstrumentation.test.ts @@ -0,0 +1,167 @@ +import * as SentryNode from '@sentry/node'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + createTransport, + getCurrentScope, + getIsolationScope, + setCurrentClient, + spanToJSON, +} from '@sentry/node'; +import { NodeClient } from '@sentry/node'; +import { solidRouterBrowserTracingIntegration } from '@sentry/solidstart/solidrouter'; +import { redirect } from '@solidjs/router'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockCaptureException = vi.spyOn(SentryNode, 'captureException').mockImplementation(() => ''); +const mockFlush = vi.spyOn(SentryNode, 'flush').mockImplementation(async () => true); +const mockGetActiveSpan = vi.spyOn(SentryNode, 'getActiveSpan'); + +const mockGetRequestEvent = vi.fn(); +vi.mock('solid-js/web', async () => { + const original = await vi.importActual('solid-js/web'); + return { + ...original, + getRequestEvent: (...args: unknown[]) => mockGetRequestEvent(...args), + }; +}); + +import { SentrySpan } from '@sentry/core'; +import { withServerActionInstrumentation } from '../../src/server'; + +describe('withServerActionInstrumentation', () => { + function createMockNodeClient(): NodeClient { + return new NodeClient({ + integrations: [], + tracesSampleRate: 1, + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], + _metadata: { + sdk: { + name: 'sentry.javascript.solidstart', + }, + }, + }); + } + + // Mimics a server action function using sentry instrumentation + const serverActionGetPrefecture = async function getPrefecture() { + return withServerActionInstrumentation('getPrefecture', () => { + return { prefecture: 'Kagoshima' }; + }); + }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + getCurrentScope().setClient(undefined); + getCurrentScope().clear(); + getIsolationScope().clear(); + }); + + afterEach(() => { + mockCaptureException.mockClear(); + }); + + it('calls captureException', async () => { + const error = new Error('Sample server action error'); + const serverAction = async function getData() { + return withServerActionInstrumentation('getData', () => { + throw error; + }); + }; + + const res = serverAction(); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(error, { mechanism: { handled: false, type: 'solidstart' } }); + }); + + it("doesn't call captureException for thrown redirects", async () => { + const serverRedirectAction = async function getData() { + return withServerActionInstrumentation('getData', () => { + throw redirect('/'); + }); + }; + + const res = serverRedirectAction(); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it('calls `startSpan`', async () => { + const spanStartMock = vi.fn(); + const client = createMockNodeClient(); + setCurrentClient(client); + + client.on('spanStart', span => spanStartMock(spanToJSON(span))); + client.addIntegration(solidRouterBrowserTracingIntegration()); + + await serverActionGetPrefecture(); + expect(spanStartMock).toHaveBeenCalledWith( + expect.objectContaining({ + op: 'function.server_action', + description: 'getPrefecture', + data: expect.objectContaining({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.server_action', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.solidstart', + }), + }), + ); + }); + + it('calls `flush` if lambda', async () => { + vi.stubEnv('LAMBDA_TASK_ROOT', '1'); + + await serverActionGetPrefecture(); + expect(mockFlush).toHaveBeenCalledTimes(1); + }); + + it('calls `flush` if vercel', async () => { + vi.stubEnv('VERCEL', '1'); + + await serverActionGetPrefecture(); + expect(mockFlush).toHaveBeenCalledTimes(1); + }); + + it('sets a server action name on the active span', async () => { + const span = new SentrySpan(); + span.setAttribute('http.target', '/_server'); + mockGetActiveSpan.mockReturnValue(span); + const mockSpanSetAttribute = vi.spyOn(span, 'setAttribute'); + + const getPrefecture = async function load() { + return withServerActionInstrumentation('getPrefecture', () => { + return { prefecture: 'Kagoshima' }; + }); + }; + + await getPrefecture(); + + expect(mockGetActiveSpan).to.toHaveBeenCalledTimes(1); + expect(mockSpanSetAttribute).to.toHaveBeenCalledWith('http.route', 'getPrefecture'); + expect(mockSpanSetAttribute).to.toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'component'); + }); + + it('does not set a server action name if the active span had a non `/_server` target', async () => { + const span = new SentrySpan(); + span.setAttribute('http.target', '/users/5'); + mockGetActiveSpan.mockReturnValue(span); + const mockSpanSetAttribute = vi.spyOn(span, 'setAttribute'); + + const getPrefecture = async function load() { + return withServerActionInstrumentation('getPrefecture', () => { + return { prefecture: 'Kagoshima' }; + }); + }; + + await getPrefecture(); + + expect(mockGetActiveSpan).to.toHaveBeenCalledTimes(1); + expect(mockSpanSetAttribute).not.toHaveBeenCalled(); + }); +}); diff --git a/scripts/node-unit-tests.ts b/scripts/node-unit-tests.ts index 81ea2b0badf3..d5fe581f2a82 100644 --- a/scripts/node-unit-tests.ts +++ b/scripts/node-unit-tests.ts @@ -41,7 +41,7 @@ const SKIP_TEST_PACKAGES: Record = { ], }, '16': { - ignoredPackages: ['@sentry/cloudflare', '@sentry/vercel-edge', '@sentry/astro'], + ignoredPackages: ['@sentry/cloudflare', '@sentry/vercel-edge', '@sentry/astro', '@sentry/solidstart'], }, '18': { ignoredPackages: [], diff --git a/yarn.lock b/yarn.lock index 6d6f295e4049..650c78d792aa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20493,10 +20493,10 @@ import-in-the-middle@1.7.1: cjs-module-lexer "^1.2.2" module-details-from-path "^1.0.3" -import-in-the-middle@^1.10.0, import-in-the-middle@^1.8.1: - version "1.10.0" - resolved "https://registry.yarnpkg.com/import-in-the-middle/-/import-in-the-middle-1.10.0.tgz#f15b0841950ded8d899b635058da5646256949b1" - integrity sha512-Z1jumVdF2GwnnYfM0a/y2ts7mZbwFMgt5rRuVmLgobgahC6iKgN5MBuXjzfTIOUpq5LSU10vJIPpVKe0X89fIw== +import-in-the-middle@^1.11.0, import-in-the-middle@^1.8.1: + version "1.11.0" + resolved "https://registry.yarnpkg.com/import-in-the-middle/-/import-in-the-middle-1.11.0.tgz#a94c4925b8da18256cde3b3b7b38253e6ca5e708" + integrity sha512-5DimNQGoe0pLUHbR9qK84iWaWjjbsxiqXnw6Qz64+azRgleqv9k2kTt5fw7QsOpmaGYtuxxursnPPsnTKEx10Q== dependencies: acorn "^8.8.2" acorn-import-attributes "^1.9.5"