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]>
nodejs · Jul 14, 2016 · 8557597 · 8557597
1 parent 7dbb0d0
commit 8557597
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 14 deletions.
diff --git a/src/node.cc b/src/node.cc
@@ -1439,7 +1439,8 @@ ssize_t DecodeWrite(Isolate* isolate,
 
 void AppendExceptionLine(Environment* env,
                          Local<Value> er,
-                         Local<Message> message) {
+                         Local<Message> message,
+                         enum ErrorHandlingMode mode) {
   if (message.IsEmpty())
     return;
 
@@ -1521,19 +1522,23 @@ void AppendExceptionLine(Environment* env,
 
   Local<String> arrow_str = String::NewFromUtf8(env->isolate(), arrow);
 
-  // Allocation failed, just print it out
-  if (arrow_str.IsEmpty() || err_obj.IsEmpty() || !err_obj->IsNativeError())
-    goto print;
-
-  err_obj->SetHiddenValue(env->arrow_message_string(), arrow_str);
-  return;
+  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);
 
- print:
-  if (env->printed_error())
+    uv_tty_reset_mode();
+    PrintErrorString("\n%s", arrow);
     return;
-  env->set_printed_error(true);
-  uv_tty_reset_mode();
-  PrintErrorString("\n%s", arrow);
+  }
+
+  err_obj->SetHiddenValue(env->arrow_message_string(), arrow_str);
 }
 
 
@@ -1542,7 +1547,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;

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -126,9 +126,11 @@ constexpr size_t arraysize(const T(&)[N]) { return N; }
 # define NO_RETURN
 #endif
 
+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 = CONTEXTIFY_ERROR);
 
 NO_RETURN void FatalError(const char* location, const char* message);
 

diff --git a/test/message/vm_caught_custom_runtime_error.js b/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');
diff --git a/test/message/vm_caught_custom_runtime_error.out b/test/message/vm_caught_custom_runtime_error.out
@@ -0,0 +1,3 @@
+beginning
+received error MyCustomError
+end