From 5910278119acbe82d545aa8a8b20cb85c4ae314c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 6 Feb 2023 23:37:33 +0100 Subject: [PATCH 1/3] bootstrap: print stack trace during environment creation failure https://github.com/nodejs/node/pull/45888 took the environment creation code out of the scope covered by the v8::TryCatch that we use to print early failures during environment creation. So e.g. when adding something that would fail in node.js, we get ``` node:internal/options:554: Uncaught Error: Should not query options before bootstrapping is done ``` This patch restores that by adding another v8::TryCatch for it: ``` node:internal/options:20 ({ options: optionsMap } = getCLIOptions()); ^ Error: Should not query options before bootstrapping is done at getCLIOptionsFromBinding (node:internal/options:20:32) at getOptionValue (node:internal/options:45:19) at node:internal/bootstrap/node:433:29 ``` --- src/api/embed_helpers.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc index 3f8463f6ae518d..602deff835ed49 100644 --- a/src/api/embed_helpers.cc +++ b/src/api/embed_helpers.cc @@ -15,6 +15,7 @@ using v8::Maybe; using v8::Nothing; using v8::SealHandleScope; using v8::SnapshotCreator; +using v8::TryCatch; namespace node { @@ -129,12 +130,21 @@ CommonEnvironmentSetup::CommonEnvironmentSetup( { Locker locker(isolate); Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + + TryCatch bootstrapCatch(isolate); + auto print_Exception = OnScopeLeave([&]() { + if (bootstrapCatch.HasCaught()) { + PrintCaughtException( + isolate, isolate->GetCurrentContext(), bootstrapCatch); + } + }); + impl_->isolate_data.reset(CreateIsolateData( isolate, loop, platform, impl_->allocator.get(), snapshot_data)); impl_->isolate_data->options()->build_snapshot = impl_->snapshot_creator.has_value(); - HandleScope handle_scope(isolate); if (snapshot_data) { impl_->env.reset(make_env(this)); if (impl_->env) { From 5547a2e25b8ff0d57a257a9c2002286307999ccc Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 7 Feb 2023 23:39:13 +0100 Subject: [PATCH 2/3] fixup! bootstrap: print stack trace during environment creation failure --- src/api/embed_helpers.cc | 4 +- src/node_errors.cc | 82 ++++++++++++++++++++++++++++------------ src/node_internals.h | 3 ++ 3 files changed, 63 insertions(+), 26 deletions(-) diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc index 602deff835ed49..5c8b733737a2e6 100644 --- a/src/api/embed_helpers.cc +++ b/src/api/embed_helpers.cc @@ -135,8 +135,8 @@ CommonEnvironmentSetup::CommonEnvironmentSetup( TryCatch bootstrapCatch(isolate); auto print_Exception = OnScopeLeave([&]() { if (bootstrapCatch.HasCaught()) { - PrintCaughtException( - isolate, isolate->GetCurrentContext(), bootstrapCatch); + errors->push_back(FormatCaughtException( + isolate, isolate->GetCurrentContext(), bootstrapCatch)); } }); diff --git a/src/node_errors.cc b/src/node_errors.cc index 05f2a848491d7c..7d9af14c21001f 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -185,7 +185,8 @@ static std::string GetErrorSource(Isolate* isolate, return buf + std::string(underline_buf, off); } -void PrintStackTrace(Isolate* isolate, Local stack) { +static std::string FormatStackTrace(Isolate* isolate, Local stack) { + std::string result; for (int i = 0; i < stack->GetFrameCount(); i++) { Local stack_frame = stack->GetFrame(isolate, i); node::Utf8Value fn_name_s(isolate, stack_frame->GetFunctionName()); @@ -195,53 +196,85 @@ void PrintStackTrace(Isolate* isolate, Local stack) { if (stack_frame->IsEval()) { if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) { - FPrintF(stderr, " at [eval]:%i:%i\n", line_number, column); + char buf[64]; + snprintf( + buf, sizeof(buf), " at [eval]:%i:%i\n", line_number, column); + result += std::string(buf); } else { - FPrintF(stderr, - " at [eval] (%s:%i:%i)\n", - *script_name, - line_number, - column); + std::vector buf(script_name.length() + 64); + snprintf(buf.data(), + buf.size(), + " at [eval] (%s:%i:%i)\n", + *script_name, + line_number, + column); + result += std::string(buf.data()); } break; } if (fn_name_s.length() == 0) { - FPrintF(stderr, " at %s:%i:%i\n", script_name, line_number, column); + std::vector buf(script_name.length() + 64); + snprintf(buf.data(), + buf.size(), + " at %s:%i:%i\n", + *script_name, + line_number, + column); + result += std::string(buf.data()); } else { - FPrintF(stderr, - " at %s (%s:%i:%i)\n", - fn_name_s, - script_name, - line_number, - column); + std::vector buf(fn_name_s.length() + script_name.length() + 64); + snprintf(buf.data(), + buf.size(), + " at %s (%s:%i:%i)\n", + *fn_name_s, + *script_name, + line_number, + column); + result += std::string(buf.data()); } } + return result; +} + +static void PrintToStderrAndFlush(const std::string& str) { + FPrintF(stderr, "%s\n", str.c_str()); fflush(stderr); } -void PrintException(Isolate* isolate, - Local context, - Local err, - Local message) { +void PrintStackTrace(Isolate* isolate, Local stack) { + PrintToStderrAndFlush(FormatStackTrace(isolate, stack)); +} + +std::string FormatCaughtException(Isolate* isolate, + Local context, + Local err, + Local message) { node::Utf8Value reason(isolate, err->ToDetailString(context) .FromMaybe(Local())); bool added_exception_line = false; std::string source = GetErrorSource(isolate, context, message, &added_exception_line); - FPrintF(stderr, "%s\n", source); - FPrintF(stderr, "%s\n", reason); + std::string result = source + '\n' + reason.ToString() + '\n'; Local stack = message->GetStackTrace(); - if (!stack.IsEmpty()) PrintStackTrace(isolate, stack); + if (!stack.IsEmpty()) result += FormatStackTrace(isolate, stack); + return result; +} + +std::string FormatCaughtException(Isolate* isolate, + Local context, + const v8::TryCatch& try_catch) { + CHECK(try_catch.HasCaught()); + return FormatCaughtException( + isolate, context, try_catch.Exception(), try_catch.Message()); } void PrintCaughtException(Isolate* isolate, Local context, const v8::TryCatch& try_catch) { - CHECK(try_catch.HasCaught()); - PrintException(isolate, context, try_catch.Exception(), try_catch.Message()); + PrintToStderrAndFlush(FormatCaughtException(isolate, context, try_catch)); } void AppendExceptionLine(Environment* env, @@ -1087,7 +1120,8 @@ void TriggerUncaughtException(Isolate* isolate, // error is supposed to be thrown at this point. // Since we don't have access to Environment here, there is not // much we can do, so we just print whatever is useful and crash. - PrintException(isolate, context, error, message); + PrintToStderrAndFlush( + FormatCaughtException(isolate, context, error, message)); Abort(); } diff --git a/src/node_internals.h b/src/node_internals.h index df90781f58e9a2..9243344eb788b5 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -83,6 +83,9 @@ void PrintStackTrace(v8::Isolate* isolate, v8::Local stack); void PrintCaughtException(v8::Isolate* isolate, v8::Local context, const v8::TryCatch& try_catch); +std::string FormatCaughtException(v8::Isolate* isolate, + v8::Local context, + const v8::TryCatch& try_catch); void ResetStdio(); // Safe to call more than once and from signal handlers. #ifdef __POSIX__ From 544b10341bb039fe942e08327c75c3eab8b9a31c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 28 Feb 2023 00:00:56 +0100 Subject: [PATCH 3/3] fixup! bootstrap: print stack trace during environment creation failure Co-authored-by: Anna Henningsen --- src/node_errors.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/node_errors.cc b/src/node_errors.cc index 7d9af14c21001f..a535bd37194f0f 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -196,10 +196,7 @@ static std::string FormatStackTrace(Isolate* isolate, Local stack) { if (stack_frame->IsEval()) { if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) { - char buf[64]; - snprintf( - buf, sizeof(buf), " at [eval]:%i:%i\n", line_number, column); - result += std::string(buf); + result += SPrintF(" at [eval]:%i:%i\n", line_number, column); } else { std::vector buf(script_name.length() + 64); snprintf(buf.data(), @@ -238,7 +235,7 @@ static std::string FormatStackTrace(Isolate* isolate, Local stack) { } static void PrintToStderrAndFlush(const std::string& str) { - FPrintF(stderr, "%s\n", str.c_str()); + FPrintF(stderr, "%s\n", str); fflush(stderr); }