From df9cdd893af83f4fec0bfc4a16ef44f1113e29c7 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sun, 5 Jul 2015 11:20:26 -0700 Subject: [PATCH] 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 --- src/env.h | 1 + src/node.cc | 41 ++++++++++++------- test/parallel/test-vm-syntax-error-message.js | 26 ++++++++++++ .../sequential/test-vm-syntax-error-stderr.js | 8 ++-- 4 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-vm-syntax-error-message.js diff --git a/src/env.h b/src/env.h index e3bae77afdceed..501c151122198e 100644 --- a/src/env.h +++ b/src/env.h @@ -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") \ diff --git a/src/node.cc b/src/node.cc index e958a6a93d8448..7283c6bb86072c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -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: @@ -1442,17 +1433,27 @@ static void ReportException(Environment* env, AppendExceptionLine(env, er, message); Local trace_value; + Local 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 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 @@ -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); + } } } diff --git a/test/parallel/test-vm-syntax-error-message.js b/test/parallel/test-vm-syntax-error-message.js new file mode 100644 index 00000000000000..4b48b0d474a88d --- /dev/null +++ b/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'); +}); diff --git a/test/sequential/test-vm-syntax-error-stderr.js b/test/sequential/test-vm-syntax-error-stderr.js index f7b1bef9e309dc..7c3c5ff1351da2 100644 --- a/test/sequential/test-vm-syntax-error-stderr.js +++ b/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, [ '-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; });