Skip to content
Permalink
Browse files

module: always decorate thrown errors

This provides more information when encountering a syntax or similar
error when executing a file with require().

Fixes: #4286
PR-URL: #4287
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information...
mscdex committed Dec 15, 2015
1 parent 4b0b991 commit 18490d3d5af3131751791e501302bc57865337b3
Showing with 94 additions and 13 deletions.
  1. +7 −3 lib/internal/util.js
  2. +12 −2 lib/module.js
  3. +2 −1 src/env.h
  4. +12 −2 src/node.cc
  5. +16 −0 src/node_util.cc
  6. +26 −4 test/parallel/test-util-decorate-error-stack.js
  7. +19 −1 test/parallel/test-util-internal.js
@@ -4,6 +4,7 @@ const binding = process.binding('util');
const prefix = `(${process.release.name}:${process.pid}) `;

exports.getHiddenValue = binding.getHiddenValue;
exports.setHiddenValue = binding.setHiddenValue;

// All the internal deprecations have to use this function only, as this will
// prepend the prefix to the actual message.
@@ -76,13 +77,16 @@ exports._deprecate = function(fn, msg) {
};

exports.decorateErrorStack = function decorateErrorStack(err) {
if (!(exports.isError(err) && err.stack))
if (!(exports.isError(err) && err.stack) ||
exports.getHiddenValue(err, 'node:decorated') === true)
return;

const arrow = exports.getHiddenValue(err, 'arrowMessage');
const arrow = exports.getHiddenValue(err, 'node:arrowMessage');

if (arrow)
if (arrow) {
err.stack = arrow + err.stack;
exports.setHiddenValue(err, 'node:decorated', true);
}
};

