vm: Put back display_errors flag
authorisaacs <i@izs.me>
Thu, 22 Aug 2013 00:19:41 +0000 (17:19 -0700)
committerisaacs <i@izs.me>
Thu, 22 Aug 2013 00:58:12 +0000 (17:58 -0700)
This is an important part of the repl use-case.

TODO: The arg parsing in vm.runIn*Context() is rather wonky.
It would be good to move more of that into the Script class,
and/or an options object.

lib/repl.js
lib/vm.js
src/node_contextify.cc
test/simple/test-repl-syntax-error-handling.js [new file with mode: 0644]

index cf33228..782e79e 100644 (file)
@@ -113,9 +113,9 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
     var err, result;
     try {
       if (self.useGlobal) {
-        result = vm.runInThisContext(code, file);
+        result = vm.runInThisContext(code, file, 0, false);
       } else {
-        result = vm.runInContext(code, context, file);
+        result = vm.runInContext(code, context, file, 0, false);
       }
     } catch (e) {
       err = e;
index 5dd1dde..c71bb81 100644 (file)
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -30,15 +30,15 @@ var util = require('util');
 // - makeContext(sandbox)
 // From this we build the entire documented API.
 
-Script.prototype.runInNewContext = function(initSandbox, timeout) {
+Script.prototype.runInNewContext = function(initSandbox, timeout, disp) {
   var context = exports.createContext(initSandbox);
-  return this.runInContext(context, timeout);
+  return this.runInContext(context, timeout, disp);
 };
 
 exports.Script = Script;
 
-exports.createScript = function(code, filename) {
-  return new Script(code, filename);
+exports.createScript = function(code, filename, disp) {
+  return new Script(code, filename, disp);
 };
 
 exports.createContext = function(initSandbox) {
@@ -51,17 +51,17 @@ exports.createContext = function(initSandbox) {
   return initSandbox;
 };
 
-exports.runInContext = function(code, sandbox, filename, timeout) {
-  var script = exports.createScript(code, filename);
-  return script.runInContext(sandbox, timeout);
+exports.runInContext = function(code, sandbox, filename, timeout, disp) {
+  var script = exports.createScript(code, filename, disp);
+  return script.runInContext(sandbox, timeout, disp);
 };
 
-exports.runInNewContext = function(code, sandbox, filename, timeout) {
-  var script = exports.createScript(code, filename);
-  return script.runInNewContext(sandbox, timeout);
+exports.runInNewContext = function(code, sandbox, filename, timeout, disp) {
+  var script = exports.createScript(code, filename, disp);
+  return script.runInNewContext(sandbox, timeout, disp);
 };
 
-exports.runInThisContext = function(code, filename, timeout) {
-  var script = exports.createScript(code, filename);
-  return script.runInThisContext(timeout);
+exports.runInThisContext = function(code, filename, timeout, disp) {
+  var script = exports.createScript(code, filename, disp);
+  return script.runInThisContext(timeout, disp);
 };
index 002cffe..b2b7324 100644 (file)
@@ -329,6 +329,7 @@ class ContextifyScript : ObjectWrap {
     contextify_script->Wrap(args.Holder());
     Local<String> code = args[0]->ToString();
     Local<String> filename = GetFilenameArg(args, 1);
+    bool display_exception = GetDisplayArg(args, 2);
 
     Local<Context> context = Context::GetCurrent();
     Context::Scope context_scope(context);
@@ -338,7 +339,8 @@ class ContextifyScript : ObjectWrap {
     Local<Script> v8_script = Script::New(code, filename);
 
     if (v8_script.IsEmpty()) {
-      DisplayExceptionLine(try_catch.Message());
+      if (display_exception)
+        DisplayExceptionLine(try_catch.Message());
       try_catch.ReThrow();
       return;
     }
@@ -364,8 +366,10 @@ class ContextifyScript : ObjectWrap {
       return;
     }
 
+    bool display_exception = GetDisplayArg(args, 1);
+
     // Do the eval within this context
-    EvalMachine(timeout, args, try_catch);
+    EvalMachine(timeout, display_exception, args, try_catch);
   }
 
   // args: sandbox, [timeout]
@@ -384,6 +388,8 @@ class ContextifyScript : ObjectWrap {
       return;
     }
 
+    bool display_exception = GetDisplayArg(args, 2);
+
     // Get the context from the sandbox
     Local<Context> context =
         ContextifyContext::ContextFromContextifiedSandbox(sandbox);
@@ -394,7 +400,7 @@ class ContextifyScript : ObjectWrap {
 
     // Do the eval within the context
     Context::Scope context_scope(context);
-    EvalMachine(timeout, args, try_catch);
+    EvalMachine(timeout, display_exception, args, try_catch);
   }
 
   static int64_t GetTimeoutArg(const FunctionCallbackInfo<Value>& args,
@@ -410,6 +416,16 @@ class ContextifyScript : ObjectWrap {
     return timeout;
   }
 
+  static bool GetDisplayArg(const FunctionCallbackInfo<Value>& args,
+                            const int i) {
+    bool display_exception = true;
+
+    if (args[i]->IsBoolean())
+      display_exception = args[i]->BooleanValue();
+
+    return display_exception;
+  }
+
 
   static Local<String> GetFilenameArg(const FunctionCallbackInfo<Value>& args,
                                       const int i) {
@@ -420,6 +436,7 @@ class ContextifyScript : ObjectWrap {
 
 
   static void EvalMachine(const int64_t timeout,
+                          const bool display_exception,
                           const FunctionCallbackInfo<Value>& args,
                           TryCatch& try_catch) {
     if (!ContextifyScript::InstanceOf(args.This())) {
@@ -432,7 +449,8 @@ class ContextifyScript : ObjectWrap {
     Local<Script> script = PersistentToLocal(node_isolate,
                                              wrapped_script->script_);
     if (script.IsEmpty()) {
-      DisplayExceptionLine(try_catch.Message());
+      if (display_exception)
+        DisplayExceptionLine(try_catch.Message());
       try_catch.ReThrow();
       return;
     }
@@ -452,7 +470,8 @@ class ContextifyScript : ObjectWrap {
 
     if (result.IsEmpty()) {
       // Error occurred during execution of the script.
-      DisplayExceptionLine(try_catch.Message());
+      if (display_exception)
+        DisplayExceptionLine(try_catch.Message());
       try_catch.ReThrow();
       return;
     }
diff --git a/test/simple/test-repl-syntax-error-handling.js b/test/simple/test-repl-syntax-error-handling.js
new file mode 100644 (file)
index 0000000..92b074b
--- /dev/null
@@ -0,0 +1,69 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+switch (process.argv[2]) {
+  case 'child':
+    return child();
+  case undefined:
+    return parent();
+  default:
+    throw new Error('wtf');
+}
+
+function parent() {
+  var spawn = require('child_process').spawn;
+  var child = spawn(process.execPath, [__filename, 'child']);
+
+  child.stderr.setEncoding('utf8');
+  child.stderr.on('data', function(c) {
+    console.error('%j', c);
+    throw new Error('should not get stderr data');
+  });
+
+  child.stdout.setEncoding('utf8');
+  var out = '';
+  child.stdout.on('data', function(c) {
+    out += c;
+  });
+  child.stdout.on('end', function() {
+    assert.equal(out, '10\n');
+    console.log('ok - got expected output');
+  });
+
+  child.on('exit', function(c) {
+    assert(!c);
+    console.log('ok - exit success');
+  });
+}
+
+function child() {
+  var vm = require('vm');
+  try {
+    vm.runInThisContext('haf!@##&$!@$*!@', 'blerg', 0, false);
+  } catch (er) {
+    var caught = true;
+  }
+  assert(caught);
+  vm.runInThisContext('console.log(10)', 'blerg', 0, false);
+}