From b2e098f2dd09819d5c6605355e9bb979efa9316a Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 11 Nov 2022 12:53:46 -0600 Subject: [PATCH] app: catch error when checking for updates ServiceWorkerRegistration.update() can raise exceptions, so we need to catch and handle them, otherwise it will crash redux sagas and the app will stop responding. Fixes: https://github.com/pybricks/pybricks-code/issues/1299 --- CHANGELOG.md | 5 +++++ src/alerts.ts | 2 ++ src/app/actions.ts | 5 +++++ src/app/alerts/UpdateServerFailure.tsx | 21 +++++++++++++++++++++ src/app/alerts/i18n.ts | 12 ++++++++++++ src/app/alerts/index.ts | 9 +++++++++ src/app/alerts/translations/en.json | 5 +++++ src/app/reducers.test.ts | 5 +++++ src/app/reducers.ts | 5 +++++ src/app/sagas.ts | 23 ++++++++++++++++++++--- 10 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 src/app/alerts/UpdateServerFailure.tsx create mode 100644 src/app/alerts/i18n.ts create mode 100644 src/app/alerts/index.ts create mode 100644 src/app/alerts/translations/en.json diff --git a/CHANGELOG.md b/CHANGELOG.md index f21d932d..545218cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ## [Unreleased] +### Fixed +- Fixed app freezing when checking for updates and update server is unreachable ([pybricks-code#1299]). + +[pybricks-code#1299]: https://github.com/pybricks/pybricks-code/issues/1299 + ## [2.0.0-beta.10] - 2022-11-11 ### Added diff --git a/src/alerts.ts b/src/alerts.ts index 318fd175..3bc2a0e1 100644 --- a/src/alerts.ts +++ b/src/alerts.ts @@ -3,6 +3,7 @@ import { ToastProps } from '@blueprintjs/core'; import alerts from './alerts/alerts'; +import app from './app/alerts'; import ble from './ble/alerts'; import explorer from './explorer/alerts'; import firmware from './firmware/alerts'; @@ -12,6 +13,7 @@ import type { CreateToast } from './toasterTypes'; /** This collects alerts from all of the subsystems of the app */ const alertDomains = { alerts, + app, ble, explorer, firmware, diff --git a/src/app/actions.ts b/src/app/actions.ts index 546c5239..540502c0 100644 --- a/src/app/actions.ts +++ b/src/app/actions.ts @@ -21,6 +21,11 @@ export const appDidCheckForUpdate = createAction((updateFound: boolean) => ({ updateFound, })); +/** Action that indicates that checking for an update failed. */ +export const appDidFailToCheckForUpdate = createAction(() => ({ + type: 'app.action.didFailToCheckForUpdate', +})); + /* Action that indicates the browser wants to prompt the use to install the app. */ export const appDidReceiveBeforeInstallPrompt = createAction(() => ({ type: 'app.action.didBeforeInstallPrompt', diff --git a/src/app/alerts/UpdateServerFailure.tsx b/src/app/alerts/UpdateServerFailure.tsx new file mode 100644 index 00000000..0abf2b2d --- /dev/null +++ b/src/app/alerts/UpdateServerFailure.tsx @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { Intent } from '@blueprintjs/core'; +import React from 'react'; +import type { CreateToast } from '../../toasterTypes'; +import { useI18n } from './i18n'; + +const UpdateServerFailure: React.VoidFunctionComponent = () => { + const i18n = useI18n(); + return

{i18n.translate('updateServerFailure.message')}

