diff --git a/src/env.h b/src/env.h index 68528d024ea..532501ed2e7 100644 --- a/src/env.h +++ b/src/env.h @@ -88,6 +88,7 @@ namespace node { V(ipv4_string, "IPv4") \ V(ipv6_string, "IPv6") \ V(issuer_string, "issuer") \ + V(message_string, "message") \ V(method_string, "method") \ V(mode_string, "mode") \ V(modulus_string, "modulus") \ @@ -111,6 +112,7 @@ namespace node { V(onstop_string, "onstop") \ V(path_string, "path") \ V(port_string, "port") \ + V(processed_string, "processed") \ V(rdev_string, "rdev") \ V(rename_string, "rename") \ V(rss_string, "rss") \ @@ -122,6 +124,7 @@ namespace node { V(smalloc_p_string, "_smalloc_p") \ V(sni_context_err_string, "Invalid SNI context") \ V(sni_context_string, "sni_context") \ + V(stack_string, "stack") \ V(status_code_string, "statusCode") \ V(status_message_string, "statusMessage") \ V(subject_string, "subject") \ diff --git a/src/node.cc b/src/node.cc index c1ecdcd30a5..642bc450e02 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1268,80 +1268,121 @@ ssize_t DecodeWrite(char *buf, return StringBytes::Write(buf, buflen, val, encoding, NULL); } -void DisplayExceptionLine(Handle message) { - // 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) +void AppendExceptionLine(Environment* env, + Handle er, + Handle message) { + static bool printed_error = false; + if (message.IsEmpty()) return; - displayed_error = true; - uv_tty_reset_mode(); + HandleScope scope(env->isolate()); + Local err_obj; + if (!er.IsEmpty() && er->IsObject()) { + err_obj = er.As(); - fprintf(stderr, "\n"); - - if (!message.IsEmpty()) { - // Print (filename):(line number): (message). - String::Utf8Value filename(message->GetScriptResourceName()); - const char* filename_string = *filename; - int linenum = message->GetLineNumber(); - fprintf(stderr, "%s:%i\n", filename_string, linenum); - // Print line of source code. - String::Utf8Value sourceline(message->GetSourceLine()); - const char* sourceline_string = *sourceline; - - // 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. 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: - // - // 1. Pass a flag around to say "this code was wrapped" - // 2. Update the stack frame output so that it is also correct. - // - // 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. - - int start = message->GetStartColumn(); - int end = message->GetEndColumn(); - - 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); - } - for (int i = start; i < end; i++) { - fputc('^', stderr); - } - fputc('\n', stderr); + // Do it only once per message + if (!err_obj->GetHiddenValue(env->processed_string()).IsEmpty()) + return; + err_obj->SetHiddenValue(env->processed_string(), True(env->isolate())); } + + static char arrow[1024]; + + // Print (filename):(line number): (message). + String::Utf8Value filename(message->GetScriptResourceName()); + const char* filename_string = *filename; + int linenum = message->GetLineNumber(); + // Print line of source code. + String::Utf8Value sourceline(message->GetSourceLine()); + const char* sourceline_string = *sourceline; + + // 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. 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: + // + // 1. Pass a flag around to say "this code was wrapped" + // 2. Update the stack frame output so that it is also correct. + // + // 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. + + int start = message->GetStartColumn(); + int end = message->GetEndColumn(); + + int off = snprintf(arrow, + sizeof(arrow), + "%s:%i\n%s\n", + filename_string, + linenum, + sourceline_string); + assert(off >= 0); + + // Print wavy underline (GetUnderline is deprecated). + for (int i = 0; i < start; i++) { + assert(static_cast(off) < sizeof(arrow)); + arrow[off++] = (sourceline_string[i] == '\t') ? '\t' : ' '; + } + for (int i = start; i < end; i++) { + assert(static_cast(off) < sizeof(arrow)); + arrow[off++] = '^'; + } + assert(static_cast(off) < sizeof(arrow) - 1); + arrow[off++] = '\n'; + arrow[off] = '\0'; + + Local arrow_str = String::NewFromUtf8(env->isolate(), arrow); + Local msg; + Local stack; + + // Allocation failed, just print it out + if (arrow_str.IsEmpty() || err_obj.IsEmpty() || !err_obj->IsNativeError()) + goto print; + + msg = err_obj->Get(env->message_string()); + stack = err_obj->Get(env->stack_string()); + + if (msg.IsEmpty() || stack.IsEmpty()) + goto print; + + err_obj->Set(env->message_string(), + String::Concat(arrow_str, msg->ToString())); + err_obj->Set(env->stack_string(), + String::Concat(arrow_str, stack->ToString())); + return; + + print: + if (printed_error) + return; + printed_error = true; + uv_tty_reset_mode(); + fprintf(stderr, "\n%s", arrow); } -static void ReportException(Handle er, Handle message) { - HandleScope scope(node_isolate); +static void ReportException(Environment* env, + Handle er, + Handle message) { + HandleScope scope(env->isolate()); - DisplayExceptionLine(message); + AppendExceptionLine(env, er, message); Local trace_value; - if (er->IsUndefined() || er->IsNull()) { - trace_value = Undefined(node_isolate); - } else { - trace_value = - er->ToObject()->Get(FIXED_ONE_BYTE_STRING(node_isolate, "stack")); - } + if (er->IsUndefined() || er->IsNull()) + trace_value = Undefined(env->isolate()); + else + trace_value = er->ToObject()->Get(env->stack_string()); String::Utf8Value trace(trace_value); @@ -1357,8 +1398,8 @@ static void ReportException(Handle er, Handle message) { if (er->IsObject()) { Local err_obj = er.As(); - message = err_obj->Get(FIXED_ONE_BYTE_STRING(node_isolate, "message")); - name = err_obj->Get(FIXED_ONE_BYTE_STRING(node_isolate, "name")); + message = err_obj->Get(env->message_string()); + name = err_obj->Get(FIXED_ONE_BYTE_STRING(env->isolate(), "name")); } if (message.IsEmpty() || @@ -1379,14 +1420,16 @@ static void ReportException(Handle er, Handle message) { } -static void ReportException(const TryCatch& try_catch) { - ReportException(try_catch.Exception(), try_catch.Message()); +static void ReportException(Environment* env, const TryCatch& try_catch) { + ReportException(env, try_catch.Exception(), try_catch.Message()); } // Executes a str within the current v8 context. -Local ExecuteString(Handle source, Handle filename) { - HandleScope scope(node_isolate); +static Local ExecuteString(Environment* env, + Handle source, + Handle filename) { + HandleScope scope(env->isolate()); TryCatch try_catch; // try_catch must be nonverbose to disable FatalException() handler, @@ -1395,13 +1438,13 @@ Local ExecuteString(Handle source, Handle filename) { Local script = v8::Script::Compile(source, filename); if (script.IsEmpty()) { - ReportException(try_catch); + ReportException(env, try_catch); exit(3); } Local result = script->Run(); if (result.IsEmpty()) { - ReportException(try_catch); + ReportException(env, try_catch); exit(4); } @@ -2018,7 +2061,7 @@ void FatalException(Handle error, Handle message) { if (!fatal_exception_function->IsFunction()) { // failed before the process._fatalException function was added! // this is probably pretty bad. Nothing to do but report and exit. - ReportException(error, message); + ReportException(env, error, message); exit(6); } @@ -2033,12 +2076,12 @@ void FatalException(Handle error, Handle message) { if (fatal_try_catch.HasCaught()) { // the fatal exception function threw, so we must exit - ReportException(fatal_try_catch); + ReportException(env, fatal_try_catch); exit(7); } if (false == caught->BooleanValue()) { - ReportException(error, message); + ReportException(env, error, message); exit(1); } } @@ -2698,10 +2741,10 @@ void Load(Environment* env) { // are not safe to ignore. try_catch.SetVerbose(false); - Local script_name = FIXED_ONE_BYTE_STRING(node_isolate, "node.js"); - Local f_value = ExecuteString(MainSource(), script_name); + Local script_name = FIXED_ONE_BYTE_STRING(env->isolate(), "node.js"); + Local f_value = ExecuteString(env, MainSource(), script_name); if (try_catch.HasCaught()) { - ReportException(try_catch); + ReportException(env, try_catch); exit(10); } assert(f_value->IsFunction()); diff --git a/src/node.h b/src/node.h index 5dbf28f1f6c..1f023a28cf1 100644 --- a/src/node.h +++ b/src/node.h @@ -190,7 +190,6 @@ enum encoding {ASCII, UTF8, BASE64, UCS2, BINARY, HEX, BUFFER}; enum encoding ParseEncoding(v8::Handle encoding_v, enum encoding _default = BINARY); NODE_EXTERN void FatalException(const v8::TryCatch& try_catch); -void DisplayExceptionLine(v8::Handle message); NODE_EXTERN v8::Local Encode(const void *buf, size_t len, enum encoding encoding = BINARY); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 40f2c17f32f..9814920b080 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -453,7 +453,7 @@ class ContextifyScript : public BaseObject { if (v8_script.IsEmpty()) { if (display_errors) { - DisplayExceptionLine(try_catch.Message()); + AppendExceptionLine(env, try_catch.Exception(), try_catch.Message()); } try_catch.ReThrow(); return; @@ -622,7 +622,7 @@ class ContextifyScript : public BaseObject { if (result.IsEmpty()) { // Error occurred during execution of the script. if (display_errors) { - DisplayExceptionLine(try_catch.Message()); + AppendExceptionLine(env, try_catch.Exception(), try_catch.Message()); } try_catch.ReThrow(); return false; diff --git a/src/node_internals.h b/src/node_internals.h index e92aa2ef4f4..8005a09ac49 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -155,6 +155,10 @@ inline static void ThrowUVException(int errorno, v8::ThrowException(UVException(errorno, syscall, message, path)); } +void AppendExceptionLine(Environment* env, + v8::Handle er, + v8::Handle message); + NO_RETURN void FatalError(const char* location, const char* message); v8::Local BuildStatsObject(Environment* env, const uv_stat_t* s); diff --git a/test/message/eval_messages.out b/test/message/eval_messages.out index 1aa0c35a9a9..809e788f7a4 100644 --- a/test/message/eval_messages.out +++ b/test/message/eval_messages.out @@ -1,5 +1,4 @@ [eval] - [eval]:1 with(this){__filename} ^^^^ @@ -12,7 +11,6 @@ SyntaxError: Strict mode code may not include a with statement at node.js:*:* 42 42 - [eval]:1 throw new Error("hello") ^ @@ -24,7 +22,6 @@ Error: hello at evalScript (node.js:*:*) at startup (node.js:*:*) at node.js:*:* - [eval]:1 throw new Error("hello") ^ @@ -37,7 +34,6 @@ Error: hello at startup (node.js:*:*) at node.js:*:* 100 - [eval]:1 var x = 100; y = x; ^ @@ -49,12 +45,10 @@ ReferenceError: y is not defined at evalScript (node.js:*:*) at startup (node.js:*:*) at node.js:*:* - [eval]:1 var ______________________________________________; throw 10 ^ 10 - [eval]:1 var ______________________________________________; throw 10 ^ diff --git a/test/message/throw_custom_error.out b/test/message/throw_custom_error.out index e8c9903a3ab..87a99c1ae3e 100644 --- a/test/message/throw_custom_error.out +++ b/test/message/throw_custom_error.out @@ -1,5 +1,4 @@ before - *test*message*throw_custom_error.js:31 throw ({ name: 'MyCustomError', message: 'This is a custom message' }); ^ diff --git a/test/message/throw_in_line_with_tabs.out b/test/message/throw_in_line_with_tabs.out index d91f8c08c33..f4e3affea67 100644 --- a/test/message/throw_in_line_with_tabs.out +++ b/test/message/throw_in_line_with_tabs.out @@ -1,5 +1,4 @@ before - *test*message*throw_in_line_with_tabs.js:32 throw ({ foo: 'bar' }); ^ diff --git a/test/message/throw_non_error.out b/test/message/throw_non_error.out index 378874f9907..5f8213eb4ff 100644 --- a/test/message/throw_non_error.out +++ b/test/message/throw_non_error.out @@ -1,5 +1,4 @@ before - *test*message*throw_non_error.js:31 throw ({ foo: 'bar' }); ^ diff --git a/test/simple/test-vm-syntax-error-stderr.js b/test/simple/test-vm-syntax-error-stderr.js new file mode 100644 index 00000000000..569d42d8010 --- /dev/null +++ b/test/simple/test-vm-syntax-error-stderr.js @@ -0,0 +1,49 @@ +// 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 path = require('path'); +var child_process = require('child_process'); + +var wrong_script = path.join(common.fixturesDir, 'cert.pem'); + +var p = child_process.spawn(process.execPath, [ + '-e', + 'try { require(' + JSON.stringify(wrong_script) + '); } ' + + 'catch (e) { console.log(e.stack); }' +]); + +p.stderr.on('data', function(data) { + assert(false, 'Unexpected stderr data: ' + data); +}); + +var output = ''; + +p.stdout.on('data', function(data) { + output += data; +}); + +process.on('exit', function() { + assert(/BEGIN CERT/.test(output)); + assert(/^\s+\^/m.test(output)); + assert(/Unexpected identifier/.test(output)); +});