Add OnSafeEvent

[koreanddinghwan]

Is there an existing issue that is already proposing this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe it

OnEvent has problem with server stops.

Describe the solution you'd like

import { applyDecorators, Logger } from '@nestjs/common';
import { OnEvent, OnEventType } from '@nestjs/event-emitter';
import { OnEventOptions } from '@nestjs/event-emitter/dist/interfaces';

function _OnSafeEvent() {
  return function (target: any, key: string, descriptor: PropertyDescriptor) {
    const originalMethod = 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 originalMethod.call(this, ...args);
      } catch (err) {
        Logger.error(err, err.stack, 'OnSafeEvent');
      }
    };
    metas.forEach(([k, v]) => Reflect.defineMetadata(k, v, descriptor.value));
  };
}

export function OnSafeEvent(event: OnEventType, options?: OnEventOptions | undefined) {
  return applyDecorators(OnEvent(event, options), _OnSafeEvent());
}

Teachability, documentation, adoption, migration strategy

No response

What is the motivation / use case for changing the behavior?

do not stop server when the exception not catched in OnEvent

[micalevisk]

So you're basically proposing a no-op on errors for event handlers.

What about adding a new option to the OnEvent instead. Both aren't that flexible tho...

[koreanddinghwan]

So you're basically proposing a no-op on errors for event handlers.

What about adding a new option to the OnEvent instead. Both aren't that flexible tho...

import { OnOptions } from 'eventemitter2';
export declare type OnEventOptions = OnOptions & {
    prependListener?: boolean;
    suppressErrors?: boolean;
};

is suppressErrors do the samething as i suggested?

[micalevisk]

yes. I didn't know about that flag

event-emitter/tests/e2e/module-e2e.spec.ts

Lines 136 to 145 in 0c5b404

	it('should be able to gracefully recover when an unexpected error occurs from provider and suppressErrors is true', async () => {
	const eventsConsumerRef = app.get(EventsProviderConsumer);
	await app.init();
	const emitter = app.get(EventEmitter2);
	const result = emitter.emit('error-handling-suppressed.provider');
	expect(eventsConsumerRef.errorHandlingCalls).toEqual(1);
	expect(result).toBeTruthy();
	});

event-emitter/tests/src/events-provider.consumer.ts

Lines 27 to 31 in 0c5b404

	@OnEvent('error-handling-suppressed.provider', { suppressErrors: true })
	onErrorHandlingSuppressedEvent() {
	this.errorHandlingCalls++;
	throw new Error('This is a test error');
	}

[koreanddinghwan]

thank you,
in korea, i saw a blog about 'when event handler do not catch exception, server stops with runtime error'

blogs

but, i cannot reproduce that behavior.

or, somewhere in

i've already make 'onSafeEvent' decorator in my project, but... should it needed?

i think that if we wanna log eventHandler's exception , then use the custom decorator is fine.

oh, i found PR #936 , the blogs is too old, thank you!

[micalevisk]

@koreanddinghwan the docs is missing that feature as well

feel free to open a PR to update it https://docs.nestjs.com/techniques/events

[koreanddinghwan]

@koreanddinghwan the docs is missing that feature as well

feel free to open a PR to update it https://docs.nestjs.com/techniques/events

got it, i'm not good at open-source contribute, and it will be the first time, but i'll gonna try it, thank you for your help!

[koreanddinghwan]

nestjs/docs.nestjs.com#2888