From 19b5b9754f5e8061f845b68d44d2ca3a4dbcd351 Mon Sep 17 00:00:00 2001 From: Patrick Sachmann <20001160+psachmann@users.noreply.github.com> Date: Thu, 24 Oct 2024 09:31:55 +0200 Subject: [PATCH] EW-1029 adding additional information for HTTP error logs (#5305) * Adding more information to HTTP error logs --- .../core/error/domain/domainErrorHandler.ts | 28 ++++++++++++++++--- .../error/filter/global-error.filter.spec.ts | 26 +++++++++++------ .../core/error/filter/global-error.filter.ts | 12 ++++---- .../src/core/error/loggable/error.loggable.ts | 7 +++-- .../src/core/logger/types/logging.types.ts | 2 +- 5 files changed, 53 insertions(+), 22 deletions(-) diff --git a/apps/server/src/core/error/domain/domainErrorHandler.ts b/apps/server/src/core/error/domain/domainErrorHandler.ts index e61daf094df..778dd48026b 100644 --- a/apps/server/src/core/error/domain/domainErrorHandler.ts +++ b/apps/server/src/core/error/domain/domainErrorHandler.ts @@ -1,6 +1,9 @@ import { Injectable } from '@nestjs/common'; +import { HttpArgumentsHost } from '@nestjs/common/interfaces'; +import { ErrorLogger, Loggable, LoggingUtils, LogMessageDataObject } from '@src/core/logger'; +import { ICurrentUser } from '@src/infra/auth-guard'; +import { Request } from 'express'; import util from 'util'; -import { ErrorLogger, Loggable, LoggingUtils } from '@src/core/logger'; import { ErrorLoggable } from '../loggable'; @Injectable() @@ -12,16 +15,33 @@ export class DomainErrorHandler { this.logger.error(loggable); } - private createErrorLoggable(error: unknown): Loggable { + public execHttpContext(error: unknown, context: HttpArgumentsHost): void { + const request: Request = context.getRequest(); + const user = request.user as ICurrentUser | undefined; + const requestInfo = { + userId: user?.userId, + request: { + method: request.method, + endpoint: request.url, + params: JSON.stringify(request.params), + query: JSON.stringify(request.query), + }, + }; + const loggable = this.createErrorLoggable(error, requestInfo); + + this.logger.error(loggable); + } + + private createErrorLoggable(error: unknown, data?: LogMessageDataObject): Loggable { let loggable: Loggable; if (LoggingUtils.isInstanceOfLoggable(error)) { loggable = error; } else if (error instanceof Error) { - loggable = new ErrorLoggable(error); + loggable = new ErrorLoggable(error, data); } else { const unknownError = new Error(util.inspect(error)); - loggable = new ErrorLoggable(unknownError); + loggable = new ErrorLoggable(unknownError, data); } return loggable; 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 8773c0bb5f3..31ce2b9f04f 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 @@ -3,14 +3,14 @@ import { NotFound } from '@feathersjs/errors'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ArgumentsHost, BadRequestException, HttpStatus, InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; +import { WsException } from '@nestjs/websockets'; import { BusinessError } from '@shared/common'; import { ErrorLogMessage, Loggable } from '@src/core/logger'; import { Response } from 'express'; -import { WsException } from '@nestjs/websockets'; +import { DomainErrorHandler } from '../domain'; import { ErrorResponse } from '../dto'; import { ErrorUtils } from '../utils'; -import { GlobalErrorFilter } from './global-error.filter'; -import { DomainErrorHandler } from '../domain'; +import { GlobalErrorFilter, UseableContextType } from './global-error.filter'; class SampleBusinessError extends BusinessError { constructor() { @@ -78,19 +78,27 @@ describe('GlobalErrorFilter', () => { describe('catch', () => { describe('when any error is passed as parameter', () => { const setup = () => { - const argumentsHost = createMock(); - argumentsHost.getType.mockReturnValueOnce('http'); + const allContextTypes = Object.keys(UseableContextType); + const contextTypes = [...allContextTypes]; + const argumentsHost = createMock({ + getType: () => contextTypes.pop() || '', + }); const error = new Error('test'); - return { error, argumentsHost }; + return { allContextTypes, argumentsHost, error }; }; - it('should be pass the error to domain error handler', () => { - const { error, argumentsHost } = setup(); + it('should call exec on domain error handler', () => { + const { allContextTypes, argumentsHost, error } = setup(); - service.catch(error, argumentsHost); + allContextTypes.forEach(() => { + service.catch(error, argumentsHost); + }); expect(domainErrorHandler.exec).toBeCalledWith(error); + expect(domainErrorHandler.exec).toBeCalledTimes(allContextTypes.length - 1); + expect(domainErrorHandler.execHttpContext).toBeCalledWith(error, {}); + expect(domainErrorHandler.execHttpContext).toBeCalledTimes(1); }); }); 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 a658d0115a7..7098b49a78b 100644 --- a/apps/server/src/core/error/filter/global-error.filter.ts +++ b/apps/server/src/core/error/filter/global-error.filter.ts @@ -1,17 +1,17 @@ import { IError, RpcMessage } from '@infra/rabbitmq'; import { ArgumentsHost, Catch, ExceptionFilter, HttpException, InternalServerErrorException } from '@nestjs/common'; +import { WsException } from '@nestjs/websockets'; import { ApiValidationError, BusinessError } from '@shared/common'; import { Response } from 'express'; import _ from 'lodash'; -import { WsException } from '@nestjs/websockets'; +import { DomainErrorHandler } from '../domain'; import { ApiValidationErrorResponse, ErrorResponse } from '../dto'; import { FeathersError } from '../interface'; import { ErrorUtils } from '../utils'; -import { DomainErrorHandler } from '../domain'; // We are receiving rmq instead of rpc and rmq is missing in context type. // @nestjs/common export type ContextType = 'http' | 'ws' | 'rpc'; -enum UseableContextType { +export enum UseableContextType { http = 'http', rpc = 'rpc', ws = 'ws', @@ -23,18 +23,20 @@ export class GlobalErrorFilter implements ExceptionFilter { constructor(private readonly domainErrorHandler: DomainErrorHandler) {} catch(error: E, host: ArgumentsHost): void | RpcMessage | WsException { - this.domainErrorHandler.exec(error); - const contextType = host.getType(); switch (contextType) { case UseableContextType.http: + this.domainErrorHandler.execHttpContext(error, host.switchToHttp()); return this.sendHttpResponse(error, host); case UseableContextType.rpc: case UseableContextType.rmq: + this.domainErrorHandler.exec(error); return this.sendRpcResponse(error); case UseableContextType.ws: + this.domainErrorHandler.exec(error); return this.sendWsResponse(error); default: + this.domainErrorHandler.exec(error); return undefined; } } diff --git a/apps/server/src/core/error/loggable/error.loggable.ts b/apps/server/src/core/error/loggable/error.loggable.ts index 518c738e8f9..b13f100a1f9 100644 --- a/apps/server/src/core/error/loggable/error.loggable.ts +++ b/apps/server/src/core/error/loggable/error.loggable.ts @@ -1,12 +1,12 @@ +import { ValidationError } from '@nestjs/common'; import { ApiValidationError } from '@shared/common'; import { getMetadataStorage } from 'class-validator'; -import { ValidationError } from '@nestjs/common'; import { Loggable } from '../../logger/interfaces'; -import { ErrorLogMessage, ValidationErrorLogMessage } from '../../logger/types'; +import { ErrorLogMessage, LogMessageDataObject, ValidationErrorLogMessage } from '../../logger/types'; import { ErrorUtils } from '../utils/error.utils'; export class ErrorLoggable implements Loggable { - constructor(private readonly error: Error) {} + constructor(private readonly error: Error, private readonly data?: LogMessageDataObject) {} private readonly classValidatorMetadataStorage = getMetadataStorage(); @@ -14,6 +14,7 @@ export class ErrorLoggable implements Loggable { let logMessage: ErrorLogMessage | ValidationErrorLogMessage = { error: this.error, type: '', + data: this.data, }; if (this.error instanceof ApiValidationError) { diff --git a/apps/server/src/core/logger/types/logging.types.ts b/apps/server/src/core/logger/types/logging.types.ts index 3bd1839fd8b..97f5699fdd1 100644 --- a/apps/server/src/core/logger/types/logging.types.ts +++ b/apps/server/src/core/logger/types/logging.types.ts @@ -26,6 +26,6 @@ export type LogMessageWithContext = { export type LogMessageData = LogMessageDataObject | string | number | boolean | undefined; -type LogMessageDataObject = { +export type LogMessageDataObject = { [key: string]: LogMessageData; };