Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle errors thrown from an EventHandler #1

Open
Sikora00 opened this issue Oct 14, 2021 · 7 comments
Open

Handle errors thrown from an EventHandler #1

Sikora00 opened this issue Oct 14, 2021 · 7 comments

Comments

@Sikora00
Copy link
Contributor

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

@Sikora00
Copy link
Contributor Author

Sikora00 commented Oct 14, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Sikora00 commented Oct 14, 2021

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

@Sikora00
Copy link
Contributor Author

Need to check nestjs/cqrs#134

@Sikora00
Copy link
Contributor Author

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

@kgajowy
Copy link

kgajowy commented Oct 18, 2021

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
Copy link
Contributor Author

@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

Sikora00 pushed a commit that referenced this issue Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants