Skip to content

Commit

Permalink
EW-1029 adding additional information for HTTP error logs (#5305)
Browse files Browse the repository at this point in the history
* Adding more information to HTTP error logs
  • Loading branch information
psachmann authored and hoeppner-dataport committed Oct 24, 2024
1 parent 81d3b30 commit cd5b84f
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 22 deletions.
28 changes: 24 additions & 4 deletions apps/server/src/core/error/domain/domainErrorHandler.ts
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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;
Expand Down
26 changes: 17 additions & 9 deletions apps/server/src/core/error/filter/global-error.filter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -78,19 +78,27 @@ describe('GlobalErrorFilter', () => {
describe('catch', () => {
describe('when any error is passed as parameter', () => {
const setup = () => {
const argumentsHost = createMock<ArgumentsHost>();
argumentsHost.getType.mockReturnValueOnce('http');
const allContextTypes = Object.keys(UseableContextType);
const contextTypes = [...allContextTypes];
const argumentsHost = createMock<ArgumentsHost>({
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);
});
});

Expand Down
12 changes: 7 additions & 5 deletions apps/server/src/core/error/filter/global-error.filter.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -23,18 +23,20 @@ export class GlobalErrorFilter<E extends IError> implements ExceptionFilter<E> {
constructor(private readonly domainErrorHandler: DomainErrorHandler) {}

catch(error: E, host: ArgumentsHost): void | RpcMessage<undefined> | WsException {
this.domainErrorHandler.exec(error);

const contextType = host.getType<UseableContextType>();
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;
}
}
Expand Down
7 changes: 4 additions & 3 deletions apps/server/src/core/error/loggable/error.loggable.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
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();

getLogMessage(): ErrorLogMessage | ValidationErrorLogMessage {
let logMessage: ErrorLogMessage | ValidationErrorLogMessage = {
error: this.error,
type: '',
data: this.data,
};

if (this.error instanceof ApiValidationError) {
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/core/logger/types/logging.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ export type LogMessageWithContext = {

export type LogMessageData = LogMessageDataObject | string | number | boolean | undefined;

type LogMessageDataObject = {
export type LogMessageDataObject = {
[key: string]: LogMessageData;
};

0 comments on commit cd5b84f

Please sign in to comment.