vm: don't print out arrow message for custom error

In `AppendExceptionLine()`, which is used both by the `vm`
module and the uncaught exception handler, don’t print anything
to stderr when called from the `vm` module, even if the
thrown object is not a native error instance.

Fixes: #7397
PR-URL: #7398
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: addaleax

src/node.cc

@@ -1514,7 +1514,8 @@ bool IsExceptionDecorated(Environment* env, Local<Value> er) {
 
 void AppendExceptionLine(Environment* env,
                          Local<Value> er,
-                         Local<Message> message) {
+                         Local<Message> message,
+                         enum ErrorHandlingMode mode) {
   if (message.IsEmpty())
     return;
 
@@ -1601,20 +1602,26 @@ void AppendExceptionLine(Environment* env,
 
   Local<String> arrow_str = String::NewFromUtf8(env->isolate(), arrow);
 
-  if (!arrow_str.IsEmpty() && !err_obj.IsEmpty() && err_obj->IsNativeError()) {
-    err_obj->SetPrivate(
-        env->context(),
-        env->arrow_message_private_symbol(),
-        arrow_str);
+  const bool can_set_arrow = !arrow_str.IsEmpty() && !err_obj.IsEmpty();
+  // If allocating arrow_str failed, print it out. There's not much else to do.
+  // If it's not an error, but something needs to be printed out because
+  // it's a fatal exception, also print it out from here.
+  // Otherwise, the arrow property will be attached to the object and handled
+  // by the caller.
+  if (!can_set_arrow || (mode == FATAL_ERROR && !err_obj->IsNativeError())) {
+    if (env->printed_error())
+      return;
+    env->set_printed_error(true);
+
+    uv_tty_reset_mode();
+    PrintErrorString("\n%s", arrow);
     return;
   }
 
-  // Allocation failed, just print it out.
-  if (env->printed_error())
-    return;
-  env->set_printed_error(true);
-  uv_tty_reset_mode();
-  PrintErrorString("\n%s", arrow);
+  CHECK(err_obj->SetPrivate(
+            env->context(),
+            env->arrow_message_private_symbol(),
+            arrow_str).FromMaybe(false));
 }
 
 
@@ -1623,7 +1630,7 @@ static void ReportException(Environment* env,
                             Local<Message> message) {
   HandleScope scope(env->isolate());
 
-  AppendExceptionLine(env, er, message);
+  AppendExceptionLine(env, er, message, FATAL_ERROR);
 
   Local<Value> trace_value;
   Local<Value> arrow;

src/node_contextify.cc

@@ -633,7 +633,7 @@ class ContextifyScript : public BaseObject {
     if (IsExceptionDecorated(env, err_obj))
       return;
 
-    AppendExceptionLine(env, exception, try_catch.Message());
+    AppendExceptionLine(env, exception, try_catch.Message(), CONTEXTIFY_ERROR);
     Local<Value> stack = err_obj->Get(env->stack_string());
     auto maybe_value =
         err_obj->GetPrivate(

src/node_internals.h

@@ -142,9 +142,11 @@ constexpr size_t arraysize(const T(&)[N]) { return N; }
 
 bool IsExceptionDecorated(Environment* env, v8::Local<v8::Value> er);
 
+enum ErrorHandlingMode { FATAL_ERROR, CONTEXTIFY_ERROR };
 void AppendExceptionLine(Environment* env,
                          v8::Local<v8::Value> er,
-                         v8::Local<v8::Message> message);
+                         v8::Local<v8::Message> message,
+                         enum ErrorHandlingMode mode);
 
 NO_RETURN void FatalError(const char* location, const char* message);
 

test/message/vm_caught_custom_runtime_error.js

@@ -0,0 +1,18 @@
+'use strict';
+require('../common');
+const vm = require('vm');
+
+console.error('beginning');
+
+// Regression test for https://github.com/nodejs/node/issues/7397:
+// vm.runInThisContext() should not print out anything to stderr by itself.
+try {
+  vm.runInThisContext(`throw ({
+    name: 'MyCustomError',
+    message: 'This is a custom message'
+  })`, { filename: 'test.vm' });
+} catch (e) {
+  console.error('received error', e.name);
+}
+
+console.error('end');

test/message/vm_caught_custom_runtime_error.out

@@ -0,0 +1,3 @@
+beginning
+received error MyCustomError
+end