From c5e5b9072c87ed6ef6cbc11ac356aa59e7245fa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Thu, 21 Nov 2024 10:30:26 +0100 Subject: [PATCH 1/7] feat(websockets): include exception cause to associate error with req --- .../exceptions/base-ws-exception-filter.ts | 71 +++++++++++++++---- 1 file changed, 58 insertions(+), 13 deletions(-) diff --git a/packages/websockets/exceptions/base-ws-exception-filter.ts b/packages/websockets/exceptions/base-ws-exception-filter.ts index 2b9961a7393..c013a859bfe 100644 --- a/packages/websockets/exceptions/base-ws-exception-filter.ts +++ b/packages/websockets/exceptions/base-ws-exception-filter.ts @@ -8,48 +8,93 @@ import { isObject } from '@nestjs/common/utils/shared.utils'; import { MESSAGES } from '@nestjs/core/constants'; import { WsException } from '../errors/ws-exception'; +interface ErrorPayload { + /** + * Error message identifier. + */ + status: 'error'; + /** + * Error message. + */ + message: string; + /** + * Data that caused the exception. + */ + cause?: unknown; +} + +interface BaseWsExceptionFilterOptions { + /** + * When true, the data that caused the exception will be included in the response. + * This is useful when you want to provide additional context to the client, or + * when you need to associate the error with a specific request. + * @default true + */ + includeCause?: boolean; +} + /** * @publicApi */ export class BaseWsExceptionFilter implements WsExceptionFilter { - private static readonly logger = new Logger('WsExceptionsHandler'); + protected static readonly logger = new Logger('WsExceptionsHandler'); + + constructor(protected readonly options: BaseWsExceptionFilterOptions = {}) { + this.options.includeCause = this.options.includeCause ?? true; + } public catch(exception: TError, host: ArgumentsHost) { const client = host.switchToWs().getClient(); - this.handleError(client, exception); + const data = host.switchToWs().getData(); + this.handleError(client, exception, data); } public handleError( client: TClient, exception: TError, + data: unknown, ) { if (!(exception instanceof WsException)) { - return this.handleUnknownError(exception, client); + return this.handleUnknownError(exception, client, data); } const status = 'error'; const result = exception.getError(); - const message = isObject(result) - ? result - : { - status, - message: result, - }; - - client.emit('exception', message); + + if (isObject(result)) { + return client.emit('exception', result); + } + + const payload: ErrorPayload = { + status, + message: result, + }; + + if (this.options?.includeCause) { + payload.cause = data; + } + + client.emit('exception', payload); } public handleUnknownError( exception: TError, client: TClient, + data: unknown, ) { const status = 'error'; - client.emit('exception', { + const payload: ErrorPayload = { status, message: MESSAGES.UNKNOWN_EXCEPTION_MESSAGE, - }); + }; + + if (this.options?.includeCause) { + payload.cause = data; + } + + client.emit('exception', payload); if (!(exception instanceof IntrinsicException)) { const logger = BaseWsExceptionFilter.logger; From 9a2b04947e247bde1d8e585ef52329ec902fda87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Thu, 21 Nov 2024 11:35:02 +0100 Subject: [PATCH 2/7] chore: include pattern too --- .../exceptions/base-ws-exception-filter.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/websockets/exceptions/base-ws-exception-filter.ts b/packages/websockets/exceptions/base-ws-exception-filter.ts index c013a859bfe..cf238192a5b 100644 --- a/packages/websockets/exceptions/base-ws-exception-filter.ts +++ b/packages/websockets/exceptions/base-ws-exception-filter.ts @@ -18,9 +18,9 @@ interface ErrorPayload { */ message: string; /** - * Data that caused the exception. + * Message that caused the exception. */ - cause?: unknown; + cause?: { pattern: string; data: unknown }; } interface BaseWsExceptionFilterOptions { @@ -47,17 +47,21 @@ export class BaseWsExceptionFilter public catch(exception: TError, host: ArgumentsHost) { const client = host.switchToWs().getClient(); - const data = host.switchToWs().getData(); - this.handleError(client, exception, data); + const pattern = host.switchToWs().getPattern(); + const data = host.switchToWs().getPattern(); + this.handleError(client, exception, { + pattern, + data, + }); } public handleError( client: TClient, exception: TError, - data: unknown, + cause: ErrorPayload['cause'], ) { if (!(exception instanceof WsException)) { - return this.handleUnknownError(exception, client, data); + return this.handleUnknownError(exception, client, cause); } const status = 'error'; @@ -73,7 +77,7 @@ export class BaseWsExceptionFilter }; if (this.options?.includeCause) { - payload.cause = data; + payload.cause = cause; } client.emit('exception', payload); @@ -82,7 +86,7 @@ export class BaseWsExceptionFilter public handleUnknownError( exception: TError, client: TClient, - data: unknown, + data: ErrorPayload['cause'], ) { const status = 'error'; const payload: ErrorPayload = { From 9020d759ca51249d2b91ea8edf8c9662fa9705e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Thu, 21 Nov 2024 11:35:24 +0100 Subject: [PATCH 3/7] chore: export error payload interface --- packages/websockets/exceptions/base-ws-exception-filter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/websockets/exceptions/base-ws-exception-filter.ts b/packages/websockets/exceptions/base-ws-exception-filter.ts index cf238192a5b..73da2a40c3b 100644 --- a/packages/websockets/exceptions/base-ws-exception-filter.ts +++ b/packages/websockets/exceptions/base-ws-exception-filter.ts @@ -8,7 +8,7 @@ import { isObject } from '@nestjs/common/utils/shared.utils'; import { MESSAGES } from '@nestjs/core/constants'; import { WsException } from '../errors/ws-exception'; -interface ErrorPayload { +export interface ErrorPayload { /** * Error message identifier. */ From db95f2009d556bdfacad79230715292fb8790fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Thu, 21 Nov 2024 12:06:35 +0100 Subject: [PATCH 4/7] test: add missing unit tests, update typo --- .../exceptions/base-ws-exception-filter.ts | 2 +- .../exceptions/ws-exceptions-handler.spec.ts | 108 +++++++++++++----- 2 files changed, 82 insertions(+), 28 deletions(-) diff --git a/packages/websockets/exceptions/base-ws-exception-filter.ts b/packages/websockets/exceptions/base-ws-exception-filter.ts index 73da2a40c3b..64da0c2b5b5 100644 --- a/packages/websockets/exceptions/base-ws-exception-filter.ts +++ b/packages/websockets/exceptions/base-ws-exception-filter.ts @@ -48,7 +48,7 @@ export class BaseWsExceptionFilter public catch(exception: TError, host: ArgumentsHost) { const client = host.switchToWs().getClient(); const pattern = host.switchToWs().getPattern(); - const data = host.switchToWs().getPattern(); + const data = host.switchToWs().getData(); this.handleError(client, exception, { pattern, data, diff --git a/packages/websockets/test/exceptions/ws-exceptions-handler.spec.ts b/packages/websockets/test/exceptions/ws-exceptions-handler.spec.ts index 9211c03ea62..b624e9478ae 100644 --- a/packages/websockets/test/exceptions/ws-exceptions-handler.spec.ts +++ b/packages/websockets/test/exceptions/ws-exceptions-handler.spec.ts @@ -7,7 +7,12 @@ import { WsExceptionsHandler } from '../../exceptions/ws-exceptions-handler'; describe('WsExceptionsHandler', () => { let handler: WsExceptionsHandler; let emitStub: sinon.SinonStub; - let client; + let client: { + emit: sinon.SinonStub; + }; + let pattern: string; + let data: unknown; + let executionContextHost: ExecutionContextHost; beforeEach(() => { handler = new WsExceptionsHandler(); @@ -15,47 +20,96 @@ describe('WsExceptionsHandler', () => { client = { emit: emitStub, }; + pattern = 'test'; + data = { foo: 'bar' }; + executionContextHost = new ExecutionContextHost([client, data, pattern]); + client.emit.returns(client); }); describe('handle', () => { - it('should method emit expected status code message when exception is unknown', () => { - handler.handle(new Error(), new ExecutionContextHost([client])); - expect( - emitStub.calledWith('exception', { - status: 'error', - message: 'Internal server error', - }), - ).to.be.true; + describe('when "includeCause" is set to true (default)', () => { + it('should method emit expected status code message when exception is unknown', () => { + handler.handle(new Error(), executionContextHost); + expect( + emitStub.calledWith('exception', { + status: 'error', + message: 'Internal server error', + cause: { + pattern, + data, + }, + }), + ).to.be.true; + }); + describe('when exception is instance of WsException', () => { + it('should method emit expected status and json object', () => { + const message = { + custom: 'Unauthorized', + }; + handler.handle(new WsException(message), executionContextHost); + expect(emitStub.calledWith('exception', message)).to.be.true; + }); + it('should method emit expected status and transform message to json', () => { + const message = 'Unauthorized'; + + handler.handle(new WsException(message), executionContextHost); + console.log(emitStub.getCall(0).args); + expect( + emitStub.calledWith('exception', { + message, + status: 'error', + cause: { + pattern, + data, + }, + }), + ).to.be.true; + }); + }); }); - describe('when exception is instance of WsException', () => { - it('should method emit expected status and json object', () => { - const message = { - custom: 'Unauthorized', - }; - handler.handle( - new WsException(message), - new ExecutionContextHost([client]), - ); - expect(emitStub.calledWith('exception', message)).to.be.true; + + describe('when "includeCause" is set to false', () => { + beforeEach(() => { + handler = new WsExceptionsHandler({ includeCause: false }); }); - it('should method emit expected status and transform message to json', () => { - const message = 'Unauthorized'; + it('should method emit expected status code message when exception is unknown', () => { handler.handle( - new WsException(message), - new ExecutionContextHost([client]), + new Error(), + new ExecutionContextHost([client, pattern, data]), ); - expect(emitStub.calledWith('exception', { message, status: 'error' })) - .to.be.true; + expect( + emitStub.calledWith('exception', { + status: 'error', + message: 'Internal server error', + }), + ).to.be.true; + }); + describe('when exception is instance of WsException', () => { + it('should method emit expected status and json object', () => { + const message = { + custom: 'Unauthorized', + }; + handler.handle(new WsException(message), executionContextHost); + expect(emitStub.calledWith('exception', message)).to.be.true; + }); + it('should method emit expected status and transform message to json', () => { + const message = 'Unauthorized'; + + handler.handle(new WsException(message), executionContextHost); + expect(emitStub.calledWith('exception', { message, status: 'error' })) + .to.be.true; + }); }); }); + describe('when "invokeCustomFilters" returns true', () => { beforeEach(() => { sinon.stub(handler, 'invokeCustomFilters').returns(true); }); it('should not call `emit`', () => { - handler.handle(new WsException(''), new ExecutionContextHost([client])); + handler.handle(new WsException(''), executionContextHost); expect(emitStub.notCalled).to.be.true; }); }); @@ -77,7 +131,7 @@ describe('WsExceptionsHandler', () => { }); }); describe('when filters array is not empty', () => { - let filters, funcSpy; + let filters: any[], funcSpy: sinon.SinonSpy; class TestException {} beforeEach(() => { From a3dbbecd61ed1bc5953fd4864ce4e5f2b4e6b582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Thu, 21 Nov 2024 12:42:40 +0100 Subject: [PATCH 5/7] feat: add cause factory to allow controlling the cause shape --- .../exceptions/base-ws-exception-filter.ts | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/websockets/exceptions/base-ws-exception-filter.ts b/packages/websockets/exceptions/base-ws-exception-filter.ts index 64da0c2b5b5..4b64c838352 100644 --- a/packages/websockets/exceptions/base-ws-exception-filter.ts +++ b/packages/websockets/exceptions/base-ws-exception-filter.ts @@ -8,7 +8,7 @@ import { isObject } from '@nestjs/common/utils/shared.utils'; import { MESSAGES } from '@nestjs/core/constants'; import { WsException } from '../errors/ws-exception'; -export interface ErrorPayload { +export interface ErrorPayload { /** * Error message identifier. */ @@ -20,7 +20,7 @@ export interface ErrorPayload { /** * Message that caused the exception. */ - cause?: { pattern: string; data: unknown }; + cause?: Cause; } interface BaseWsExceptionFilterOptions { @@ -31,6 +31,13 @@ interface BaseWsExceptionFilterOptions { * @default true */ includeCause?: boolean; + + /** + * A factory function that can be used to control the shape of the "cause" object. + * This is useful when you need a custom structure for the cause object. + * @default (pattern, data) => ({ pattern, data }) + */ + causeFactory?: (pattern: string, data: unknown) => Record; } /** @@ -43,6 +50,8 @@ export class BaseWsExceptionFilter constructor(protected readonly options: BaseWsExceptionFilterOptions = {}) { this.options.includeCause = this.options.includeCause ?? true; + this.options.causeFactory = + this.options.causeFactory ?? ((pattern, data) => ({ pattern, data })); } public catch(exception: TError, host: ArgumentsHost) { @@ -71,13 +80,13 @@ export class BaseWsExceptionFilter return client.emit('exception', result); } - const payload: ErrorPayload = { + const payload: ErrorPayload = { status, message: result, }; if (this.options?.includeCause) { - payload.cause = cause; + payload.cause = this.options.causeFactory(cause.pattern, cause.data); } client.emit('exception', payload); @@ -89,13 +98,13 @@ export class BaseWsExceptionFilter data: ErrorPayload['cause'], ) { const status = 'error'; - const payload: ErrorPayload = { + const payload: ErrorPayload = { status, message: MESSAGES.UNKNOWN_EXCEPTION_MESSAGE, }; if (this.options?.includeCause) { - payload.cause = data; + payload.cause = this.options.causeFactory(data.pattern, data.data); } client.emit('exception', payload); From 5c7724b6492b747025ff3efa090c058633e799ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Thu, 21 Nov 2024 12:46:20 +0100 Subject: [PATCH 6/7] test: update integration test --- integration/websockets/e2e/error-gateway.spec.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/integration/websockets/e2e/error-gateway.spec.ts b/integration/websockets/e2e/error-gateway.spec.ts index 95a7d4d2840..b1911bc4863 100644 --- a/integration/websockets/e2e/error-gateway.spec.ts +++ b/integration/websockets/e2e/error-gateway.spec.ts @@ -11,20 +11,27 @@ describe('ErrorGateway', () => { const testingModule = await Test.createTestingModule({ providers: [ErrorGateway], }).compile(); - app = await testingModule.createNestApplication(); + + app = testingModule.createNestApplication(); await app.listen(3000); }); it(`should handle error`, async () => { const ws = io('http://localhost:8080'); - ws.emit('push', { - test: 'test', - }); + const pattern = 'push'; + const data = { test: 'test' }; + + ws.emit(pattern, data); + await new Promise(resolve => ws.on('exception', data => { expect(data).to.be.eql({ status: 'error', message: 'test', + cause: { + pattern, + data, + }, }); resolve(); }), From f80984fb7816f83d948e3d4ccaaa697130b13416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Thu, 21 Nov 2024 14:17:57 +0100 Subject: [PATCH 7/7] test: update integration test --- integration/websockets/e2e/error-gateway.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/websockets/e2e/error-gateway.spec.ts b/integration/websockets/e2e/error-gateway.spec.ts index b1911bc4863..5e5335a00b3 100644 --- a/integration/websockets/e2e/error-gateway.spec.ts +++ b/integration/websockets/e2e/error-gateway.spec.ts @@ -24,8 +24,8 @@ describe('ErrorGateway', () => { ws.emit(pattern, data); await new Promise(resolve => - ws.on('exception', data => { - expect(data).to.be.eql({ + ws.on('exception', error => { + expect(error).to.be.eql({ status: 'error', message: 'test', cause: {