Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: attach error to stack on displayErrors #4874

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions doc/api/vm.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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.

Expand Down
18 changes: 6 additions & 12 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand Down
28 changes: 26 additions & 2 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -640,6 +640,30 @@ class ContextifyScript : public BaseObject {
}
}

static void DecorateErrorStack(Environment* env, const TryCatch& try_catch) {
Local<Value> exception = try_catch.Exception();

if (!exception->IsObject())
return;

Local<Object> err_obj = exception.As<Object>();

if (IsExceptionDecorated(env, err_obj))
return;

AppendExceptionLine(env, exception, try_catch.Message());
Local<Value> stack = err_obj->Get(env->stack_string());
Local<Value> arrow = err_obj->GetHiddenValue(env->arrow_message_string());

if (!(stack->IsString() && arrow->IsString()))
return;

Local<String> decorated_stack = String::Concat(arrow.As<String>(),
stack.As<String>());
err_obj->Set(env->stack_string(), decorated_stack);
err_obj->SetHiddenValue(env->decorated_string(), True(env->isolate()));
}

static int64_t GetTimeoutArg(const FunctionCallbackInfo<Value>& args,
const int i) {
if (args[i]->IsUndefined() || args[i]->IsString()) {
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<v8::Value> er);

void AppendExceptionLine(Environment* env,
v8::Local<v8::Value> er,
v8::Local<v8::Message> message);
Expand Down
6 changes: 6 additions & 0 deletions test/message/vm_display_syntax_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
13 changes: 13 additions & 0 deletions test/message/vm_display_syntax_error.out
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
beginning

foo.vm:1
var 4;
^
SyntaxError: Unexpected number
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> (*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;
^
Expand Down