Browse Source

node: do not override `message`/`stack` of error

Put the `...^` arrow string to the hidden property of the object, and
use it only when printing error to the stderr.

Fix: https://github.com/nodejs/io.js/issues/2104
PR-URL: https://github.com/nodejs/io.js/pull/2108
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
v4.0.0-rc
Fedor Indutny 10 years ago
committed by Rod Vagg
parent
commit
ef65321083
  1. 1
      src/env.h
  2. 37
      src/node.cc
  3. 26
      test/parallel/test-vm-syntax-error-message.js
  4. 8
      test/sequential/test-vm-syntax-error-stderr.js

1
src/env.h

@ -44,6 +44,7 @@ namespace node {
V(address_string, "address") \ V(address_string, "address") \
V(args_string, "args") \ V(args_string, "args") \
V(argv_string, "argv") \ V(argv_string, "argv") \
V(arrow_message_string, "arrowMessage") \
V(async, "async") \ V(async, "async") \
V(async_queue_string, "_asyncQueue") \ V(async_queue_string, "_asyncQueue") \
V(atime_string, "atime") \ V(atime_string, "atime") \

37
src/node.cc

@ -1413,16 +1413,7 @@ void AppendExceptionLine(Environment* env,
if (arrow_str.IsEmpty() || err_obj.IsEmpty() || !err_obj->IsNativeError()) if (arrow_str.IsEmpty() || err_obj.IsEmpty() || !err_obj->IsNativeError())
goto print; goto print;
msg = err_obj->Get(env->message_string()); err_obj->SetHiddenValue(env->arrow_message_string(), arrow_str);
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())));
return; return;
print: print:
@ -1442,17 +1433,27 @@ static void ReportException(Environment* env,
AppendExceptionLine(env, er, message); AppendExceptionLine(env, er, message);
Local<Value> trace_value; Local<Value> trace_value;
Local<Value> arrow;
if (er->IsUndefined() || er->IsNull()) if (er->IsUndefined() || er->IsNull()) {
trace_value = Undefined(env->isolate()); trace_value = Undefined(env->isolate());
else } else {
trace_value = er->ToObject(env->isolate())->Get(env->stack_string()); 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); node::Utf8Value trace(env->isolate(), trace_value);
// range errors have a trace member set to undefined // range errors have a trace member set to undefined
if (trace.length() > 0 && !trace_value->IsUndefined()) { if (trace.length() > 0 && !trace_value->IsUndefined()) {
if (arrow.IsEmpty() || !arrow->IsString()) {
fprintf(stderr, "%s\n", *trace); fprintf(stderr, "%s\n", *trace);
} else {
node::Utf8Value arrow_string(env->isolate(), arrow);
fprintf(stderr, "%s\n%s\n", *arrow_string, *trace);
}
} else { } else {
// this really only happens for RangeErrors, since they're the only // 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 // kind that won't have all this info in the trace, or when non-Error
@ -1476,7 +1477,17 @@ static void ReportException(Environment* env,
} else { } else {
node::Utf8Value name_string(env->isolate(), name); node::Utf8Value name_string(env->isolate(), name);
node::Utf8Value message_string(env->isolate(), message); node::Utf8Value message_string(env->isolate(), message);
if (arrow.IsEmpty() || !arrow->IsString()) {
fprintf(stderr, "%s: %s\n", *name_string, *message_string); 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);
}
} }
} }

26
test/parallel/test-vm-syntax-error-message.js

@ -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
test/sequential/test-vm-syntax-error-stderr.js

@ -8,17 +8,17 @@ var wrong_script = path.join(common.fixturesDir, 'cert.pem');
var p = child_process.spawn(process.execPath, [ var p = child_process.spawn(process.execPath, [
'-e', '-e',
'try { require(process.argv[1]); } catch (e) { console.log(e.stack); }', 'require(process.argv[1]);',
wrong_script wrong_script
]); ]);
p.stderr.on('data', function(data) { p.stdout.on('data', function(data) {
assert(false, 'Unexpected stderr data: ' + data); assert(false, 'Unexpected stdout data: ' + data);
}); });
var output = ''; var output = '';
p.stdout.on('data', function(data) { p.stderr.on('data', function(data) {
output += data; output += data;
}); });

Loading…
Cancel
Save