feat(nestjs): Filter RPC exceptions (#13227)

`RpcExceptions` are always explicitly thrown in nest. Therefore, they are expected for users and should not be sent to Sentry. This PR filters these exceptions. In `@sentry/nestjs` we can simply use `instanceof RpcExceptions` to achieve this. In `@sentry/node` we do not have access to this class, so we need to check based on a property. [ref](#13190)
getsentry · Aug 5, 2024 · c1052ab · c1052ab
1 parent d6b7279
commit c1052ab
Show file tree

Hide file tree

Showing 12 changed files with 96 additions and 7 deletions.
diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/package.json b/dev-packages/e2e-tests/test-applications/nestjs-basic/package.json
@@ -17,6 +17,7 @@
   "dependencies": {
     "@nestjs/common": "^10.0.0",
     "@nestjs/core": "^10.0.0",
+    "@nestjs/microservices": "^10.0.0",
     "@nestjs/schedule": "^4.1.0",
     "@nestjs/platform-express": "^10.0.0",
     "@sentry/nestjs": "latest || *",

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
@@ -49,6 +49,11 @@ export class AppController {
     return this.appService.testExpected500Exception(id);
   }
 
+  @Get('test-expected-rpc-exception/:id')
+  async testExpectedRpcException(@Param('id') id: string) {
+    return this.appService.testExpectedRpcException(id);
+  }
+
   @Get('test-span-decorator-async')
   async testSpanDecoratorAsync() {
     return { result: await this.appService.testSpanDecoratorAsync() };

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
@@ -1,4 +1,5 @@
 import { HttpException, HttpStatus, Injectable } from '@nestjs/common';
+import { RpcException } from '@nestjs/microservices';
 import { Cron, SchedulerRegistry } from '@nestjs/schedule';
 import * as Sentry from '@sentry/nestjs';
 import { SentryCron, SentryTraced } from '@sentry/nestjs';
@@ -38,6 +39,10 @@ export class AppService {
     throw new HttpException(`This is an expected 500 exception with id ${id}`, HttpStatus.INTERNAL_SERVER_ERROR);
   }
 
+  testExpectedRpcException(id: string) {
+    throw new RpcException(`This is an expected RPC exception with id ${id}`);
+  }
+
   @SentryTraced('wait and return a string')
   async wait() {
     await new Promise(resolve => setTimeout(resolve, 500));

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
@@ -69,3 +69,28 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
 
   expect(errorEventOccurred).toBe(false);
 });
+
+test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => {
+  let errorEventOccurred = false;
+
+  waitForError('nestjs-basic', event => {
+    if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected RPC exception with id 123') {
+      errorEventOccurred = true;
+    }
+
+    return event?.transaction === 'GET /test-expected-rpc-exception/:id';
+  });
+
+  const transactionEventPromise = waitForTransaction('nestjs-basic', transactionEvent => {
+    return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/:id';
+  });
+
+  const response = await fetch(`${baseURL}/test-expected-rpc-exception/123`);
+  expect(response.status).toBe(500);
+
+  await transactionEventPromise;
+
+  await new Promise(resolve => setTimeout(resolve, 10000));
+
+  expect(errorEventOccurred).toBe(false);
+});
diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/package.json b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/package.json
@@ -17,6 +17,7 @@
   "dependencies": {
     "@nestjs/common": "^10.0.0",
     "@nestjs/core": "^10.0.0",
+    "@nestjs/microservices": "^10.0.0",
     "@nestjs/schedule": "^4.1.0",
     "@nestjs/platform-express": "^10.0.0",
     "@sentry/nestjs": "latest || *",

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
@@ -49,6 +49,11 @@ export class AppController {
     return this.appService.testExpected500Exception(id);
   }
 
+  @Get('test-expected-rpc-exception/:id')
+  async testExpectedRpcException(@Param('id') id: string) {
+    return this.appService.testExpectedRpcException(id);
+  }
+
   @Get('test-span-decorator-async')
   async testSpanDecoratorAsync() {
     return { result: await this.appService.testSpanDecoratorAsync() };

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
@@ -1,4 +1,5 @@
 import { HttpException, HttpStatus, Injectable } from '@nestjs/common';
+import { RpcException } from '@nestjs/microservices';
 import { Cron, SchedulerRegistry } from '@nestjs/schedule';
 import * as Sentry from '@sentry/nestjs';
 import { SentryCron, SentryTraced } from '@sentry/nestjs';
@@ -38,6 +39,10 @@ export class AppService {
     throw new HttpException(`This is an expected 500 exception with id ${id}`, HttpStatus.INTERNAL_SERVER_ERROR);
   }
 
+  testExpectedRpcException(id: string) {
+    throw new RpcException(`This is an expected RPC exception with id ${id}`);
+  }
+
   @SentryTraced('wait and return a string')
   async wait() {
     await new Promise(resolve => setTimeout(resolve, 500));

diff --git a/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-nestjs-basic/tests/errors.test.ts
@@ -69,3 +69,28 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
 
   expect(errorEventOccurred).toBe(false);
 });
+
+test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => {
+  let errorEventOccurred = false;
+
+  waitForError('node-nestjs-basic', event => {
+    if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected RPC exception with id 123') {
+      errorEventOccurred = true;
+    }
+
+    return event?.transaction === 'GET /test-expected-rpc-exception/:id';
+  });
+
+  const transactionEventPromise = waitForTransaction('node-nestjs-basic', transactionEvent => {
+    return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/:id';
+  });
+
+  const response = await fetch(`${baseURL}/test-expected-rpc-exception/123`);
+  expect(response.status).toBe(500);
+
+  await transactionEventPromise;
+
+  await new Promise(resolve => setTimeout(resolve, 10000));
+
+  expect(errorEventOccurred).toBe(false);
+});
diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json
@@ -51,11 +51,13 @@
   },
   "devDependencies": {
     "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0",
-    "@nestjs/core": "^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"
   },
   "peerDependencies": {
     "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0",
-    "@nestjs/core": "^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"
   },
   "scripts": {
     "build": "run-p build:transpile build:types",

diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts
@@ -11,6 +11,7 @@ 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 {
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -68,7 +69,7 @@ class SentryGlobalFilter extends BaseExceptionFilter {
    */
   public catch(exception: unknown, host: ArgumentsHost): void {
     // don't report expected errors
-    if (exception instanceof HttpException) {
+    if (exception instanceof HttpException || exception instanceof RpcException) {
       return super.catch(exception, host);
     }
 

diff --git a/packages/node/src/integrations/tracing/nest/nest.ts b/packages/node/src/integrations/tracing/nest/nest.ts
@@ -87,10 +87,16 @@ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsE
         const originalCatch = Reflect.get(target, prop, receiver);
 
         return (exception: unknown, host: unknown) => {
-          const status_code = (exception as { status?: number }).status;
-
-          // don't report expected errors
-          if (status_code !== undefined) {
+          const exceptionIsObject = typeof exception === 'object' && exception !== null;
+          const exceptionStatusCode = exceptionIsObject && 'status' in exception ? exception.status : null;
+          const exceptionErrorProperty = exceptionIsObject && 'error' in exception ? exception.error : null;
+
+          /*
+          Don't report expected NestJS control flow errors
+          - `HttpException` errors will have a `status` property
+          - `RpcException` errors will have an `error` property
+           */
+          if (exceptionStatusCode !== null || exceptionErrorProperty !== null) {
             return originalCatch.apply(target, [exception, host]);
           }
 

diff --git a/yarn.lock b/yarn.lock
@@ -6135,6 +6135,14 @@
     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"