Fix: promises returned by the Promise.finally callback not being handled

src/Promise.js

@@ -217,14 +217,12 @@ Promise.prototype.always = function (fn) {
  * @returns {Promise} promise
  */
 Promise.prototype.finally = function (fn) {
- var me = this;
- const res = function() {
- return me;
- }
+ const me = this;
+
  const final = function() {
- return new Promise(function (resolve) {
- resolve(fn())
- }).then(res);
+ return new Promise((resolve) => resolve())
+ .then(fn)
+  .then(() => me);
  };
 
  return this.then(final, final);

test/Pool.test.js

@@ -1315,6 +1315,20 @@ describe('Pool', function () {
  });
  });
 
+ it('should terminate inside finally', function () {
+ const pool = new Pool()
+
+ function forever() {
+ return new Promise(resolve => setTimeout(resolve, Infinity))
+ }
+
+ return pool.exec(forever)
+ .timeout(50)
+ .finally(() => {
+ return pool.terminate()
+ });
+ });
+
  describe('validate', () => {
  it('should not allow unknown properties in forkOpts', function() {
  var pool = createPool({

test/Promise.test.js

@@ -257,7 +257,7 @@ describe ('Promise', function () {
  assert.strictEqual(p.pending, false);
  });
 
- it('should continue promsie chain from finally if not rejected', function(done) {
+ it('should continue promise chain from finally if not rejected', function(done) {
  var isFullfilled = false;
  var finallyRan = false;
  var p = new Promise(function(resolve, _reject) {
@@ -278,6 +278,119 @@ describe ('Promise', function () {
  done();
  });
  });
+
+ it('should not pass arguments to finally (resolving)', function(done) {
+ new Promise(resolve => resolve(42))
+ .finally((arg) => {
+ assert.strictEqual(arg, undefined)
+ done()
+ })
+ });
+
+ it('should not pass arguments to finally (rejecting)', function(done) {
+ new Promise((resolve, reject) => reject('Some error'))
+ .finally((arg) => {
+ assert.strictEqual(arg, undefined)
+ done()
+ })
+ });
+
+ it('should not return arguments from finally', function(done) {
+ new Promise((resolve) => resolve())
+ .finally(() => {
+ return 42
+ })
+ .then((arg) => {
+ assert.strictEqual(arg, undefined)
+ done()
+ })
+ });
+
+ it('should pass previous value along when calling finally', function(done) {
+ let finallyCalled = false
+
+ new Promise((resolve) => resolve(42))
+ .finally(() => {
+ finallyCalled = true
+ return 123
+ })
+ .then((arg) => {
+ assert.strictEqual(arg, 42)
+ assert.strictEqual(finallyCalled, true)
+ done()
+ })
+ });
+
+ it('should rethrow previous error along when calling finally', function(done) {
+ let finallyCalled = false
+
+ new Promise((resolve, reject) => reject('some error'))
+ .finally(() => {
+ finallyCalled = true
+ return 123
+ })
+ .catch((err) => {
+ assert.strictEqual(err, 'some error')
+ assert.strictEqual(finallyCalled, true)
+ done()
+ })
+ });
+
+ it('should await a Promise returned by finally (resolving)', function(done) {
+ let finallyCalled = false
+
+ new Promise((resolve) => resolve(42))
+ .finally(() => {
+ return new Promise(resolve => setTimeout(resolve, 100)).then(() => {
+ finallyCalled = true
+ })
+ })
+ .then((arg) => {
+ assert.strictEqual(arg, 42)
+ assert.strictEqual(finallyCalled, true)
+ done()
+ })
+ });
+
+ it('should await a Promise returned by finally (rejecting)', function(done) {
+ let finallyCalled = false
+
+ new Promise((resolve, reject) => reject('some error'))
+ .finally(() => {
+ return new Promise(resolve => setTimeout(resolve, 100)).then(() => {
+ finallyCalled = true
+ })
+ })
+ .catch((err) => {
+ assert.strictEqual(err, 'some error')
+ assert.strictEqual(finallyCalled, true)
+ done()
+ })
+ });
+
+ it('should propagate cancelling a Promise via finally', function(done) {
+ let isResolved = false
+ let finallyCalled = false
+
+ const promise = new Promise((resolve, _reject) => {
+ setTimeout(resolve, 100)
+ })
+ .then(() => {
+ // we should not reach this
+ isResolved = true
+ })
+ .finally(() => {
+ finallyCalled = true
+ })
+
+ promise.cancel()
+
+ setTimeout(() => {
+ assert.strictEqual(isResolved, false)
+ assert.strictEqual(finallyCalled, true)
+ done()
+ }, 200)
+ });
  });
 
  describe('status', function () {