From 1c17b327ce5d25c8db26a5dd0835369de49980b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Zahola?= Date: Thu, 30 Nov 2023 13:45:48 +0100 Subject: [PATCH 1/4] fix: don't reject `loadURL()` promise from `did-fail-load` - use `did-finish-load` instead --- lib/browser/api/web-contents.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index a9f5a52213b57..0658d058e4883 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -345,12 +345,13 @@ WebContents.prototype.loadURL = function (url, options) { removeListeners(); reject(err); }; + let doFinish = () => resolveAndCleanup(); const finishListener = () => { - resolveAndCleanup(); + doFinish(); }; const failListener = (event: Electron.Event, errorCode: number, errorDescription: string, validatedURL: string, isMainFrame: boolean) => { if (isMainFrame) { - rejectAndCleanup(errorCode, errorDescription, validatedURL); + doFinish = () => rejectAndCleanup(errorCode, errorDescription, validatedURL); } }; From 08e8b672a4e1b4a919eed1ef812084303a0f2dc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Zahola?= Date: Thu, 30 Nov 2023 23:44:37 +0100 Subject: [PATCH 2/4] Only react to the first `did-fail-load` --- lib/browser/api/web-contents.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index 0658d058e4883..b1d7ae61791dc 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -345,12 +345,14 @@ WebContents.prototype.loadURL = function (url, options) { removeListeners(); reject(err); }; + let didFail = false; let doFinish = () => resolveAndCleanup(); const finishListener = () => { doFinish(); }; const failListener = (event: Electron.Event, errorCode: number, errorDescription: string, validatedURL: string, isMainFrame: boolean) => { - if (isMainFrame) { + if (!didFail && isMainFrame) { + didFail = true; doFinish = () => rejectAndCleanup(errorCode, errorDescription, validatedURL); } }; From 48d67485e6ed8d93107dddccb1fd9a6ba10211ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Zahola?= Date: Fri, 1 Dec 2023 15:50:57 +0100 Subject: [PATCH 3/4] Also handle did-stop-loading after did-fail-load --- lib/browser/api/web-contents.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index b1d7ae61791dc..f64b00fbeb499 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -333,27 +333,31 @@ WebContents.prototype.loadFile = function (filePath, options = {}) { })); }; +type LoadError = { errorCode: number, errorDescription: string, url: string }; + WebContents.prototype.loadURL = function (url, options) { const p = new Promise((resolve, reject) => { const resolveAndCleanup = () => { removeListeners(); resolve(); }; - const rejectAndCleanup = (errorCode: number, errorDescription: string, url: string) => { + let error: LoadError | undefined; + const rejectAndCleanup = ({ errorCode, errorDescription, url }: LoadError) => { const err = new Error(`${errorDescription} (${errorCode}) loading '${typeof url === 'string' ? url.substr(0, 2048) : url}'`); Object.assign(err, { errno: errorCode, code: errorDescription, url }); removeListeners(); reject(err); }; - let didFail = false; - let doFinish = () => resolveAndCleanup(); const finishListener = () => { - doFinish(); + if (error) { + rejectAndCleanup(error); + } else { + resolveAndCleanup(); + } }; const failListener = (event: Electron.Event, errorCode: number, errorDescription: string, validatedURL: string, isMainFrame: boolean) => { - if (!didFail && isMainFrame) { - didFail = true; - doFinish = () => rejectAndCleanup(errorCode, errorDescription, validatedURL); + if (!error && isMainFrame) { + error = { errorCode, errorDescription, url: validatedURL }; } }; @@ -371,7 +375,7 @@ WebContents.prototype.loadURL = function (url, options) { // considered navigation events but are triggered with isSameDocument. // We can ignore these to allow virtual routing on page load as long // as the routing does not leave the document - return rejectAndCleanup(-3, 'ERR_ABORTED', url); + return rejectAndCleanup({ errorCode: -3, errorDescription: 'ERR_ABORTED', url }); } browserInitiatedInPageNavigation = navigationStarted && isSameDocument; navigationStarted = true; @@ -386,7 +390,10 @@ WebContents.prototype.loadURL = function (url, options) { // TODO(jeremy): enumerate all the cases in which this can happen. If // the only one is with a bad scheme, perhaps ERR_INVALID_ARGUMENT // would be more appropriate. - rejectAndCleanup(-2, 'ERR_FAILED', url); + if (!error) { + error = { errorCode: -2, errorDescription: 'ERR_FAILED', url: url }; + } + finishListener(); }; const finishListenerWhenUserInitiatedNavigation = () => { if (!browserInitiatedInPageNavigation) { From 834a472eb2efacd0fe26963694388939bc5f2564 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Zahola?= Date: Fri, 1 Dec 2023 15:54:45 +0100 Subject: [PATCH 4/4] Add test --- spec/api-web-contents-spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index cfd366554f32b..53c2fffe332bd 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -514,6 +514,11 @@ describe('webContents module', () => { .and.have.property('errno', -355); // ERR_INCOMPLETE_CHUNKED_ENCODING s.close(); }); + + it('subsequent load failures reject each time', async () => { + await expect(w.loadURL('file:non-existent')).to.eventually.be.rejected(); + await expect(w.loadURL('file:non-existent')).to.eventually.be.rejected(); + }); }); describe('getFocusedWebContents() API', () => {