Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 8557597

Browse filesBrowse files
addaleaxMyles Borins
authored andcommitted
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 <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 7dbb0d0 commit 8557597
Copy full SHA for 8557597

File tree

Expand file treeCollapse file tree

4 files changed

+42
-14
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+42
-14
lines changed
Open diff view settings
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+18-13Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,8 @@ ssize_t DecodeWrite(Isolate* isolate,
14391439

14401440
void AppendExceptionLine(Environment* env,
14411441
Local<Value> er,
1442-
Local<Message> message) {
1442+
Local<Message> message,
1443+
enum ErrorHandlingMode mode) {
14431444
if (message.IsEmpty())
14441445
return;
14451446

@@ -1521,19 +1522,23 @@ void AppendExceptionLine(Environment* env,
15211522

15221523
Local<String> arrow_str = String::NewFromUtf8(env->isolate(), arrow);
15231524

1524-
// Allocation failed, just print it out
1525-
if (arrow_str.IsEmpty() || err_obj.IsEmpty() || !err_obj->IsNativeError())
1526-
goto print;
1527-
1528-
err_obj->SetHiddenValue(env->arrow_message_string(), arrow_str);
1529-
return;
1525+
const bool can_set_arrow = !arrow_str.IsEmpty() && !err_obj.IsEmpty();
1526+
// If allocating arrow_str failed, print it out. There's not much else to do.
1527+
// If it's not an error, but something needs to be printed out because
1528+
// it's a fatal exception, also print it out from here.
1529+
// Otherwise, the arrow property will be attached to the object and handled
1530+
// by the caller.
1531+
if (!can_set_arrow || (mode == FATAL_ERROR && !err_obj->IsNativeError())) {
1532+
if (env->printed_error())
1533+
return;
1534+
env->set_printed_error(true);
15301535

1531-
print:
1532-
if (env->printed_error())
1536+
uv_tty_reset_mode();
1537+
PrintErrorString("\n%s", arrow);
15331538
return;
1534-
env->set_printed_error(true);
1535-
uv_tty_reset_mode();
1536-
PrintErrorString("\n%s", arrow);
1539+
}
1540+
1541+
err_obj->SetHiddenValue(env->arrow_message_string(), arrow_str);
15371542
}
15381543

15391544

@@ -1542,7 +1547,7 @@ static void ReportException(Environment* env,
15421547
Local<Message> message) {
15431548
HandleScope scope(env->isolate());
15441549

1545-
AppendExceptionLine(env, er, message);
1550+
AppendExceptionLine(env, er, message, FATAL_ERROR);
15461551

15471552
Local<Value> trace_value;
15481553
Local<Value> arrow;
Collapse file

‎src/node_internals.h‎

Copy file name to clipboardExpand all lines: src/node_internals.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,11 @@ constexpr size_t arraysize(const T(&)[N]) { return N; }
126126
# define NO_RETURN
127127
#endif
128128

129+
enum ErrorHandlingMode { FATAL_ERROR, CONTEXTIFY_ERROR };
129130
void AppendExceptionLine(Environment* env,
130131
v8::Local<v8::Value> er,
131-
v8::Local<v8::Message> message);
132+
v8::Local<v8::Message> message,
133+
enum ErrorHandlingMode mode = CONTEXTIFY_ERROR);
132134

133135
NO_RETURN void FatalError(const char* location, const char* message);
134136

Collapse file
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
require('../common');
3+
const vm = require('vm');
4+
5+
console.error('beginning');
6+
7+
// Regression test for https://github.com/nodejs/node/issues/7397:
8+
// vm.runInThisContext() should not print out anything to stderr by itself.
9+
try {
10+
vm.runInThisContext(`throw ({
11+
name: 'MyCustomError',
12+
message: 'This is a custom message'
13+
})`, { filename: 'test.vm' });
14+
} catch (e) {
15+
console.error('received error', e.name);
16+
}
17+
18+
console.error('end');
Collapse file
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
beginning
2+
received error MyCustomError
3+
end

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.