Bug 1534392 - Part 2: Fix error handling in %AsyncFromSyncIteratorPrototype% methods. r=anba
☠☠ backed out by 00919ccaa29f ☠ ☠
authorJason Orendorff <jorendorff@mozilla.com>
Tue, 12 Mar 2019 20:49:29 +0000
changeset 524603 4c52efb1430f4b71a272998add34b63e53b00210
parent 524602 47e570e513851c5e4a7b930af82ad9de21e0bb22
child 524604 da793f0fe362720f58903a2ea16d674e50feaa2b
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1534392
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1534392 - Part 2: Fix error handling in %AsyncFromSyncIteratorPrototype% methods. r=anba This makes %AsyncFromSyncIteratorPrototype%.next/return/throw return a rejected promise, not throw, when PromiseResolve throws, following the usual convention for methods that return promises. This follows proposed spec change <https://github.com/tc39/ecma262/pull/1470>, which I expect will land with little controversy. Differential Revision: https://phabricator.services.mozilla.com/D23030
js/src/builtin/Promise.cpp
js/src/tests/non262/async-functions/ecma262-issue-1461.js
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -3793,37 +3793,41 @@ bool js::AsyncFromSyncIteratorMethod(JSC
 
   // Step 3: Let value be IteratorValue(result).
   // Step 4: IfAbruptRejectPromise(value, promiseCapability).
   RootedValue value(cx);
   if (!GetProperty(cx, resultObj, resultObj, cx->names().value, &value)) {
     return AbruptRejectPromise(cx, args, resultPromise, nullptr);
   }
 
-  // Steps 6-8 (reordered).
-  // Step 6: Let steps be the algorithm steps defined in Async-from-Sync
+  // Step numbers below include the changes in
+  // <https://github.com/tc39/ecma262/pull/1470>, which inserted a new step 6.
+  //
+  // Steps 7-9 (reordered).
+  // Step 7: Let steps be the algorithm steps defined in Async-from-Sync
   //         Iterator Value Unwrap Functions.
-  // Step 7: Let onFulfilled be CreateBuiltinFunction(steps, « [[Done]] »).
-  // Step 8: Set onFulfilled.[[Done]] to done.
+  // Step 8: Let onFulfilled be CreateBuiltinFunction(steps, « [[Done]] »).
+  // Step 9: Set onFulfilled.[[Done]] to done.
   RootedValue onFulfilled(
       cx,
       Int32Value(done ? PromiseHandlerAsyncFromSyncIteratorValueUnwrapDone
                       : PromiseHandlerAsyncFromSyncIteratorValueUnwrapNotDone));
   RootedValue onRejected(cx, Int32Value(PromiseHandlerThrower));
 
-  // These steps are identical to some steps in Await; we have a utility
+  // Steps 5 and 10 are identical to some steps in Await; we have a utility
   // function InternalAwait() that implements the idiom.
   //
-  // Step 5: Let valueWrapper be ? PromiseResolve(%Promise%, « value »).
-  // Step 9: Perform ! PerformPromiseThen(valueWrapper, onFulfilled,
+  // Step 5: Let valueWrapper be PromiseResolve(%Promise%, « value »).
+  // Step 6: IfAbruptRejectPromise(valueWrapper, promiseCapability).
+  // Step 10: Perform ! PerformPromiseThen(valueWrapper, onFulfilled,
   //                                      undefined, promiseCapability).
   auto extra = [](Handle<PromiseReactionRecord*> reaction) {};
   if (!InternalAwait(cx, value, resultPromise, onFulfilled, onRejected,
                      extra)) {
-    return false;
+    return AbruptRejectPromise(cx, args, resultPromise, nullptr);
   }
 
   // Step 11: Return promiseCapability.[[Promise]].
   args.rval().setObject(*resultPromise);
   return true;
 }
 
 enum class ResumeNextKind { Enqueue, Reject, Resolve };
new file mode 100644
--- /dev/null
+++ b/js/src/tests/non262/async-functions/ecma262-issue-1461.js
@@ -0,0 +1,25 @@
+// <https://github.com/tc39/ecma262/pull/1470> changes a detail of
+// error-handling in %AsyncFromSyncIteratorPrototype% methods. This test is
+// based on a comment in the thread where the issue was first reported,
+// <https://github.com/tc39/ecma262/issues/1461#issuecomment-468602852>
+
+let log = [];
+
+{
+  async function f() {
+    var p = Promise.resolve(0);
+    Object.defineProperty(p, "constructor", {get() { throw "hi" }});
+    for await (var x of [p]);
+  }
+  Promise.resolve(0)
+         .then(() => log.push("tick 1"))
+         .then(() => log.push("tick 2"))
+         .then(() => log.push("tick 3"));
+  f().catch(exc => log.push(exc));
+}
+
+drainJobQueue();
+assertEq(log.join(), "tick 1,tick 2,hi,tick 3");
+
+if (typeof reportCompare === 'function')
+  reportCompare(0, 0);