How to handle errors thrown from defineCommand / defineEvent handlers? about node-cqrs-domain HOT 9 OPEN

When Designing this piece of code I discussed this with other persons.
The decision was to always “punish” programmer errors this way, but feel free to suggest a PR.

from node-cqrs-domain.

Always happy for a PR.

from node-cqrs-domain.

Always happy for a PR.

currently we are at stage of investigating and collecting potential issues before the real start of development. As soon as we will do it wait PR from us :)

from node-cqrs-domain.

These type of errors should theoretically be catched via normal, error argument in the handle callback or as uncaughtException, i.e:

process.on('uncaughtException', function (err) {
})

Can you try to update to v2.14.78 ?

from node-cqrs-domain.

Regarding the custom command handler, you are free to use it and to call external operations, etc...
But I would suggest to solve this via a dedicated saga, which will react on an "preparation"-event, do an external api call and generate the "complete"-command

from node-cqrs-domain.

These type of errors should theoretically be catched via normal, error argument in the handle callback or as uncaughtException, i.e:

process.on('uncaughtException', function (err) {
})

Can you try to update to v2.14.78 ?

thank you, now it works.
I think we should move list of errors which produces commandRejected events into domain options, because now it is hardcoded in createCommandRejectedEvent of domain.js

    if (err instanceof ValidationError || err instanceof BusinessRuleError ||
        err instanceof AggregateDestroyedError || err instanceof AggregateConcurrencyError ||
        err instanceof DuplicateCommandError) {
      dotty.put(evt, this.definitions.event.payload, {
        command: cmd,
        reason: {
          name: err.name,
          message: err.message,
          more: err.more
        }
      });

      if (!!this.definitions.event.revision && dotty.get(evt, this.definitions.event.revision) === undefined && err.more && err.more.aggregateRevision > -1) {
        dotty.put(evt, this.definitions.event.revision, err.more.aggregateRevision);
      }
    } else {
      evt = null;
    }

in reason of this, if error occurred in commandHandler (for example TypeError), we can't rollback our Saga and as result end user will see the hanged operation. I think we should do rollbacks where it possible even if error was made by programmer on code level.

from node-cqrs-domain.

About

process.on('uncaughtException', function (err) {
})

I understand that we can use it in emergency cases if by some reason callback handler was not executed with error object in first argument. But this situation does not allow us to log this error by our OpenTracing server (Jaeger) because to close current trace we need to pass trace id to error handler of Jaeger. In uncaughtException we do not have access to such trace ID because Error object does not contains it itself.

I made this note to explain why we should try to pass error in handler callback in all cases where it possible and leave handling by uncaughtException only for unpredicted errors, for example on library level. This regards all libs not only Domain.

from node-cqrs-domain.

But then in case of the exception inside command handler, we have to deal with unfinished (not completed nor rolled back) saga and manually rollback actions performed before this exception. On the other hand, imagine that command handler catch any exceptions and thrown them as, let me say, CommandHandlerError and this error type would have listed in the code @imissyouso noted above (with ValidationError, BusinessRuleError etc). Then "commandRejected" event will be produced in a natural way, as it was ValidationError or BusinessRuleError, and the saga would be rolled back.

I understand that having an unhandled exception in command is an unusual situation, but now we have to catch it and do produce "commandRejected" manually, and it looks kinda strange to me because other types of exceptions produce it by themselves.

from node-cqrs-domain.

When Designing this piece of code I discussed this with other persons.
The decision was to always “punish” programmer errors this way, but feel free to suggest a PR.

the strange way to punish programmers especially if we are talking about highload systems where in reason of programmer error we will have to rollback all hanged sagas manually - sounds bad for business :)

from node-cqrs-domain.