Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Handle error when offscreen document already exists #25138

Merged
merged 2 commits into from
Jul 18, 2024
Merged

fix: Handle error when offscreen document already exists #25138

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05d3384
fix: Handle error when offscreen document already exists
Gudahtt Jun 7, 2024
8341502
Update error handling to avoid crashing
Gudahtt Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions app/scripts/offscreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { captureException } from '@sentry/browser';
import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-communication';

/**
Expand All @@ -9,29 +10,51 @@ 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 (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');
} 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);
Copy link
Member Author

@Gudahtt Gudahtt Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just updated this to catch all platform errors here (as suggested by @FrederikBolding on Slack) to avoid bricking the wallet if something else goes wrong here. We're still capturing the error in Sentry, and this is a non-essential step, so this should be a safe way to approach this for now.

See commit ffb2c3a

}
return;
}

// 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) => {
Expand Down