Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,8 @@ bool IsExceptionDecorated(Environment* env, Local<Value> er) {

void AppendExceptionLine(Environment* env,
Local<Value> er,
Local<Message> message) {
Local<Message> message,
enum ErrorHandlingMode mode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enum keyword is not strictly necessary (but it doesn't hurt and I realize we use it in other places.)

if (message.IsEmpty())
return;

Expand Down Expand Up @@ -1510,20 +1511,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));
}


Expand All @@ -1532,7 +1539,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
2 changes: 1 addition & 1 deletion src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,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(
Expand Down
4 changes: 3 additions & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,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);

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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also do another print from the catch to make sure it routes that way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fishrock123 sounds good, done


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