Skip to content

Commit f1de13b

Browse files
indutnyindutny
committed
node: do not print SyntaxError hints to stderr
Try embedding the ` ... ^` lines inside the `SyntaxError` (or any other native error) object before giving up and printing them to the stderr. fix nodejs#6920 fix nodejs#1310
1 parent 6cbfcda commit f1de13b

11 files changed

Lines changed: 190 additions & 89 deletions

‎src/env-inl.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ inline Environment::Environment(v8::Local<v8::Context> context)
224224
isolate_data_(IsolateData::GetOrCreate(context->GetIsolate())),
225225
using_smalloc_alloc_cb_(false),
226226
using_domains_(false),
227+
printed_error_(false),
227228
context_(context->GetIsolate(), context) {
228229
// We'll be creating new objects so make sure we've entered the context.
229230
v8::HandleScope handle_scope(isolate());
@@ -330,6 +331,14 @@ inline void Environment::set_using_domains(bool value) {
330331
using_domains_ = value;
331332
}
332333

334+
inline bool Environment::printed_error() const {
335+
return printed_error_;
336+
}
337+
338+
inline void Environment::set_printed_error(bool value) {
339+
printed_error_ = value;
340+
}
341+
333342
inline Environment* Environment::from_cares_timer_handle(uv_timer_t* handle) {
334343
return CONTAINER_OF(handle, Environment, cares_timer_handle_);
335344
}

‎src/env.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ namespace node {
9292
V(ipv6_string, "IPv6") \
9393
V(issuer_string, "issuer") \
9494
V(mark_sweep_compact_string, "mark-sweep-compact") \
95+
V(message_string, "message") \
9596
V(method_string, "method") \
9697
V(mode_string, "mode") \
9798
V(modulus_string, "modulus") \
@@ -115,6 +116,7 @@ namespace node {
115116
V(onstop_string, "onstop") \
116117
V(path_string, "path") \
117118
V(port_string, "port") \
119+
V(processed_string, "processed") \
118120
V(rdev_string, "rdev") \
119121
V(rename_string, "rename") \
120122
V(rss_string, "rss") \
@@ -127,6 +129,7 @@ namespace node {
127129
V(smalloc_p_string, "_smalloc_p") \
128130
V(sni_context_err_string, "Invalid SNI context") \
129131
V(sni_context_string, "sni_context") \
132+
V(stack_string, "stack") \
130133
V(status_code_string, "statusCode") \
131134
V(status_message_string, "statusMessage") \
132135
V(subject_string, "subject") \
@@ -302,6 +305,9 @@ class Environment {
302305
inline bool using_domains() const;
303306
inline void set_using_domains(bool value);
304307

308+
inline bool printed_error() const;
309+
inline void set_printed_error(bool value);
310+
305311
// Strings are shared across shared contexts. The getters simply proxy to
306312
// the per-isolate primitive.
307313
#define V(PropertyName, StringValue) \
@@ -343,6 +349,7 @@ class Environment {
343349
bool using_smalloc_alloc_cb_;
344350
bool using_domains_;
345351
QUEUE gc_tracker_queue_;
352+
bool printed_error_;
346353

347354
#define V(PropertyName, TypeName) \
348355
v8::Persistent<TypeName> PropertyName ## _;

‎src/node.cc‎

Lines changed: 119 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,80 +1275,120 @@ ssize_t DecodeWrite(char *buf,
12751275
return StringBytes::Write(buf, buflen, val, encoding, NULL);
12761276
}
12771277

1278-
void DisplayExceptionLine(Handle<Message> message) {
1279-
// Prevent re-entry into this function. For example, if there is
1280-
// a throw from a program in vm.runInThisContext(code, filename, true),
1281-
// then we want to show the original failure, not the secondary one.
1282-
static bool displayed_error = false;
1283-
1284-
if (displayed_error)
1278+
void AppendExceptionLine(Environment* env,
1279+
Handle<Value> er,
1280+
Handle<Message> message) {
1281+
if (message.IsEmpty())
12851282
return;
1286-
displayed_error = true;
12871283

1288-
uv_tty_reset_mode();
1284+
HandleScope scope(env->isolate());
1285+
Local<Object> err_obj;
1286+
if (!er.IsEmpty() && er->IsObject()) {
1287+
err_obj = er.As<Object>();
12891288

1290-
fprintf(stderr, "\n");
1291-
1292-
if (!message.IsEmpty()) {
1293-
// Print (filename):(line number): (message).
1294-
String::Utf8Value filename(message->GetScriptResourceName());
1295-
const char* filename_string = *filename;
1296-
int linenum = message->GetLineNumber();
1297-
fprintf(stderr, "%s:%i\n", filename_string, linenum);
1298-
// Print line of source code.
1299-
String::Utf8Value sourceline(message->GetSourceLine());
1300-
const char* sourceline_string = *sourceline;
1301-
1302-
// Because of how node modules work, all scripts are wrapped with a
1303-
// "function (module, exports, __filename, ...) {"
1304-
// to provide script local variables.
1305-
//
1306-
// When reporting errors on the first line of a script, this wrapper
1307-
// function is leaked to the user. There used to be a hack here to
1308-
// truncate off the first 62 characters, but it caused numerous other
1309-
// problems when vm.runIn*Context() methods were used for non-module
1310-
// code.
1311-
//
1312-
// If we ever decide to re-instate such a hack, the following steps
1313-
// must be taken:
1314-
//
1315-
// 1. Pass a flag around to say "this code was wrapped"
1316-
// 2. Update the stack frame output so that it is also correct.
1317-
//
1318-
// It would probably be simpler to add a line rather than add some
1319-
// number of characters to the first line, since V8 truncates the
1320-
// sourceline to 78 characters, and we end up not providing very much
1321-
// useful debugging info to the user if we remove 62 characters.
1322-
1323-
int start = message->GetStartColumn();
1324-
int end = message->GetEndColumn();
1325-
1326-
fprintf(stderr, "%s\n", sourceline_string);
1327-
// Print wavy underline (GetUnderline is deprecated).
1328-
for (int i = 0; i < start; i++) {
1329-
fputc((sourceline_string[i] == '\t') ? '\t' : ' ', stderr);
1330-
}
1331-
for (int i = start; i < end; i++) {
1332-
fputc('^', stderr);
1333-
}
1334-
fputc('\n', stderr);
1289+
// Do it only once per message
1290+
if (!err_obj->GetHiddenValue(env->processed_string()).IsEmpty())
1291+
return;
1292+
err_obj->SetHiddenValue(env->processed_string(), True(env->isolate()));
13351293
}
1294+
1295+
static char arrow[1024];
1296+
1297+
// Print (filename):(line number): (message).
1298+
String::Utf8Value filename(message->GetScriptResourceName());
1299+
const char* filename_string = *filename;
1300+
int linenum = message->GetLineNumber();
1301+
// Print line of source code.
1302+
String::Utf8Value sourceline(message->GetSourceLine());
1303+
const char* sourceline_string = *sourceline;
1304+
1305+
// Because of how node modules work, all scripts are wrapped with a
1306+
// "function (module, exports, __filename, ...) {"
1307+
// to provide script local variables.
1308+
//
1309+
// When reporting errors on the first line of a script, this wrapper
1310+
// function is leaked to the user. There used to be a hack here to
1311+
// truncate off the first 62 characters, but it caused numerous other
1312+
// problems when vm.runIn*Context() methods were used for non-module
1313+
// code.
1314+
//
1315+
// If we ever decide to re-instate such a hack, the following steps
1316+
// must be taken:
1317+
//
1318+
// 1. Pass a flag around to say "this code was wrapped"
1319+
// 2. Update the stack frame output so that it is also correct.
1320+
//
1321+
// It would probably be simpler to add a line rather than add some
1322+
// number of characters to the first line, since V8 truncates the
1323+
// sourceline to 78 characters, and we end up not providing very much
1324+
// useful debugging info to the user if we remove 62 characters.
1325+
1326+
int start = message->GetStartColumn();
1327+
int end = message->GetEndColumn();
1328+
1329+
int off = snprintf(arrow,
1330+
sizeof(arrow),
1331+
"%s:%i\n%s\n",
1332+
filename_string,
1333+
linenum,
1334+
sourceline_string);
1335+
assert(off >= 0);
1336+
1337+
// Print wavy underline (GetUnderline is deprecated).
1338+
for (int i = 0; i < start; i++) {
1339+
assert(static_cast<size_t>(off) < sizeof(arrow));
1340+
arrow[off++] = (sourceline_string[i] == '\t') ? '\t' : ' ';
1341+
}
1342+
for (int i = start; i < end; i++) {
1343+
assert(static_cast<size_t>(off) < sizeof(arrow));
1344+
arrow[off++] = '^';
1345+
}
1346+
assert(static_cast<size_t>(off) < sizeof(arrow) - 1);
1347+
arrow[off++] = '\n';
1348+
arrow[off] = '\0';
1349+
1350+
Local<String> arrow_str = String::NewFromUtf8(env->isolate(), arrow);
1351+
Local<Value> msg;
1352+
Local<Value> stack;
1353+
1354+
// Allocation failed, just print it out
1355+
if (arrow_str.IsEmpty() || err_obj.IsEmpty() || !err_obj->IsNativeError())
1356+
goto print;
1357+
1358+
msg = err_obj->Get(env->message_string());
1359+
stack = err_obj->Get(env->stack_string());
1360+
1361+
if (msg.IsEmpty() || stack.IsEmpty())
1362+
goto print;
1363+
1364+
err_obj->Set(env->message_string(),
1365+
String::Concat(arrow_str, msg->ToString()));
1366+
err_obj->Set(env->stack_string(),
1367+
String::Concat(arrow_str, stack->ToString()));
1368+
return;
1369+
1370+
print:
1371+
if (env->printed_error())
1372+
return;
1373+
env->set_printed_error(true);
1374+
uv_tty_reset_mode();
1375+
fprintf(stderr, "\n%s", arrow);
13361376
}
13371377

13381378

1339-
static void ReportException(Handle<Value> er, Handle<Message> message) {
1340-
HandleScope scope(node_isolate);
1379+
static void ReportException(Environment* env,
1380+
Handle<Value> er,
1381+
Handle<Message> message) {
1382+
HandleScope scope(env->isolate());
13411383

1342-
DisplayExceptionLine(message);
1384+
AppendExceptionLine(env, er, message);
13431385

13441386
Local<Value> trace_value;
13451387

1346-
if (er->IsUndefined() || er->IsNull()) {
1347-
trace_value = Undefined(node_isolate);
1348-
} else {
1349-
trace_value =
1350-
er->ToObject()->Get(FIXED_ONE_BYTE_STRING(node_isolate, "stack"));
1351-
}
1388+
if (er->IsUndefined() || er->IsNull())
1389+
trace_value = Undefined(env->isolate());
1390+
else
1391+
trace_value = er->ToObject()->Get(env->stack_string());
13521392

13531393
String::Utf8Value trace(trace_value);
13541394

@@ -1364,8 +1404,8 @@ static void ReportException(Handle<Value> er, Handle<Message> message) {
13641404

13651405
if (er->IsObject()) {
13661406
Local<Object> err_obj = er.As<Object>();
1367-
message = err_obj->Get(FIXED_ONE_BYTE_STRING(node_isolate, "message"));
1368-
name = err_obj->Get(FIXED_ONE_BYTE_STRING(node_isolate, "name"));
1407+
message = err_obj->Get(env->message_string());
1408+
name = err_obj->Get(FIXED_ONE_BYTE_STRING(env->isolate(), "name"));
13691409
}
13701410

13711411
if (message.IsEmpty() ||
@@ -1386,14 +1426,16 @@ static void ReportException(Handle<Value> er, Handle<Message> message) {
13861426
}
13871427

13881428

1389-
static void ReportException(const TryCatch& try_catch) {
1390-
ReportException(try_catch.Exception(), try_catch.Message());
1429+
static void ReportException(Environment* env, const TryCatch& try_catch) {
1430+
ReportException(env, try_catch.Exception(), try_catch.Message());
13911431
}
13921432

13931433

13941434
// Executes a str within the current v8 context.
1395-
Local<Value> ExecuteString(Handle<String> source, Handle<Value> filename) {
1396-
HandleScope scope(node_isolate);
1435+
static Local<Value> ExecuteString(Environment* env,
1436+
Handle<String> source,
1437+
Handle<Value> filename) {
1438+
HandleScope scope(env->isolate());
13971439
TryCatch try_catch;
13981440

13991441
// try_catch must be nonverbose to disable FatalException() handler,
@@ -1402,13 +1444,13 @@ Local<Value> ExecuteString(Handle<String> source, Handle<Value> filename) {
14021444

14031445
Local<v8::Script> script = v8::Script::Compile(source, filename);
14041446
if (script.IsEmpty()) {
1405-
ReportException(try_catch);
1447+
ReportException(env, try_catch);
14061448
exit(3);
14071449
}
14081450

14091451
Local<Value> result = script->Run();
14101452
if (result.IsEmpty()) {
1411-
ReportException(try_catch);
1453+
ReportException(env, try_catch);
14121454
exit(4);
14131455
}
14141456

@@ -2025,7 +2067,7 @@ void FatalException(Handle<Value> error, Handle<Message> message) {
20252067
if (!fatal_exception_function->IsFunction()) {
20262068
// failed before the process._fatalException function was added!
20272069
// this is probably pretty bad. Nothing to do but report and exit.
2028-
ReportException(error, message);
2070+
ReportException(env, error, message);
20292071
exit(6);
20302072
}
20312073

@@ -2040,12 +2082,12 @@ void FatalException(Handle<Value> error, Handle<Message> message) {
20402082

20412083
if (fatal_try_catch.HasCaught()) {
20422084
// the fatal exception function threw, so we must exit
2043-
ReportException(fatal_try_catch);
2085+
ReportException(env, fatal_try_catch);
20442086
exit(7);
20452087
}
20462088

20472089
if (false == caught->BooleanValue()) {
2048-
ReportException(error, message);
2090+
ReportException(env, error, message);
20492091
exit(1);
20502092
}
20512093
}
@@ -2705,10 +2747,10 @@ void Load(Environment* env) {
27052747
// are not safe to ignore.
27062748
try_catch.SetVerbose(false);
27072749

2708-
Local<String> script_name = FIXED_ONE_BYTE_STRING(node_isolate, "node.js");
2709-
Local<Value> f_value = ExecuteString(MainSource(), script_name);
2750+
Local<String> script_name = FIXED_ONE_BYTE_STRING(env->isolate(), "node.js");
2751+
Local<Value> f_value = ExecuteString(env, MainSource(), script_name);
27102752
if (try_catch.HasCaught()) {
2711-
ReportException(try_catch);
2753+
ReportException(env, try_catch);
27122754
exit(10);
27132755
}
27142756
assert(f_value->IsFunction());

‎src/node.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ enum encoding {ASCII, UTF8, BASE64, UCS2, BINARY, HEX, BUFFER};
190190
enum encoding ParseEncoding(v8::Handle<v8::Value> encoding_v,
191191
enum encoding _default = BINARY);
192192
NODE_EXTERN void FatalException(const v8::TryCatch& try_catch);
193-
void DisplayExceptionLine(v8::Handle<v8::Message> message);
194193

195194
NODE_EXTERN v8::Local<v8::Value> Encode(const void *buf, size_t len,
196195
enum encoding encoding = BINARY);

‎src/node_contextify.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ class ContextifyScript : public BaseObject {
453453

454454
if (v8_script.IsEmpty()) {
455455
if (display_errors) {
456-
DisplayExceptionLine(try_catch.Message());
456+
AppendExceptionLine(env, try_catch.Exception(), try_catch.Message());
457457
}
458458
try_catch.ReThrow();
459459
return;
@@ -622,7 +622,7 @@ class ContextifyScript : public BaseObject {
622622
if (result.IsEmpty()) {
623623
// Error occurred during execution of the script.
624624
if (display_errors) {
625-
DisplayExceptionLine(try_catch.Message());
625+
AppendExceptionLine(env, try_catch.Exception(), try_catch.Message());
626626
}
627627
try_catch.ReThrow();
628628
return false;

‎src/node_internals.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ inline static void ThrowUVException(int errorno,
155155
v8::ThrowException(UVException(errorno, syscall, message, path));
156156
}
157157

158+
void AppendExceptionLine(Environment* env,
159+
v8::Handle<v8::Value> er,
160+
v8::Handle<v8::Message> message);
161+
158162
NO_RETURN void FatalError(const char* location, const char* message);
159163

160164
v8::Local<v8::Object> BuildStatsObject(Environment* env, const uv_stat_t* s);

0 commit comments

Comments
 (0)