diff --git a/src/app.module.ts b/src/app.module.ts index b589dcc..b003a35 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/require-await */ import { Logger, Module } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; import { CallModule } from './modules/call/call.module'; import { ConfigModule, ConfigService } from '@nestjs/config'; import configuration from './config/configuration'; @@ -7,6 +8,7 @@ import { SequelizeModule, SequelizeModuleOptions } from '@nestjs/sequelize'; import { format } from 'sql-formatter'; import { SharedModule } from './shared/shared.module'; import { LoggerModule } from './common/logger/logger.module'; +import { HttpGlobalExceptionFilter } from './common/http-exception-filter'; const defaultDbConfig = ( configService: ConfigService, @@ -79,5 +81,11 @@ const defaultDbConfig = ( SharedModule, ], controllers: [], + providers: [ + { + provide: APP_FILTER, + useClass: HttpGlobalExceptionFilter, + }, + ], }) export class AppModule {} diff --git a/src/common/http-exception-filter.spec.ts b/src/common/http-exception-filter.spec.ts new file mode 100644 index 0000000..b41babb --- /dev/null +++ b/src/common/http-exception-filter.spec.ts @@ -0,0 +1,416 @@ +import { + ArgumentsHost, + HttpException, + HttpStatus, + Logger, +} from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { BaseExceptionFilter, HttpAdapterHost } from '@nestjs/core'; +import { ValidationError } from 'sequelize'; +import { AxiosError, AxiosHeaders, InternalAxiosRequestConfig } from 'axios'; +import { + HttpGlobalExceptionFilter, + RequestWithUser, +} from './http-exception-filter'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { v4 } from 'uuid'; + +interface LogPayload { + errorType: string; + requestId: string; + statusCode: number; + method: string; + path: string; + name?: string; + user?: RequestWithUser['user']; + error?: { + message: string; + stack?: string; + }; +} + +describe('HttpGlobalExceptionFilter', () => { + let filter: HttpGlobalExceptionFilter; + let mockHttpAdapter: DeepMocked; + let mockHttpAdapterHost: DeepMocked; + let loggerMock: DeepMocked; + + beforeEach(async () => { + mockHttpAdapter = createMock(); + mockHttpAdapterHost = createMock({ + httpAdapter: mockHttpAdapter, + }); + loggerMock = createMock(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + HttpGlobalExceptionFilter, + { + provide: HttpAdapterHost, + useValue: mockHttpAdapterHost, + }, + ], + }) + .setLogger(loggerMock) + .compile(); + filter = module.get(HttpGlobalExceptionFilter); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('HttpException handling', () => { + it('When a 4xx HttpException is thrown, it should reply with its status and log a warning', () => { + const requestId = v4(); + const exception = new HttpException( + 'Bad Request', + HttpStatus.BAD_REQUEST, + ); + const host = createMockArgumentsHost( + '/call/123', + 'GET', + null, + {}, + requestId, + ); + + filter.catch(exception, host); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + { + statusCode: HttpStatus.BAD_REQUEST, + message: 'Bad Request', + requestId, + }, + HttpStatus.BAD_REQUEST, + ); + expect(loggerMock.warn).toHaveBeenCalled(); + const [payload, message] = loggerMock.warn.mock.calls[0] as [ + LogPayload, + string, + ]; + expect(payload).toMatchObject({ + errorType: 'HTTP_ERROR', + requestId, + statusCode: 400, + method: 'GET', + path: '/call/123', + }); + expect(message).toBe('GET /call/123 400'); + }); + + it.each([ + HttpStatus.UNAUTHORIZED, + HttpStatus.FORBIDDEN, + HttpStatus.NOT_FOUND, + ])( + 'When a %s HttpException is thrown, it should reply but not log', + (status) => { + const exception = new HttpException('Ignored', status); + const host = createMockArgumentsHost('/call', 'GET'); + + filter.catch(exception, host); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + status, + ); + expect(loggerMock.error).not.toHaveBeenCalled(); + expect(loggerMock.warn).not.toHaveBeenCalled(); + }, + ); + + it('When a 5xx HttpException is thrown, it should reply with its status and log an error', () => { + const requestId = v4(); + const exception = new HttpException( + 'Something broke', + HttpStatus.INTERNAL_SERVER_ERROR, + ); + const host = createMockArgumentsHost( + '/call', + 'POST', + { uuid: 'user-uuid' }, + {}, + requestId, + ); + + filter.catch(exception, host); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + { + statusCode: HttpStatus.INTERNAL_SERVER_ERROR, + message: 'Something broke', + requestId, + }, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + expect(loggerMock.error).toHaveBeenCalled(); + const [payload, message] = loggerMock.error.mock.calls[0] as [ + LogPayload, + string, + ]; + expect(payload).toMatchObject({ + errorType: 'HTTP_ERROR', + requestId, + statusCode: 500, + method: 'POST', + path: '/call', + user: { uuid: 'user-uuid' }, + }); + expect(message).toBe('POST /call 500'); + }); + + it('When a 3xx or lower HttpException is thrown, it should not log', () => { + const exception = new HttpException( + 'Redirect', + HttpStatus.MOVED_PERMANENTLY, + ); + const host = createMockArgumentsHost('/old-path', 'GET'); + + filter.catch(exception, host); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + HttpStatus.MOVED_PERMANENTLY, + ); + expect(loggerMock.error).not.toHaveBeenCalled(); + expect(loggerMock.warn).not.toHaveBeenCalled(); + }); + + it('When HttpException has an object response, it should pass it through', () => { + const responseBody = { + statusCode: 400, + message: ['field must be a string'], + error: 'Bad Request', + }; + const exception = new HttpException(responseBody, HttpStatus.BAD_REQUEST); + const host = createMockArgumentsHost('/call', 'POST'); + + filter.catch(exception, host); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + responseBody, + HttpStatus.BAD_REQUEST, + ); + }); + }); + + describe('Unexpected errors', () => { + it('When a generic Error is thrown, it should log with UNEXPECTED_ERROR and return 500', () => { + const requestId = v4(); + const exception = new Error('Something unexpected'); + const host = createMockArgumentsHost( + '/call/123/users/join', + 'POST', + { email: 'user@test.com', uuid: 'user-uuid', id: 1 }, + { name: 'John' }, + requestId, + ); + + filter.catch(exception, host); + + expect(loggerMock.error).toHaveBeenCalled(); + const [payload, message] = loggerMock.error.mock.calls[0] as [ + LogPayload, + string, + ]; + expect(payload).toMatchObject({ + errorType: 'UNEXPECTED_ERROR', + requestId, + statusCode: 500, + name: 'Error', + method: 'POST', + path: '/call/123/users/join', + user: { uuid: 'user-uuid', id: 1 }, + error: { + message: 'Something unexpected', + }, + }); + expect(payload.error?.stack).toBeDefined(); + expect(message).toBe('POST /call/123/users/join 500 - UNEXPECTED_ERROR'); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + { + statusCode: HttpStatus.INTERNAL_SERVER_ERROR, + message: 'Internal Server Error', + requestId, + }, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + }); + + it('When a SequelizeError is thrown, it should log with DATABASE subtype', () => { + const exception = new ValidationError('Database validation error', []); + const host = createMockArgumentsHost('/call', 'POST', { + uuid: 'user-uuid', + }); + + filter.catch(exception, host); + + expect(loggerMock.error).toHaveBeenCalled(); + const [payload, message] = loggerMock.error.mock.calls[0] as [ + LogPayload, + string, + ]; + expect(payload.errorType).toBe('UNEXPECTED_ERROR/DATABASE'); + expect(message).toBe('POST /call 500 - UNEXPECTED_ERROR/DATABASE'); + }); + + it('When an AxiosError is thrown, it should log with EXTERNAL_SERVICE subtype', () => { + const config = { + headers: new AxiosHeaders(), + } as InternalAxiosRequestConfig; + const exception = new AxiosError( + 'External service error', + 'ETIMEDOUT', + config, + {}, + { + status: 500, + data: 'Error', + statusText: 'Error', + headers: {}, + config, + }, + ); + const host = createMockArgumentsHost('/call', 'POST', { + uuid: 'user-uuid', + }); + + filter.catch(exception, host); + + expect(loggerMock.error).toHaveBeenCalled(); + const [payload, message] = loggerMock.error.mock.calls[0] as [ + LogPayload, + string, + ]; + expect(payload.errorType).toBe('UNEXPECTED_ERROR/EXTERNAL_SERVICE'); + expect(message).toBe( + 'POST /call 500 - UNEXPECTED_ERROR/EXTERNAL_SERVICE', + ); + }); + + it('When request has no user, it should handle it gracefully', () => { + const exception = new Error('Unexpected error'); + const host = createMockArgumentsHost('/call', 'GET', null); + + filter.catch(exception, host); + + expect(loggerMock.error).toHaveBeenCalled(); + const [payload] = loggerMock.error.mock.calls[0] as [LogPayload]; + expect(payload.user).toEqual({ + uuid: undefined, + id: undefined, + }); + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + statusCode: HttpStatus.INTERNAL_SERVER_ERROR, + }), + HttpStatus.INTERNAL_SERVER_ERROR, + ); + }); + }); + + describe('Database connection errors', () => { + it('When a Sequelize connection error is thrown, it should return 503 and log', () => { + const exception = new Error('Connection refused'); + exception.name = 'SequelizeConnectionRefusedError'; + const host = createMockArgumentsHost('/call', 'GET', { + uuid: 'user-uuid', + }); + + filter.catch(exception, host); + + expect(loggerMock.error).toHaveBeenCalled(); + const [payload, message] = loggerMock.error.mock.calls[0] as [ + LogPayload, + string, + ]; + expect(payload).toMatchObject({ + errorType: 'DATABASE_CONNECTION_ERROR', + statusCode: HttpStatus.SERVICE_UNAVAILABLE, + name: 'SequelizeConnectionRefusedError', + method: 'GET', + path: '/call', + }); + expect(message).toContain('GET /call 503'); + + expect(mockHttpAdapter.reply).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + statusCode: HttpStatus.SERVICE_UNAVAILABLE, + message: 'Service temporarily unavailable', + }), + HttpStatus.SERVICE_UNAVAILABLE, + ); + }); + + it.each([ + 'SequelizeConnectionAcquireTimeoutError', + 'SequelizeConnectionError', + 'SequelizeConnectionRefusedError', + 'SequelizeConnectionTimedOutError', + ])('should detect %s as a database connection error', (errorName) => { + const exception = new Error('db error'); + exception.name = errorName; + const host = createMockArgumentsHost('/call', 'GET'); + + filter.catch(exception, host); + + const [payload] = loggerMock.error.mock.calls[0] as [LogPayload]; + expect(payload.errorType).toBe('DATABASE_CONNECTION_ERROR'); + }); + }); + + describe('Filter self-protection', () => { + it('When the filter itself throws, it should fallback to the parent BaseExceptionFilter', () => { + const exception = new Error('Original error'); + const host = createMockArgumentsHost('/call', 'GET', { + uuid: 'user-uuid', + }); + + loggerMock.error.mockImplementationOnce(() => { + throw new Error('Error in filter'); + }); + + const superCatchSpy = jest.spyOn(BaseExceptionFilter.prototype, 'catch'); + + filter.catch(exception, host); + + expect(superCatchSpy).toHaveBeenCalledWith( + expect.objectContaining({ message: 'Error in filter' }), + host, + ); + + superCatchSpy.mockRestore(); + }); + }); +}); + +const createMockArgumentsHost = ( + url: string, + method: string, + user: RequestWithUser['user'] | null = null, + body: RequestWithUser['body'] = {}, + id: string = v4(), +) => + createMock({ + switchToHttp: () => ({ + getRequest: () => ({ + url, + method, + user, + body, + id, + }), + getResponse: () => ({}), + }), + }); diff --git a/src/common/http-exception-filter.ts b/src/common/http-exception-filter.ts new file mode 100644 index 0000000..4ec964a --- /dev/null +++ b/src/common/http-exception-filter.ts @@ -0,0 +1,210 @@ +import { + Catch, + HttpException, + ArgumentsHost, + HttpStatus, + Logger, +} from '@nestjs/common'; +import { BaseError as SequelizeError } from 'sequelize'; +import { AxiosError } from 'axios'; +import { BaseExceptionFilter } from '@nestjs/core'; +import { Response } from 'express'; + +export interface RequestWithUser { + id: string; + url: string; + method: string; + body: Record; + user?: { + email?: string; + uuid?: string; + id?: number; + }; +} + +@Catch() +export class HttpGlobalExceptionFilter extends BaseExceptionFilter { + private readonly logger = new Logger(HttpGlobalExceptionFilter.name); + + catch(exception: unknown, host: ArgumentsHost): void { + const { httpAdapter } = this.httpAdapterHost; + const ctx = host.switchToHttp(); + const request = ctx.getRequest(); + const response = ctx.getResponse(); + const requestId = request.id; + + try { + if (exception instanceof HttpException) { + const status = exception.getStatus(); + + this.logHttpException(exception, request, status); + + const res = exception.getResponse(); + const message = + typeof res === 'object' && res !== null + ? res + : { statusCode: status, message: res, requestId }; + + httpAdapter.reply(response, message, status); + return; + } + + const error = + exception instanceof Error ? exception : new Error('Unknown error'); + + if (this.isDatabaseConnectionError(exception)) { + this.logDatabaseConnectionError(error, request); + + httpAdapter.reply( + response, + { + statusCode: HttpStatus.SERVICE_UNAVAILABLE, + message: 'Service temporarily unavailable', + requestId, + }, + HttpStatus.SERVICE_UNAVAILABLE, + ); + return; + } + + this.logUnexpectedError(error, request); + + httpAdapter.reply( + response, + { + statusCode: HttpStatus.INTERNAL_SERVER_ERROR, + message: 'Internal Server Error', + requestId, + }, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + } catch (error: unknown) { + const err = + error instanceof Error ? error : new Error('Unknown error in filter'); + this.logger.error( + { + errorType: 'EXCEPTION_FILTER_ERROR', + method: request.method, + path: request.url, + user: { + uuid: request?.user?.uuid, + }, + error: { + message: err.message, + stack: err.stack, + }, + }, + 'EXCEPTION_FILTER_ERROR', + ); + + super.catch(err, host); + } + } + + private static readonly IGNORED_HTTP_STATUSES = new Set([ + HttpStatus.UNAUTHORIZED, + HttpStatus.FORBIDDEN, + HttpStatus.NOT_FOUND, + ]); + + private logHttpException( + exception: HttpException, + request: RequestWithUser, + status: number, + ) { + if (HttpGlobalExceptionFilter.IGNORED_HTTP_STATUSES.has(status)) { + return; + } + + const logPayload = { + errorType: 'HTTP_ERROR', + requestId: request.id, + statusCode: status, + method: request.method, + path: request.url, + user: { + uuid: request.user?.uuid, + }, + error: { + message: exception.message, + }, + }; + + const logMessage = `${request.method} ${request.url} ${status}`; + + if (status >= 500) { + this.logger.error(logPayload, logMessage); + } else if (status >= 400) { + this.logger.warn(logPayload, logMessage); + } + } + + private isDatabaseConnectionError(exception: unknown): boolean { + const connectionErrorNames = [ + 'SequelizeConnectionAcquireTimeoutError', + 'SequelizeConnectionError', + 'SequelizeConnectionRefusedError', + 'SequelizeConnectionTimedOutError', + ]; + + return connectionErrorNames.includes( + (exception as { name?: string })?.name ?? '', + ); + } + + private logDatabaseConnectionError( + exception: Error, + request: RequestWithUser, + ) { + this.logger.error( + { + errorType: 'DATABASE_CONNECTION_ERROR', + requestId: request.id, + statusCode: HttpStatus.SERVICE_UNAVAILABLE, + name: exception.name, + method: request.method, + path: request.url, + user: { + uuid: request.user?.uuid, + }, + error: { + message: exception.message, + }, + }, + `${request.method} ${request.url} 503 - ${exception.name}`, + ); + } + + private logUnexpectedError(exception: Error, request: RequestWithUser) { + let errorSubtype = ''; + if (exception instanceof SequelizeError) { + errorSubtype = 'DATABASE'; + } else if (exception instanceof AxiosError) { + errorSubtype = 'EXTERNAL_SERVICE'; + } + + const errorType = errorSubtype + ? `UNEXPECTED_ERROR/${errorSubtype}` + : 'UNEXPECTED_ERROR'; + + this.logger.error( + { + errorType, + requestId: request.id, + statusCode: HttpStatus.INTERNAL_SERVER_ERROR, + name: exception.name, + method: request.method, + path: request.url, + user: { + uuid: request.user?.uuid, + id: request.user?.id, + }, + error: { + message: exception.message, + stack: exception.stack, + }, + }, + `${request.method} ${request.url} 500 - ${errorType}`, + ); + } +} diff --git a/src/modules/call/call.controller.spec.ts b/src/modules/call/call.controller.spec.ts index 06d8e0c..fcb4561 100644 --- a/src/modules/call/call.controller.spec.ts +++ b/src/modules/call/call.controller.spec.ts @@ -96,7 +96,7 @@ describe('Testing Call Endpoints', () => { ).rejects.toThrow(ConflictException); }); - it('When an unexpected error occurs, then an error indicating so is thrown', async () => { + it('When an unexpected error occurs, then the error propagates to the global exception filter', async () => { const mockUserToken = createMockUserToken(); callUseCase.createCallAndRoom.mockRejectedValueOnce( @@ -105,7 +105,7 @@ describe('Testing Call Endpoints', () => { await expect( callController.createCall(mockUserToken.payload), - ).rejects.toThrow(InternalServerErrorException); + ).rejects.toThrow(Error); }); }); diff --git a/src/modules/call/call.controller.ts b/src/modules/call/call.controller.ts index 83027f8..4b44e90 100644 --- a/src/modules/call/call.controller.ts +++ b/src/modules/call/call.controller.ts @@ -4,8 +4,6 @@ import { Controller, Get, HttpCode, - HttpException, - InternalServerErrorException, Logger, Param, Post, @@ -77,32 +75,7 @@ export class CallController { throw new BadRequestException('The user id is needed to create a call'); } - try { - const call = await this.callUseCase.createCallAndRoom( - user, - createCallDto?.scheduled, - ); - return call; - } catch (error) { - const err = error as Error; - this.logger.error( - { - userId: uuid, - email: email, - err, - }, - 'Failed to create a call and room', - ); - - if (error instanceof HttpException) { - throw error; - } - - throw new InternalServerErrorException( - 'An unexpected error occurred while creating the call', - { cause: err.message }, - ); - } + return this.callUseCase.createCallAndRoom(user, createCallDto?.scheduled); } @Post('/:id/users/join')