Improve error reporting

- No more single line "node.js:176:9" errors
- No more strange output when error happens on first line due to
  module wrapper function.
- A few tests to check these things

lib/module.js

@@ -376,16 +376,12 @@ Module.prototype._compile = function (content, filename) {
                 + content
                 + "\n});";
 
-    try {
       var compiledWrapper = process.compile(wrapper, filename);
       var dirName = path.dirname(filename);
       if (filename === process.argv[1]) {
         process.checkBreak();
       }
       compiledWrapper.apply(self.exports, [self.exports, require, self, filename, dirName]);
-    } catch (e) {
-      return e;
-    }
   } else {
     self.exports = content;
   }
@@ -460,7 +456,5 @@ exports.runMain = function () {
 
   // Load the main module--the command line argument.
   process.mainModule = new Module(".");
-  process.mainModule.load(process.argv[1], function (err) {
-    if (err) throw err;
-  });
+  process.mainModule.loadSync(process.argv[1]);
 }

src/node.cc

@@ -954,7 +954,7 @@ const char* ToCString(const v8::String::Utf8Value& value) {
   return *value ? *value : "<str conversion failed>";
 }
 
-static void ReportException(TryCatch &try_catch, bool show_line = false) {
+static void ReportException(TryCatch &try_catch, bool show_line) {
   Handle<Message> message = try_catch.Message();
 
   Handle<Value> error = try_catch.Exception();
@@ -975,10 +975,31 @@ static void ReportException(TryCatch &try_catch, bool show_line = false) {
     // Print line of source code.
     String::Utf8Value sourceline(message->GetSourceLine());
     const char* sourceline_string = ToCString(sourceline);
-    fprintf(stderr, "%s\n", sourceline_string);
+
+    // HACK HACK HACK
+    //
+    // FIXME
+    //
+    // Because of how CommonJS modules work, all scripts are wrapped with a
+    // "function (function (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 lib/module.js
+    //
+    // 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.
+    //
+    // Even better would be to get support into V8 for wrappers that
+    // shouldn't be reported to users.
+    int offset = linenum == 1 ? 62 : 0;
+
+    fprintf(stderr, "%s\n", sourceline_string + offset);
     // Print wavy underline (GetUnderline is deprecated).
     int start = message->GetStartColumn();
-    for (int i = 0; i < start; i++) {
+    for (int i = offset; i < start; i++) {
       fprintf(stderr, " ");
     }
     int end = message->GetEndColumn();
@@ -1004,13 +1025,13 @@ Local<Value> ExecuteString(Local<String> source, Local<Value> filename) {
 
   Local<v8::Script> script = v8::Script::Compile(source, filename);
   if (script.IsEmpty()) {
-    ReportException(try_catch);
+    ReportException(try_catch, true);
     exit(1);
   }
 
   Local<Value> result = script->Run();
   if (result.IsEmpty()) {
-    ReportException(try_catch);
+    ReportException(try_catch, true);
     exit(1);
   }
 
@@ -1523,7 +1544,10 @@ Handle<Value> Compile(const Arguments& args) {
   }
 
   Local<Value> result = script->Run();
-  if (try_catch.HasCaught()) return try_catch.ReThrow();
+  if (try_catch.HasCaught()) {
+    ReportException(try_catch, false);
+    exit(1);
+  }
 
   return scope.Close(result);
 }
@@ -1544,7 +1568,7 @@ void FatalException(TryCatch &try_catch) {
 
   // Check if uncaught_exception_counter indicates a recursion
   if (uncaught_exception_counter > 0) {
-    ReportException(try_catch);
+    ReportException(try_catch, true);
     exit(1);
   }
 
@@ -1570,7 +1594,7 @@ void FatalException(TryCatch &try_catch) {
   uint32_t length = listener_array->Length();
   // Report and exit if process has no "uncaughtException" listener
   if (length == 0) {
-    ReportException(try_catch);
+    ReportException(try_catch, true);
     exit(1);
   }
 
@@ -1899,18 +1923,14 @@ static void Load(int argc, char *argv[]) {
 
   // The node.js file returns a function 'f'
 
-#ifndef NDEBUG
   TryCatch try_catch;
-#endif
 
   Local<Value> f_value = ExecuteString(String::New(native_node),
                                        String::New("node.js"));
-#ifndef NDEBUG
   if (try_catch.HasCaught())  {
-    ReportException(try_catch);
+    ReportException(try_catch, true);
     exit(10);
   }
-#endif
   assert(f_value->IsFunction());
   Local<Function> f = Local<Function>::Cast(f_value);
 
@@ -1926,12 +1946,10 @@ static void Load(int argc, char *argv[]) {
 
   f->Call(global, 1, args);
 
-#ifndef NDEBUG
   if (try_catch.HasCaught())  {
-    ReportException(try_catch);
+    ReportException(try_catch, true);
     exit(11);
   }
-#endif
 }
 
 static void PrintHelp();

test/disabled/test-http-stress.js

@@ -1,40 +1,40 @@
 require("../common");
 
+http = require("http");
+
 var request_count = 1000;
-var response_body = '{"ok": true}';
+var body = '{"ok": true}';
 
-var server = process.http.createServer(function(req, res) {
- res.sendHeader(200, {'Content-Type': 'text/javascript'});
- res.sendBody(response_body);
- res.finish();
+var server = http.createServer(function(req, res) {
+ res.writeHead(200, {'Content-Type': 'text/javascript'});
+ res.write(body);
+ res.end();
 });
-server.listen(PORT, 4024);
+server.listen(PORT);
 
 var requests_ok = 0;
 var requests_complete = 0;
 
-function onLoad () {
+server.addListener('listening', function () {
  for (var i = 0; i < request_count; i++) {
-   process.http.cat('http://localhost:'+PORT+'/', 'utf8')
-     .addCallback(function (content) {
-       assert.equal(response_body, content)
+   http.cat('http://localhost:'+PORT+'/', 'utf8', function (err, content) {
+     requests_complete++;
+     if (err) {
+       print("-");
+     } else {
+       assert.equal(body, content)
        print(".");
        requests_ok++;
-       requests_complete++;
-       if (requests_ok == request_count) {
-         puts("\nrequests ok: " + requests_ok);
-         server.close();
-       }
-     })
-     .addErrback(function() {
-       print("-");
-       requests_complete++;
-       //process.debug("error " + i);
-     });
+     }
+     if (requests_complete == request_count) {
+       puts("\nrequests ok: " + requests_ok);
+       server.close();
+     }
+   });
  }
-}
+});
 
 process.addListener("exit", function () {
-  assert.equal(request_count, requests_complete); 
-  assert.equal(request_count, requests_ok); 
+  assert.equal(request_count, requests_complete);
+  assert.equal(request_count, requests_ok);
 });

test/fixtures/throws_error2.js

@@ -0,0 +1,3 @@
+
+JSON.parse(undefined);
+

test/fixtures/throws_error3.js

@@ -0,0 +1,3 @@
+process.nextTick(function () {
+  JSON.parse(undefined);
+});

test/simple/test-error-reporting.js

@@ -0,0 +1,50 @@
+require("../common");
+exec = require('child_process').exec,
+path = require('path');
+
+exits = 0;
+
+function errExec (script, callback) {
+  var cmd = process.argv[0] + ' ' + path.join(fixturesDir, script);
+  return exec(cmd, function (err, stdout, stderr) {
+    // There was some error
+    assert.ok(err);
+
+    // More than one line of error output.
+    assert.ok(stderr.split('\n').length > 2);
+
+    // Assert the script is mentioned in error output.
+    assert.ok(stderr.indexOf(script) >= 0);
+
+    // Proxy the args for more tests.
+    callback(err, stdout, stderr);
+
+    // Count the tests
+    exits++;
+
+    puts('.');
+  });
+}
+
+
+// Simple throw error
+errExec('throws_error.js', function (err, stdout, stderr) {
+  assert.ok(/blah/.test(stderr));
+});
+
+
+// Trying to JSON.parse(undefined)
+errExec('throws_error2.js', function (err, stdout, stderr) {
+  assert.ok(/JSON/.test(stderr));
+});
+
+
+// Trying to JSON.parse(undefined) in nextTick
+errExec('throws_error3.js', function (err, stdout, stderr) {
+  assert.ok(/JSON/.test(stderr));
+});
+
+
+process.addListener('exit', function () {
+  assert.equal(3, exits);
+});