Bug 1794127 - Wrap errors from AsyncIterableNextImpl::GetNextResult/AsyncIterableReturnImpl::GetReturnPromise in a promise. r=edgar

Differential Revision: https://phabricator.services.mozilla.com/D158842

dom/bindings/BindingUtils.cpp

@@ -4361,25 +4361,16 @@ bool IsGetterEnabled(JSContext* aCx, JS::Handle<JSObject*> aObj,
 
 already_AddRefed<Promise> CreateRejectedPromiseFromThrownException(
     JSContext* aCx, ErrorResult& aError) {
-  JS::Rooted<JS::Value> exn(aCx);
-  if (!JS_GetPendingException(aCx, &exn)) {
+  if (!JS_IsExceptionPending(aCx)) {
     // If there is no pending exception here but we're ending up in this code,
     // that means the callee threw an uncatchable exception. Just propagate that
-    // out as-is.
+    // out as-is. Promise::RejectWithExceptionFromContext also checks this, but
+    // we want to bail out here before trying to get the globals.
     aError.ThrowUncatchableException();
     return nullptr;
   }
 
-  JS_ClearPendingException(aCx);
-
-  JS::Rooted<JSObject*> globalObj(aCx, GetEntryGlobal()->GetGlobalJSObject());
-  JSAutoRealm ar(aCx, globalObj);
-  if (!JS_WrapValue(aCx, &exn)) {
-    aError.StealExceptionFromJSContext(aCx);
-    return nullptr;
-  }
-
-  GlobalObject promiseGlobal(aCx, globalObj);
+  GlobalObject promiseGlobal(aCx, GetEntryGlobal()->GetGlobalJSObject());
   if (promiseGlobal.Failed()) {
     aError.StealExceptionFromJSContext(aCx);
     return nullptr;
@@ -4392,7 +4383,7 @@ already_AddRefed<Promise> CreateRejectedPromiseFromThrownException(
     return nullptr;
   }
 
-  return Promise::Reject(global, aCx, exn, aError);
+  return Promise::RejectWithExceptionFromContext(global, aCx, aError);
 }
 
 }  // namespace binding_detail

dom/bindings/IterableIterator.cpp

@@ -109,7 +109,14 @@ already_AddRefed<Promise> AsyncIterableNextImpl::NextSteps(
 
   // 4. Let nextPromise be the result of getting the next iteration result with
   //    object’s target and object.
-  RefPtr<Promise> nextPromise = GetNextResult(aRv);
+  RefPtr<Promise> nextPromise;
+  {
+    ErrorResult error;
+    nextPromise = GetNextResult(error);
+    if (error.Failed()) {
+      nextPromise = Promise::Reject(aGlobalObject, std::move(error), aRv);
+    }
+  }
 
   // 5. Let fulfillSteps be the following steps, given next:
   auto fulfillSteps = [](JSContext* aCx, JS::Handle<JS::Value> aNext,
@@ -246,7 +253,13 @@ already_AddRefed<Promise> AsyncIterableReturnImpl::ReturnSteps(
 
   // 4. Return the result of running the asynchronous iterator return algorithm
   // for interface, given object’s target, object, and value.
-  return GetReturnPromise(aCx, aValue, aRv);
+  ErrorResult error;
+  RefPtr<Promise> returnPromise = GetReturnPromise(aCx, aValue, error);
+  if (error.Failed()) {
+    return Promise::Reject(aGlobalObject, std::move(error), aRv);
+  }
+
+  return returnPromise.forget();
 }
 
 already_AddRefed<Promise> AsyncIterableReturnImpl::Return(

dom/bindings/test/TestInterfaceAsyncIterableSingle.h

@@ -52,6 +52,7 @@ class TestInterfaceAsyncIterableSingle : public nsISupports,
     uint32_t mMultiplier = 1;
     Sequence<OwningNonNull<Promise>> mBlockingPromises;
     size_t mBlockingPromisesIndex = 0;
+    uint32_t mFailNextAfter = 4294967295;
   };
 
   void InitAsyncIteratorData(IteratorData& aData, Iterator::IteratorType aType,

dom/bindings/test/TestInterfaceAsyncIterableSingleWithArgs.cpp

@@ -65,11 +65,16 @@ void TestInterfaceAsyncIterableSingleWithArgs::InitAsyncIteratorData(
     const TestInterfaceAsyncIteratorOptions& aOptions, ErrorResult& aError) {
   aData.mMultiplier = aOptions.mMultiplier;
   aData.mBlockingPromises = aOptions.mBlockingPromises;
+  aData.mFailNextAfter = aOptions.mFailNextAfter;
 }
 
 already_AddRefed<Promise>
 TestInterfaceAsyncIterableSingleWithArgs::GetNextIterationResult(
     Iterator* aIterator, ErrorResult& aRv) {
+  if (aIterator->Data().mIndex == aIterator->Data().mFailNextAfter) {
+    aRv.ThrowTypeError("Failed because we of 'failNextAfter'.");
+    return nullptr;
+  }
   return TestInterfaceAsyncIterableSingle::GetNextIterationResult(
       aIterator, aIterator->Data(), aRv);
 }

dom/bindings/test/test_async_iterable.html

@@ -163,6 +163,25 @@ async function test_data_single() {
      `AsyncIterableSingleWithArgs: "return("efgh")" should return { done: true, value: "efgh" }`);
   is(itr.returnLastCalledWith, "abcd",
      `AsyncIterableSingleWithArgs: the asynchronous iterator return algorithm shouldn't be called if the iterator's 'is finished' flag is true already.`);
+
+  // eslint-disable-next-line no-undef
+  itr = new TestInterfaceAsyncIterableSingleWithArgs();
+  itrValues = itr.values({ failNextAfter: 1 });
+  itrValues.next().then(({ value, done }) => {
+    is(value, singleValues[0], "First value is correct");
+    ok(!done, "Expecting more values");
+    return itrValues.next();
+  }).then(() => {
+    ok(false, "Second call to next() should convert failure to a rejected promise.");
+    return itrValues.next();
+  }).catch(() => {
+    ok(true, "Second call to next() should convert failure to a rejected promise.");
+    return itrValues.next();
+  }).then(({ done }) => {
+    ok(done, "An earlier failure in next() should set the async iterator's 'is finished' flag to true.");
+  }).catch(() => {
+    ok(false, "An earlier failure in next() shouldn't cause subsequent calls to return a rejected promise.");
+  });
 }
 
 async function test_data_double() {

dom/promise/Promise.cpp

@@ -546,6 +546,38 @@ void Promise::HandleException(JSContext* aCx) {
   }
 }
 
+// static
+already_AddRefed<Promise> Promise::RejectWithExceptionFromContext(
+    nsIGlobalObject* aGlobal, JSContext* aCx, ErrorResult& aError) {
+  JS::Rooted<JS::Value> exn(aCx);
+  if (!JS_GetPendingException(aCx, &exn)) {
+    // This is very important: if there is no pending exception here but we're
+    // ending up in this code, that means the callee threw an uncatchable
+    // exception.  Just propagate that out as-is.
+    aError.ThrowUncatchableException();
+    return nullptr;
+  }
+
+  JSAutoRealm ar(aCx, aGlobal->GetGlobalJSObject());
+  if (!JS_WrapValue(aCx, &exn)) {
+    // We just give up.
+    aError.StealExceptionFromJSContext(aCx);
+    return nullptr;
+  }
+
+  JS_ClearPendingException(aCx);
+
+  IgnoredErrorResult error;
+  RefPtr<Promise> promise = Promise::Reject(aGlobal, aCx, exn, error);
+  if (!promise) {
+    // We just give up, let's store the exception in the ErrorResult.
+    aError.ThrowJSException(aCx, exn);
+    return nullptr;
+  }
+
+  return promise.forget();
+}
+
 // static
 already_AddRefed<Promise> Promise::CreateFromExisting(
     nsIGlobalObject* aGlobal, JS::Handle<JSObject*> aPromiseObj,

dom/promise/Promise.h

@@ -212,6 +212,27 @@ class Promise : public SupportsWeakPtr {
                                           JS::Handle<JS::Value> aValue,
                                           ErrorResult& aRv);
 
+  template <typename T>
+  static already_AddRefed<Promise> Reject(nsIGlobalObject* aGlobal, T&& aValue,
+                                          ErrorResult& aError) {
+    AutoJSAPI jsapi;
+    if (!jsapi.Init(aGlobal)) {
+      aError.Throw(NS_ERROR_UNEXPECTED);
+      return nullptr;
+    }
+
+    JSContext* cx = jsapi.cx();
+    JS::Rooted<JS::Value> val(cx);
+    if (!ToJSValue(cx, std::forward<T>(aValue), &val)) {
+      return Promise::RejectWithExceptionFromContext(aGlobal, cx, aError);
+    }
+
+    return Reject(aGlobal, cx, val, aError);
+  }
+
+  static already_AddRefed<Promise> RejectWithExceptionFromContext(
+      nsIGlobalObject* aGlobal, JSContext* aCx, ErrorResult& aError);
+
   // Do the equivalent of Promise.all in the current compartment of aCx.  Errors
   // are reported on the ErrorResult; if aRv comes back !Failed(), this function
   // MUST return a non-null value.

dom/webidl/TestInterfaceJSMaplikeSetlikeIterable.webidl

@@ -112,6 +112,7 @@ interface TestInterfaceAsyncIterableSingle {
 dictionary TestInterfaceAsyncIteratorOptions {
   unsigned long multiplier = 1;
   sequence<Promise<any>> blockingPromises = [];
+  unsigned long failNextAfter = 4294967295;
 };
 
 [Pref="dom.expose_test_interfaces",