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 6151544

Browse filesBrowse files
addaleaxFishrock123
addaleax
authored and
Fishrock123
committed
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 8d18aed commit 6151544
Copy full SHA for 6151544

File tree

Expand file treeCollapse file tree

5 files changed

+45
-15
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+45
-15
lines changed
Open diff view settings
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+20-13Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,7 +1514,8 @@ bool IsExceptionDecorated(Environment* env, Local<Value> er) {
15141514

15151515
void AppendExceptionLine(Environment* env,
15161516
Local<Value> er,
1517-
Local<Message> message) {
1517+
Local<Message> message,
1518+
enum ErrorHandlingMode mode) {
15181519
if (message.IsEmpty())
15191520
return;
15201521

@@ -1601,20 +1602,26 @@ void AppendExceptionLine(Environment* env,
16011602

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

1604-
if (!arrow_str.IsEmpty() && !err_obj.IsEmpty() && err_obj->IsNativeError()) {
1605-
err_obj->SetPrivate(
1606-
env->context(),
1607-
env->arrow_message_private_symbol(),
1608-
arrow_str);
1605+
const bool can_set_arrow = !arrow_str.IsEmpty() && !err_obj.IsEmpty();
1606+
// If allocating arrow_str failed, print it out. There's not much else to do.
1607+
// If it's not an error, but something needs to be printed out because
1608+
// it's a fatal exception, also print it out from here.
1609+
// Otherwise, the arrow property will be attached to the object and handled
1610+
// by the caller.
1611+
if (!can_set_arrow || (mode == FATAL_ERROR && !err_obj->IsNativeError())) {
1612+
if (env->printed_error())
1613+
return;
1614+
env->set_printed_error(true);
1615+
1616+
uv_tty_reset_mode();
1617+
PrintErrorString("\n%s", arrow);
16091618
return;
16101619
}
16111620

1612-
// Allocation failed, just print it out.
1613-
if (env->printed_error())
1614-
return;
1615-
env->set_printed_error(true);
1616-
uv_tty_reset_mode();
1617-
PrintErrorString("\n%s", arrow);
1621+
CHECK(err_obj->SetPrivate(
1622+
env->context(),
1623+
env->arrow_message_private_symbol(),
1624+
arrow_str).FromMaybe(false));
16181625
}
16191626

16201627

@@ -1623,7 +1630,7 @@ static void ReportException(Environment* env,
16231630
Local<Message> message) {
16241631
HandleScope scope(env->isolate());
16251632

1626-
AppendExceptionLine(env, er, message);
1633+
AppendExceptionLine(env, er, message, FATAL_ERROR);
16271634

16281635
Local<Value> trace_value;
16291636
Local<Value> arrow;
Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,7 @@ class ContextifyScript : public BaseObject {
633633
if (IsExceptionDecorated(env, err_obj))
634634
return;
635635

636-
AppendExceptionLine(env, exception, try_catch.Message());
636+
AppendExceptionLine(env, exception, try_catch.Message(), CONTEXTIFY_ERROR);
637637
Local<Value> stack = err_obj->Get(env->stack_string());
638638
auto maybe_value =
639639
err_obj->GetPrivate(
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
@@ -142,9 +142,11 @@ constexpr size_t arraysize(const T(&)[N]) { return N; }
142142

143143
bool IsExceptionDecorated(Environment* env, v8::Local<v8::Value> er);
144144

145+
enum ErrorHandlingMode { FATAL_ERROR, CONTEXTIFY_ERROR };
145146
void AppendExceptionLine(Environment* env,
146147
v8::Local<v8::Value> er,
147-
v8::Local<v8::Message> message);
148+
v8::Local<v8::Message> message,
149+
enum ErrorHandlingMode mode);
148150

149151
NO_RETURN void FatalError(const char* location, const char* message);
150152

Collapse file

‎test/message/vm_caught_custom_runtime_error.js‎

Copy file name to clipboard
+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

‎test/message/vm_caught_custom_runtime_error.out‎

Copy file name to clipboard
+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.