From b2e098f2dd09819d5c6605355e9bb979efa9316a Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 11 Nov 2022 12:53:46 -0600 Subject: [PATCH 1/3] 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()); + } } /** From 6f91c57743cf8d692d0f73d1583c34a93463baf3 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 11 Nov 2022 12:57:16 -0600 Subject: [PATCH 2/3] firmware/sagas: add delay after connecting This adds a delay after connecting to a hub when flashing firmware via BLE before sending any commands. This gives the OS Bluetooth stack time to finish enumerating the Bluetooth device before we start trying to interact with the device. Hopefully this fixes issues for some people who are seeing problems while flashing firmware. Issue: https://github.com/orgs/pybricks/discussions/792 --- CHANGELOG.md | 2 ++ src/firmware/sagas.ts | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 545218cf..3d4c607d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,10 @@ ### Fixed - Fixed app freezing when checking for updates and update server is unreachable ([pybricks-code#1299]). +- Added delay to try to mitigate errors when flashing firmware on city hubs ([support#792]). [pybricks-code#1299]: https://github.com/pybricks/pybricks-code/issues/1299 +[support#792]: https://github.com/orgs/pybricks/discussions/792 ## [2.0.0-beta.10] - 2022-11-11 diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 6ef85cc5..f64bb5ea 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -393,6 +393,12 @@ function* handleFlashFirmware(action: ReturnType): Generat return; } + // istanbul ignore if + if (process.env.NODE_ENV !== 'test') { + // give OS Bluetooth stack some time to settle + yield* delay(1000); + } + const nextMessageId = yield* getContext<() => number>('nextMessageId'); const infoAction = yield* put(infoRequest(nextMessageId())); From abb4578450fa7df7c41207023f98d5fea054eeda Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 11 Nov 2022 13:00:07 -0600 Subject: [PATCH 3/3] v2.0.0-beta.11 --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d4c607d..bae2219e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ## [Unreleased] +## [2.0.0-beta.11] - 2022-11-11 + ### Fixed - Fixed app freezing when checking for updates and update server is unreachable ([pybricks-code#1299]). - Added delay to try to mitigate errors when flashing firmware on city hubs ([support#792]). @@ -493,7 +495,8 @@ Prerelease changes are documented at [support#48]. -[Unreleased]: https://github.com/pybricks/pybricks-code/compare/v2.0.0-beta.10...HEAD +[Unreleased]: https://github.com/pybricks/pybricks-code/compare/v2.0.0-beta.11...HEAD +[2.0.0-beta.11]: https://github.com/pybricks/pybricks-code/compare/v2.0.0-beta.10...v2.0.0-beta.11 [2.0.0-beta.10]: https://github.com/pybricks/pybricks-code/compare/v2.0.0-beta.9...v2.0.0-beta.10 [2.0.0-beta.9]: https://github.com/pybricks/pybricks-code/compare/v2.0.0-beta.8...v2.0.0-beta.9 [2.0.0-beta.8]: https://github.com/pybricks/pybricks-code/compare/v2.0.0-beta.7...v2.0.0-beta.8 diff --git a/package.json b/package.json index 593df064..ea7dfdd8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pybricks/pybricks-code", - "version": "2.0.0-beta.10", + "version": "2.0.0-beta.11", "license": "MIT", "author": "The Pybricks Authors", "repository": {