nestjs-architects / cqrs

MIT License
3 stars 0 forks source link

Handle errors thrown from an EventHandler #1

Open Sikora00 opened 3 years ago

Sikora00 commented 3 years ago

How do we want to handle this problem? nestjs/cqrs#409

Sikora00 commented 3 years ago

Option 1

@CatchEventFailture(CatAdopted)
export class CatAdpotedErrorHandler {

  handle(failture) {
    const exception: Error = failture.exception;
    const producer: typeof NotifyCatOwnerEventHandler = failture.producer;
    const event = failture.event
  }
}

Option 2

// in the place the event is dispatched
cat.commit().catch(() => {
  this.commandBus.exec(new RevertCatAdopted())
})

What do you think @kgajowy @Dyostiq?

Sikora00 commented 3 years ago

Anyway. Do we want the EventBus to be a part of the CQRS? It could be a part of a ddd package, especially as this one base on Aggregates. @kgajowy @Dyostiq

Sikora00 commented 3 years ago

I really like the second option. Simply, intuitive, the producer is often the best "person" to decide how to react to the error. :two:

Sikora00 commented 3 years ago

Need to check https://github.com/nestjs/cqrs/issues/134

Sikora00 commented 3 years ago

I see that there is already something like that https://github.com/bradsheppard/nestjs-async-cqrs

kgajowy commented 3 years ago

I think we shall support community needs as well, thus how about various ways of allowing to handle errors?

  1. (global) Logging/Exception like:
providers: [
    {
      provide: APP_CQRS_ERROR_HANDLER,
      useClass: CqrsErrorHandler,
    },
  ],

1A. "global" within module for specific handlers

  1. Decorator you mentioned
@CatchEventFailture(CatAdopted)
  1. catch you mentioned (not really sure about this one, as it sometimes could lead to catching within Sagas, moving compensate actions to different part of the application)

2A / 3A. Extending handler interface with optional (?) method that handles the error

First of all we could add (1) to prevent application to crash and Logger.error it - it isn't any intrusive yet safe option for others.

Sikora00 commented 3 years ago

@kgajowy 1 and 1A could be the same. Like Exception Filters, the one without specified event will catch all. Regarding 3. Sagas are independent units they just could execute a command handler and react for its errors instead of just returning a command. What makes it Independent unit from what I described above. Ofc, what if the saga is triggered by an event which we need to revert? It's easy with RxJS. We just need a place to dispatch an event about that what is possible when aggregate.commit returns a promise