Permalink
Browse files

Report errors properly from --eval and stdin

  • Loading branch information...
1 parent 72bc4dc commit b3cf3f35fcf192c717e7a393fb984bf6f879d0cc @isaacs isaacs committed Jul 28, 2012
Showing with 163 additions and 27 deletions.
  1. +30 −19 src/node.cc
  2. +14 −2 src/node.js
  3. +1 −0 src/node_script.cc
  4. +53 −0 test/message/eval_messages.js
  5. +58 −0 test/message/eval_messages.out
  6. +7 −6 test/simple/test-cli-eval.js
View
@@ -1214,8 +1214,15 @@ ssize_t DecodeWrite(char *buf,
return buflen;
}
-
void DisplayExceptionLine (TryCatch &try_catch) {
+ // Prevent re-entry into this function. For example, if there is
+ // a throw from a program in vm.runInThisContext(code, filename, true),
+ // then we want to show the original failure, not the secondary one.
+ static bool displayed_error = false;
+
+ if (displayed_error) return;
+ displayed_error = true;
+
HandleScope scope;
Handle<Message> message = try_catch.Message();
@@ -1234,33 +1241,37 @@ void DisplayExceptionLine (TryCatch &try_catch) {
String::Utf8Value sourceline(message->GetSourceLine());
const char* sourceline_string = *sourceline;
- // HACK HACK HACK
- //
- // FIXME
- //
- // Because of how CommonJS modules work, all scripts are wrapped with a
- // "function (function (exports, __filename, ...) {"
+ // Because of how node modules work, all scripts are wrapped with a
+ // "function (module, exports, __filename, ...) {"
// to provide script local variables.
//
// When reporting errors on the first line of a script, this wrapper
- // function is leaked to the user. This HACK is to remove it. The length
- // of the wrapper is 62. That wrapper is defined in src/node.js
+ // function is leaked to the user. There used to be a hack here to
+ // truncate off the first 62 characters, but it caused numerous other
+ // problems when vm.runIn*Context() methods were used for non-module
+ // code.
+ //
+ // If we ever decide to re-instate such a hack, the following steps
+ // must be taken:
//
- // If that wrapper is ever changed, then this number also has to be
- // updated. Or - someone could clean this up so that the two peices
- // don't need to be changed.
+ // 1. Pass a flag around to say "this code was wrapped"
+ // 2. Update the stack frame output so that it is also correct.
//
- // Even better would be to get support into V8 for wrappers that
- // shouldn't be reported to users.
- int offset = linenum == 1 ? 62 : 0;
+ // It would probably be simpler to add a line rather than add some
+ // number of characters to the first line, since V8 truncates the
+ // sourceline to 78 characters, and we end up not providing very much
+ // useful debugging info to the user if we remove 62 characters.
- fprintf(stderr, "%s\n", sourceline_string + offset);
- // Print wavy underline (GetUnderline is deprecated).
int start = message->GetStartColumn();
- for (int i = offset; i < start; i++) {
+ int end = message->GetEndColumn();
+
+ // fprintf(stderr, "---\nsourceline:%s\noffset:%d\nstart:%d\nend:%d\n---\n", sourceline_string, start, end);
+
+ fprintf(stderr, "%s\n", sourceline_string);
+ // Print wavy underline (GetUnderline is deprecated).
+ for (int i = 0; i < start; i++) {
fputc((sourceline_string[i] == '\t') ? '\t' : ' ', stderr);
}
- int end = message->GetEndColumn();
for (int i = start; i < end; i++) {
fputc('^', stderr);
}
View
@@ -73,7 +73,7 @@
} else if (process._eval != null) {
// User passed '-e' or '--eval' arguments to Node.
- evalScript('eval');
+ evalScript('[eval]');
} else if (process.argv[1]) {
// make process.argv[1] into a full path
var path = NativeModule.require('path');
@@ -267,7 +267,19 @@
var module = new Module(name);
module.filename = path.join(cwd, name);
module.paths = Module._nodeModulePaths(cwd);
- var result = module._compile('return eval(process._eval)', name);
+ var script = process._eval;
+ if (!Module._contextLoad) {
+ var body = script;
+ script = 'global.__filename = ' + JSON.stringify(name) + ';\n' +
+ 'global.exports = exports;\n' +
+ 'global.module = module;\n' +
+ 'global.__dirname = __dirname;\n' +
+ 'global.require = require;\n' +
+ 'return require("vm").runInThisContext(' +
+ JSON.stringify(body) + ', ' +
+ JSON.stringify(name) + ', true);\n';
+ }
+ var result = module._compile(script, name + '-wrapper');
if (process._print_eval) console.log(result);
}
View
@@ -417,6 +417,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
if (output_flag == returnResult) {
result = script->Run();
if (result.IsEmpty()) {
+ if (display_error) DisplayExceptionLine(try_catch);
return try_catch.ReThrow();
}
} else {
@@ -0,0 +1,53 @@
+
+// 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');
+
+var spawn = require('child_process').spawn;
+
+function run(cmd, strict, cb) {
+ var args = [];
+ if (strict) args.push('--use_strict');
+ args.push('-pe', cmd);
+ var child = spawn(process.execPath, args);
+ child.stdout.pipe(process.stdout);
+ child.stderr.pipe(process.stdout);
+ child.on('close', cb);
+}
+
+var queue =
+ [ 'with(this){__filename}',
+ '42',
+ 'throw new Error("hello")',
+ 'var x = 100; y = x;',
+ 'var ______________________________________________; throw 10' ];
+
+function go() {
+ var c = queue.shift();
+ if (!c) return console.log('done');
+ run(c, false, function() {
+ run(c, true, go);
+ });
+}
+
+go();
@@ -0,0 +1,58 @@
+[eval]
+
+[eval]:1
+with(this){__filename}
+^^^^
+SyntaxError: Strict mode code may not include a with statement
+ at Object.<anonymous> ([eval]-wrapper:6:22)
+ at Module._compile (module.js:449:26)
+ at evalScript (node.js:282:25)
+ at startup (node.js:76:7)
+ at node.js:623:3
+42
+42
+
+[eval]:1
+throw new Error("hello")
+ ^
+Error: hello
+ at [eval]:1:7
+ at Object.<anonymous> ([eval]-wrapper:6:22)
+ at Module._compile (module.js:449:26)
+ at evalScript (node.js:282:25)
+ at startup (node.js:76:7)
+ at node.js:623:3
+
+[eval]:1
+throw new Error("hello")
+ ^
+Error: hello
+ at [eval]:1:7
+ at Object.<anonymous> ([eval]-wrapper:6:22)
+ at Module._compile (module.js:449:26)
+ at evalScript (node.js:282:25)
+ at startup (node.js:76:7)
+ at node.js:623:3
+100
+
+[eval]:1
+var x = 100; y = x;
+ ^
+ReferenceError: y is not defined
+ at [eval]:1:16
+ at Object.<anonymous> ([eval]-wrapper:6:22)
+ at Module._compile (module.js:449:26)
+ at evalScript (node.js:282:25)
+ at startup (node.js:76:7)
+ at node.js:623:3
+
+[eval]:1
+var ______________________________________________; throw 10
+ ^
+10
+
+[eval]:1
+var ______________________________________________; throw 10
+ ^
+10
+done
@@ -19,21 +19,22 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.
+if (module.parent) {
+ // signal we've been loaded as a module
+ console.log('Loaded as a module, exiting with status code 42.');
+ process.exit(42);
+}
+
var common = require('../common.js'),
assert = require('assert'),
child = require('child_process'),
nodejs = '"' + process.execPath + '"';
+
// replace \ by / because windows uses backslashes in paths, but they're still
// interpreted as the escape character when put between quotes.
var filename = __filename.replace(/\\/g, '/');
-if (module.parent) {
- // signal we've been loaded as a module
- console.log('Loaded as a module, exiting with status code 42.');
- process.exit(42);
-}
-
// assert that nothing is written to stdout
child.exec(nodejs + ' --eval 42',
function(err, stdout, stderr) {

0 comments on commit b3cf3f3

Please sign in to comment.