From b66fdd1f5d790de8afbd9daac96f53e15dd9bda8 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Thu, 26 Oct 2023 16:59:41 +0200 Subject: [PATCH 1/3] BC-5489 - add tests --- .../error/filter/global-error.filter.spec.ts | 99 ++++++++++++++++--- 1 file changed, 88 insertions(+), 11 deletions(-) diff --git a/apps/server/src/core/error/filter/global-error.filter.spec.ts b/apps/server/src/core/error/filter/global-error.filter.spec.ts index ca40620515f..df52063da90 100644 --- a/apps/server/src/core/error/filter/global-error.filter.spec.ts +++ b/apps/server/src/core/error/filter/global-error.filter.spec.ts @@ -1,7 +1,7 @@ /* eslint-disable promise/valid-params */ import { NotFound } from '@feathersjs/errors'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { ArgumentsHost, BadRequestException, HttpStatus } from '@nestjs/common'; +import { ArgumentsHost, BadRequestException, HttpStatus, InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { BusinessError } from '@shared/common'; import { ErrorLogger, ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; @@ -9,6 +9,7 @@ import { Response } from 'express'; import util from 'util'; import { ErrorResponse } from '../dto'; import { ErrorLoggable } from '../loggable/error.loggable'; +import { ErrorUtils } from '../utils'; import { GlobalErrorFilter } from './global-error.filter'; class SampleBusinessError extends BusinessError { @@ -42,6 +43,24 @@ class SampleLoggableException extends BadRequestException implements Loggable { } } +class LoggableExceptionWithCause extends InternalServerErrorException implements Loggable { + constructor(private readonly testValue: string, error?: unknown) { + super(ErrorUtils.createHttpExceptionOptions(error)); + } + + getLogMessage(): ErrorLogMessage { + const message: ErrorLogMessage = { + type: 'WITH_CAUSE', + stack: this.stack, + data: { + testValue: this.testValue, + }, + }; + + return message; + } +} + describe('GlobalErrorFilter', () => { let module: TestingModule; let service: GlobalErrorFilter; @@ -304,24 +323,82 @@ describe('GlobalErrorFilter', () => { ).toBeCalledWith(expectedResponse); }); }); + + describe('when error has a cause error', () => { + const setup = () => { + const causeError = new Error('Cause error'); + const error = new LoggableExceptionWithCause('test', causeError); + const expectedResponse = new ErrorResponse( + 'WITH_CAUSE', + 'With Cause', + 'Loggable Exception With Cause', + HttpStatus.INTERNAL_SERVER_ERROR + ); + + const argumentsHost = setupHttpArgumentsHost(); + + return { error, argumentsHost, expectedResponse }; + }; + + it('should set response status appropriately', () => { + const { error, argumentsHost } = setup(); + + service.catch(error, argumentsHost); + + expect(argumentsHost.switchToHttp().getResponse().status).toBeCalledWith( + HttpStatus.INTERNAL_SERVER_ERROR + ); + }); + + it('should send appropriate error response', () => { + const { error, argumentsHost, expectedResponse } = setup(); + + service.catch(error, argumentsHost); + + expect( + argumentsHost.switchToHttp().getResponse().status(HttpStatus.INTERNAL_SERVER_ERROR).json + ).toBeCalledWith(expectedResponse); + }); + }); }); describe('when context is rmq', () => { - const setup = () => { - const argumentsHost = createMock(); - argumentsHost.getType.mockReturnValueOnce('rmq'); + describe('Name of the group', () => { + const setup = () => { + const argumentsHost = createMock(); + argumentsHost.getType.mockReturnValueOnce('rmq'); - const error = new Error(); + const error = new Error(); - return { error, argumentsHost }; - }; + return { error, argumentsHost }; + }; - it('should return an RpcMessage with the error', () => { - const { error, argumentsHost } = setup(); + it('should return an RpcMessage with the error', () => { + const { error, argumentsHost } = setup(); - const result = service.catch(error, argumentsHost); + const result = service.catch(error, argumentsHost); - expect(result).toEqual({ message: undefined, error }); + expect(result).toEqual({ message: undefined, error }); + }); + }); + + describe('when error is a LoggableError', () => { + const setup = () => { + const causeError = new Error('Cause error'); + const error = new LoggableExceptionWithCause('test', causeError); + const argumentsHost = createMock(); + argumentsHost.getType.mockReturnValueOnce('rmq'); + + return { error, argumentsHost }; + }; + + it('should return appropriate error', () => { + const { error, argumentsHost } = setup(); + + const result = service.catch(error, argumentsHost); + + expect(result).toEqual({ message: undefined, error }); + }); }); }); }); From f1756ff9a5b626eec96942c65a23c72d92b64943 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Mon, 30 Oct 2023 10:20:37 +0100 Subject: [PATCH 2/3] BC-5489 - code review --- .../core/error/filter/global-error.filter.spec.ts | 14 +++++++------- .../src/core/error/filter/global-error.filter.ts | 4 +++- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/apps/server/src/core/error/filter/global-error.filter.spec.ts b/apps/server/src/core/error/filter/global-error.filter.spec.ts index df52063da90..3b6786be7cb 100644 --- a/apps/server/src/core/error/filter/global-error.filter.spec.ts +++ b/apps/server/src/core/error/filter/global-error.filter.spec.ts @@ -43,7 +43,7 @@ class SampleLoggableException extends BadRequestException implements Loggable { } } -class LoggableExceptionWithCause extends InternalServerErrorException implements Loggable { +class SampleLoggableExceptionWithCause extends InternalServerErrorException implements Loggable { constructor(private readonly testValue: string, error?: unknown) { super(ErrorUtils.createHttpExceptionOptions(error)); } @@ -327,11 +327,11 @@ describe('GlobalErrorFilter', () => { describe('when error has a cause error', () => { const setup = () => { const causeError = new Error('Cause error'); - const error = new LoggableExceptionWithCause('test', causeError); + const error = new SampleLoggableExceptionWithCause('test', causeError); const expectedResponse = new ErrorResponse( - 'WITH_CAUSE', - 'With Cause', - 'Loggable Exception With Cause', + 'SAMPLE_WITH_CAUSE', + 'Sample With Cause', + 'Sample Loggable Exception With Cause', HttpStatus.INTERNAL_SERVER_ERROR ); @@ -363,7 +363,7 @@ describe('GlobalErrorFilter', () => { }); describe('when context is rmq', () => { - describe('Name of the group', () => { + describe('when error is unknown error', () => { const setup = () => { const argumentsHost = createMock(); argumentsHost.getType.mockReturnValueOnce('rmq'); @@ -385,7 +385,7 @@ describe('GlobalErrorFilter', () => { describe('when error is a LoggableError', () => { const setup = () => { const causeError = new Error('Cause error'); - const error = new LoggableExceptionWithCause('test', causeError); + const error = new SampleLoggableExceptionWithCause('test', causeError); const argumentsHost = createMock(); argumentsHost.getType.mockReturnValueOnce('rmq'); diff --git a/apps/server/src/core/error/filter/global-error.filter.ts b/apps/server/src/core/error/filter/global-error.filter.ts index 7e0d1dc3c3f..56760b18dd9 100644 --- a/apps/server/src/core/error/filter/global-error.filter.ts +++ b/apps/server/src/core/error/filter/global-error.filter.ts @@ -24,7 +24,9 @@ export class GlobalErrorFilter implements Exceptio if (contextType === 'http') { this.sendHttpResponse(error, host); - } else if (contextType === 'rmq') { + } + + if (contextType === 'rmq') { return { message: undefined, error }; } } From 3728f5231b77420ddbdc7fe69ea4870ea3da1bd1 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Mon, 30 Oct 2023 10:25:32 +0100 Subject: [PATCH 3/3] BC-5489 - add test --- .../error/filter/global-error.filter.spec.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/apps/server/src/core/error/filter/global-error.filter.spec.ts b/apps/server/src/core/error/filter/global-error.filter.spec.ts index 3b6786be7cb..c45c13e4bff 100644 --- a/apps/server/src/core/error/filter/global-error.filter.spec.ts +++ b/apps/server/src/core/error/filter/global-error.filter.spec.ts @@ -401,5 +401,24 @@ describe('GlobalErrorFilter', () => { }); }); }); + + describe('when context is other than rmq and http', () => { + const setup = () => { + const argumentsHost = createMock(); + argumentsHost.getType.mockReturnValueOnce('other'); + + const error = new Error(); + + return { error, argumentsHost }; + }; + + it('should return undefined', () => { + const { error, argumentsHost } = setup(); + + const result = service.catch(error, argumentsHost); + + expect(result).toBeUndefined(); + }); + }); }); });