fix: handle electron script errors better (#25328)

build/webpack/run-compiler.js

@@ -10,10 +10,9 @@ config.output = {
   filename: path.basename(outPath)
 };
 
-const { wrapInitWithProfilingTimeout } = config;
+const { wrapInitWithProfilingTimeout, wrapInitWithTryCatch, ...webpackConfig } = config;
-delete config.wrapInitWithProfilingTimeout;
 
-webpack(config, (err, stats) => {
+webpack(webpackConfig, (err, stats) => {
   if (err) {
     console.error(err);
     process.exit(1);
@@ -21,9 +20,17 @@ webpack(config, (err, stats) => {
     console.error(stats.toString('normal'));
     process.exit(1);
   } else {
+    let contents = fs.readFileSync(outPath, 'utf8');
+    if (wrapInitWithTryCatch) {
+      contents = `try {
+${contents}
+} catch (err) {
+  console.error('Electron ${webpackConfig.output.filename} script failed to run');
+  console.error(err);
+}`;
+    }
     if (wrapInitWithProfilingTimeout) {
-      const contents = fs.readFileSync(outPath, 'utf8');
+      contents = `function ___electron_webpack_init__() {
-      const newContents = `function ___electron_webpack_init__() {
 ${contents}
 };
 if ((globalThis.process || binding.process).argv.includes("--profile-electron-init")) {
@@ -31,8 +38,8 @@ if ((globalThis.process || binding.process).argv.includes("--profile-electron-in
 } else {
   ___electron_webpack_init__();
 }`;
-      fs.writeFileSync(outPath, newContents);
     }
+    fs.writeFileSync(outPath, contents);
     process.exit(0);
   }
 });

build/webpack/webpack.config.base.js

@@ -69,7 +69,8 @@ module.exports = ({
   loadElectronFromAlternateTarget,
   targetDeletesNodeGlobals,
   target,
-  wrapInitWithProfilingTimeout
+  wrapInitWithProfilingTimeout,
+  wrapInitWithTryCatch
 }) => {
   let entry = path.resolve(electronRoot, 'lib', target, 'init.ts');
   if (!fs.existsSync(entry)) {
@@ -87,6 +88,7 @@ module.exports = ({
       filename: `${target}.bundle.js`
     },
     wrapInitWithProfilingTimeout,
+    wrapInitWithTryCatch,
     resolve: {
       alias: {
         '@electron/internal': path.resolve(electronRoot, 'lib'),

build/webpack/webpack.config.isolated_renderer.js

@@ -1,4 +1,5 @@
 module.exports = require('./webpack.config.base')({
   target: 'isolated_renderer',
-  alwaysHasNode: false
+  alwaysHasNode: false,
+  wrapInitWithTryCatch: true
 });

build/webpack/webpack.config.renderer.js

@@ -2,5 +2,6 @@ module.exports = require('./webpack.config.base')({
   target: 'renderer',
   alwaysHasNode: true,
   targetDeletesNodeGlobals: true,
-  wrapInitWithProfilingTimeout: true
+  wrapInitWithProfilingTimeout: true,
+  wrapInitWithTryCatch: true
 });

build/webpack/webpack.config.sandboxed_renderer.js

@@ -1,5 +1,6 @@
 module.exports = require('./webpack.config.base')({
   target: 'sandboxed_renderer',
   alwaysHasNode: false,
-  wrapInitWithProfilingTimeout: true
+  wrapInitWithProfilingTimeout: true,
+  wrapInitWithTryCatch: true
 });

build/webpack/webpack.config.worker.js

@@ -2,5 +2,6 @@ module.exports = require('./webpack.config.base')({
   target: 'worker',
   loadElectronFromAlternateTarget: 'renderer',
   alwaysHasNode: true,
-  targetDeletesNodeGlobals: true
+  targetDeletesNodeGlobals: true,
+  wrapInitWithTryCatch: true
 });

shell/common/node_bindings.cc

@@ -400,9 +400,24 @@ node::Environment* NodeBindings::CreateEnvironment(
   std::unique_ptr<const char*[]> c_argv = StringVectorToArgArray(args);
   isolate_data_ =
       node::CreateIsolateData(context->GetIsolate(), uv_loop_, platform);
-  node::Environment* env = node::CreateEnvironment(
+
-      isolate_data_, context, args.size(), c_argv.get(), 0, nullptr);
+  node::Environment* env;
-  DCHECK(env);
+  if (browser_env_ != BrowserEnvironment::BROWSER) {
+    v8::TryCatch try_catch(context->GetIsolate());
+    env = node::CreateEnvironment(isolate_data_, context, args.size(),
+                                  c_argv.get(), 0, nullptr);
+    DCHECK(env);
+    // This will only be caught when something has gone terrible wrong as all
+    // electron scripts are wrapped in a try {} catch {} in run-compiler.js
+    if (try_catch.HasCaught()) {
+      LOG(ERROR) << "Failed to initialize node environment in process: "
+                 << process_type;
+    }
+  } else {
+    env = node::CreateEnvironment(isolate_data_, context, args.size(),
+                                  c_argv.get(), 0, nullptr);
+    DCHECK(env);
+  }
 
   // Clean up the global _noBrowserGlobals that we unironically injected into
   // the global scope

shell/common/node_util.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "shell/common/node_util.h"
+#include "base/logging.h"
 #include "shell/common/node_includes.h"
 #include "third_party/electron_node/src/node_native_module_env.h"
 
@@ -17,6 +18,7 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
     std::vector<v8::Local<v8::Value>>* arguments,
     node::Environment* optional_env) {
   v8::Isolate* isolate = context->GetIsolate();
+  v8::TryCatch try_catch(isolate);
   v8::MaybeLocal<v8::Function> compiled =
       node::native_module::NativeModuleEnv::LookupAndCompile(
           context, id, parameters, optional_env);
@@ -24,8 +26,14 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
     return v8::MaybeLocal<v8::Value>();
   }
   v8::Local<v8::Function> fn = compiled.ToLocalChecked().As<v8::Function>();
-  return fn->Call(context, v8::Null(isolate), arguments->size(),
+  v8::MaybeLocal<v8::Value> ret = fn->Call(
-                  arguments->data());
+      context, v8::Null(isolate), arguments->size(), arguments->data());
+  // This will only be caught when something has gone terrible wrong as all
+  // electron scripts are wrapped in a try {} catch {} in run-compiler.js
+  if (try_catch.HasCaught()) {
+    LOG(ERROR) << "Failed to CompileAndCall electron script: " << id;
+  }
+  return ret;
 }
 
 }  // namespace util