; +}; + +export const updateServerFailure: CreateToast = (onAction) => { + return { + message: , + icon: 'error', + intent: Intent.DANGER, + onDismiss: () => onAction('dismiss'), + }; +}; diff --git a/src/app/alerts/i18n.ts b/src/app/alerts/i18n.ts new file mode 100644 index 00000000..eb8dc486 --- /dev/null +++ b/src/app/alerts/i18n.ts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { useI18n as useShopifyI18n } from '@shopify/react-i18n'; +import type { TypedI18n } from '../../i18n'; +import type translations from './translations/en.json'; + +export function useI18n(): TypedI18n { + // istanbul ignore next: babel-loader rewrites this line + const [i18n] = useShopifyI18n(); + return i18n; +} diff --git a/src/app/alerts/index.ts b/src/app/alerts/index.ts new file mode 100644 index 00000000..35084e53 --- /dev/null +++ b/src/app/alerts/index.ts @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { updateServerFailure } from './UpdateServerFailure'; + +// gathers all of the alert creation functions for passing up to the top level +export default { + updateServerFailure, +}; diff --git a/src/app/alerts/translations/en.json b/src/app/alerts/translations/en.json new file mode 100644 index 00000000..bd914faa --- /dev/null +++ b/src/app/alerts/translations/en.json @@ -0,0 +1,5 @@ +{ + "updateServerFailure": { + "message": "Failed to connect to update server. The Internet connection or the server may be down. Try again later." + } +} diff --git a/src/app/reducers.test.ts b/src/app/reducers.test.ts index 0b7b5629..0cb38745 100644 --- a/src/app/reducers.test.ts +++ b/src/app/reducers.test.ts @@ -9,6 +9,7 @@ import { import { appCheckForUpdate, appDidCheckForUpdate, + appDidFailToCheckForUpdate, appDidReceiveBeforeInstallPrompt, appDidResolveInstallPrompt, appShowInstallPrompt, @@ -59,6 +60,10 @@ test('checkingForUpdate', () => { reducers({ checkingForUpdate: true } as State, appDidCheckForUpdate(false)) .checkingForUpdate, ).toBe(false); + expect( + reducers({ checkingForUpdate: true } as State, appDidFailToCheckForUpdate()) + .checkingForUpdate, + ).toBe(false); expect( reducers({ checkingForUpdate: true } as State, serviceWorkerDidUpdate()) .checkingForUpdate, diff --git a/src/app/reducers.ts b/src/app/reducers.ts index 7b450fb4..0849e4f8 100644 --- a/src/app/reducers.ts +++ b/src/app/reducers.ts @@ -11,6 +11,7 @@ import { import { appCheckForUpdate, appDidCheckForUpdate, + appDidFailToCheckForUpdate, appDidReceiveBeforeInstallPrompt, appDidResolveInstallPrompt, appShowInstallPrompt, @@ -39,6 +40,10 @@ const checkingForUpdate: Reducer = (state = false, action) => { return state; } + if (appDidFailToCheckForUpdate.matches(action)) { + return false; + } + if (serviceWorkerDidUpdate.matches(action)) { return false; } diff --git a/src/app/sagas.ts b/src/app/sagas.ts index 31ee6393..db9c0515 100644 --- a/src/app/sagas.ts +++ b/src/app/sagas.ts @@ -3,15 +3,18 @@ import { eventChannel } from 'redux-saga'; import { call, delay, fork, put, take, takeEvery } from 'typed-redux-saga/macro'; +import { alertsShowAlert } from '../alerts/actions'; import { serviceWorkerDidSucceed, serviceWorkerDidUpdate, } from '../service-worker/actions'; import * as serviceWorkerRegistration from '../serviceWorkerRegistration'; +import { ensureError } from '../utils'; import { BeforeInstallPromptEvent } from '../utils/dom'; import { appCheckForUpdate, appDidCheckForUpdate, + appDidFailToCheckForUpdate, appDidReceiveBeforeInstallPrompt, appDidResolveInstallPrompt, appReload, @@ -36,9 +39,23 @@ function* handleAppReload(registration: ServiceWorkerRegistration): Generator { * Must be called (forked) with serviceWorkerRegistration context set. */ function* handleAppCheckForUpdate(registration: ServiceWorkerRegistration): Generator { - yield* call(() => registration.update()); - const updateFound = registration.installing !== null; - yield* put(appDidCheckForUpdate(updateFound)); + try { + yield* call(() => registration.update()); + const updateFound = registration.installing !== null; + yield* put(appDidCheckForUpdate(updateFound)); + } catch (err) { + if (err instanceof TypeError) { + yield* put(alertsShowAlert('app', 'updateServerFailure')); + } else { + yield* put( + alertsShowAlert('alerts', 'unexpectedError', { + error: ensureError(err), + }), + ); + } + + yield* put(appDidFailToCheckForUpdate()); + } } /**