From 8412773fac2acee0fa096dcb957e78159e7f8753 Mon Sep 17 00:00:00 2001 From: pooreumu Date: Tue, 25 Jul 2023 16:01:28 +0900 Subject: [PATCH 1/6] test(): add test for error handling --- tests/e2e/module-e2e.spec.ts | 11 +++++++++++ tests/src/events-provider.consumer.ts | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/tests/e2e/module-e2e.spec.ts b/tests/e2e/module-e2e.spec.ts index 7ac0bf46..dee977a3 100644 --- a/tests/e2e/module-e2e.spec.ts +++ b/tests/e2e/module-e2e.spec.ts @@ -122,6 +122,17 @@ describe('EventEmitterModule - e2e', () => { expect(customConsumer.isEmitted).toBeTruthy(); }); + it('should be able to handled when an unexpected error occurs', async () => { + const eventsConsumerRef = app.get(EventsProviderConsumer); + await app.init(); + + const emitter = app.get(EventEmitter2); + const result = emitter.emit('error-handling.event'); + + expect(eventsConsumerRef.errorHandlingCalls).toEqual(1); + expect(result).toBeTruthy(); + }); + afterEach(async () => { await app.close(); }); diff --git a/tests/src/events-provider.consumer.ts b/tests/src/events-provider.consumer.ts index d5688638..614af83e 100644 --- a/tests/src/events-provider.consumer.ts +++ b/tests/src/events-provider.consumer.ts @@ -5,6 +5,7 @@ import { OnEvent } from '../../lib'; export class EventsProviderConsumer { public eventPayload = {}; public stackedEventCalls = 0; + public errorHandlingCalls = 0; @OnEvent('test.*') onTestEvent(payload: Record) { @@ -16,4 +17,10 @@ export class EventsProviderConsumer { onStackedEvent() { this.stackedEventCalls++; } + + @OnEvent('error-handling.*') + onErrorHandlingEvent() { + this.errorHandlingCalls++; + throw new Error('This is a test error'); + } } From f84ed00af252f9e0e8e5487571071f1117dfde15 Mon Sep 17 00:00:00 2001 From: pooreumu Date: Tue, 25 Jul 2023 16:25:23 +0900 Subject: [PATCH 2/6] fix(): catch unhandled exceptions --- lib/decorators/on-event.decorator.ts | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/decorators/on-event.decorator.ts b/lib/decorators/on-event.decorator.ts index 469a660c..d92827ac 100644 --- a/lib/decorators/on-event.decorator.ts +++ b/lib/decorators/on-event.decorator.ts @@ -1,6 +1,7 @@ import { extendArrayMetadata } from '@nestjs/common/utils/extend-metadata.util'; import { EVENT_LISTENER_METADATA } from '../constants'; import { OnEventOptions } from '../interfaces'; +import { Logger } from '@nestjs/common'; /** * `@OnEvent` decorator metadata @@ -32,10 +33,30 @@ export const OnEvent = ( options?: OnEventOptions, ): MethodDecorator => { const decoratorFactory = (target: object, key?: any, descriptor?: any) => { + const logger = new Logger('OnEvent'); + const originMethod = descriptor.value; + const metaKeys = Reflect.getOwnMetadataKeys(descriptor.value); + const metas = metaKeys.map(key => [ + key, + Reflect.getMetadata(key, descriptor.value), + ]); + descriptor.value = async function (...args: any[]) { + try { + await originMethod.call(this, ...args); + } catch (error) { + if (error instanceof Error) { + logger.error(error, error.stack); + } else { + logger.error(error); + } + } + }; + metas.forEach(([k, v]) => Reflect.defineMetadata(k, v, descriptor.value)); + extendArrayMetadata( - EVENT_LISTENER_METADATA, - [{ event, options } as OnEventMetadata], - descriptor.value, + EVENT_LISTENER_METADATA, + [{ event, options } as OnEventMetadata], + descriptor.value, ); return descriptor; }; From c5b4ff852867ca6ef21efde9fca4647e9e28815c Mon Sep 17 00:00:00 2001 From: pooreumu Date: Thu, 3 Aug 2023 11:58:42 +0900 Subject: [PATCH 3/6] refactor(): extract function --- lib/decorators/on-event.decorator.ts | 36 +++++++++++++--------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/decorators/on-event.decorator.ts b/lib/decorators/on-event.decorator.ts index d92827ac..97029439 100644 --- a/lib/decorators/on-event.decorator.ts +++ b/lib/decorators/on-event.decorator.ts @@ -22,6 +22,22 @@ export interface OnEventMetadata { */ export type OnEventType = string | symbol | Array; +/** + * Wraps the method in try/catch blocks. + * @param descriptor + */ +const wrapDescriptorInTryCatchBlocks = (descriptor: any) => { + const originMethod = descriptor.value; + descriptor.value = async function (...args: any[]) { + try { + await originMethod.call(this, ...args); + } catch (error) { + Logger.error(error, 'OnEvent'); + } + }; + Object.setPrototypeOf(descriptor.value, originMethod); +}; + /** * Event listener decorator. * Subscribes to events based on the specified name(s). @@ -33,25 +49,7 @@ export const OnEvent = ( options?: OnEventOptions, ): MethodDecorator => { const decoratorFactory = (target: object, key?: any, descriptor?: any) => { - const logger = new Logger('OnEvent'); - const originMethod = descriptor.value; - const metaKeys = Reflect.getOwnMetadataKeys(descriptor.value); - const metas = metaKeys.map(key => [ - key, - Reflect.getMetadata(key, descriptor.value), - ]); - descriptor.value = async function (...args: any[]) { - try { - await originMethod.call(this, ...args); - } catch (error) { - if (error instanceof Error) { - logger.error(error, error.stack); - } else { - logger.error(error); - } - } - }; - metas.forEach(([k, v]) => Reflect.defineMetadata(k, v, descriptor.value)); + wrapDescriptorInTryCatchBlocks(descriptor); extendArrayMetadata( EVENT_LISTENER_METADATA, From fcacc9e1c00bb5649551f9a1f2c8067b1842bbf6 Mon Sep 17 00:00:00 2001 From: pooreumu Date: Thu, 3 Aug 2023 15:41:08 +0900 Subject: [PATCH 4/6] refactor(): rename test --- tests/e2e/module-e2e.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/module-e2e.spec.ts b/tests/e2e/module-e2e.spec.ts index dee977a3..a441535b 100644 --- a/tests/e2e/module-e2e.spec.ts +++ b/tests/e2e/module-e2e.spec.ts @@ -122,7 +122,7 @@ describe('EventEmitterModule - e2e', () => { expect(customConsumer.isEmitted).toBeTruthy(); }); - it('should be able to handled when an unexpected error occurs', async () => { + it('should be able to gracefully recover when an unexpected error occurs', async () => { const eventsConsumerRef = app.get(EventsProviderConsumer); await app.init(); From 498fbb3013c10f8566da1d4f992a06b8528661ee Mon Sep 17 00:00:00 2001 From: pooreumu Date: Sat, 5 Aug 2023 23:56:23 +0900 Subject: [PATCH 5/6] test(): add test for error handling in request scoped --- tests/e2e/module-e2e.spec.ts | 13 +++++++++++-- tests/src/events-provider.consumer.ts | 2 +- .../src/events-provider.request-scoped.consumer.ts | 5 +++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/e2e/module-e2e.spec.ts b/tests/e2e/module-e2e.spec.ts index a441535b..5cd6ef3b 100644 --- a/tests/e2e/module-e2e.spec.ts +++ b/tests/e2e/module-e2e.spec.ts @@ -122,17 +122,26 @@ describe('EventEmitterModule - e2e', () => { expect(customConsumer.isEmitted).toBeTruthy(); }); - it('should be able to gracefully recover when an unexpected error occurs', async () => { + it('should be able to gracefully recover when an unexpected error occurs from provider', async () => { const eventsConsumerRef = app.get(EventsProviderConsumer); await app.init(); const emitter = app.get(EventEmitter2); - const result = emitter.emit('error-handling.event'); + const result = emitter.emit('error-handling.provider'); expect(eventsConsumerRef.errorHandlingCalls).toEqual(1); expect(result).toBeTruthy(); }); + it('should be able to gracefully recover when an unexpected error occurs from request scoped', async () => { + await app.init(); + + const eventEmitter = app.get(EventEmitter2); + const result = eventEmitter.emit('error-handling.request-scoped'); + + expect(result).toBeTruthy(); + }); + afterEach(async () => { await app.close(); }); diff --git a/tests/src/events-provider.consumer.ts b/tests/src/events-provider.consumer.ts index 614af83e..b0f51a24 100644 --- a/tests/src/events-provider.consumer.ts +++ b/tests/src/events-provider.consumer.ts @@ -18,7 +18,7 @@ export class EventsProviderConsumer { this.stackedEventCalls++; } - @OnEvent('error-handling.*') + @OnEvent('error-handling.provider') onErrorHandlingEvent() { this.errorHandlingCalls++; throw new Error('This is a test error'); diff --git a/tests/src/events-provider.request-scoped.consumer.ts b/tests/src/events-provider.request-scoped.consumer.ts index cf5233e9..0477aa47 100644 --- a/tests/src/events-provider.request-scoped.consumer.ts +++ b/tests/src/events-provider.request-scoped.consumer.ts @@ -21,4 +21,9 @@ export class EventsProviderRequestScopedConsumer { @OnEvent('string.*') onStringPayloadEvent() {} + + @OnEvent('error-handling.request-scoped') + onErrorHandlingEvent() { + throw new Error('This is a test error'); + } } From ff091ec93689fddb627b94093ae33ee8e26d8d90 Mon Sep 17 00:00:00 2001 From: pooreumu Date: Sat, 5 Aug 2023 23:58:09 +0900 Subject: [PATCH 6/6] fix(): move try-catch to event-subscribers.loader.ts --- lib/decorators/on-event.decorator.ts | 19 ------------------- lib/event-subscribers.loader.ts | 22 +++++++++++++++++++--- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/lib/decorators/on-event.decorator.ts b/lib/decorators/on-event.decorator.ts index 97029439..b080b890 100644 --- a/lib/decorators/on-event.decorator.ts +++ b/lib/decorators/on-event.decorator.ts @@ -1,7 +1,6 @@ import { extendArrayMetadata } from '@nestjs/common/utils/extend-metadata.util'; import { EVENT_LISTENER_METADATA } from '../constants'; import { OnEventOptions } from '../interfaces'; -import { Logger } from '@nestjs/common'; /** * `@OnEvent` decorator metadata @@ -22,22 +21,6 @@ export interface OnEventMetadata { */ export type OnEventType = string | symbol | Array; -/** - * Wraps the method in try/catch blocks. - * @param descriptor - */ -const wrapDescriptorInTryCatchBlocks = (descriptor: any) => { - const originMethod = descriptor.value; - descriptor.value = async function (...args: any[]) { - try { - await originMethod.call(this, ...args); - } catch (error) { - Logger.error(error, 'OnEvent'); - } - }; - Object.setPrototypeOf(descriptor.value, originMethod); -}; - /** * Event listener decorator. * Subscribes to events based on the specified name(s). @@ -49,8 +32,6 @@ export const OnEvent = ( options?: OnEventOptions, ): MethodDecorator => { const decoratorFactory = (target: object, key?: any, descriptor?: any) => { - wrapDescriptorInTryCatchBlocks(descriptor); - extendArrayMetadata( EVENT_LISTENER_METADATA, [{ event, options } as OnEventMetadata], diff --git a/lib/event-subscribers.loader.ts b/lib/event-subscribers.loader.ts index fd11a155..e7d1303b 100644 --- a/lib/event-subscribers.loader.ts +++ b/lib/event-subscribers.loader.ts @@ -1,5 +1,6 @@ import { Injectable, + Logger, OnApplicationBootstrap, OnApplicationShutdown, } from '@nestjs/common'; @@ -24,6 +25,7 @@ export class EventSubscribersLoader implements OnApplicationBootstrap, OnApplicationShutdown { private readonly injector = new Injector(); + private readonly logger = new Logger('Event'); constructor( private readonly discoveryService: DiscoveryService, @@ -92,7 +94,8 @@ export class EventSubscribersLoader } else { listenerMethod( event, - (...args: unknown[]) => instance[methodKey].call(instance, ...args), + (...args: unknown[]) => + this.wrapFunctionInTryCatchBlocks(instance, methodKey, args), options, ); } @@ -135,9 +138,10 @@ export class EventSubscribersLoader moduleRef.providers, contextId, ); - return contextInstance[listenerMethodKey].call( + return this.wrapFunctionInTryCatchBlocks( contextInstance, - ...args, + listenerMethodKey, + args, ); }, options, @@ -168,4 +172,16 @@ export class EventSubscribersLoader this.moduleRef.registerRequestByContextId(payloadObjectOrArray, contextId); } + + private async wrapFunctionInTryCatchBlocks( + instance: Record, + methodKey: string, + args: unknown[], + ) { + try { + return await instance[methodKey].call(instance, ...args); + } catch (e) { + this.logger.error(e); + } + } }