cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Sequence regression in 3.6.1 #401

Closed benurb closed 9 years ago

benurb commented 9 years ago

Good evening everybody :smile:

after the update from when 3.6.0 to 3.6.1 I noticed a regression in the sequence()-logic. The following simplified test produces different results in the two versions:

var sequence = require("when/sequence");

sequence([function () {
    return 1;
}, function () {
    throw new Error("Should be catched");
}]).done(function () {
    console.log("This should not be triggered");
}, function (err) {
    console.log("This should output 'Should be catched': " + err.message);
});

Output 3.6.0

This should output 'Should be catched': Should be catched

Output 3.6.1

/home/myusername/workspace/test/node_modules/when/lib/decorators/unhandledRejection.js:100
        throw e;
              ^
Error: Should be catched
    at /home/myusername/workspace/test/whenbug/test.js:6:11
    at /home/myusername/workspace/test/node_modules/when/sequence.js:33:22
    at dispatch (/home/myusername/workspace/test/node_modules/when/lib/decorators/array.js:284:21)
    at callAndResolve (/home/myusername/workspace/test/node_modules/when/lib/apply.js:30:12)
    at callAndResolveNext (/home/myusername/workspace/test/node_modules/when/lib/apply.js:40:4)
    at Fold.fulfilled (/home/myusername/workspace/test/node_modules/when/lib/makePromise.js:760:11)
    at tryCatchReject (/home/myusername/workspace/test/node_modules/when/lib/makePromise.js:838:30)
    at runContinuation1 (/home/myusername/workspace/test/node_modules/when/lib/makePromise.js:797:4)
    at Fulfilled.when (/home/myusername/workspace/test/node_modules/when/lib/makePromise.js:588:4)
    at Pending.run (/home/myusername/workspace/test/node_modules/when/lib/makePromise.js:479:13)

Thanks for your great work and I hope this is enough information to identify the bug.

- Benjamin

unscriptable commented 9 years ago

Thanks for the simple test case, @benurb!

briancavalier commented 9 years ago

@benurb Thanks for the test case. Yep, it's a regression. Fix coming ...

briancavalier commented 9 years ago

@benurb A candidate fix is in ad0a10a. Can you try it out and let us know if it works for you use case? Thanks!

benurb commented 9 years ago

Yep, that fixes the code from which I derived the test case. Thanks!

briancavalier commented 9 years ago

Great, thanks for confirming!