Skip to content

Commit

Permalink
Merge pull request #339 from codeworrior/finally-and-non-callable-cal…
Browse files Browse the repository at this point in the history
…lback

[Fixes #336] Promise.prototype.finally must not fail for a non-callable callback
  • Loading branch information
stefanpenner committed Sep 10, 2018
2 parents 314e483 + a770571 commit df63bb8
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/es6-promise/promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,12 @@ class Promise {
let promise = this;
let constructor = promise.constructor;

return promise.then(value => constructor.resolve(callback()).then(() => value),
reason => constructor.resolve(callback()).then(() => { throw reason; }));
if ( isFunction(callback) ) {
return promise.then(value => constructor.resolve(callback()).then(() => value),
reason => constructor.resolve(callback()).then(() => { throw reason; }));
}

return promise.then(callback, callback);
}
}

Expand Down
20 changes: 20 additions & 0 deletions test/extension-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,26 @@ describe('Promise.prototype.finally', function() {
done();
});
});

it("preserves the original fulfillment value even if a non-callable callback is given", function(done) {
var fulfillmentValue = 1;
var promise = Promise.resolve(fulfillmentValue);

promise['finally']().then(function(value) {
assert.equal(fulfillmentValue, value);
done();
});
});

it("preserves the original rejection reason even if a non-callable callback is given", function(done) {
var rejectionReason = new Error();
var promise = Promise.reject(rejectionReason);

promise['finally']().then(undefined, function(reason) {
assert.equal(rejectionReason, reason);
done();
});
});
});

describe("exception cases do propogate the failure", function(){
Expand Down

0 comments on commit df63bb8

Please sign in to comment.