refactor: simplify code

safe-global · Mar 4, 2025 · e9c6997 · e9c6997
1 parent cb13325
commit e9c6997
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 33 deletions.
diff --git a/src/domain/common/errors/http-exception-no-log.error.ts b/src/domain/common/errors/http-exception-no-log.error.ts
@@ -8,6 +8,8 @@ import type { HttpStatus } from '@nestjs/common';
  */
 export class HttpExceptionNoLog extends HttpException {
   constructor(message: string, code: HttpStatus) {
+    // Create body in a similar fashion to code-specific exceptions
+    // @see https://github.com/nestjs/nest/blob/3eaa07ae17245c43732c852084928012c745fa71/packages/common/exceptions/bad-gateway.exception.ts#L44-L49
     super(HttpExceptionNoLog.createBody(message, undefined!, code), code);
   }
 }
diff --git a/src/routes/common/filters/global-error.filter.spec.ts b/src/routes/common/filters/global-error.filter.spec.ts
@@ -73,9 +73,7 @@ describe('GlobalErrorFilter tests', () => {
 
   describe('responses', () => {
     it('http exception returns correct error code and message', async () => {
-      const code = faker.internet.httpStatusCode({
-        types: ['clientError', 'serverError'],
-      });
+      const code = faker.helpers.arrayElement([422, 502]);
 
       await request(app.getHttpServer())
         .post('/http-exception')
@@ -85,9 +83,7 @@ describe('GlobalErrorFilter tests', () => {
     });
 
     it('no log http exception returns correct error code and message', async () => {
-      const code = faker.internet.httpStatusCode({
-        types: ['clientError', 'serverError'],
-      });
+      const code = faker.helpers.arrayElement([422, 502]);
 
       await request(app.getHttpServer())
         .post('/no-log-http-exception')
@@ -109,7 +105,7 @@ describe('GlobalErrorFilter tests', () => {
 
   describe('logs', () => {
     it('should log server errors as errors', async () => {
-      const code = faker.internet.httpStatusCode({ types: ['serverError'] });
+      const code = 502;
 
       await request(app.getHttpServer())
         .post('/http-exception')
@@ -126,9 +122,7 @@ describe('GlobalErrorFilter tests', () => {
     });
 
     it('should log non-server errors as info', async () => {
-      const code = faker.internet.httpStatusCode({
-        types: ['clientError'],
-      });
+      const code = 422;
 
       await request(app.getHttpServer())
         .post('/http-exception')
@@ -145,9 +139,7 @@ describe('GlobalErrorFilter tests', () => {
     });
 
     it('should not log no log http exception', async () => {
-      const code = faker.internet.httpStatusCode({
-        types: ['clientError', 'serverError'],
-      });
+      const code = faker.helpers.arrayElement([422, 502]);
 
       await request(app.getHttpServer())
         .post('/no-log-http-exception')

diff --git a/src/routes/common/filters/global-error.filter.ts b/src/routes/common/filters/global-error.filter.ts
@@ -23,24 +23,18 @@ export class GlobalErrorFilter implements ExceptionFilter {
     const ctx = host.switchToHttp();
 
     const isHttpException = exception instanceof HttpException;
-    const isHttpExceptionNoLog = exception.constructor === HttpExceptionNoLog;
+    const isHttpExceptionNoLog = exception instanceof HttpExceptionNoLog;
 
     const httpStatus =
       isHttpException || isHttpExceptionNoLog
         ? exception.getStatus()
         : HttpStatus.INTERNAL_SERVER_ERROR;
 
     if (!isHttpExceptionNoLog) {
-      const logMessage = {
-        name: exception.name,
-        message: exception.message,
-        stacktrace: exception.stack,
-      };
-      if (httpStatus >= 500 && httpStatus < 600) {
-        this.loggingService.error(logMessage);
-      } else {
-        this.loggingService.info(logMessage);
-      }
+      this.log({
+        exception,
+        httpStatus,
+      });
     }
 
     const responseBody =
@@ -52,4 +46,19 @@ export class GlobalErrorFilter implements ExceptionFilter {
           };
     httpAdapter.reply(ctx.getResponse(), responseBody, httpStatus);
   }
+
+  private log(args: { exception: Error; httpStatus: number }): void {
+    const logMessage = {
+      name: args.exception.name,
+      message: args.exception.message,
+      stacktrace: args.exception.stack,
+    };
+
+    const isServerError = args.httpStatus >= 500 && args.httpStatus < 600;
+    if (isServerError) {
+      this.loggingService.error(logMessage);
+    } else {
+      this.loggingService.info(logMessage);
+    }
+  }
 }
diff --git a/...tions/__tests__/controllers/add-transaction-confirmations.transactions.controller.spec.ts b/...tions/__tests__/controllers/add-transaction-confirmations.transactions.controller.spec.ts
@@ -28,8 +28,6 @@ import { addConfirmationDtoBuilder } from '@/routes/transactions/__tests__/entit
 import type { Server } from 'net';
 import { rawify } from '@/validation/entities/raw.entity';
 import { generatePrivateKey, privateKeyToAccount } from 'viem/accounts';
-import { GlobalErrorFilter } from '@/routes/common/filters/global-error.filter';
-import { APP_FILTER } from '@nestjs/core';
 
 describe('Add transaction confirmations - Transactions Controller (Unit)', () => {
   let app: INestApplication<Server>;
@@ -49,13 +47,6 @@ describe('Add transaction confirmations - Transactions Controller (Unit)', () =>
         TestLoggingModule,
         TestNetworkModule,
       ],
-      providers: [
-        // TODO: Add to all tests to reflect app implementation
-        {
-          provide: APP_FILTER,
-          useClass: GlobalErrorFilter,
-        },
-      ],
     }).compile();
 
     const configurationService = moduleFixture.get<IConfigurationService>(