Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
Report errors properly from --eval and stdin
  • Loading branch information
isaacs committed Jul 30, 2012
1 parent 72bc4dc commit b3cf3f3
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 27 deletions.
49 changes: 30 additions & 19 deletions src/node.cc
Expand Up @@ -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();
Expand All @@ -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);
}
Expand Down
16 changes: 14 additions & 2 deletions src/node.js
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions src/node_script.cc
Expand Up @@ -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 {
Expand Down
53 changes: 53 additions & 0 deletions test/message/eval_messages.js
@@ -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();
58 changes: 58 additions & 0 deletions test/message/eval_messages.out
@@ -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
13 changes: 7 additions & 6 deletions test/simple/test-cli-eval.js
Expand Up @@ -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) {
Expand Down

0 comments on commit b3cf3f3

Please sign in to comment.