[js] Fix a promise/a+ compliance bug revealed when running against version

2.1.1 of the compliance test suite.

Author: jleyba

javascript/node/selenium-webdriver/CHANGES.md

@@ -2,6 +2,11 @@
 
 * Node v0.12.x users must run with --harmony. _This is the last release that
     will support v0.12.x_
+* FIXED: (Promise/A+ compliance) When a promise is rejected with a thenable,
+    the promise adopts the thenable as its rejection reason instead of waiting
+    for it to settle. The previous (incorrect) behavior was hidden by bugs in
+    the `promises-aplus-tests` compliance test suite that were fixed in version
+    `2.1.1`.
 * FIXED: the `webdriver.promise.ControlFlow` now has a consistent execution
     order for tasks/callbacks scheduled in different turns of the JS event loop.
     Refer to the `webdriver.promise` documentation for more details.

javascript/webdriver/promise.js

@@ -1080,38 +1080,40 @@ promise.Promise = goog.defineClass(null, {
     this.parent_ = null;
     this.state_ = PromiseState.BLOCKED;
 
-    if (promise.Thenable.isImplementation(newValue)) {
-      // 2.3.2
-      newValue = /** @type {!promise.Thenable} */(newValue);
-      newValue.then(
-          this.unblockAndResolve_.bind(this, PromiseState.FULFILLED),
-          this.unblockAndResolve_.bind(this, PromiseState.REJECTED));
-      return;
+    if (newState !== PromiseState.REJECTED) {
+      if (promise.Thenable.isImplementation(newValue)) {
+        // 2.3.2
+        newValue = /** @type {!promise.Thenable} */(newValue);
+        newValue.then(
+            this.unblockAndResolve_.bind(this, PromiseState.FULFILLED),
+            this.unblockAndResolve_.bind(this, PromiseState.REJECTED));
+        return;
 
-    } else if (goog.isObject(newValue)) {
-      // 2.3.3
+      } else if (goog.isObject(newValue)) {
+        // 2.3.3
 
-      try {
-        // 2.3.3.1
-        var then = newValue['then'];
-      } catch (e) {
-        // 2.3.3.2
-        this.state_ = PromiseState.REJECTED;
-        this.value_ = e;
-        this.scheduleNotifications_();
-        return;
-      }
+        try {
+          // 2.3.3.1
+          var then = newValue['then'];
+        } catch (e) {
+          // 2.3.3.2
+          this.state_ = PromiseState.REJECTED;
+          this.value_ = e;
+          this.scheduleNotifications_();
+          return;
+        }
 
-      // NB: goog.isFunction is loose and will accept instanceof Function.
-      if (typeof then === 'function') {
-        // 2.3.3.3
-        this.invokeThen_(newValue, then);
-        return;
+        // NB: goog.isFunction is loose and will accept instanceof Function.
+        if (typeof then === 'function') {
+          // 2.3.3.3
+          this.invokeThen_(newValue, then);
+          return;
+        }
       }
     }
 
     if (newState === PromiseState.REJECTED &&
-      isError(newValue) && newValue.stack && this.stack_) {
+        isError(newValue) && newValue.stack && this.stack_) {
       newValue.stack += '\nFrom: ' + (this.stack_.stack || this.stack_);
     }
 

javascript/webdriver/test/promise_error_test.js

@@ -465,26 +465,28 @@ function testThrownPromiseIsHandledSameAsReturningPromise_promiseIsFulfilled() {
 }
 
 
-function testThrownPromiseIsHandledSameAsReturningPromise_promiseIsRejected() {
-  return webdriver.promise.fulfilled().then(function() {
-    throw webdriver.promise.rejected(new StubError);
-  }).then(fail, assertIsStubError);
-}
-
-
-function testTaskThrowsPromise_taskSucceedsIfPromiseIsFulfilled() {
+function testTaskThrowsPromise_promiseWasFulfiled() {
+  var toThrow = webdriver.promise.fulfilled(1234);
   flow.execute(function() {
-    throw webdriver.promise.fulfilled(1234);
+    throw toThrow;
+  }).then(fail, function(value) {
+    assertEquals(toThrow, value);
+    return toThrow;
   }).then(function(value) {
     assertEquals(1234, value);
   });
   return waitForIdle();
 }
 
 
-function testTaskThrowsPromise_taskFailsIfPromiseIsRejected() {
+function testTaskThrowsPromise_promiseWasRejected() {
+  var toThrow = webdriver.promise.rejected(new StubError);
+  toThrow.thenCatch(goog.nullFunction);  // For tearDown.
   flow.execute(function() {
-    throw webdriver.promise.rejected(new StubError);
+    throw toThrow;
+  }).then(fail, function(e) {
+    assertEquals(toThrow, e);
+    return e;
   }).then(fail, assertIsStubError);
   return waitForIdle();
 }
@@ -842,3 +844,49 @@ function testErrorsInAsyncFunctionsAreReportedAsUnhandledRejection() {
     });
   });
 }
+
+
+function testDoesNotWaitForValuesThrownFromCallbacksToBeResolved_1() {
+  var p1 = webdriver.promise.fulfilled();
+  var reason = webdriver.promise.fulfilled('should not see me');
+  return p1.then(function() {
+    throw reason;
+  }).then(fail, function(e) {
+    assertEquals(reason, e);
+  });
+}
+
+
+function testDoesNotWaitForValuesThrownFromCallbacksToBeResolved_2() {
+  var p1 = webdriver.promise.fulfilled();
+  var reason = webdriver.promise.rejected('should not see me');
+  reason.thenCatch(goog.nullFunction);  // For tearDown.
+  return p1.then(function() {
+    throw reason;
+  }).then(fail, function(e) {
+    assertEquals(reason, e);
+  });
+}
+
+
+function testDoesNotWaitForValuesThrownFromCallbacksToBeResolved_3() {
+  var p1 = webdriver.promise.fulfilled();
+  var reason = webdriver.promise.defer();
+  setTimeout(() => reason.fulfill('should not see me'), 100);
+  return p1.then(function() {
+    throw reason.promise;
+  }).then(fail, function(e) {
+    assertEquals(reason.promise, e);
+  });
+}
+
+
+function testDoesNotWaitForValuesThrownFromCallbacksToBeResolved_4() {
+  var p1 = webdriver.promise.fulfilled();
+  var reason = {then: function() {}};  // A thenable like object.
+  return p1.then(function() {
+    throw reason;
+  }).then(fail, function(e) {
+    assertEquals(reason, e);
+  });
+}

javascript/webdriver/test/promise_test.js

@@ -574,7 +574,7 @@ function testRejectForcesValueToAnError_errorLike() {
 }
 
 
-function testRejectingAPromiseWithAnotherPromiseCreatesAChain_ourPromise() {
+function testRejectingAPromiseWithAnotherPromise_ourPromise() {
   var d1 = new webdriver.promise.Deferred();
   var d2 = new webdriver.promise.Deferred();
   var pair1 = callbackPair(assertIsStubError, null);
@@ -583,7 +583,11 @@ function testRejectingAPromiseWithAnotherPromiseCreatesAChain_ourPromise() {
     return new StubError;
   });
 
-  d1.then(pair1.callback, pair1.errback);
+  d1.then(fail, function(e) {
+    assertEquals(d2promise, e);
+    return d2promise;
+  }).then(pair1.callback, pair1.errback);
+
   var d2promise = d2.then(pair2.callback, pair2.errback);
 
   pair1.assertNeither();
@@ -600,11 +604,7 @@ function testRejectingAPromiseWithAnotherPromiseCreatesAChain_ourPromise() {
 }
 
 
-function testRejectingAPromiseWithAnotherPromiseCreatesAChain_otherPromise() {
-  var d = new webdriver.promise.Deferred(), callback, errback;
-  d.then(callback = callbackHelper(assertIsStubError),
-         errback = callbackHelper());
-
+function testRejectingAPromiseWithAnotherPromise_otherPromise() {
   var otherPromise = {
     then: function(callback) {
       this.callback = callback;
@@ -614,15 +614,20 @@ function testRejectingAPromiseWithAnotherPromiseCreatesAChain_otherPromise() {
     }
   };
 
+  var pair = callbackPair(null, assertIsStubError);
+  var d = new webdriver.promise.Deferred();
+  d.promise.then(fail, function(e) {
+    assertEquals(otherPromise, e);
+    return otherPromise;
+  }).then(pair.callback, pair.errback);
+
   d.reject(otherPromise);
   clock.tick();
-  callback.assertNotCalled();
-  errback.assertNotCalled();
+  pair.assertNeither();
 
   otherPromise.fulfill(new StubError);
   clock.tick();
-  callback.assertCalled();
-  errback.assertNotCalled();
+  pair.assertCallback();
 }