exports.isError = function isError(e) {
@@ -23,6 +23,16 @@ function hasOwnProperty(obj, prop) {
}


function tryWrapper(wrapper, opts) {
try {
return runInThisContext(wrapper, opts);
} catch (e) {
internalUtil.decorateErrorStack(e);
throw e;
}
}


function Module(id, parent) {
this.id = id;
this.exports = {};
@@ -371,8 +381,8 @@ Module.prototype._compile = function(content, filename) {
// create wrapper function
var wrapper = Module.wrap(content);

var compiledWrapper = runInThisContext(wrapper,
{ filename: filename, lineOffset: 0 });
var compiledWrapper = tryWrapper(wrapper,
{ filename: filename, lineOffset: 0 });
if (global.v8debug) {
if (!resolvedArgv) {
// we enter the repl if we're not given a filename argument.
@@ -51,7 +51,7 @@ namespace node {
V(alpn_buffer_string, "alpnBuffer") \
V(args_string, "args") \
V(argv_string, "argv") \
V(arrow_message_string, "arrowMessage") \
V(arrow_message_string, "node:arrowMessage") \
V(async, "async") \
V(async_queue_string, "_asyncQueue") \
V(atime_string, "atime") \
@@ -71,6 +71,7 @@ namespace node {
V(cwd_string, "cwd") \
V(debug_port_string, "debugPort") \
V(debug_string, "debug") \
V(decorated_string, "node:decorated") \
V(dest_string, "dest") \
V(detached_string, "detached") \
V(dev_string, "dev") \
@@ -1399,6 +1399,15 @@ ssize_t DecodeWrite(Isolate* isolate,
return StringBytes::Write(isolate, buf, buflen, val, encoding, nullptr);
}

bool IsExceptionDecorated(Environment* env, Local<Value> er) {
if (!er.IsEmpty() && er->IsObject()) {
Local<Object> err_obj = er.As<Object>();
Local<Value> decorated = err_obj->GetHiddenValue(env->decorated_string());
return !decorated.IsEmpty() && decorated->IsTrue();
}
return false;
}

void AppendExceptionLine(Environment* env,
Local<Value> er,
Local<Message> message) {
@@ -1508,6 +1517,7 @@ static void ReportException(Environment* env,

Local<Value> trace_value;
Local<Value> arrow;
const bool decorated = IsExceptionDecorated(env, er);

if (er->IsUndefined() || er->IsNull()) {
trace_value = Undefined(env->isolate());
@@ -1522,7 +1532,7 @@ static void ReportException(Environment* env,

// range errors have a trace member set to undefined
if (trace.length() > 0 && !trace_value->IsUndefined()) {
if (arrow.IsEmpty() || !arrow->IsString()) {
if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
PrintErrorString("%s\n", *trace);
} else {
node::Utf8Value arrow_string(env->isolate(), arrow);
@@ -1554,7 +1564,7 @@ static void ReportException(Environment* env,
node::Utf8Value name_string(env->isolate(), name);
node::Utf8Value message_string(env->isolate(), message);

if (arrow.IsEmpty() || !arrow->IsString()) {
if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
PrintErrorString("%s: %s\n", *name_string, *message_string);
} else {
node::Utf8Value arrow_string(env->isolate(), arrow);
@@ -52,6 +52,21 @@ static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(obj->GetHiddenValue(name));
}

static void SetHiddenValue(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

if (!args[0]->IsObject())
return env->ThrowTypeError("obj must be an object");

if (!args[1]->IsString())
return env->ThrowTypeError("name must be a string");

Local<Object> obj = args[0].As<Object>();
Local<String> name = args[1].As<String>();

args.GetReturnValue().Set(obj->SetHiddenValue(name, args[2]));
}


void Initialize(Local<Object> target,
Local<Value> unused,
@@ -63,6 +78,7 @@ void Initialize(Local<Object> target,
#undef V

env->SetMethod(target, "getHiddenValue", GetHiddenValue);
env->SetMethod(target, "setHiddenValue", SetHiddenValue);
}

} // namespace util
@@ -3,6 +3,8 @@
const common = require('../common');
const assert = require('assert');
const internalUtil = require('internal/util');
const spawnSync = require('child_process').spawnSync;
const path = require('path');

assert.doesNotThrow(function() {
internalUtil.decorateErrorStack();
@@ -17,17 +19,37 @@ internalUtil.decorateErrorStack(obj);
assert.strictEqual(obj.stack, undefined);

// Verify that the stack is decorated when possible
function checkStack(stack) {
const matches = stack.match(/var foo bar;/g);
assert.strictEqual(Array.isArray(matches), true);
assert.strictEqual(matches.length, 1);
}
let err;
const badSyntaxPath =
path.resolve(__dirname, '..', 'fixtures', 'syntax', 'bad_syntax')
.replace(/\\/g, '\\\\');

try {
require('../fixtures/syntax/bad_syntax');
require(badSyntaxPath);
} catch (e) {
err = e;
assert(!/var foo bar;/.test(err.stack));
internalUtil.decorateErrorStack(err);
}

assert(/var foo bar;/.test(err.stack));
assert(typeof err, 'object');
checkStack(err.stack);

// Verify that the stack is only decorated once
internalUtil.decorateErrorStack(err);
internalUtil.decorateErrorStack(err);
checkStack(err.stack);

// Verify that the stack is only decorated once for uncaught exceptions
const args = [
'-e',
`require('${badSyntaxPath}')`
];
const result = spawnSync(process.argv[0], args, { encoding: 'utf8' });
checkStack(result.stderr);

// Verify that the stack is unchanged when there is no arrow message
err = new Error('foo');
@@ -12,6 +12,12 @@ function getHiddenValue(obj, name) {
};
}

function setHiddenValue(obj, name, val) {
return function() {
internalUtil.setHiddenValue(obj, name, val);
};
}

assert.throws(getHiddenValue(), /obj must be an object/);
assert.throws(getHiddenValue(null, 'foo'), /obj must be an object/);
assert.throws(getHiddenValue(undefined, 'foo'), /obj must be an object/);
@@ -22,12 +28,24 @@ assert.throws(getHiddenValue({}, null), /name must be a string/);
assert.throws(getHiddenValue({}, []), /name must be a string/);
assert.deepEqual(internalUtil.getHiddenValue({}, 'foo'), undefined);

assert.throws(setHiddenValue(), /obj must be an object/);
assert.throws(setHiddenValue(null, 'foo'), /obj must be an object/);
assert.throws(setHiddenValue(undefined, 'foo'), /obj must be an object/);
assert.throws(setHiddenValue('bar', 'foo'), /obj must be an object/);
assert.throws(setHiddenValue(85, 'foo'), /obj must be an object/);
assert.throws(setHiddenValue({}), /name must be a string/);
assert.throws(setHiddenValue({}, null), /name must be a string/);
assert.throws(setHiddenValue({}, []), /name must be a string/);
const obj = {};
assert.strictEqual(internalUtil.setHiddenValue(obj, 'foo', 'bar'), true);
assert.strictEqual(internalUtil.getHiddenValue(obj, 'foo'), 'bar');

let arrowMessage;

try {
require('../fixtures/syntax/bad_syntax');
} catch (err) {
arrowMessage = internalUtil.getHiddenValue(err, 'arrowMessage');
arrowMessage = internalUtil.getHiddenValue(err, 'node:arrowMessage');
}

assert(/bad_syntax\.js:1/.test(arrowMessage));

0 comments on commit 18490d3

Please sign in to comment.
You can’t perform that action at this time.