From 107525bb1dbc213cdf1b7abb08449c7bd8f268d9 Mon Sep 17 00:00:00 2001 From: Olusegun Akintayo Date: Thu, 3 Nov 2022 21:25:13 +0400 Subject: [PATCH] Show error message if service worker did not load (respond to the UI) (#15774) * Show error message if service worker did not load (respond to the UI) after 1 minute. Signed-off-by: Akintayo A. Olusegun * Remove console.log Signed-off-by: Akintayo A. Olusegun * New Error message design Signed-off-by: Akintayo A. Olusegun * Fix tests Signed-off-by: Akintayo A. Olusegun * Use lastTimeStamp instead of keeping track of message ids Signed-off-by: Akintayo A. Olusegun * Do not initial channe every second. Signed-off-by: Akintayo A. Olusegun * New logic to check if SW is stuck Signed-off-by: Akintayo A. Olusegun Signed-off-by: Akintayo A. Olusegun --- app/_locales/en/messages.json | 3 ++ app/scripts/background.js | 11 +++++++ app/scripts/ui.js | 55 ++++++++++++++++++++++++++++++---- shared/lib/error-utils.js | 50 ++++++++++++++++++++----------- shared/lib/error-utils.test.js | 6 +++- ui/css/errors.scss | 44 +++++++++++++++++---------- 6 files changed, 129 insertions(+), 40 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index e785cd4fd..7fcf47431 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -3272,6 +3272,9 @@ "someNetworksMayPoseSecurity": { "message": "Some networks may pose security and/or privacy risks. Understand the risks before adding & using a network." }, + "somethingIsWrong": { + "message": "Something's gone wrong. Try reloading the page." + }, "somethingWentWrong": { "message": "Oops! Something went wrong." }, diff --git a/app/scripts/background.js b/app/scripts/background.js index f3ed3dc3a..b2ed07e8b 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -88,6 +88,9 @@ const ONE_SECOND_IN_MILLISECONDS = 1_000; // Timeout for initializing phishing warning page. const PHISHING_WARNING_PAGE_TIMEOUT = ONE_SECOND_IN_MILLISECONDS; +const ACK_KEEP_ALIVE_MESSAGE = 'ACK_KEEP_ALIVE_MESSAGE'; +const WORKER_KEEP_ALIVE_MESSAGE = 'WORKER_KEEP_ALIVE_MESSAGE'; + /** * In case of MV3 we attach a "onConnect" event listener as soon as the application is initialised. * Reason is that in case of MV3 a delay in doing this was resulting in missing first connect event after service worker is re-activated. @@ -439,6 +442,14 @@ function setupController(initState, initLangCode, remoteSourcePort) { // This ensures that UI is initialised only after background is ready // It fixes the issue of blank screen coming when extension is loaded, the issue is very frequent in MV3 remotePort.postMessage({ name: 'CONNECTION_READY' }); + + // If we get a WORKER_KEEP_ALIVE message, we respond with an ACK + remotePort.onMessage.addListener((message) => { + if (message.name === WORKER_KEEP_ALIVE_MESSAGE) { + // To test un-comment this line and wait for 1 minute. An error should be shown on MetaMask UI. + remotePort.postMessage({ name: ACK_KEEP_ALIVE_MESSAGE }); + } + }); } if (processName === ENVIRONMENT_TYPE_POPUP) { diff --git a/app/scripts/ui.js b/app/scripts/ui.js index d9569f043..13516becd 100644 --- a/app/scripts/ui.js +++ b/app/scripts/ui.js @@ -30,22 +30,65 @@ const container = document.getElementById('app-content'); const ONE_SECOND_IN_MILLISECONDS = 1_000; const WORKER_KEEP_ALIVE_INTERVAL = ONE_SECOND_IN_MILLISECONDS; +// Service Worker Keep Alive Message Constants const WORKER_KEEP_ALIVE_MESSAGE = 'WORKER_KEEP_ALIVE_MESSAGE'; +const ACK_KEEP_ALIVE_WAIT_TIME = 60_000; // 1 minute +const ACK_KEEP_ALIVE_MESSAGE = 'ACK_KEEP_ALIVE_MESSAGE'; // Timeout for initializing phishing warning page. const PHISHING_WARNING_PAGE_TIMEOUT = ONE_SECOND_IN_MILLISECONDS; const PHISHING_WARNING_SW_STORAGE_KEY = 'phishing-warning-sw-registered'; +let lastMessageRecievedTimestamp = Date.now(); /* * As long as UI is open it will keep sending messages to service worker * In service worker as this message is received * if service worker is inactive it is reactivated and script re-loaded * Time has been kept to 1000ms but can be reduced for even faster re-activation of service worker */ +let extensionPort; +let timeoutHandle; + if (isManifestV3) { - setInterval(() => { + // Checking for SW aliveness (or stuckness) flow + // 1. Check if we have an extensionPort, if yes + // 2a. Send a keep alive message to the background via extensionPort + // 2b. Add a listener to it (if not already added) + // 3a. Set a timeout to check if we have received an ACK from background + // 3b. If we have not received an ACK within Xs, we know the background is stuck or dead + // 4. If we recieve an ACK_KEEP_ALIVE_MESSAGE from the service worker, we know it is alive + + const ackKeepAliveListener = (message) => { + if (message.name === ACK_KEEP_ALIVE_MESSAGE) { + lastMessageRecievedTimestamp = Date.now(); + clearTimeout(timeoutHandle); + } + }; + + const handle = setInterval(() => { browser.runtime.sendMessage({ name: WORKER_KEEP_ALIVE_MESSAGE }); + + if (extensionPort !== null && extensionPort !== undefined) { + extensionPort.postMessage({ name: WORKER_KEEP_ALIVE_MESSAGE }); + + if (extensionPort.onMessage.hasListener(ackKeepAliveListener) === false) { + extensionPort.onMessage.addListener(ackKeepAliveListener); + } + } + + timeoutHandle = setTimeout(() => { + if ( + Date.now() - lastMessageRecievedTimestamp > + ACK_KEEP_ALIVE_WAIT_TIME + ) { + clearInterval(handle); + displayCriticalError( + 'somethingIsWrong', + new Error("Something's gone wrong. Try reloading the page."), + ); + } + }, ACK_KEEP_ALIVE_WAIT_TIME); }, WORKER_KEEP_ALIVE_INTERVAL); } @@ -61,7 +104,7 @@ async function start() { let isUIInitialised = false; // setup stream to background - let extensionPort = browser.runtime.connect({ name: windowType }); + extensionPort = browser.runtime.connect({ name: windowType }); let connectionStream = new PortStream(extensionPort); const activeTab = await queryCurrentActiveTab(windowType); @@ -208,7 +251,7 @@ async function start() { initializeUi(tab, connectionStream, (err, store) => { if (err) { // if there's an error, store will be = metamaskState - displayCriticalError(err, store); + displayCriticalError('troubleStarting', err, store); return; } isUIInitialised = true; @@ -226,7 +269,7 @@ async function start() { function updateUiStreams() { connectToAccountManager(connectionStream, (err, backgroundConnection) => { if (err) { - displayCriticalError(err); + displayCriticalError('troubleStarting', err); return; } @@ -277,8 +320,8 @@ function initializeUi(activeTab, connectionStream, cb) { }); } -async function displayCriticalError(err, metamaskState) { - const html = await getErrorHtml(SUPPORT_LINK, metamaskState); +async function displayCriticalError(errorKey, err, metamaskState) { + const html = await getErrorHtml(errorKey, SUPPORT_LINK, metamaskState); container.innerHTML = html; diff --git a/shared/lib/error-utils.js b/shared/lib/error-utils.js index c0baad2c1..0dc11e91c 100644 --- a/shared/lib/error-utils.js +++ b/shared/lib/error-utils.js @@ -32,7 +32,7 @@ const getLocaleContext = (currentLocaleMessages, enLocaleMessages) => { }; }; -export async function getErrorHtml(supportLink, metamaskState) { +export async function getErrorHtml(errorKey, supportLink, metamaskState) { let response, preferredLocale; if (metamaskState?.currentLocale) { preferredLocale = metamaskState.currentLocale; @@ -50,26 +50,40 @@ export async function getErrorHtml(supportLink, metamaskState) { const { currentLocaleMessages, enLocaleMessages } = response; const t = getLocaleContext(currentLocaleMessages, enLocaleMessages); + /** + * The pattern ${errorKey === 'troubleStarting' ? t('troubleStarting') : ''} + * is neccessary because we we need linter to see the string + * of the locale keys. If we use the variable directly, the linter will not + * see the string and will not be able to check if the locale key exists. + */ return `
-
-

- ${t('troubleStarting')} -

- +
+ + + +
+
+
+

+ ${errorKey === 'troubleStarting' ? t('troubleStarting') : ''} + ${errorKey === 'somethingIsWrong' ? t('somethingIsWrong') : ''} +

+ + ${t('restartMetamask')} + +
+

+ ${t('stillGettingMessage')} + + ${t('sendBugReport')} + +

-

- ${t('stillGettingMessage')} - - ${t('sendBugReport')} - -

`; } diff --git a/shared/lib/error-utils.test.js b/shared/lib/error-utils.test.js index e1a121d7f..de846aa8b 100644 --- a/shared/lib/error-utils.test.js +++ b/shared/lib/error-utils.test.js @@ -33,7 +33,11 @@ describe('Error utils Tests', function () { }; fetchLocale.mockReturnValue(mockStore.localeMessages.current); - const errorHtml = await getErrorHtml(SUPPORT_LINK, mockStore.metamask); + const errorHtml = await getErrorHtml( + 'troubleStarting', + SUPPORT_LINK, + mockStore.metamask, + ); const currentLocale = mockStore.localeMessages.current; const troubleStartingMessage = currentLocale.troubleStarting.message; const restartMetamaskMessage = currentLocale.restartMetamask.message; diff --git a/ui/css/errors.scss b/ui/css/errors.scss index 14daf4262..c1f516f31 100644 --- a/ui/css/errors.scss +++ b/ui/css/errors.scss @@ -1,37 +1,51 @@ .critical-error { - padding: 16px; + padding: 1em; max-width: 600px; margin: 0 auto; + border-radius: 4px; + border-left: #f66a0a 4px solid; + background-color: rgba(255, 211, 61, 0.1); + display: flex; + padding: 0 4px 0 4px; + gap: 12px; + + &__icon { + color: var(--color-error-default); + height: 100px; + padding: 4px 4px 0 4px; + } + + &__description { + flex-grow: 1; + } &__alert { - color: var(--color-text-default); - background-color: var(--color-error-muted); - border: 1px solid var(--color-error-default); - border-radius: 8px; - padding: 16px; display: flex; flex-direction: column; margin-bottom: 16px; + font-style: normal; + font-weight: 400; + font-size: 16px; + line-height: 24px; &__message { margin-bottom: 16px; + text-align: left; + color: var(--color-text-default); } - &__button { + &__action-link { + text-align: left; height: 40px; - border-radius: 20px; - padding-left: 16px; - padding-right: 16px; - background-color: var(--color-primary-default); - color: var(--color-primary-inverse); - border: 1px solid var(--color-primary-default); - margin: 0 auto; + color: var(--color-primary-default); + text-decoration: none; + cursor: pointer; } } &__paragraph { + text-align: left; color: var(--color-text-default); - text-align: center; &__link { color: var(--color-primary-default);