[top-level-await] Stop throwing exceptions with TLA.

Bug: v8:9970
Change-Id: I0e542fc63211e78800eab82257ccab9583305433
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1946534Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Joshua Litt <joshualitt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65351}

src/execution/execution.cc

@@ -40,7 +40,7 @@ struct InvokeParams {
       Isolate* isolate, Handle<Object> callable, Handle<Object> receiver,
       int argc, Handle<Object>* argv,
       Execution::MessageHandling message_handling,
-      MaybeHandle<Object>* exception_out);
+      MaybeHandle<Object>* exception_out, bool reschedule_terminate);
 
   static InvokeParams SetUpForRunMicrotasks(Isolate* isolate,
                                             MicrotaskQueue* microtask_queue,
@@ -59,6 +59,7 @@ struct InvokeParams {
 
   bool is_construct;
   Execution::Target execution_target;
+  bool reschedule_terminate;
 };
 
 // static
@@ -77,6 +78,7 @@ InvokeParams InvokeParams::SetUpForNew(Isolate* isolate,
   params.exception_out = nullptr;
   params.is_construct = true;
   params.execution_target = Execution::Target::kCallable;
+  params.reschedule_terminate = true;
   return params;
 }
 
@@ -96,6 +98,7 @@ InvokeParams InvokeParams::SetUpForCall(Isolate* isolate,
   params.exception_out = nullptr;
   params.is_construct = false;
   params.execution_target = Execution::Target::kCallable;
+  params.reschedule_terminate = true;
   return params;
 }
 
@@ -103,7 +106,7 @@ InvokeParams InvokeParams::SetUpForCall(Isolate* isolate,
 InvokeParams InvokeParams::SetUpForTryCall(
     Isolate* isolate, Handle<Object> callable, Handle<Object> receiver,
     int argc, Handle<Object>* argv, Execution::MessageHandling message_handling,
-    MaybeHandle<Object>* exception_out) {
+    MaybeHandle<Object>* exception_out, bool reschedule_terminate) {
   InvokeParams params;
   params.target = callable;
   params.receiver = NormalizeReceiver(isolate, receiver);
@@ -115,6 +118,7 @@ InvokeParams InvokeParams::SetUpForTryCall(
   params.exception_out = exception_out;
   params.is_construct = false;
   params.execution_target = Execution::Target::kCallable;
+  params.reschedule_terminate = reschedule_terminate;
   return params;
 }
 
@@ -134,6 +138,7 @@ InvokeParams InvokeParams::SetUpForRunMicrotasks(
   params.exception_out = exception_out;
   params.is_construct = false;
   params.execution_target = Execution::Target::kRunMicrotasks;
+  params.reschedule_terminate = true;
   return params;
 }
 
@@ -343,7 +348,7 @@ MaybeHandle<Object> InvokeWithTryCatch(Isolate* isolate,
     }
   }
 
-  if (is_termination) {
+  if (is_termination && params.reschedule_terminate) {
     // Reschedule terminate execution exception.
     isolate->OptionalRescheduleException(false);
   }
@@ -386,16 +391,14 @@ MaybeHandle<Object> Execution::New(Isolate* isolate, Handle<Object> constructor,
 }
 
 // static
-MaybeHandle<Object> Execution::TryCall(Isolate* isolate,
-                                       Handle<Object> callable,
-                                       Handle<Object> receiver, int argc,
-                                       Handle<Object> argv[],
-                                       MessageHandling message_handling,
-                                       MaybeHandle<Object>* exception_out) {
+MaybeHandle<Object> Execution::TryCall(
+    Isolate* isolate, Handle<Object> callable, Handle<Object> receiver,
+    int argc, Handle<Object> argv[], MessageHandling message_handling,
+    MaybeHandle<Object>* exception_out, bool reschedule_terminate) {
   return InvokeWithTryCatch(
-      isolate,
-      InvokeParams::SetUpForTryCall(isolate, callable, receiver, argc, argv,
-                                    message_handling, exception_out));
+      isolate, InvokeParams::SetUpForTryCall(
+                   isolate, callable, receiver, argc, argv, message_handling,
+                   exception_out, reschedule_terminate));
 }
 
 // static

src/execution/execution.h

@@ -54,7 +54,7 @@ class Execution final : public AllStatic {
   V8_EXPORT_PRIVATE static MaybeHandle<Object> TryCall(
       Isolate* isolate, Handle<Object> callable, Handle<Object> receiver,
       int argc, Handle<Object> argv[], MessageHandling message_handling,
-      MaybeHandle<Object>* exception_out);
+      MaybeHandle<Object>* exception_out, bool reschedule_terminate = true);
   // Convenience method for performing RunMicrotasks
   static MaybeHandle<Object> TryRunMicrotasks(
       Isolate* isolate, MicrotaskQueue* microtask_queue,

src/objects/source-text-module.cc

@@ -906,7 +906,9 @@ MaybeHandle<Object> SourceTextModule::InnerExecuteAsyncModule(
   Handle<Object> result;
   ASSIGN_RETURN_ON_EXCEPTION(
       isolate, result,
-      Execution::Call(isolate, resume, async_function_object, 0, nullptr),
+      Execution::TryCall(isolate, resume, async_function_object, 0, nullptr,
+                         Execution::MessageHandling::kKeepPending, nullptr,
+                         false),
       Object);
   return result;
 }
@@ -919,9 +921,21 @@ MaybeHandle<Object> SourceTextModule::ExecuteModule(
   Handle<JSFunction> resume(
       isolate->native_context()->generator_next_internal(), isolate);
   Handle<Object> result;
-  ASSIGN_RETURN_ON_EXCEPTION(
-      isolate, result, Execution::Call(isolate, resume, generator, 0, nullptr),
-      Object);
+
+  // With top_level_await, we need to catch any exceptions and reject
+  // the top level capability.
+  if (FLAG_harmony_top_level_await) {
+    ASSIGN_RETURN_ON_EXCEPTION(
+        isolate, result,
+        Execution::TryCall(isolate, resume, generator, 0, nullptr,
+                           Execution::MessageHandling::kKeepPending, nullptr,
+                           false),
+        Object);
+  } else {
+    ASSIGN_RETURN_ON_EXCEPTION(
+        isolate, result,
+        Execution::Call(isolate, resume, generator, 0, nullptr), Object);
+  }
   DCHECK(JSIteratorResult::cast(*result).done().BooleanValue(isolate));
   return handle(JSIteratorResult::cast(*result).value(), isolate);
 }

test/cctest/test-modules.cc

@@ -258,49 +258,49 @@ TEST(ModuleEvaluationError1) {
               .FromJust());
     CHECK_EQ(Module::kInstantiated, module->GetStatus());
 
-    MaybeLocal<Value> result_1;
     {
       v8::TryCatch inner_try_catch(isolate);
-      result_1 = module->Evaluate(env.local());
+      MaybeLocal<Value> result = module->Evaluate(env.local());
       CHECK_EQ(Module::kErrored, module->GetStatus());
       Local<Value> exception = module->GetException();
       CHECK(exception->StrictEquals(v8_str("boom")));
       ExpectInt32("Object.x", 1);
-      CHECK(inner_try_catch.HasCaught());
-      CHECK(inner_try_catch.Exception()->StrictEquals(v8_str("boom")));
+
+      if (i::FLAG_harmony_top_level_await) {
+        // With top level await, we do not throw and errored evaluation returns
+        // a rejected promise with the exception.
+        CHECK(!inner_try_catch.HasCaught());
+        Local<Promise> promise = Local<Promise>::Cast(result.ToLocalChecked());
+        CHECK_EQ(promise->State(), v8::Promise::kRejected);
+        CHECK_EQ(promise->Result(), module->GetException());
+      } else {
+        CHECK(inner_try_catch.HasCaught());
+        CHECK(inner_try_catch.Exception()->StrictEquals(v8_str("boom")));
+        CHECK(result.IsEmpty());
+      }
     }
 
-    MaybeLocal<Value> result_2;
     {
       v8::TryCatch inner_try_catch(isolate);
-      result_2 = module->Evaluate(env.local());
+      MaybeLocal<Value> result = module->Evaluate(env.local());
       CHECK_EQ(Module::kErrored, module->GetStatus());
       Local<Value> exception = module->GetException();
       CHECK(exception->StrictEquals(v8_str("boom")));
       ExpectInt32("Object.x", 1);
 
       if (i::FLAG_harmony_top_level_await) {
-        // With top level await we do not rethrow the exception.
+        // With top level await, we do not throw and errored evaluation returns
+        // a rejected promise with the exception.
         CHECK(!inner_try_catch.HasCaught());
+        Local<Promise> promise = Local<Promise>::Cast(result.ToLocalChecked());
+        CHECK_EQ(promise->State(), v8::Promise::kRejected);
+        CHECK_EQ(promise->Result(), module->GetException());
       } else {
         CHECK(inner_try_catch.HasCaught());
         CHECK(inner_try_catch.Exception()->StrictEquals(v8_str("boom")));
+        CHECK(result.IsEmpty());
       }
     }
-    if (i::FLAG_harmony_top_level_await) {
-      // With top level await, errored evaluation returns a rejected promise
-      // with the exception.
-      Local<Promise> promise_1 =
-          Local<Promise>::Cast(result_1.ToLocalChecked());
-      Local<Promise> promise_2 =
-          Local<Promise>::Cast(result_2.ToLocalChecked());
-      CHECK_EQ(promise_1->State(), v8::Promise::kRejected);
-      CHECK_EQ(promise_2->State(), v8::Promise::kRejected);
-      CHECK_EQ(promise_1->Result(), module->GetException());
-      CHECK_EQ(promise_2->Result(), module->GetException());
-    } else {
-      CHECK(result_1.IsEmpty() && result_2.IsEmpty());
-    }
 
     CHECK(!try_catch.HasCaught());
   }
@@ -342,15 +342,25 @@ TEST(ModuleEvaluationError2) {
               .FromJust());
     CHECK_EQ(Module::kInstantiated, failure_module->GetStatus());
 
-    MaybeLocal<Value> result_1;
     {
       v8::TryCatch inner_try_catch(isolate);
-      result_1 = failure_module->Evaluate(env.local());
+      MaybeLocal<Value> result = failure_module->Evaluate(env.local());
       CHECK_EQ(Module::kErrored, failure_module->GetStatus());
       Local<Value> exception = failure_module->GetException();
       CHECK(exception->StrictEquals(v8_str("boom")));
-      CHECK(inner_try_catch.HasCaught());
-      CHECK(inner_try_catch.Exception()->StrictEquals(v8_str("boom")));
+
+      if (i::FLAG_harmony_top_level_await) {
+        // With top level await, we do not throw and errored evaluation returns
+        // a rejected promise with the exception.
+        CHECK(!inner_try_catch.HasCaught());
+        Local<Promise> promise = Local<Promise>::Cast(result.ToLocalChecked());
+        CHECK_EQ(promise->State(), v8::Promise::kRejected);
+        CHECK_EQ(promise->Result(), failure_module->GetException());
+      } else {
+        CHECK(inner_try_catch.HasCaught());
+        CHECK(inner_try_catch.Exception()->StrictEquals(v8_str("boom")));
+        CHECK(result.IsEmpty());
+      }
     }
 
     Local<String> dependent_text =
@@ -367,39 +377,28 @@ TEST(ModuleEvaluationError2) {
               .FromJust());
     CHECK_EQ(Module::kInstantiated, dependent_module->GetStatus());
 
-    MaybeLocal<Value> result_2;
     {
       v8::TryCatch inner_try_catch(isolate);
-      result_2 = dependent_module->Evaluate(env.local());
+      MaybeLocal<Value> result = dependent_module->Evaluate(env.local());
       CHECK_EQ(Module::kErrored, dependent_module->GetStatus());
       Local<Value> exception = dependent_module->GetException();
       CHECK(exception->StrictEquals(v8_str("boom")));
       CHECK_EQ(exception, failure_module->GetException());
 
       if (i::FLAG_harmony_top_level_await) {
-        // With top level await we do not rethrow the exception.
+        // With top level await, we do not throw and errored evaluation returns
+        // a rejected promise with the exception.
         CHECK(!inner_try_catch.HasCaught());
+        Local<Promise> promise = Local<Promise>::Cast(result.ToLocalChecked());
+        CHECK_EQ(promise->State(), v8::Promise::kRejected);
+        CHECK_EQ(promise->Result(), failure_module->GetException());
       } else {
         CHECK(inner_try_catch.HasCaught());
         CHECK(inner_try_catch.Exception()->StrictEquals(v8_str("boom")));
+        CHECK(result.IsEmpty());
       }
     }
 
-    if (i::FLAG_harmony_top_level_await) {
-      // With top level await, errored evaluation returns a rejected promise
-      // with the exception.
-      Local<Promise> promise_1 =
-          Local<Promise>::Cast(result_1.ToLocalChecked());
-      Local<Promise> promise_2 =
-          Local<Promise>::Cast(result_2.ToLocalChecked());
-      CHECK_EQ(promise_1->State(), v8::Promise::kRejected);
-      CHECK_EQ(promise_2->State(), v8::Promise::kRejected);
-      CHECK_EQ(promise_1->Result(), failure_module->GetException());
-      CHECK_EQ(promise_2->Result(), failure_module->GetException());
-    } else {
-      CHECK(result_1.IsEmpty() && result_2.IsEmpty());
-    }
-
     CHECK(!try_catch.HasCaught());
   }
   i::FLAG_harmony_top_level_await = prev_top_level_await;