From fd3657610e49005dfc778c3f060dbba0a34f286a Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Sat, 24 Aug 2013 18:53:24 -0400 Subject: [PATCH] vm: update API to use options argument Passing a filename is still supported in place of certain options arguments, for backward-compatibility, but timeout and display-errors are not translated since those were undocumented. Also managed to eliminate an extra stack trace line by not calling through the `createScript` export. Added a few message tests to show how `displayErrors` works. --- lib/module.js | 6 +- lib/repl.js | 10 +- lib/vm.js | 50 +++++---- src/node.js | 10 +- src/node_contextify.cc | 105 ++++++++++++------ test/message/eval_messages.out | 1 - test/message/stdin_messages.out | 1 - test/message/vm_display_runtime_error.js | 30 +++++ test/message/vm_display_runtime_error.out | 16 +++ test/message/vm_display_syntax_error.js | 30 +++++ test/message/vm_display_syntax_error.out | 15 +++ test/message/vm_dont_display_runtime_error.js | 42 +++++++ .../message/vm_dont_display_runtime_error.out | 17 +++ test/message/vm_dont_display_syntax_error.js | 42 +++++++ test/message/vm_dont_display_syntax_error.out | 16 +++ .../simple/test-repl-syntax-error-handling.js | 4 +- test/simple/test-vm-timeout.js | 24 ++-- 17 files changed, 331 insertions(+), 88 deletions(-) create mode 100644 test/message/vm_display_runtime_error.js create mode 100644 test/message/vm_display_runtime_error.out create mode 100644 test/message/vm_display_syntax_error.js create mode 100644 test/message/vm_display_syntax_error.out create mode 100644 test/message/vm_dont_display_runtime_error.js create mode 100644 test/message/vm_dont_display_runtime_error.out create mode 100644 test/message/vm_dont_display_syntax_error.js create mode 100644 test/message/vm_dont_display_syntax_error.out diff --git a/lib/module.js b/lib/module.js index d9e9744c0a..c0ac340793 100644 --- a/lib/module.js +++ b/lib/module.js @@ -412,7 +412,7 @@ Module.prototype._compile = function(content, filename) { sandbox.global = sandbox; sandbox.root = root; - return runInNewContext(content, sandbox, filename); + return runInNewContext(content, sandbox, { filename: filename }); } debug('load root module'); @@ -423,13 +423,13 @@ Module.prototype._compile = function(content, filename) { global.__dirname = dirname; global.module = self; - return runInThisContext(content, filename); + return runInThisContext(content, { filename: filename }); } // create wrapper function var wrapper = Module.wrap(content); - var compiledWrapper = runInThisContext(wrapper, filename); + var compiledWrapper = runInThisContext(wrapper, { filename: filename }); if (global.v8debug) { if (!resolvedArgv) { // we enter the repl if we're not given a filename argument. diff --git a/lib/repl.js b/lib/repl.js index 782e79e153..82a3c70069 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -113,9 +113,15 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) { var err, result; try { if (self.useGlobal) { - result = vm.runInThisContext(code, file, 0, false); + result = vm.runInThisContext(code, { + filename: file, + displayErrors: false + }); } else { - result = vm.runInContext(code, context, file, 0, false); + result = vm.runInContext(code, context, { + filename: file, + displayErrors: false + }); } } catch (e) { err = e; diff --git a/lib/vm.js b/lib/vm.js index 8fee40a8f2..83b78b874b 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -24,48 +24,50 @@ var Script = binding.ContextifyScript; var util = require('util'); // The binding provides a few useful primitives: -// - ContextifyScript(code, [filename]), with methods: -// - runInThisContext() -// - runInContext(sandbox, [timeout]) +// - ContextifyScript(code, { filename = "evalmachine.anonymous", +// displayErrors = true } = {}) +// with methods: +// - runInThisContext({ displayErrors = true } = {}) +// - runInContext(sandbox, { displayErrors = true, timeout = undefined } = {}) // - makeContext(sandbox) // - isContext(sandbox) // From this we build the entire documented API. -Script.prototype.runInNewContext = function(initSandbox, timeout, disp) { - var context = exports.createContext(initSandbox); - return this.runInContext(context, timeout, disp); +Script.prototype.runInNewContext = function(sandbox, options) { + var context = exports.createContext(sandbox); + return this.runInContext(context, options); }; exports.Script = Script; -exports.createScript = function(code, filename, disp) { - return new Script(code, filename, disp); +exports.createScript = function(code, options) { + return new Script(code, options); }; -exports.createContext = function(initSandbox) { - if (util.isUndefined(initSandbox)) { - initSandbox = {}; - } else if (binding.isContext(initSandbox)) { - return initSandbox; +exports.createContext = function(sandbox) { + if (util.isUndefined(sandbox)) { + sandbox = {}; + } else if (binding.isContext(sandbox)) { + return sandbox; } - binding.makeContext(initSandbox); - return initSandbox; + binding.makeContext(sandbox); + return sandbox; }; -exports.runInContext = function(code, sandbox, filename, timeout, disp) { - var script = exports.createScript(code, filename, disp); - return script.runInContext(sandbox, timeout, disp); +exports.runInContext = function(code, contextifiedSandbox, options) { + var script = new Script(code, options); + return script.runInContext(contextifiedSandbox, options); }; -exports.runInNewContext = function(code, sandbox, filename, timeout, disp) { - var script = exports.createScript(code, filename, disp); - return script.runInNewContext(sandbox, timeout, disp); +exports.runInNewContext = function(code, sandbox, options) { + var script = new Script(code, options); + return script.runInNewContext(sandbox, options); }; -exports.runInThisContext = function(code, filename, timeout, disp) { - var script = exports.createScript(code, filename, disp); - return script.runInThisContext(timeout, disp); +exports.runInThisContext = function(code, options) { + var script = new Script(code, options); + return script.runInThisContext(options); }; exports.isContext = binding.isContext; diff --git a/src/node.js b/src/node.js index 131745f9c7..50e654402a 100644 --- a/src/node.js +++ b/src/node.js @@ -382,8 +382,8 @@ 'global.__dirname = __dirname;\n' + 'global.require = require;\n' + 'return require("vm").runInThisContext(' + - JSON.stringify(body) + ', ' + - JSON.stringify(name) + ');\n'; + JSON.stringify(body) + ', { filename: ' + + JSON.stringify(name) + ' });\n'; } var result = module._compile(script, name + '-wrapper'); if (process._print_eval) console.log(result); @@ -675,8 +675,8 @@ // node binary, so they can be loaded faster. var ContextifyScript = process.binding('contextify').ContextifyScript; - function runInThisContext(code, filename) { - var script = new ContextifyScript(code, filename); + function runInThisContext(code, options) { + var script = new ContextifyScript(code, options); return script.runInThisContext(); } @@ -739,7 +739,7 @@ var source = NativeModule.getSource(this.id); source = NativeModule.wrap(source); - var fn = runInThisContext(source, this.filename); + var fn = runInThisContext(source, { filename: this.filename }); fn(this.exports, NativeModule.require, this, this.filename); this.loaded = true; diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 8d3b531a38..331dbdb6e9 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -321,7 +321,7 @@ class ContextifyScript : ObjectWrap { } - // args: code, [filename] + // args: code, [options] static void New(const FunctionCallbackInfo& args) { HandleScope scope(node_isolate); @@ -331,20 +331,25 @@ class ContextifyScript : ObjectWrap { ContextifyScript *contextify_script = new ContextifyScript(); contextify_script->Wrap(args.Holder()); + + TryCatch try_catch; Local code = args[0]->ToString(); Local filename = GetFilenameArg(args, 1); - bool display_exception = GetDisplayArg(args, 2); + bool display_errors = GetDisplayErrorsArg(args, 1); + if (try_catch.HasCaught()) { + try_catch.ReThrow(); + return; + } Local context = Context::GetCurrent(); Context::Scope context_scope(context); - TryCatch try_catch; - Local