fix(nestjs): Check arguments before instrumenting with @Injectable

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();
+  }
 }

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!');
+  }
 }

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);
+});

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();
+  }
 }

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!');
+  }
 }

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);
+});

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<unknown>, 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);
+      }
+    },
+  });
+}

packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts

@@ -8,8 +8,15 @@ 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 type { CallHandler, CatchTarget, InjectableTarget, MinimalNestJsExecutionContext, Observable } from './types';
+import { getMiddlewareSpanOptions, getNextProxy, instrumentObservable, isPatched } from './helpers';
+import type {
+  CallHandler,
+  CatchTarget,
+  InjectableTarget,
+  MinimalNestJsExecutionContext,
+  NextFunction,
+  Observable,
+} from './types';
 
 const supportedVersions = ['>=8.0.0 <11'];
 
@@ -103,21 +110,16 @@ export class SentryNestInstrumentation extends InstrumentationBase {
                 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();
-
-                      if (prevSpan) {
-                        return withActiveSpan(prevSpan, () => {
-                          return Reflect.apply(originalNext, thisArgNext, argsNext);
-                        });
-                      } else {
-                        return Reflect.apply(originalNext, thisArgNext, argsNext);
-                      }
-                    },
-                  });
+                // Check that we can reasonably assume that the target is a middleware.
+                // Without this guard, instrumentation will fail if a function named 'use' on a service, which is
+                // decorated with @Injectable, is called.
+                if (!(next satisfies NextFunction)) {
+                  return originalUse.apply(thisArgUse, argsUse);
+                }
 
+                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]);
                 });
               },

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;