X Tutup
Skip to content

Commit 4ad9bcb

Browse files
MarshallOfSoundMarshallOfSound
authored
fix: handle electron script errors better (electron#25328)
1 parent 29c1248 commit 4ad9bcb

File tree

8 files changed

+52
-16
lines changed

8 files changed

+52
-16
lines changed

‎build/webpack/run-compiler.js‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,36 @@ config.output = {
1010
filename: path.basename(outPath)
1111
};
1212

13-
const { wrapInitWithProfilingTimeout } = config;
14-
delete config.wrapInitWithProfilingTimeout;
13+
const { wrapInitWithProfilingTimeout, wrapInitWithTryCatch, ...webpackConfig } = config;
1514

16-
webpack(config, (err, stats) => {
15+
webpack(webpackConfig, (err, stats) => {
1716
if (err) {
1817
console.error(err);
1918
process.exit(1);
2019
} else if (stats.hasErrors()) {
2120
console.error(stats.toString('normal'));
2221
process.exit(1);
2322
} else {
23+
let contents = fs.readFileSync(outPath, 'utf8');
24+
if (wrapInitWithTryCatch) {
25+
contents = `try {
26+
${contents}
27+
} catch (err) {
28+
console.error('Electron ${webpackConfig.output.filename} script failed to run');
29+
console.error(err);
30+
}`;
31+
}
2432
if (wrapInitWithProfilingTimeout) {
25-
const contents = fs.readFileSync(outPath, 'utf8');
26-
const newContents = `function ___electron_webpack_init__() {
33+
contents = `function ___electron_webpack_init__() {
2734
${contents}
2835
};
2936
if ((globalThis.process || binding.process).argv.includes("--profile-electron-init")) {
3037
setTimeout(___electron_webpack_init__, 0);
3138
} else {
3239
___electron_webpack_init__();
3340
}`;
34-
fs.writeFileSync(outPath, newContents);
3541
}
42+
fs.writeFileSync(outPath, contents);
3643
process.exit(0);
3744
}
3845
});

‎build/webpack/webpack.config.base.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ module.exports = ({
6969
loadElectronFromAlternateTarget,
7070
targetDeletesNodeGlobals,
7171
target,
72-
wrapInitWithProfilingTimeout
72+
wrapInitWithProfilingTimeout,
73+
wrapInitWithTryCatch
7374
}) => {
7475
let entry = path.resolve(electronRoot, 'lib', target, 'init.ts');
7576
if (!fs.existsSync(entry)) {
@@ -87,6 +88,7 @@ module.exports = ({
8788
filename: `${target}.bundle.js`
8889
},
8990
wrapInitWithProfilingTimeout,
91+
wrapInitWithTryCatch,
9092
resolve: {
9193
alias: {
9294
'@electron/internal': path.resolve(electronRoot, 'lib'),

‎build/webpack/webpack.config.isolated_renderer.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
module.exports = require('./webpack.config.base')({
22
target: 'isolated_renderer',
3-
alwaysHasNode: false
3+
alwaysHasNode: false,
4+
wrapInitWithTryCatch: true
45
});

‎build/webpack/webpack.config.renderer.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@ module.exports = require('./webpack.config.base')({
22
target: 'renderer',
33
alwaysHasNode: true,
44
targetDeletesNodeGlobals: true,
5-
wrapInitWithProfilingTimeout: true
5+
wrapInitWithProfilingTimeout: true,
6+
wrapInitWithTryCatch: true
67
});

‎build/webpack/webpack.config.sandboxed_renderer.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
module.exports = require('./webpack.config.base')({
22
target: 'sandboxed_renderer',
33
alwaysHasNode: false,
4-
wrapInitWithProfilingTimeout: true
4+
wrapInitWithProfilingTimeout: true,
5+
wrapInitWithTryCatch: true
56
});

‎build/webpack/webpack.config.worker.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@ module.exports = require('./webpack.config.base')({
22
target: 'worker',
33
loadElectronFromAlternateTarget: 'renderer',
44
alwaysHasNode: true,
5-
targetDeletesNodeGlobals: true
5+
targetDeletesNodeGlobals: true,
6+
wrapInitWithTryCatch: true
67
});

‎shell/common/node_bindings.cc‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,9 +400,24 @@ node::Environment* NodeBindings::CreateEnvironment(
400400
std::unique_ptr<const char*[]> c_argv = StringVectorToArgArray(args);
401401
isolate_data_ =
402402
node::CreateIsolateData(context->GetIsolate(), uv_loop_, platform);
403-
node::Environment* env = node::CreateEnvironment(
404-
isolate_data_, context, args.size(), c_argv.get(), 0, nullptr);
405-
DCHECK(env);
403+
404+
node::Environment* env;
405+
if (browser_env_ != BrowserEnvironment::BROWSER) {
406+
v8::TryCatch try_catch(context->GetIsolate());
407+
env = node::CreateEnvironment(isolate_data_, context, args.size(),
408+
c_argv.get(), 0, nullptr);
409+
DCHECK(env);
410+
// This will only be caught when something has gone terrible wrong as all
411+
// electron scripts are wrapped in a try {} catch {} in run-compiler.js
412+
if (try_catch.HasCaught()) {
413+
LOG(ERROR) << "Failed to initialize node environment in process: "
414+
<< process_type;
415+
}
416+
} else {
417+
env = node::CreateEnvironment(isolate_data_, context, args.size(),
418+
c_argv.get(), 0, nullptr);
419+
DCHECK(env);
420+
}
406421

407422
// Clean up the global _noBrowserGlobals that we unironically injected into
408423
// the global scope

‎shell/common/node_util.cc‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
#include "shell/common/node_util.h"
6+
#include "base/logging.h"
67
#include "shell/common/node_includes.h"
78
#include "third_party/electron_node/src/node_native_module_env.h"
89

@@ -17,15 +18,22 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
1718
std::vector<v8::Local<v8::Value>>* arguments,
1819
node::Environment* optional_env) {
1920
v8::Isolate* isolate = context->GetIsolate();
21+
v8::TryCatch try_catch(isolate);
2022
v8::MaybeLocal<v8::Function> compiled =
2123
node::native_module::NativeModuleEnv::LookupAndCompile(
2224
context, id, parameters, optional_env);
2325
if (compiled.IsEmpty()) {
2426
return v8::MaybeLocal<v8::Value>();
2527
}
2628
v8::Local<v8::Function> fn = compiled.ToLocalChecked().As<v8::Function>();
27-
return fn->Call(context, v8::Null(isolate), arguments->size(),
28-
arguments->data());
29+
v8::MaybeLocal<v8::Value> ret = fn->Call(
30+
context, v8::Null(isolate), arguments->size(), arguments->data());
31+
// This will only be caught when something has gone terrible wrong as all
32+
// electron scripts are wrapped in a try {} catch {} in run-compiler.js
33+
if (try_catch.HasCaught()) {
34+
LOG(ERROR) << "Failed to CompileAndCall electron script: " << id;
35+
}
36+
return ret;
2937
}
3038

3139
} // namespace util

0 commit comments

Comments
 (0)
X Tutup