fix: promises returned by the Promise.finally callback not being ha…

…ndled (#470)

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 () {