From 05d33843a994f2437689c8f9b283cf31eb243f2a Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 7 Jun 2024 09:21:36 -0230 Subject: [PATCH 1/2] fix: Handle error when offscreen document already exists A `try..catch` block has been added to catch errors about the Offscreen document already existing. This circumstance is not a bug, it's expected to happen sometimes due to race conditions between `hasDocument` and the creation step. The code in question is just meant to create the document if it doesn't exist, so it's OK if we get this error. The `hasDocument` call has been removed as well, since it's now redundant. This function wasn't safe to call anyway; it's an undocumented private function. See here for more information: * https://stackoverflow.com/a/7725839 * https://groups.google.com/a/chromium.org/g/chromium-extensions/c/D5Jg2ukyvUc4 Fixes #25118 --- app/scripts/offscreen.js | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/app/scripts/offscreen.js b/app/scripts/offscreen.js index ba796874f2fc..556561c7dd46 100644 --- a/app/scripts/offscreen.js +++ b/app/scripts/offscreen.js @@ -9,29 +9,49 @@ import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-c */ export async function createOffscreen() { const { chrome } = globalThis; - if (!chrome.offscreen || (await chrome.offscreen.hasDocument())) { + if (!chrome.offscreen) { return; } + let offscreenDocumentLoadedListener; const loadPromise = new Promise((resolve) => { - const messageListener = (msg) => { + offscreenDocumentLoadedListener = (msg) => { if ( msg.target === OffscreenCommunicationTarget.extensionMain && msg.isBooted ) { - chrome.runtime.onMessage.removeListener(messageListener); + chrome.runtime.onMessage.removeListener( + offscreenDocumentLoadedListener, + ); resolve(); } }; - chrome.runtime.onMessage.addListener(messageListener); + chrome.runtime.onMessage.addListener(offscreenDocumentLoadedListener); }); - await chrome.offscreen.createDocument({ - url: './offscreen.html', - reasons: ['IFRAME_SCRIPTING'], - justification: - 'Used for Hardware Wallet and Snaps scripts to communicate with the extension.', - }); + try { + await chrome.offscreen.createDocument({ + url: './offscreen.html', + reasons: ['IFRAME_SCRIPTING'], + justification: + 'Used for Hardware Wallet and Snaps scripts to communicate with the extension.', + }); + } catch (error) { + if ( + error?.message?.startsWith( + 'Only a single offscreen document may be created', + ) + ) { + console.debug('Offscreen document already exists; skipping creation'); + if (offscreenDocumentLoadedListener) { + chrome.runtime.onMessage.removeListener( + offscreenDocumentLoadedListener, + ); + } + return; + } + throw error; + } // In case we are in a bad state where the offscreen document is not loading, timeout and let execution continue. const timeoutPromise = new Promise((resolve) => { From 83415021e11240611258fcc59ccc934ea5193747 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 24 Jun 2024 16:42:56 -0230 Subject: [PATCH 2/2] Update error handling to avoid crashing We now capture unrecognized errors and report them to Sentry, rather than throwing them. This ensures that any new failures would not brick the wallet. --- app/scripts/offscreen.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/app/scripts/offscreen.js b/app/scripts/offscreen.js index 556561c7dd46..cb18f393e11d 100644 --- a/app/scripts/offscreen.js +++ b/app/scripts/offscreen.js @@ -1,3 +1,4 @@ +import { captureException } from '@sentry/browser'; import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-communication'; /** @@ -37,20 +38,22 @@ export async function createOffscreen() { 'Used for Hardware Wallet and Snaps scripts to communicate with the extension.', }); } catch (error) { + if (offscreenDocumentLoadedListener) { + chrome.runtime.onMessage.removeListener(offscreenDocumentLoadedListener); + } if ( error?.message?.startsWith( 'Only a single offscreen document may be created', ) ) { console.debug('Offscreen document already exists; skipping creation'); - if (offscreenDocumentLoadedListener) { - chrome.runtime.onMessage.removeListener( - offscreenDocumentLoadedListener, - ); - } - return; + } else { + // Report unrecongized errors without halting wallet initialization + // Failures to create the offscreen document does not compromise wallet data integrity or + // core functionality, it's just needed for specific features. + captureException(error); } - throw error; + return; } // In case we are in a bad state where the offscreen document is not loading, timeout and let execution continue.