[d8] Catch errors in any message

Currently, errors somewhere deep in a promise chain are not reported as
test failures. We would print the error, but still return with exit
status 0. This CL fixes this to treat any uncaught exception as
failure, as we already do in {Shell::ExecuteString}.

R=yangguo@chromium.org

Change-Id: Idc4e58cc0d45f8fca3f7b2c5070ab68d317fc00b
Reviewed-on: https://chromium-review.googlesource.com/c/1439378Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59165}

src/d8.cc

@@ -2409,8 +2409,8 @@ bool ends_with(const char* input, const char* suffix) {
   return false;
 }
 
-void SourceGroup::Execute(Isolate* isolate) {
-  bool exception_was_thrown = false;
+bool SourceGroup::Execute(Isolate* isolate) {
+  bool success = true;
   for (int i = begin_offset_; i < end_offset_; ++i) {
     const char* arg = argv_[i];
     if (strcmp(arg, "-e") == 0 && i + 1 < end_offset_) {
@@ -2426,7 +2426,7 @@ void SourceGroup::Execute(Isolate* isolate) {
       if (!Shell::ExecuteString(isolate, source, file_name,
                                 Shell::kNoPrintResult, Shell::kReportExceptions,
                                 Shell::kNoProcessMessageQueue)) {
-        exception_was_thrown = true;
+        success = false;
         break;
       }
       ++i;
@@ -2434,7 +2434,7 @@ void SourceGroup::Execute(Isolate* isolate) {
     } else if (ends_with(arg, ".mjs")) {
       Shell::set_script_executed();
       if (!Shell::ExecuteModule(isolate, arg)) {
-        exception_was_thrown = true;
+        success = false;
         break;
       }
       continue;
@@ -2443,7 +2443,7 @@ void SourceGroup::Execute(Isolate* isolate) {
       arg = argv_[++i];
       Shell::set_script_executed();
       if (!Shell::ExecuteModule(isolate, arg)) {
-        exception_was_thrown = true;
+        success = false;
         break;
       }
       continue;
@@ -2466,13 +2466,11 @@ void SourceGroup::Execute(Isolate* isolate) {
     if (!Shell::ExecuteString(isolate, source, file_name, Shell::kNoPrintResult,
                               Shell::kReportExceptions,
                               Shell::kProcessMessageQueue)) {
-      exception_was_thrown = true;
+      success = false;
       break;
     }
   }
-  if (exception_was_thrown != Shell::options.expected_to_throw) {
-    base::OS::ExitProcess(1);
-  }
+  return success;
 }
 
 Local<String> SourceGroup::ReadFile(Isolate* isolate, const char* name) {
@@ -2929,6 +2927,7 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
   for (int i = 1; i < options.num_isolates; ++i) {
     options.isolate_sources[i].StartExecuteInThread();
   }
+  bool success = true;
   {
     SetWaitUntilDone(isolate, false);
     if (options.lcov_file) {
@@ -2945,8 +2944,8 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
       Context::Scope cscope(context);
       InspectorClient inspector_client(context, options.enable_inspector);
       PerIsolateData::RealmScope realm_scope(PerIsolateData::Get(isolate));
-      options.isolate_sources[0].Execute(isolate);
-      CompleteMessageLoop(isolate);
+      if (!options.isolate_sources[0].Execute(isolate)) success = false;
+      if (!CompleteMessageLoop(isolate)) success = false;
     }
     if (!use_existing_context) {
       DisposeModuleEmbedderData(context);
@@ -2962,7 +2961,8 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
     }
   }
   CleanupWorkers();
-  return 0;
+  // In order to finish successfully, success must be != expected_to_throw.
+  return success == Shell::options.expected_to_throw ? 1 : 0;
 }
 
 
@@ -3025,7 +3025,7 @@ bool ProcessMessages(
 }
 }  // anonymous namespace
 
-void Shell::CompleteMessageLoop(Isolate* isolate) {
+bool Shell::CompleteMessageLoop(Isolate* isolate) {
   auto get_waiting_behaviour = [isolate]() {
     base::MutexGuard guard(isolate_status_lock_.Pointer());
     DCHECK_GT(isolate_status_.count(isolate), 0);
@@ -3037,7 +3037,7 @@ void Shell::CompleteMessageLoop(Isolate* isolate) {
     return should_wait ? platform::MessageLoopBehavior::kWaitForWork
                        : platform::MessageLoopBehavior::kDoNotWait;
   };
-  ProcessMessages(isolate, get_waiting_behaviour);
+  return ProcessMessages(isolate, get_waiting_behaviour);
 }
 
 bool Shell::EmptyMessageQueues(Isolate* isolate) {

src/d8.h

@@ -94,7 +94,8 @@ class SourceGroup {
 
   void End(int offset) { end_offset_ = offset; }
 
-  void Execute(Isolate* isolate);
+  // Returns true on success, false if an uncaught exception was thrown.
+  bool Execute(Isolate* isolate);
 
   void StartExecuteInThread();
   void WaitForThread();
@@ -417,7 +418,7 @@ class Shell : public i::AllStatic {
   static void OnExit(Isolate* isolate);
   static void CollectGarbage(Isolate* isolate);
   static bool EmptyMessageQueues(Isolate* isolate);
-  static void CompleteMessageLoop(Isolate* isolate);
+  static bool CompleteMessageLoop(Isolate* isolate);
 
   static std::unique_ptr<SerializationData> SerializeValue(
       Isolate* isolate, Local<Value> value, Local<Value> transfer);

test/message/mjsunit/fail/assert-promise-result-wasm-compile-fail.js

+// Copyright 2019 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.
+
+// Test wasm compilation explicitly, since this creates a promise which is only
+// resolved later, i.e. the message queue gets empty in-between.
+// The important part here is that d8 exits with a non-zero exit code.
+
+load('test/mjsunit/mjsunit.js');
+load('test/mjsunit/wasm/wasm-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+assertPromiseResult((async function test() {
+  let ok_buffer = (() => {
+    let builder = new WasmModuleBuilder();
+    builder.addFunction(undefined, kSig_i_v).addBody([kExprI32Const, 42]);
+    return builder.toBuffer();
+  })();
+  let bad_buffer = new ArrayBuffer(0);
+  let kNumCompiles = 3;
+
+  // Three compilations of the OK module should succeed.
+  for (var i = 0; i < kNumCompiles; ++i) {
+    await WebAssembly.compile(ok_buffer);
+  }
+
+  // Three compilations of the bad module should fail.
+  for (var i = 0; i < kNumCompiles; ++i) {
+    await WebAssembly.compile(bad_buffer);
+  }
+})());

test/message/mjsunit/fail/assert-promise-result-wasm-compile-fail.out

+test/mjsunit/mjsunit.js:{NUMBER}: CompileError: WebAssembly.compile(): BufferSource argument is empty
+              throw concatenateErrors(stack, e);
+              ^
+Error
+    at assertPromiseResult (test/mjsunit/mjsunit.js:{NUMBER}:{NUMBER})
+    at *%(basename)s:13:1
+
+CompileError: WebAssembly.compile(): BufferSource argument is empty
+    at test (*%(basename)s:29:23)
+