thenativeweb / node-cqrs-saga

Node-cqrs-saga is a node.js module that helps to implement the sagas in cqrs. It can be very useful as domain component if you work with (d)ddd, cqrs, eventdenormalizer, host, etc.
http://cqrs.js.org/pages/saga.html
MIT License
61 stars 16 forks source link

Must timeouted commands really be dispatched manually? #37

Closed IRT-fbachmann closed 6 years ago

IRT-fbachmann commented 7 years ago

Hi,

eventually, I got to the point I needed timeouted commands. I managed to get it working, however, it took a while till I realized that timeouted commands must be dispatched manually. Could you clarify this to me? I do this:

sagas.forEach(saga => {
    const cmds = saga.getTimeoutCommands();

    // Multiple commands may be registered for the case of timeout
    logger.info(`Saga ${saga.id} has ${cmds.length} timeouted command(s).`);
    cmds.forEach(cmd => saga.addCommandToSend(cmd));

    // Save commands to be sent
    saga.commit(errCommit => {
        if (errCommit) {
            return logger.error(saga.id, 'Error while committing.');
        }

        // If successful, remove timeout
        saga.removeTimeout();

        // ... and (manually?????) dispatch that commands to be sent
        dispatch(saga, cmds);
    });
});

My main questions are:

Furthermore (maybe theoretic), if I commit those commandsToSend here, and during the callback of that commit, but before the dispatch, another event occurs that regards that saga, and that saga will commit while handling that event, may the commands be dispatched twice? (Since there are _commands commited to the sagaStore.)

adrai commented 7 years ago

Hi,

until today the timeouted commands feature was always used in a separate process (like a watchdog), where you don't need to register the onCommand function... that's why... but I think in theory this could be done... (I think I wanted to do this some time ago ;-) should I have a look at it?)

The saga.addCommandToSend will additionally set the optional meta information and internally will when commiting the saga the commands will optionally get a generated id...

adrai commented 7 years ago

can you try v1.7.0 ?

IRT-fbachmann commented 7 years ago

Awesome you tried to fix this so fast!! Sadly, there seems to be a fault somewhere. This is my saga handler:

const cmdUncacheItemCount = createUncacheItemCount(event.payload.id);
if (topic.cachedCount >= maxCountCached) {
    logger.info('Going to uncache item counts.');
    saga.addCommandToSend(cmdUncacheItemCount);
} else {
    saga.defineTimeout(new Date(Date.now() + 10000), cmdUncacheItemCount);
}

addCommandToSend produces:

{
  name: 'UNCACHE_ITEM_COUNT',
  context: 'ctx',
  aggregate: 'agg',
  payload: { id: '1234' },
  meta: undefined,
  id: '13ba038d-6f1b-40ad-98b1-a166f5edef0b'
}

defineTimeout:

{
  id: '8992cfa5-c2be-480b-a85c-468364e410c7',
  payload:
  {
     name: 'UNCACHE_ITEM_COUNT',
     context: 'ctx',
     aggregate: 'agg',
     payload: { id: '1234' },
     id: '8992cfa5-c2be-480b-a85c-468364e410c7'
  }
}

I guess the problem is here.

adrai commented 7 years ago

sorry.... I see the issue... will fix asap

adrai commented 7 years ago

v1.7.1

IRT-fbachmann commented 7 years ago

Perfect :) Makes everything clearer!