diff --git a/lib/internal/util.js b/lib/internal/util.js index 78254ceb82e887..21aafff21824b3 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.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) { diff --git a/lib/module.js b/lib/module.js index 8feba15b0fe7c0..82b1971e8bf009 100644 --- a/lib/module.js +++ b/lib/module.js @@ -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. diff --git a/src/env.h b/src/env.h index 6151c57069ad7b..743bf057e8584d 100644 --- a/src/env.h +++ b/src/env.h @@ -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") \ diff --git a/src/node.cc b/src/node.cc index f9797e664ef937..894c8f76416275 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1399,6 +1399,15 @@ ssize_t DecodeWrite(Isolate* isolate, return StringBytes::Write(isolate, buf, buflen, val, encoding, nullptr); } +bool IsExceptionDecorated(Environment* env, Local er) { + if (!er.IsEmpty() && er->IsObject()) { + Local err_obj = er.As(); + Local decorated = err_obj->GetHiddenValue(env->decorated_string()); + return !decorated.IsEmpty() && decorated->IsTrue(); + } + return false; +} + void AppendExceptionLine(Environment* env, Local er, Local message) { @@ -1508,6 +1517,7 @@ static void ReportException(Environment* env, Local trace_value; Local 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); diff --git a/src/node_util.cc b/src/node_util.cc index 1e0f214ae4470a..8475468c1f4afe 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -52,6 +52,21 @@ static void GetHiddenValue(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(obj->GetHiddenValue(name)); } +static void SetHiddenValue(const FunctionCallbackInfo& 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 obj = args[0].As(); + Local name = args[1].As(); + + args.GetReturnValue().Set(obj->SetHiddenValue(name, args[2])); +} + void Initialize(Local target, Local unused, @@ -63,6 +78,7 @@ void Initialize(Local target, #undef V env->SetMethod(target, "getHiddenValue", GetHiddenValue); + env->SetMethod(target, "setHiddenValue", SetHiddenValue); } } // namespace util diff --git a/test/parallel/test-util-decorate-error-stack.js b/test/parallel/test-util-decorate-error-stack.js index 24fee56df7b655..f289d52be404d3 100644 --- a/test/parallel/test-util-decorate-error-stack.js +++ b/test/parallel/test-util-decorate-error-stack.js @@ -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'); diff --git a/test/parallel/test-util-internal.js b/test/parallel/test-util-internal.js index ac7ff3da629f9c..06fddb7e942c85 100644 --- a/test/parallel/test-util-internal.js +++ b/test/parallel/test-util-internal.js @@ -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));