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 1649cdb94b5f..75308e8f0ea9 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 @@ -96,4 +96,24 @@ export class AppController { async exampleExceptionLocalFilter() { throw new ExampleExceptionLocalFilter(); } + + @Get('test-service-use') + testServiceWithUseMethod() { + return this.appService.use(); + } + + @Get('test-service-transform') + testServiceWithTransform() { + return this.appService.transform(); + } + + @Get('test-service-intercept') + testServiceWithIntercept() { + return this.appService.intercept(); + } + + @Get('test-service-canActivate') + testServiceWithCanActivate() { + return this.appService.canActivate(); + } } 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 f1c935257013..3e4639040a7e 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 @@ -78,4 +78,20 @@ export class AppService { async killTestCron() { this.schedulerRegistry.deleteCronJob('test-cron-job'); } + + use() { + console.log('Test use!'); + } + + transform() { + console.log('Test transform!'); + } + + intercept() { + console.log('Test intercept!'); + } + + canActivate() { + console.log('Test canActivate!'); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts index e33e63f319ca..a9c8ffea9117 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 @@ -707,3 +707,23 @@ test('API route transaction includes exactly one nest async interceptor span aft // 'Interceptor - After Route' is NOT the parent of 'test-controller-span' expect(testControllerSpan.parent_span_id).not.toBe(exampleInterceptorSpanAfterRouteId); }); + +test('Calling use method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-use`); + expect(response.status).toBe(200); +}); + +test('Calling transform method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-transform`); + expect(response.status).toBe(200); +}); + +test('Calling intercept method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-intercept`); + expect(response.status).toBe(200); +}); + +test('Calling canActivate method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-canActivate`); + expect(response.status).toBe(200); +}); 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 1f141dc0f966..70c734c61d73 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 @@ -82,4 +82,24 @@ export class AppController { async flush() { await flush(); } + + @Get('test-service-use') + testServiceWithUseMethod() { + return this.appService.use(); + } + + @Get('test-service-transform') + testServiceWithTransform() { + return this.appService.transform(); + } + + @Get('test-service-intercept') + testServiceWithIntercept() { + return this.appService.intercept(); + } + + @Get('test-service-canActivate') + testServiceWithCanActivate() { + return this.appService.canActivate(); + } } 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 f1c935257013..3e4639040a7e 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 @@ -78,4 +78,20 @@ export class AppService { async killTestCron() { this.schedulerRegistry.deleteCronJob('test-cron-job'); } + + use() { + console.log('Test use!'); + } + + transform() { + console.log('Test transform!'); + } + + intercept() { + console.log('Test intercept!'); + } + + canActivate() { + console.log('Test canActivate!'); + } } diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/transactions.test.ts index eb52dfe98585..23855d1f55b8 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 @@ -705,3 +705,23 @@ test('API route transaction includes exactly one nest async interceptor span aft // 'Interceptor - After Route' is NOT the parent of 'test-controller-span' expect(testControllerSpan.parent_span_id).not.toBe(exampleInterceptorSpanAfterRouteId); }); + +test('Calling use method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-use`); + expect(response.status).toBe(200); +}); + +test('Calling transform method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-transform`); + expect(response.status).toBe(200); +}); + +test('Calling intercept method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-intercept`); + expect(response.status).toBe(200); +}); + +test('Calling canActivate method on service with Injectable decorator returns 200', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-service-canActivate`); + expect(response.status).toBe(200); +}); diff --git a/packages/node/src/integrations/tracing/nest/helpers.ts b/packages/node/src/integrations/tracing/nest/helpers.ts index e2d3c4c573b9..cc83dda3855d 100644 --- a/packages/node/src/integrations/tracing/nest/helpers.ts +++ b/packages/node/src/integrations/tracing/nest/helpers.ts @@ -1,7 +1,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, withActiveSpan } from '@sentry/core'; import type { Span } from '@sentry/types'; import { addNonEnumerableProperty } from '@sentry/utils'; -import type { CatchTarget, InjectableTarget, Observable, Subscription } from './types'; +import type { CatchTarget, InjectableTarget, NextFunction, Observable, Subscription } from './types'; const sentryPatched = 'sentryPatched'; @@ -53,3 +53,22 @@ export function instrumentObservable(observable: Observable, activeSpan }); } } + +/** + * Proxies the next() call in a nestjs middleware to end the span when it is called. + */ +export function getNextProxy(next: NextFunction, span: Span, prevSpan: undefined | Span): NextFunction { + return new Proxy(next, { + apply: (originalNext, thisArgNext, argsNext) => { + span.end(); + + if (prevSpan) { + return withActiveSpan(prevSpan, () => { + return Reflect.apply(originalNext, thisArgNext, argsNext); + }); + } else { + return Reflect.apply(originalNext, thisArgNext, argsNext); + } + }, + }); +} 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 b8ea782ee66f..2d59d97d87fd 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -8,7 +8,7 @@ import { import { getActiveSpan, startInactiveSpan, startSpan, startSpanManual, withActiveSpan } from '@sentry/core'; import type { Span } from '@sentry/types'; import { SDK_VERSION, addNonEnumerableProperty, isThenable } from '@sentry/utils'; -import { getMiddlewareSpanOptions, instrumentObservable, isPatched } from './helpers'; +import { getMiddlewareSpanOptions, getNextProxy, instrumentObservable, isPatched } from './helpers'; import type { CallHandler, CatchTarget, InjectableTarget, MinimalNestJsExecutionContext, Observable } from './types'; const supportedVersions = ['>=8.0.0 <11']; @@ -101,23 +101,19 @@ export class SentryNestInstrumentation extends InstrumentationBase { target.prototype.use = new Proxy(target.prototype.use, { apply: (originalUse, thisArgUse, argsUse) => { const [req, res, next, ...args] = argsUse; - const prevSpan = getActiveSpan(); - return startSpanManual(getMiddlewareSpanOptions(target), (span: Span) => { - const nextProxy = new Proxy(next, { - apply: (originalNext, thisArgNext, argsNext) => { - span.end(); + // Check that we can reasonably assume that the target is a middleware. + // Without these guards, instrumentation will fail if a function named 'use' on a service, which is + // decorated with @Injectable, is called. + if (!req || !res || !next || typeof next !== 'function') { + return originalUse.apply(thisArgUse, argsUse); + } - if (prevSpan) { - return withActiveSpan(prevSpan, () => { - return Reflect.apply(originalNext, thisArgNext, argsNext); - }); - } else { - return Reflect.apply(originalNext, thisArgNext, argsNext); - } - }, - }); + const prevSpan = getActiveSpan(); + return startSpanManual(getMiddlewareSpanOptions(target), (span: Span) => { + // proxy next to end span on call + const nextProxy = getNextProxy(next, span, prevSpan); return originalUse.apply(thisArgUse, [req, res, nextProxy, args]); }); }, @@ -133,6 +129,12 @@ export class SentryNestInstrumentation extends InstrumentationBase { target.prototype.canActivate = new Proxy(target.prototype.canActivate, { apply: (originalCanActivate, thisArgCanActivate, argsCanActivate) => { + const context: MinimalNestJsExecutionContext = argsCanActivate[0]; + + if (!context) { + return originalCanActivate.apply(thisArgCanActivate, argsCanActivate); + } + return startSpan(getMiddlewareSpanOptions(target), () => { return originalCanActivate.apply(thisArgCanActivate, argsCanActivate); }); @@ -148,6 +150,13 @@ export class SentryNestInstrumentation extends InstrumentationBase { target.prototype.transform = new Proxy(target.prototype.transform, { apply: (originalTransform, thisArgTransform, argsTransform) => { + const value = argsTransform[0]; + const metadata = argsTransform[1]; + + if (!value || !metadata) { + return originalTransform.apply(thisArgTransform, argsTransform); + } + return startSpan(getMiddlewareSpanOptions(target), () => { return originalTransform.apply(thisArgTransform, argsTransform); }); @@ -169,6 +178,11 @@ export class SentryNestInstrumentation extends InstrumentationBase { const parentSpan = getActiveSpan(); let afterSpan: Span; + // Check that we can reasonably assume that the target is an interceptor. + if (!context || !next || typeof next.handle !== 'function') { + return originalIntercept.apply(thisArgIntercept, argsIntercept); + } + return startSpanManual(getMiddlewareSpanOptions(target), (beforeSpan: Span) => { // eslint-disable-next-line @typescript-eslint/unbound-method next.handle = new Proxy(next.handle, { @@ -263,6 +277,13 @@ export class SentryNestInstrumentation extends InstrumentationBase { target.prototype.catch = new Proxy(target.prototype.catch, { apply: (originalCatch, thisArgCatch, argsCatch) => { + const exception = argsCatch[0]; + const host = argsCatch[1]; + + if (!exception || !host) { + return originalCatch.apply(thisArgCatch, argsCatch); + } + return startSpan(getMiddlewareSpanOptions(target), () => { return originalCatch.apply(thisArgCatch, argsCatch); }); diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index 18bd26b31ccb..0590462c09d5 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -73,3 +73,8 @@ export interface CatchTarget { catch?: (...args: any[]) => any; }; } + +/** + * Represents an express NextFunction. + */ +export type NextFunction = (err?: any) => void;