From 6c3e96994dea11ecc859c04ffddf2f0c77771b4c Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 25 Jan 2016 22:44:17 -0500 Subject: [PATCH] src: attach error to stack on displayErrors The vm module's displayErrors option attaches error arrow messages as a hidden property. Later, core JavaScript code can optionally decorate the error stack with the arrow message. However, when user code catches an error, it has no way to access the arrow message. This commit changes the behavior of displayErrors to mean "decorate the error stack if an error occurs." Fixes: https://github.com/nodejs/node/issues/4835 --- doc/api/vm.markdown | 24 ++++++++++---------- lib/module.js | 18 +++++---------- src/node.js | 5 +++-- src/node_contextify.cc | 28 ++++++++++++++++++++++-- src/node_internals.h | 2 ++ test/message/vm_display_syntax_error.js | 6 +++++ test/message/vm_display_syntax_error.out | 13 +++++++++++ 7 files changed, 68 insertions(+), 28 deletions(-) diff --git a/doc/api/vm.markdown b/doc/api/vm.markdown index 612d69945ff778..1b494a468c947d 100644 --- a/doc/api/vm.markdown +++ b/doc/api/vm.markdown @@ -32,10 +32,10 @@ The options when creating a script are: displayed in stack traces - `columnOffset`: allows you to add an offset to the column number that is displayed in stack traces -- `displayErrors`: whether or not to print any errors to stderr, with the - line of code that caused them highlighted, before throwing an exception. - Applies only to syntax errors compiling the code; errors while running the - code are controlled by the options to the script's methods. +- `displayErrors`: if `true`, on error, attach the line of code that caused + the error to the stack trace. Applies only to syntax errors compiling the + code; errors while running the code are controlled by the options to the + script's methods. - `timeout`: a number of milliseconds to execute `code` before terminating execution. If execution is terminated, an [`Error`][] will be thrown. - `cachedData`: an optional `Buffer` with V8's code cache data for the supplied @@ -150,10 +150,10 @@ The options for running a script are: displayed in stack traces - `columnOffset`: allows you to add an offset to the column number that is displayed in stack traces -- `displayErrors`: whether or not to print any errors to stderr, with the - line of code that caused them highlighted, before throwing an exception. - Applies only to runtime errors executing the code; it is impossible to create - a `Script` instance with syntax errors, as the constructor will throw. +- `displayErrors`: if `true`, on error, attach the line of code that caused + the error to the stack trace. Applies only to runtime errors executing the + code; it is impossible to create a `Script` instance with syntax errors, as + the constructor will throw. - `timeout`: a number of milliseconds to execute the script before terminating execution. If execution is terminated, an [`Error`][] will be thrown. @@ -290,10 +290,10 @@ e.g. `(0,eval)('code')`. However, it also has the following additional options: displayed in stack traces - `columnOffset`: allows you to add an offset to the column number that is displayed in stack traces -- `displayErrors`: whether or not to print any errors to stderr, with the - line of code that caused them highlighted, before throwing an exception. - Will capture both syntax errors from compiling `code` and runtime errors - thrown by executing the compiled code. Defaults to `true`. +- `displayErrors`: if `true`, on error, attach the line of code that caused + the error to the stack trace. Will capture both syntax errors from compiling + `code` and runtime errors thrown by executing the compiled code. Defaults to + `true`. - `timeout`: a number of milliseconds to execute `code` before terminating execution. If execution is terminated, an [`Error`][] will be thrown. diff --git a/lib/module.js b/lib/module.js index a068bffa7d9056..eabc27a0f8715e 100644 --- a/lib/module.js +++ b/lib/module.js @@ -23,16 +23,6 @@ function hasOwnProperty(obj, prop) { } -function tryWrapper(wrapper, opts) { - try { - return runInThisContext(wrapper, opts); - } catch (e) { - internalUtil.decorateErrorStack(e); - throw e; - } -} - - function stat(filename) { filename = path._makeLong(filename); const cache = stat.cache; @@ -394,8 +384,12 @@ Module.prototype._compile = function(content, filename) { // create wrapper function var wrapper = Module.wrap(content); - var compiledWrapper = tryWrapper(wrapper, - { filename: filename, lineOffset: 0 }); + var compiledWrapper = runInThisContext(wrapper, { + filename: filename, + lineOffset: 0, + displayErrors: true + }); + if (global.v8debug) { if (!resolvedArgv) { // we enter the repl if we're not given a filename argument. diff --git a/src/node.js b/src/node.js index 60bffc454065c7..1f29386a87714f 100644 --- a/src/node.js +++ b/src/node.js @@ -605,7 +605,7 @@ 'global.require = require;\n' + 'return require("vm").runInThisContext(' + JSON.stringify(body) + ', { filename: ' + - JSON.stringify(name) + ' });\n'; + JSON.stringify(name) + ', displayErrors: true });\n'; // Defer evaluation for a tick. This is a workaround for deferred // events not firing when evaluating scripts from the command line, // see https://github.com/nodejs/node/issues/1600. @@ -988,7 +988,8 @@ var fn = runInThisContext(source, { filename: this.filename, - lineOffset: 0 + lineOffset: 0, + displayErrors: true }); fn(this.exports, NativeModule.require, this, this.filename); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 31d98853d3617d..7ca622de43425b 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -541,7 +541,7 @@ class ContextifyScript : public BaseObject { if (v8_script.IsEmpty()) { if (display_errors) { - AppendExceptionLine(env, try_catch.Exception(), try_catch.Message()); + DecorateErrorStack(env, try_catch); } try_catch.ReThrow(); return; @@ -640,6 +640,30 @@ class ContextifyScript : public BaseObject { } } + static void DecorateErrorStack(Environment* env, const TryCatch& try_catch) { + Local exception = try_catch.Exception(); + + if (!exception->IsObject()) + return; + + Local err_obj = exception.As(); + + if (IsExceptionDecorated(env, err_obj)) + return; + + AppendExceptionLine(env, exception, try_catch.Message()); + Local stack = err_obj->Get(env->stack_string()); + Local arrow = err_obj->GetHiddenValue(env->arrow_message_string()); + + if (!(stack->IsString() && arrow->IsString())) + return; + + Local decorated_stack = String::Concat(arrow.As(), + stack.As()); + err_obj->Set(env->stack_string(), decorated_stack); + err_obj->SetHiddenValue(env->decorated_string(), True(env->isolate())); + } + static int64_t GetTimeoutArg(const FunctionCallbackInfo& args, const int i) { if (args[i]->IsUndefined() || args[i]->IsString()) { @@ -816,7 +840,7 @@ class ContextifyScript : public BaseObject { if (result.IsEmpty()) { // Error occurred during execution of the script. if (display_errors) { - AppendExceptionLine(env, try_catch.Exception(), try_catch.Message()); + DecorateErrorStack(env, try_catch); } try_catch.ReThrow(); return false; diff --git a/src/node_internals.h b/src/node_internals.h index c720f70eda0d62..ea83d4d9fc1238 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -125,6 +125,8 @@ inline static int snprintf(char *buffer, size_t n, const char *format, ...) { # define NO_RETURN #endif +bool IsExceptionDecorated(Environment* env, v8::Local er); + void AppendExceptionLine(Environment* env, v8::Local er, v8::Local message); diff --git a/test/message/vm_display_syntax_error.js b/test/message/vm_display_syntax_error.js index 23525c14d822fb..62bbd432a2f98a 100644 --- a/test/message/vm_display_syntax_error.js +++ b/test/message/vm_display_syntax_error.js @@ -4,6 +4,12 @@ var vm = require('vm'); console.error('beginning'); +try { + vm.runInThisContext('var 4;', { filename: 'foo.vm', displayErrors: true }); +} catch (err) { + console.error(err.stack); +} + vm.runInThisContext('var 5;', { filename: 'test.vm' }); console.error('end'); diff --git a/test/message/vm_display_syntax_error.out b/test/message/vm_display_syntax_error.out index 086944870263f6..14df3a8284a64f 100644 --- a/test/message/vm_display_syntax_error.out +++ b/test/message/vm_display_syntax_error.out @@ -1,5 +1,18 @@ beginning +foo.vm:1 +var 4; + ^ +SyntaxError: Unexpected number + at Object.exports.runInThisContext (vm.js:*) + at Object. (*test*message*vm_display_syntax_error.js:*) + at Module._compile (module.js:*) + at Object.Module._extensions..js (module.js:*) + at Module.load (module.js:*) + at Function.Module._load (module.js:*) + at Function.Module.runMain (module.js:*) + at startup (node.js:*) + at node.js:* test.vm:1 var 5; ^