fix: handle electron script errors better (electron#25331) (electron#…

…25454)

Co-authored-by: Samuel Attard <samuel.r.attard@gmail.com>

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>

build/webpack/run-compiler.js

@@ -1,3 +1,4 @@
+const fs = require('fs')
 const path = require('path')
 const webpack = require('webpack')
 
@@ -9,14 +10,26 @@ config.output = {
   filename: path.basename(outPath)
 }
 
-webpack(config, (err, stats) => {
+const { wrapInitWithTryCatch, ...webpackConfig } = config;
+
+webpack(webpackConfig, (err, stats) => {
   if (err) {
     console.error(err)
     process.exit(1)
   } else if (stats.hasErrors()) {
     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);
+}`;
+    }
+    fs.writeFileSync(outPath, contents)
     process.exit(0)
   }
 })

build/webpack/webpack.config.base.js

@@ -24,7 +24,8 @@ module.exports = ({
   alwaysHasNode,
   loadElectronFromAlternateTarget,
   targetDeletesNodeGlobals,
-  target
+  target,
+  wrapInitWithTryCatch
 }) => {
   let entry = path.resolve(electronRoot, 'lib', target, 'init.ts')
   if (!fs.existsSync(entry)) {
@@ -39,6 +40,7 @@ module.exports = ({
     output: {
       filename: `${target}.bundle.js`
     },
+    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

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

build/webpack/webpack.config.sandboxed_renderer.js

@@ -1,4 +1,5 @@
 module.exports = require('./webpack.config.base')({
   target: 'sandboxed_renderer',
-  alwaysHasNode: false
+  alwaysHasNode: false,
+  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

@@ -328,10 +328,25 @@ node::Environment* NodeBindings::CreateEnvironment(
   args.insert(args.begin() + 1, init_script);
 
   std::unique_ptr<const char*[]> c_argv = StringVectorToArgArray(args);
-  node::Environment* env = node::CreateEnvironment(
-      node::CreateIsolateData(context->GetIsolate(), uv_loop_, platform),
-      context, args.size(), c_argv.get(), 0, nullptr, bootstrap_env);
-  DCHECK(env);
+  node::Environment* env;
+  if (browser_env_ != BrowserEnvironment::BROWSER) {
+    v8::TryCatch try_catch(context->GetIsolate());
+    env = node::CreateEnvironment(
+        node::CreateIsolateData(context->GetIsolate(), uv_loop_, platform),
+        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(
+        node::CreateIsolateData(context->GetIsolate(), uv_loop_, platform),
+        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,20 +3,20 @@
 // 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"
 
 namespace electron {
-
 namespace util {
-
 v8::MaybeLocal<v8::Value> CompileAndCall(
     v8::Local<v8::Context> context,
     const char* id,
     std::vector<v8::Local<v8::String>>* parameters,
     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);
@@ -26,8 +26,15 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
   v8::Local<v8::Function> fn = compiled.ToLocalChecked().As<v8::Function>();
   return fn->Call(context, v8::Null(isolate), arguments->size(),
                   arguments->data());
+  v8::MaybeLocal<v8::Value> ret = fn->Call(
+      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
-
 }  // namespace electron