cqrs: Exceptions thrown from within event handlers crashes the app
When starting to deal with a type of application that receives Webhooks from an external service, my application reacts by triggering events based on the type of object received. For example, an event that triggers an email to the customer. So, when dealing with some smtp plugins, I accidentally realized that if any exception is thrown from within EventHandlers, the application completely crashes.
Current behavior
Exception thrown from within EventHandlers craches the app.
Expected behavior
A way to handle and don’t crash the app.
Minimal reproduction of the problem with instructions
import { Controller, Get } from '@nestjs/common';
import { EventBus } from '@nestjs/cqrs';
import { HeroKilledDragonEvent } from './events/impl/hero-killed-dragon.event';
@Controller('hero')
export class HeroesGameController {
constructor(
private readonly eventBus: EventBus,
) {}
@Get()
async index(): Promise<any> {
//publishes an event which throws an exceptions and craches the app
return this.eventBus.publish(new HeroKilledDragonEvent('123', '432'));
}
@Get('health')
async health(): Promise<any> {
return 'ok';
}
}
export class HeroKilledDragonEvent {
constructor(
public readonly heroId: string,
public readonly dragonId: string,
) {}
}
import { BadRequestException } from '@nestjs/common';
import { IEventHandler, EventsHandler } from '@nestjs/cqrs';
import { HeroKilledDragonEvent } from '../impl/hero-killed-dragon.event';
@EventsHandler(HeroKilledDragonEvent)
export class HeroKilledDragonHandler implements IEventHandler<HeroKilledDragonEvent> {
handle(event: HeroKilledDragonEvent) {
throw new BadRequestException();
}
}
Environment
Nest version: 7.0.11
This is still an issue in the most recent Nest version
For Tooling issues:
- Node version: 12.13.0
- Platform: Windows
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 18
- Comments: 41 (13 by maintainers)
Commits related to this issue
- bugfix: log errors from event handlers Add log when event handler faced an error before the app crash Refs nestjs/cqrs#409 — committed to Sikora00/cqrs by Sikora00 3 years ago
- bugfix: log errors from event handlers Add log when event handler faced an error before the app crash Refs nestjs/cqrs#409 — committed to Sikora00/cqrs by Sikora00 3 years ago
- bugfix: log errors from event handlers Add log when event handler faced an error before the app crash Refs nestjs/cqrs#409 — committed to Sikora00/cqrs by Sikora00 3 years ago
I can confirm the issue of a crashing application.
Hey guys. I think I’m facing the same problem with notifications about exceptions occuring outside “request/response” scope. I mean, for instance, CRON tasks and CQRS handlers could throws exceptions which
BaseExceptionFiltermiddleware will ignore.Do you already have a recommended approach to make the error tracking more concise in these situations?
I confirm the issue. I thought it’s related to node version and to unhandled promise rejections: https://developer.ibm.com/blogs/nodejs-15-release-blog/
So if you do this, at least the application won’t crash.
process.on(‘unhandledRejection’, (reason, promise) => { // do something, but don’t throw the error });
it is not a unhandled error. im throwing an exception that is well handled by my application. but if by handled you mean putting all my event handlers inside try catchs, yes, they are unhandled. im stuck at 8.0.3 where this behaviour doesn’t exists.
@incompletude The lib is not the one who crashes your app. The unhandled error is 😉
@dudematthew Our solution, for now, is to use a try/catch in our event handlers’
handlemethod. This ensures whatever processing is being done in the event handler has a catch, ensuring that your server will not crash.I would love to see a more holistic solution from the NestJS team. This library is a core utility in our application. I’d like to contribute if there was direction to do so. @kamilmysliwiec @Sikora00
@mciureanu I’m not sure this is a best practice as described in this article. https://blog.heroku.com/best-practices-nodejs-errors
This could be a workaround for now, but we definitely need a way to handle this kind of error at the module, even handler or sagas level.
@collinc777 I agree with you on this.
There seems to be a disconnect in this thread with some who:
This issue is about (1). The app shouldn’t just kill the process unexpectedly. This does not happen for anything else in the app, and we can’t use Exception Filters to deal with it, because it’s outside of the request pipeline.
@kgajowy but actually there can be an event without handlers