feat(api): Add api exception mapping (#6601)

Co-authored-by: Biswajeet Das <[email protected]> Co-authored-by: Sokratis Vidros <[email protected]> Co-authored-by: Paweł Tymczuk <[email protected]> Co-authored-by: Richard Fontein <[email protected]> Co-authored-by: Dima Grossman <[email protected]> Co-authored-by: Himanshu Garg <[email protected]> Co-authored-by: Sokratis Vidros <[email protected]> Co-authored-by: Gali Ainouz Baum <[email protected]> Co-authored-by: Adam Chmara <[email protected]>
novuhq · Oct 7, 2024 · fcc12b0 · fcc12b0
1 parent 3911c6d
commit fcc12b0
Show file tree

Hide file tree

Showing 30 changed files with 1,055 additions and 325 deletions.
diff --git a/.idea/nx-angular-config.xml b/.idea/nx-angular-config.xml
diff --git a/.idea/runConfigurations/API.xml b/.idea/runConfigurations/API.xml
diff --git a/apps/api/package.json b/apps/api/package.json
@@ -48,8 +48,11 @@
     "@novu/stateless": "workspace:*",
     "@novu/testing": "workspace:*",
     "@sendgrid/mail": "^8.1.0",
-    "@sentry/hub": "^7.40.0",
-    "@sentry/node": "^7.40.0",
+    "@sentry/browser": "^8.33.1",
+    "@sentry/hub": "^7.114.0",
+    "@sentry/node": "^8.33.1",
+    "@sentry/nestjs": "^8.33.1",
+    "@sentry/profiling-node": "^8.33.1",
     "@sentry/tracing": "^7.40.0",
     "@types/newrelic": "^9.14.0",
     "@upstash/ratelimit": "^0.4.4",
@@ -68,8 +71,8 @@
     "helmet": "^6.0.1",
     "i18next": "^23.7.6",
     "ioredis": "5.3.2",
-    "jsonwebtoken": "9.0.0",
     "json-schema-defaults": "^0.4.0",
+    "jsonwebtoken": "9.0.0",
     "lodash": "^4.17.15",
     "nanoid": "^3.1.20",
     "nest-raven": "10.1.0",

diff --git a/apps/api/src/app.module.ts b/apps/api/src/app.module.ts
@@ -1,11 +1,10 @@
-/* eslint-disable global-require */
-import { DynamicModule, HttpException, Logger, Module, Provider } from '@nestjs/common';
-import { RavenInterceptor, RavenModule } from 'nest-raven';
+/* eslint-disable global-require */ import { DynamicModule, Logger, Module, Provider } from '@nestjs/common';
 import { APP_INTERCEPTOR } from '@nestjs/core';
 import { Type } from '@nestjs/common/interfaces/type.interface';
 import { ForwardReference } from '@nestjs/common/interfaces/modules/forward-reference.interface';
 import { ProfilingModule, TracingModule } from '@novu/application-generic';
 import { isClerkEnabled } from '@novu/shared';
+import { SentryModule } from '@sentry/nestjs/setup';
 import packageJson from '../package.json';
 import { SharedModule } from './app/shared/shared.module';
 import { UserModule } from './app/user/user.module';
@@ -135,20 +134,7 @@ const providers: Provider[] = [
 ];
 
 if (process.env.SENTRY_DSN) {
-  modules.push(RavenModule);
-  providers.push({
-    provide: APP_INTERCEPTOR,
-    useValue: new RavenInterceptor({
-      filters: [
-        /*
-         * Filter exceptions to type HttpException. Ignore those that
-         * have status code of less than 500
-         */
-        { type: HttpException, filter: (exception: HttpException) => exception.getStatus() < 500 },
-      ],
-      user: ['_id', 'firstName', 'organizationId', 'environmentId', 'roles', 'domain'],
-    }),
-  });
+  modules.unshift(SentryModule.forRoot());
 }
 
 if (process.env.SEGMENT_TOKEN) {

diff --git a/apps/api/src/app/events/e2e/trigger-event.e2e.ts b/apps/api/src/app/events/e2e/trigger-event.e2e.ts
@@ -2082,11 +2082,9 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () {
 
       const { body } = response;
 
-      expect(body).to.eql({
-        statusCode: 422,
-        message: 'workflow_not_found',
-        error: 'Unprocessable Entity',
-      });
+      expect(body.statusCode).to.equal(422);
+      expect(body.message).to.equal('workflow_not_found');
+      expect(body.error).to.equal('Unprocessable Entity');
     });
 
     it('should handle empty workflow scenario', async function () {

diff --git a/apps/api/src/app/layouts/e2e/create-layout.e2e.ts b/apps/api/src/app/layouts/e2e/create-layout.e2e.ts
@@ -78,11 +78,11 @@ describe('Layout creation - /layouts (POST)', async () => {
     });
 
     expect(response.statusCode).to.eql(409);
-    expect(response.body).to.eql({
-      error: 'Conflict',
-      message: `Layout with identifier: ${existingLayoutIdentifier} already exists under environment ${session.environment._id}`,
-      statusCode: 409,
-    });
+    expect(response.body.error).to.eql('Conflict');
+    expect(response.body.message).to.eql(
+      `Layout with identifier: ${existingLayoutIdentifier} already exists under environment ${session.environment._id}`
+    );
+    expect(response.body.statusCode).to.eql(409);
   });
 
   it('if the layout created is assigned as default it should set as non default the existing default layout', async () => {

diff --git a/apps/api/src/app/layouts/e2e/delete-layout.e2e.ts b/apps/api/src/app/layouts/e2e/delete-layout.e2e.ts
@@ -64,11 +64,11 @@ describe('Delete a layout - /layouts/:layoutId (DELETE)', async () => {
     const deleteResponse = await session.testAgent.delete(url);
 
     expect(deleteResponse.statusCode).to.eql(409);
-    expect(deleteResponse.body).to.eql({
-      error: 'Conflict',
-      message: `Layout with id ${createdLayout._id} is being used as your default layout, so it can not be deleted`,
-      statusCode: 409,
-    });
+    expect(deleteResponse.body.error).to.eql('Conflict');
+    expect(deleteResponse.body.message).to.eql(
+      `Layout with id ${createdLayout._id} is being used as your default layout, so it can not be deleted`
+    );
+    expect(deleteResponse.body.statusCode).to.eql(409);
   });
 
   it('should throw a conflict error when the layout ID to soft delete is associated to message templates', async () => {
@@ -100,10 +100,10 @@ describe('Delete a layout - /layouts/:layoutId (DELETE)', async () => {
     const deleteResponse = await session.testAgent.delete(url);
 
     expect(deleteResponse.statusCode).to.eql(409);
-    expect(deleteResponse.body).to.eql({
-      error: 'Conflict',
-      message: `Layout with id ${createdLayout._id} is being used so it can not be deleted`,
-      statusCode: 409,
-    });
+    expect(deleteResponse.body.error).to.eql('Conflict');
+    expect(deleteResponse.body.message).to.eql(
+      `Layout with id ${createdLayout._id} is being used so it can not be deleted`
+    );
+    expect(deleteResponse.body.statusCode).to.eql(409);
   });
 });
diff --git a/apps/api/src/app/layouts/e2e/update-layout.e2e.ts b/apps/api/src/app/layouts/e2e/update-layout.e2e.ts
@@ -105,12 +105,9 @@ describe('Layout update - /layouts (PATCH)', async () => {
       isDefault: updatedIsDefault,
     });
 
-    expect(updateResponse.statusCode).to.eql(409);
-    expect(updateResponse.body).to.eql({
-      error: 'Conflict',
-      message: `One default layout is required`,
-      statusCode: 409,
-    });
+    expect(updateResponse.body.error).to.eql('Conflict');
+    expect(updateResponse.body.message).to.eql('One default layout is required');
+    expect(updateResponse.body.statusCode).to.eql(409);
   });
 
   it('should throw error for an update of layout identifier that already exists in the environment', async function () {
@@ -122,12 +119,11 @@ describe('Layout update - /layouts (PATCH)', async () => {
     const updateResponse = await session.testAgent.patch(url).send({
       identifier: updatedLayoutIdentifier,
     });
-    expect(updateResponse.statusCode).to.eql(409);
-    expect(updateResponse.body).to.eql({
-      error: 'Conflict',
-      message: `Layout with identifier: ${updatedLayoutIdentifier} already exists under environment ${session.environment._id}`,
-      statusCode: 409,
-    });
+    expect(updateResponse.body.message).to.eq(
+      `Layout with identifier: ${updatedLayoutIdentifier} already exists under environment ${session.environment._id}`
+    );
+    expect(updateResponse.body.error).to.eq('Conflict');
+    expect(updateResponse.body.statusCode).to.eq(409);
   });
 
   it('if the layout updated is assigned as default it should set as non default the existing default layout', async () => {

diff --git a/apps/api/src/app/shared/framework/idempotency.e2e.ts b/apps/api/src/app/shared/framework/idempotency.e2e.ts
@@ -108,13 +108,11 @@ describe('Idempotency Test', async () => {
       expect(headerDupe[HttpResponseHeaderKeysEnum.IDEMPOTENCY_KEY.toLowerCase()]).to.eq(key);
       expect(headerDupe[HttpResponseHeaderKeysEnum.LINK.toLowerCase()]).to.eq(DOCS_LINK);
       expect(retryHeader).to.eq(`1`);
-      expect(JSON.stringify(conflictBody)).to.eq(
-        JSON.stringify({
-          message: `Request with key "${key}" is currently being processed. Please retry after 1 second`,
-          error: 'Conflict',
-          statusCode: 409,
-        })
+      expect(conflictBody.message).to.eq(
+        `Request with key "${key}" is currently being processed. Please retry after 1 second`
       );
+      expect(conflictBody.error).to.eq('Conflict');
+      expect(conflictBody.statusCode).to.eq(409);
     });
     it('should return conflict when different body is sent for same key', async () => {
       const key = '5';
@@ -137,13 +135,9 @@ describe('Idempotency Test', async () => {
       expect(headers[HttpResponseHeaderKeysEnum.IDEMPOTENCY_KEY.toLowerCase()]).to.eq(key);
       expect(headerDupe[HttpResponseHeaderKeysEnum.IDEMPOTENCY_KEY.toLowerCase()]).to.eq(key);
       expect(headerDupe[HttpResponseHeaderKeysEnum.LINK.toLowerCase()]).to.eq(DOCS_LINK);
-      expect(JSON.stringify(conflictBody)).to.eq(
-        JSON.stringify({
-          message: `Request with key "${key}" is being reused for a different body`,
-          error: 'Unprocessable Entity',
-          statusCode: 422,
-        })
-      );
+      expect(conflictBody.message).to.eq(`Request with key "${key}" is being reused for a different body`);
+      expect(conflictBody.error).to.eq('Unprocessable Entity');
+      expect(conflictBody.statusCode).to.eq(422);
     });
     it('should return non cached response for unique requests', async () => {
       const key = '6';
@@ -201,7 +195,8 @@ describe('Idempotency Test', async () => {
         .set('authorization', `ApiKey ${session.apiKey}`)
         .send({ data: 422 })
         .expect(422);
-      expect(JSON.stringify(body)).to.equal(JSON.stringify(bodyDupe));
+      expect(body.message).to.equal(bodyDupe.message);
+      expect(body.statusCode).to.equal(bodyDupe.statusCode);
 
       expect(headers[HttpResponseHeaderKeysEnum.IDEMPOTENCY_KEY.toLowerCase()]).to.eq(key);
       expect(headerDupe[HttpResponseHeaderKeysEnum.IDEMPOTENCY_KEY.toLowerCase()]).to.eq(key);
@@ -217,13 +212,9 @@ describe('Idempotency Test', async () => {
         .set('authorization', `ApiKey ${session.apiKey}`)
         .send({ data: 250 })
         .expect(400);
-      expect(JSON.stringify(body)).to.eq(
-        JSON.stringify({
-          message: `idempotencyKey "${key}" has exceeded the maximum allowed length of 255 characters`,
-          error: 'Bad Request',
-          statusCode: 400,
-        })
-      );
+      const { statusCode, message } = body;
+      expect(statusCode).to.eq(400);
+      expect(message).to.eq(`idempotencyKey "${key}" has exceeded the maximum allowed length of 255 characters`);
     });
 
     describe('Allowed Authentication Security Schemes', () => {

diff --git a/apps/api/src/app/testing/product-feature.e2e.ts b/apps/api/src/app/testing/product-feature.e2e.ts
@@ -27,6 +27,7 @@ describe('Product feature Test @skip-in-ee', async () => {
 
   it('should return a 402 response when required api service level does not exists on organization for feature', async () => {
     const { body } = await session.testAgent.get(path).set('authorization', `ApiKey ${session.apiKey}`).expect(402);
-    expect(body).to.deep.equal({ statusCode: 402, message: 'Payment Required' });
+    expect(body.statusCode).to.equal(402);
+    expect(body.message).to.equal('Payment Required');
   });
 });
diff --git a/apps/api/src/bootstrap.ts b/apps/api/src/bootstrap.ts
@@ -1,22 +1,20 @@
 import './config/env.config';
+import './instrument';
 import 'newrelic';
-import '@sentry/tracing';
 
 import helmet from 'helmet';
 import { INestApplication, Logger, ValidationPipe, VersioningType } from '@nestjs/common';
 import { NestFactory, Reflector } from '@nestjs/core';
 import bodyParser from 'body-parser';
-import { Handlers, init, Integrations } from '@sentry/node';
+
 import { BullMqService, getErrorInterceptor, Logger as PinoLogger } from '@novu/application-generic';
 import { ExpressAdapter } from '@nestjs/platform-express';
-
 import { CONTEXT_PATH, corsOptionsDelegate, validateEnv } from './config';
 import { AppModule } from './app.module';
-
-import packageJson from '../package.json';
 import { setupSwagger } from './app/shared/framework/swagger/swagger.controller';
 import { SubscriberRouteGuard } from './app/auth/framework/subscriber-route.guard';
 import { ResponseInterceptor } from './app/shared/framework/response.interceptor';
+import { AllExceptionsFilter } from './exception-filter';
 
 const passport = require('passport');
 const compression = require('compression');
@@ -30,19 +28,6 @@ const extendedBodySizeRoutes = [
   '/v1/bridge/diff',
 ];
 
-if (process.env.SENTRY_DSN) {
-  init({
-    dsn: process.env.SENTRY_DSN,
-    environment: process.env.NODE_ENV,
-    release: `v${packageJson.version}`,
-    ignoreErrors: ['Non-Error exception captured'],
-    integrations: [
-      // enable HTTP calls tracing
-      new Integrations.Http({ tracing: true }),
-    ],
-  });
-}
-
 // Validate the ENV variables after launching SENTRY, so missing variables will report to sentry
 validateEnv();
 
@@ -88,11 +73,6 @@ export async function bootstrap(expressApp?): Promise<INestApplication> {
   server.headersTimeout = 65 * 1000;
   Logger.verbose(`Server headersTimeout: ${server.headersTimeout / 1000}s `);
 
-  if (process.env.SENTRY_DSN) {
-    app.use(Handlers.requestHandler());
-    app.use(Handlers.tracingHandler());
-  }
-
   app.use(helmet());
   app.enableCors(corsOptionsDelegate);
 
@@ -120,8 +100,8 @@ export async function bootstrap(expressApp?): Promise<INestApplication> {
 
   await setupSwagger(app);
 
+  app.useGlobalFilters(new AllExceptionsFilter(app.get(PinoLogger)));
   Logger.log('BOOTSTRAPPED SUCCESSFULLY');
-
   if (expressApp) {
     await app.init();
   } else {