Browse Source

chore: better logging if Node initialization fails (#45286)

feat: better logging if Node initialization fails
Charles Kerr 2 months ago
parent
commit
db7ef90159
4 changed files with 74 additions and 21 deletions
  1. 2 2
      shell/app/node_main.cc
  2. 4 19
      shell/common/node_bindings.cc
  3. 52 0
      shell/common/node_util.cc
  4. 16 0
      shell/common/node_util.h

+ 2 - 2
shell/app/node_main.cc

@@ -250,8 +250,8 @@ int NodeMain() {
 
       uint64_t env_flags = node::EnvironmentFlags::kDefaultFlags |
                            node::EnvironmentFlags::kHideConsoleWindows;
-      env = node::CreateEnvironment(
-          isolate_data, isolate->GetCurrentContext(), result->args(),
+      env = electron::util::CreateEnvironment(
+          isolate, isolate_data, isolate->GetCurrentContext(), result->args(),
           result->exec_args(),
           static_cast<node::EnvironmentFlags::Flags>(env_flags));
       CHECK_NE(nullptr, env);

+ 4 - 19
shell/common/node_bindings.cc

@@ -649,7 +649,6 @@ std::shared_ptr<node::Environment> NodeBindings::CreateEnvironment(
   context->SetAlignedPointerInEmbedderData(kElectronContextEmbedderDataIndex,
                                            static_cast<void*>(isolate_data));
 
-  node::Environment* env;
   uint64_t env_flags = node::EnvironmentFlags::kDefaultFlags |
                        node::EnvironmentFlags::kHideConsoleWindows |
                        node::EnvironmentFlags::kNoGlobalSearchPaths |
@@ -675,24 +674,10 @@ std::shared_ptr<node::Environment> NodeBindings::CreateEnvironment(
     env_flags |= node::EnvironmentFlags::kNoStartDebugSignalHandler;
   }
 
-  {
-    v8::TryCatch try_catch(isolate);
-    env = node::CreateEnvironment(
-        static_cast<node::IsolateData*>(isolate_data), context, args, exec_args,
-        static_cast<node::EnvironmentFlags::Flags>(env_flags));
-
-    if (try_catch.HasCaught()) {
-      std::string err_msg =
-          "Failed to initialize node environment in process: " + process_type;
-      v8::Local<v8::Message> message = try_catch.Message();
-      std::string msg;
-      if (!message.IsEmpty() &&
-          gin::ConvertFromV8(isolate, message->Get(), &msg))
-        err_msg += " , with error: " + msg;
-      LOG(ERROR) << err_msg;
-    }
-  }
-
+  node::Environment* env = electron::util::CreateEnvironment(
+      isolate, static_cast<node::IsolateData*>(isolate_data), context, args,
+      exec_args, static_cast<node::EnvironmentFlags::Flags>(env_flags),
+      process_type);
   DCHECK(env);
 
   node::IsolateSettings is;

+ 52 - 0
shell/common/node_util.cc

@@ -5,7 +5,12 @@
 #include "shell/common/node_util.h"
 
 #include "base/compiler_specific.h"
+#include "base/containers/to_value_list.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
+#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/values.h"
 #include "gin/converter.h"
 #include "gin/dictionary.h"
 #include "shell/browser/javascript_environment.h"
@@ -76,4 +81,51 @@ base::span<uint8_t> as_byte_span(v8::Local<v8::Value> node_buffer) {
   return UNSAFE_BUFFERS(base::span{data, size});
 }
 
+node::Environment* CreateEnvironment(v8::Isolate* isolate,
+                                     node::IsolateData* isolate_data,
+                                     v8::Local<v8::Context> context,
+                                     const std::vector<std::string>& args,
+                                     const std::vector<std::string>& exec_args,
+                                     node::EnvironmentFlags::Flags env_flags,
+                                     std::string_view process_type) {
+  v8::TryCatch try_catch{isolate};
+  node::Environment* env = node::CreateEnvironment(isolate_data, context, args,
+                                                   exec_args, env_flags);
+  if (auto message = try_catch.Message(); !message.IsEmpty()) {
+    base::Value::Dict dict;
+
+    if (std::string str; gin::ConvertFromV8(isolate, message->Get(), &str))
+      dict.Set("message", std::move(str));
+
+    if (std::string str; gin::ConvertFromV8(
+            isolate, message->GetScriptOrigin().ResourceName(), &str)) {
+      const auto line_num = message->GetLineNumber(context).FromJust();
+      const auto line_str = base::NumberToString(line_num);
+      dict.Set("location", base::StrCat({", at ", str, ":", line_str}));
+    }
+
+    if (std::string str; gin::ConvertFromV8(
+            isolate, message->GetSourceLine(context).ToLocalChecked(), &str))
+      dict.Set("source_line", std::move(str));
+
+    if (!std::empty(process_type))
+      dict.Set("process_type", process_type);
+
+    if (auto list = base::ToValueList(args); !std::empty(list))
+      dict.Set("args", std::move(list));
+
+    if (auto list = base::ToValueList(exec_args); !std::empty(list))
+      dict.Set("exec_args", std::move(list));
+
+    std::string errstr = "Failed to initialize Node.js.";
+    if (std::optional<std::string> jsonstr = base::WriteJsonWithOptions(
+            dict, base::JsonOptions::OPTIONS_PRETTY_PRINT))
+      errstr += base::StrCat({" ", *jsonstr});
+
+    LOG(ERROR) << errstr;
+  }
+
+  return env;
+}
+
 }  // namespace electron::util

+ 16 - 0
shell/common/node_util.h

@@ -5,6 +5,7 @@
 #ifndef ELECTRON_SHELL_COMMON_NODE_UTIL_H_
 #define ELECTRON_SHELL_COMMON_NODE_UTIL_H_
 
+#include <string>
 #include <string_view>
 #include <vector>
 
@@ -13,7 +14,13 @@
 
 namespace node {
 class Environment;
+class IsolateData;
+struct ThreadId;
+
+namespace EnvironmentFlags {
+enum Flags : uint64_t;
 }
+}  // namespace node
 
 namespace electron::util {
 
@@ -37,6 +44,15 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
     std::vector<v8::Local<v8::String>>* parameters,
     std::vector<v8::Local<v8::Value>>* arguments);
 
+// Wrapper for node::CreateEnvironment that logs failure
+node::Environment* CreateEnvironment(v8::Isolate* isolate,
+                                     node::IsolateData* isolate_data,
+                                     v8::Local<v8::Context> context,
+                                     const std::vector<std::string>& args,
+                                     const std::vector<std::string>& exec_args,
+                                     node::EnvironmentFlags::Flags env_flags,
+                                     std::string_view process_type = "");
+
 // Convenience function to view a Node buffer's data as a base::span().
 // Analogous to base::as_byte_span()
 [[nodiscard]] base::span<uint8_t> as_byte_span(