Skip to content

Commit

Permalink
node: do not override message/stack of error
Browse files Browse the repository at this point in the history
Put the `...^` arrow string to the hidden property of the object, and
use it only when printing error to the stderr.

Fix: #2104
PR-URL: #2108
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
indutny authored and rvagg committed Aug 4, 2015
1 parent 9d34bd1 commit ef65321
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 19 deletions.
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ namespace node {
V(address_string, "address") \
V(args_string, "args") \
V(argv_string, "argv") \
V(arrow_message_string, "arrowMessage") \
V(async, "async") \
V(async_queue_string, "_asyncQueue") \
V(atime_string, "atime") \
Expand Down
41 changes: 26 additions & 15 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1413,16 +1413,7 @@ void AppendExceptionLine(Environment* env,
if (arrow_str.IsEmpty() || err_obj.IsEmpty() || !err_obj->IsNativeError())
goto print;

msg = err_obj->Get(env->message_string());
stack = err_obj->Get(env->stack_string());

if (msg.IsEmpty() || stack.IsEmpty())
goto print;

err_obj->Set(env->message_string(),
String::Concat(arrow_str, msg->ToString(env->isolate())));
err_obj->Set(env->stack_string(),
String::Concat(arrow_str, stack->ToString(env->isolate())));
err_obj->SetHiddenValue(env->arrow_message_string(), arrow_str);
return;

print:
Expand All @@ -1442,17 +1433,27 @@ static void ReportException(Environment* env,
AppendExceptionLine(env, er, message);

Local<Value> trace_value;
Local<Value> arrow;

if (er->IsUndefined() || er->IsNull())
if (er->IsUndefined() || er->IsNull()) {
trace_value = Undefined(env->isolate());
else
trace_value = er->ToObject(env->isolate())->Get(env->stack_string());
} else {
Local<Object> err_obj = er->ToObject(env->isolate());

trace_value = err_obj->Get(env->stack_string());
arrow = err_obj->GetHiddenValue(env->arrow_message_string());
}

node::Utf8Value trace(env->isolate(), trace_value);

// range errors have a trace member set to undefined
if (trace.length() > 0 && !trace_value->IsUndefined()) {
fprintf(stderr, "%s\n", *trace);
if (arrow.IsEmpty() || !arrow->IsString()) {
fprintf(stderr, "%s\n", *trace);
} else {
node::Utf8Value arrow_string(env->isolate(), arrow);
fprintf(stderr, "%s\n%s\n", *arrow_string, *trace);
}
} else {
// this really only happens for RangeErrors, since they're the only
// kind that won't have all this info in the trace, or when non-Error
Expand All @@ -1476,7 +1477,17 @@ static void ReportException(Environment* env,
} else {
node::Utf8Value name_string(env->isolate(), name);
node::Utf8Value message_string(env->isolate(), message);
fprintf(stderr, "%s: %s\n", *name_string, *message_string);

if (arrow.IsEmpty() || !arrow->IsString()) {
fprintf(stderr, "%s: %s\n", *name_string, *message_string);
} else {
node::Utf8Value arrow_string(env->isolate(), arrow);
fprintf(stderr,
"%s\n%s: %s\n",
*arrow_string,
*name_string,
*message_string);
}
}
}

Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-vm-syntax-error-message.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var child_process = require('child_process');

var p = child_process.spawn(process.execPath, [
'-e',
'vm = require("vm");' +
'context = vm.createContext({});' +
'try { vm.runInContext("throw new Error(\'boo\')", context); } ' +
'catch (e) { console.log(e.message); }'
]);

p.stderr.on('data', function(data) {
assert(false, 'Unexpected stderr data: ' + data);
});

var output = '';

p.stdout.on('data', function(data) {
output += data;
});

process.on('exit', function() {
assert.equal(output.replace(/[\r\n]+/g, ''), 'boo');
});
8 changes: 4 additions & 4 deletions test/sequential/test-vm-syntax-error-stderr.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ var wrong_script = path.join(common.fixturesDir, 'cert.pem');

var p = child_process.spawn(process.execPath, [
'-e',
'try { require(process.argv[1]); } catch (e) { console.log(e.stack); }',
'require(process.argv[1]);',
wrong_script
]);

p.stderr.on('data', function(data) {
assert(false, 'Unexpected stderr data: ' + data);
p.stdout.on('data', function(data) {
assert(false, 'Unexpected stdout data: ' + data);
});

var output = '';

p.stdout.on('data', function(data) {
p.stderr.on('data', function(data) {
output += data;
});

Expand Down

0 comments on commit ef65321

Please sign in to comment.