From 829fb4f586ee4f0c5b6bbbc6ae9c9853a710eba6 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 6 Mar 2023 11:04:43 +0100 Subject: [PATCH] fix: don't double-log unhandled rejections (#37464) --- shell/browser/electron_browser_main_parts.cc | 2 +- shell/renderer/electron_renderer_client.cc | 2 +- .../api/unhandled-rejection-handled.js | 13 +++++++ spec/fixtures/api/unhandled-rejection.js | 5 +++ spec/node-spec.ts | 36 +++++++++++++++++++ 5 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/api/unhandled-rejection-handled.js create mode 100644 spec/fixtures/api/unhandled-rejection.js diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index 96cebf05534..9d7ffe7fc94 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -279,7 +279,7 @@ void ElectronBrowserMainParts::PostEarlyInitialization() { env->set_trace_sync_io(env->options()->trace_sync_io); // We do not want to crash the main process on unhandled rejections. - env->options()->unhandled_rejections = "warn"; + env->options()->unhandled_rejections = "warn-with-error-code"; // Add Electron extended APIs. electron_bindings_->BindTo(js_env_->isolate(), env->process_object()); diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index bbdea3035c6..21991ba8389 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -96,7 +96,7 @@ void ElectronRendererClient::DidCreateScriptContext( env->options()->force_context_aware = true; // We do not want to crash the renderer process on unhandled rejections. - env->options()->unhandled_rejections = "warn"; + env->options()->unhandled_rejections = "warn-with-error-code"; environments_.insert(env); diff --git a/spec/fixtures/api/unhandled-rejection-handled.js b/spec/fixtures/api/unhandled-rejection-handled.js new file mode 100644 index 00000000000..17557fbfaec --- /dev/null +++ b/spec/fixtures/api/unhandled-rejection-handled.js @@ -0,0 +1,13 @@ +const { app } = require('electron'); + +const handleUnhandledRejection = (reason) => { + console.error(`Unhandled Rejection: ${reason.stack}`); + app.quit(); +}; + +const main = async () => { + process.on('unhandledRejection', handleUnhandledRejection); + throw new Error('oops'); +}; + +main(); diff --git a/spec/fixtures/api/unhandled-rejection.js b/spec/fixtures/api/unhandled-rejection.js new file mode 100644 index 00000000000..fa47d6c56a7 --- /dev/null +++ b/spec/fixtures/api/unhandled-rejection.js @@ -0,0 +1,5 @@ +const main = async () => { + throw new Error('oops'); +}; + +main(); diff --git a/spec/node-spec.ts b/spec/node-spec.ts index 3612bcde182..47991c2177a 100644 --- a/spec/node-spec.ts +++ b/spec/node-spec.ts @@ -226,6 +226,42 @@ describe('node feature', () => { })); expect(result).to.equal('hello'); }); + + it('does not log the warning more than once when the rejection is unhandled', async () => { + const appPath = path.join(mainFixturesPath, 'api', 'unhandled-rejection.js'); + const appProcess = childProcess.spawn(process.execPath, [appPath]); + + let output = ''; + const out = (data: string) => { + output += data; + if (/UnhandledPromiseRejectionWarning/.test(data)) { + appProcess.kill(); + } + }; + appProcess.stdout!.on('data', out); + appProcess.stderr!.on('data', out); + + await once(appProcess, 'exit'); + expect(/UnhandledPromiseRejectionWarning/.test(output)).to.equal(true); + const matches = output.match(/Error: oops/gm); + expect(matches).to.have.lengthOf(1); + }); + + it('does not log the warning more than once when the rejection is handled', async () => { + const appPath = path.join(mainFixturesPath, 'api', 'unhandled-rejection-handled.js'); + const appProcess = childProcess.spawn(process.execPath, [appPath]); + + let output = ''; + const out = (data: string) => { output += data; }; + appProcess.stdout!.on('data', out); + appProcess.stderr!.on('data', out); + + const [code] = await once(appProcess, 'exit'); + expect(code).to.equal(0); + expect(/UnhandledPromiseRejectionWarning/.test(output)).to.equal(false); + const matches = output.match(/Error: oops/gm); + expect(matches).to.have.lengthOf(1); + }); }); });