fix(node): Fix nest.js error handler (#11874)

This was brought up here
#5578 (comment),

our error handler implementation was too naive, and our tests not ideal
- the tests only checked that stuff is sent to sentry (which it was!)
but not that the page otherwise worked.

Now, I updated the test to ensure this works as expected.

With this PR, the signature for `Sentry.setupNestErrorHandler()` changes
( breaking change, but sadly required at this point). You have to pass
in an exception filter, which we'll extend to _also_ send exceptions to
Sentry:


```js
import { BaseExceptionFilter, HttpAdapterHost } from '@nestjs/core';

const { httpAdapter } = app1.get(HttpAdapterHost);
Sentry.setupNestErrorHandler(app1, new BaseExceptionFilter(httpAdapter));
```

This is a bit more involved, but also allows you to use a custom filter
if needed (we can extend _any_ exception filter).

Author: mydea

dev-packages/e2e-tests/test-applications/node-nestjs-app/src/main.ts

@@ -1,4 +1,4 @@
-import { NestFactory } from '@nestjs/core';
+import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
 import * as Sentry from '@sentry/node';
 import { AppModule1, AppModule2 } from './app.module';
 
@@ -15,7 +15,9 @@ async function bootstrap() {
   });
 
   const app1 = await NestFactory.create(AppModule1);
-  Sentry.setupNestErrorHandler(app1);
+
+  const { httpAdapter } = app1.get(HttpAdapterHost);
+  Sentry.setupNestErrorHandler(app1, new BaseExceptionFilter(httpAdapter));
 
   await app1.listen(app1Port);
 

dev-packages/e2e-tests/test-applications/node-nestjs-app/tests/errors.test.ts

@@ -47,9 +47,12 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
   });
 
   try {
-    axios.get(`${baseURL}/test-exception/123`);
-  } catch {
-    // this results in an error, but we don't care - we want to check the error event
+    await axios.get(`${baseURL}/test-exception/123`);
+    // Should never be reached!
+    expect(false).toBe(true);
+  } catch (error) {
+    expect(error).toBeInstanceOf(AxiosError);
+    expect(error.response?.status).toBe(500);
   }
 
   const errorEvent = await errorEventPromise;

dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts

@@ -15,7 +15,7 @@ Sentry.init({
 });
 
 import { Controller, Get, Injectable, Module, Param } from '@nestjs/common';
-import { NestFactory } from '@nestjs/core';
+import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
 
 const port = 3480;
 
@@ -49,7 +49,8 @@ class AppModule {}
 
 async function init(): Promise<void> {
   const app = await NestFactory.create(AppModule);
-  Sentry.setupNestErrorHandler(app);
+  const { httpAdapter } = app.get(HttpAdapterHost);
+  Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter));
   await app.listen(port);
   sendPortToRunner(port);
 }

dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts

@@ -13,7 +13,7 @@ Sentry.init({
 });
 
 import { Controller, Get, Injectable, Module, Param } from '@nestjs/common';
-import { NestFactory } from '@nestjs/core';
+import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
 
 const port = 3460;
 
@@ -47,7 +47,8 @@ class AppModule {}
 
 async function init(): Promise<void> {
   const app = await NestFactory.create(AppModule);
-  Sentry.setupNestErrorHandler(app);
+  const { httpAdapter } = app.get(HttpAdapterHost);
+  Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter));
   await app.listen(port);
   sendPortToRunner(port);
 }

dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts

@@ -15,7 +15,7 @@ Sentry.init({
 });
 
 import { Controller, Get, Injectable, Module, Param } from '@nestjs/common';
-import { NestFactory } from '@nestjs/core';
+import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
 
 const port = 3470;
 
@@ -49,7 +49,8 @@ class AppModule {}
 
 async function init(): Promise<void> {
   const app = await NestFactory.create(AppModule);
-  Sentry.setupNestErrorHandler(app);
+  const { httpAdapter } = app.get(HttpAdapterHost);
+  Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter));
   await app.listen(port);
   sendPortToRunner(port);
 }

packages/node/src/integrations/tracing/nest.ts

@@ -17,8 +17,14 @@ interface MinimalNestJsExecutionContext {
     };
   };
 }
+
+interface NestJsErrorFilter {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  catch(exception: any, host: any): void;
+}
+
 interface MinimalNestJsApp {
-  useGlobalFilters: (arg0: { catch(exception: unknown): void }) => void;
+  useGlobalFilters: (arg0: NestJsErrorFilter) => void;
   useGlobalInterceptors: (interceptor: {
     intercept: (context: MinimalNestJsExecutionContext, next: { handle: () => void }) => void;
   }) => void;
@@ -40,16 +46,10 @@ const _nestIntegration = (() => {
  */
 export const nestIntegration = defineIntegration(_nestIntegration);
 
-const SentryNestExceptionFilter = {
-  catch(exception: unknown) {
-    captureException(exception);
-  },
-};
-
 /**
  * Setup an error handler for Nest.
  */
-export function setupNestErrorHandler(app: MinimalNestJsApp): void {
+export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsErrorFilter): void {
   app.useGlobalInterceptors({
     intercept(context, next) {
       if (getIsolationScope() === getDefaultIsolationScope()) {
@@ -65,5 +65,19 @@ export function setupNestErrorHandler(app: MinimalNestJsApp): void {
     },
   });
 
-  app.useGlobalFilters(SentryNestExceptionFilter);
+  const wrappedFilter = new Proxy(baseFilter, {
+    get(target, prop, receiver) {
+      if (prop === 'catch') {
+        const originalCatch = Reflect.get(target, prop, receiver);
+
+        return (exception: unknown, host: unknown) => {
+          captureException(exception);
+          return originalCatch.apply(target, [exception, host]);
+        };
+      }
+      return Reflect.get(target, prop, receiver);
+    },
+  });
+
+  app.useGlobalFilters(wrappedFilter);
 }