Skip to content

Commit

Permalink
vm: fix displayErrors in runIn.. functions
Browse files Browse the repository at this point in the history
This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
laverdet authored and jasnell committed May 28, 2017
1 parent 4cbdac3 commit 191bb5a
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 13 deletions.
2 changes: 1 addition & 1 deletion lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ function REPLServer(prompt,
try {
try {
const scriptOptions = {
displayErrors: true,
displayErrors: false,
breakOnSigint: self.breakEvalOnSigint
};

Expand Down
18 changes: 7 additions & 11 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -736,8 +736,10 @@ class ContextifyScript : public BaseObject {
return;
}

Local<String> decorated_stack = String::Concat(arrow.As<String>(),
stack.As<String>());
Local<String> decorated_stack = String::Concat(
String::Concat(arrow.As<String>(),
FIXED_ONE_BYTE_STRING(env->isolate(), "\n")),
stack.As<String>());
err_obj->Set(env->stack_string(), decorated_stack);
err_obj->SetPrivate(
env->context(),
Expand Down Expand Up @@ -984,6 +986,9 @@ class ContextifyScript : public BaseObject {
env->ThrowError("Script execution timed out.");
} else if (received_signal) {
env->ThrowError("Script execution interrupted.");
} else if (display_errors) {
// We should decorate non-termination exceptions
DecorateErrorStack(env, *try_catch);
}

// If there was an exception thrown during script execution, re-throw it.
Expand All @@ -996,15 +1001,6 @@ class ContextifyScript : public BaseObject {
return false;
}

if (result.IsEmpty()) {
// Error occurred during execution of the script.
if (display_errors) {
DecorateErrorStack(env, *try_catch);
}
try_catch->ReThrow();
return false;
}

args.GetReturnValue().Set(result);
return true;
}
Expand Down
8 changes: 7 additions & 1 deletion test/message/vm_display_runtime_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ const vm = require('vm');

console.error('beginning');

vm.runInThisContext('throw new Error("boo!")', { filename: 'test.vm' });
try {
vm.runInThisContext('throw new Error("boo!")', { filename: 'test.vm'});
} catch (err) {
console.error(err.stack);
}

vm.runInThisContext('throw new Error("spooky!")', { filename: 'test.vm' });

console.error('end');
15 changes: 15 additions & 0 deletions test/message/vm_display_runtime_error.out
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,18 @@ Error: boo!
at tryModuleLoad (module.js:*:*)
at Function.Module._load (module.js:*)
at Function.Module.runMain (module.js:*)
test.vm:1
throw new Error("spooky!")
^

Error: spooky!
at test.vm:1:7
at ContextifyScript.Script.runInThisContext (vm.js:*)
at Object.runInThisContext (vm.js:*)
at Object.<anonymous> (*test*message*vm_display_runtime_error.js:*)
at Module._compile (module.js:*)
at Object.Module._extensions..js (module.js:*)
at Module.load (module.js:*)
at tryModuleLoad (module.js:*:*)
at Function.Module._load (module.js:*)
at Function.Module.runMain (module.js:*)

0 comments on commit 191bb5a

Please sign in to comment.