From bff7be8210ca445e53d7b7852726f711c264c386 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 6 Feb 2023 23:37:33 +0100 Subject: [PATCH] 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 ``` PR-URL: https://github.com/nodejs/node/pull/46533 Reviewed-By: Chengzhong Wu Reviewed-By: Anna Henningsen --- src/api/embed_helpers.cc | 12 +++++- src/node_errors.cc | 79 ++++++++++++++++++++++++++++------------ src/node_internals.h | 3 ++ 3 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc index 3f8463f6ae518d..5c8b733737a2e6 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()) { + errors->push_back(FormatCaughtException( + 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) { diff --git a/src/node_errors.cc b/src/node_errors.cc index 7f2128dacb5e37..ec9ae0f13eac0a 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,82 @@ 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); + result += SPrintF(" at [eval]:%i:%i\n", line_number, column); } 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); 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, @@ -1089,7 +1119,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__