Skip to content

Commit

Permalink
vm: don't print out arrow message for custom error
Browse files Browse the repository at this point in the history
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 <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
addaleax authored and Myles Borins committed Jul 14, 2016
1 parent 7dbb0d0 commit 8557597
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 14 deletions.
31 changes: 18 additions & 13 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}


Expand All @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
18 changes: 18 additions & 0 deletions test/message/vm_caught_custom_runtime_error.js
Original file line number Diff line number Diff line change
@@ -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');
3 changes: 3 additions & 0 deletions test/message/vm_caught_custom_runtime_error.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
beginning
received error MyCustomError
end

0 comments on commit 8557597

Please sign in to comment.