[async] Improve error handling when running async hooks

If an exception is thrown in instrumented async code, for instance
  await import('non-existing-module')
it should be correctly reported by the hooks that run around this code.
Also calling ToLocalChecked() on the hook result is wrong if the hook
has thrown an exception.

Bug: chromium:865892
Change-Id: I5712376fe4426a3e49223d821e4647150887a258
Reviewed-on: https://chromium-review.googlesource.com/1146561
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54610}

src/async-hooks-wrapper.cc

@@ -4,6 +4,7 @@
 
 #include "src/async-hooks-wrapper.h"
 #include "src/d8.h"
+#include "src/isolate-inl.h"
 
 namespace v8 {
 
@@ -216,6 +217,15 @@ void AsyncHooks::PromiseHookDispatch(PromiseHookType type,
   TryCatch try_catch(hooks->isolate_);
   try_catch.SetVerbose(true);
 
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(hooks->isolate_);
+  if (isolate->has_scheduled_exception()) {
+    isolate->ScheduleThrow(isolate->scheduled_exception());
+
+    DCHECK(try_catch.HasCaught());
+    Shell::ReportException(hooks->isolate_, &try_catch);
+    return;
+  }
+
   Local<Value> rcv = Undefined(hooks->isolate_);
   Local<Context> context = hooks->isolate_->GetCurrentContext();
   Local<Value> async_id =
@@ -223,6 +233,10 @@ void AsyncHooks::PromiseHookDispatch(PromiseHookType type,
           .ToLocalChecked();
   Local<Value> args[1] = {async_id};
 
+  // This is unused. It's here to silence the warning about
+  // not using the MaybeLocal return value from Call.
+  MaybeLocal<Value> result;
+
   // Sacrifice the brevity for readability and debugfulness
   if (type == PromiseHookType::kInit) {
     if (!wrap->init_function().IsEmpty()) {
@@ -235,21 +249,19 @@ void AsyncHooks::PromiseHookDispatch(PromiseHookType type,
               ->GetPrivate(context, hooks->trigger_id_smb.Get(hooks->isolate_))
               .ToLocalChecked(),
           promise};
-      wrap->init_function()->Call(context, rcv, 4, initArgs).ToLocalChecked();
+      result = wrap->init_function()->Call(context, rcv, 4, initArgs);
     }
   } else if (type == PromiseHookType::kBefore) {
     if (!wrap->before_function().IsEmpty()) {
-      wrap->before_function()->Call(context, rcv, 1, args).ToLocalChecked();
+      result = wrap->before_function()->Call(context, rcv, 1, args);
     }
   } else if (type == PromiseHookType::kAfter) {
     if (!wrap->after_function().IsEmpty()) {
-      wrap->after_function()->Call(context, rcv, 1, args).ToLocalChecked();
+      result = wrap->after_function()->Call(context, rcv, 1, args);
     }
   } else if (type == PromiseHookType::kResolve) {
     if (!wrap->promiseResolve_function().IsEmpty()) {
-      wrap->promiseResolve_function()
-          ->Call(context, rcv, 1, args)
-          .ToLocalChecked();
+      result = wrap->promiseResolve_function()->Call(context, rcv, 1, args);
     }
   }
 

src/isolate.h

@@ -820,7 +820,7 @@ class Isolate : private HiddenFactory {
   };
   CatchType PredictExceptionCatcher();
 
-  void ScheduleThrow(Object* exception);
+  V8_EXPORT_PRIVATE void ScheduleThrow(Object* exception);
   // Re-set pending message, script and positions reported to the TryCatch
   // back to the TLS for re-use when rethrowing.
   void RestorePendingMessageFromTryCatch(v8::TryCatch* handler);

test/mjsunit/regress/regress-crbug-865892.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-async-hooks
+
+let ah = async_hooks.createHook(
+{
+  init(asyncId, type) {
+    if (type !== 'PROMISE') { return; }
+    assertThrows('asyncIds.push(asyncId);');
+  }
+});
+ah.enable();
+
+async function foo() {
+  let x = { toString() { return 'modules-skip-1.js' } };
+  assertThrows('await import(x);');
+}
+foo();