Edge Case: Deferred calls can have the wrong context in `processQueue`

[arobson]

I haven't had a chance to track down how this is happening yet, but 2.0.1 contains a breaking change where the context can get set incorrectly during the processQueue call post transition for deferred events.

While I don't understand the test setups well enough yet to reproduce this in tests, I do know that the following change to the code doesn't break additional tests and fixes the issue I was seeing:

starting at line 223 in BehavioralFsm.js in the function processQueue

                _.each( toProcess, function( item ) {
            var context = this.handle ? this : client
            context.handle.apply( context, [ client ].concat( item.args ) );
        }, this );

Without this change in place, the fsms in Rabbot blow up with the latest machina complaining that an unhandled promise rejection occurred because cannot read property apply of undefined. After going through all of my uses of apply, I began looking at where the issue was happening in my tests and narrowed it down to this edge case.

In the meantime, I can just use machina 2.0.0 without any issues from what I've seen so far.

If it helps, in my edge case, this has become the global context and not the fsm where client appears to still be an instance of the fsm containing the handle call (which does get called as expected).

[arobson]

Well, this is odd - 2.0.0 is broken on Node 6.13.2 in the same way that 2.0.1 behaves for Node 8. Oddly enough the behavior changes for 2.0.0 depending on which version of Node I run it under.

[ifandelse]

@arobson could you send me the stack trace leading to the cannot read property apply of undefined error? It will help to see how the FSM was wired up - the fact that it changes between diff node versions is baffling. The wire-up of the original processQueue is correct AFAICT. Do you still see the same issue if you update that part of the method to this:

_.each( toProcess, function( item ) {
    this.handle.apply( this, [ client ].concat( item.args ) );
}.bind( this ) ); // native bind instead of passing this via lodash

[arobson]

@ifandelse

The simplest thing I can change in the code that makes it work again (in Node 6) is:

_.each( toProcess, function( item ) {
        client.handle.apply( client, [ client ].concat( item.args ) );
}, this );

All the FSMs do this. I'm using the following pattern:

const Fsm = machina.Fsm.extend({
})
Monologue.mixInto(Fsm);
const fsm = new Fsm();

The stack always looks like this:

TypeError: Cannot read property 'apply' of undefined
    at /git/node/rabbot/node_modules/machina/lib/machina.js:702:16
    at arrayEach (/git/node/rabbot/node_modules/lodash/lodash.js:508:11)
    at Function.forEach (/git/node/rabbot/node_modules/lodash/lodash.js:9334:14)
    at fsm.processQueue (/git/node/rabbot/node_modules/machina/lib/machina.js:701:6)
    at fsm.Fsm.(anonymous function) [as processQueue] (/git/node/rabbot/node_modules/machina/lib/machina.js:466:63)
    at fsm.transition (/git/node/rabbot/node_modules/machina/lib/machina.js:661:12)
    at fsm.Fsm.(anonymous function) [as transition] (/git/node/rabbot/node_modules/machina/lib/machina.js:466:63)
    at fsm._onAcquisition (/git/node/rabbot/src/amqp/iomonad.js:109:12)
    at process._tickCallback (internal/process/next_tick.js:109:7) }

[arobson]

Thanks, @ifandelse! Your suggestion was exactly what fixed it 🥇