Skip to content

Commit b2e098f

Browse files
committed
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
1 parent 6c906c9 commit b2e098f

File tree

10 files changed

+89
-3
lines changed

10 files changed

+89
-3
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44

55
## [Unreleased]
66

7+
### Fixed
8+
- Fixed app freezing when checking for updates and update server is unreachable ([pybricks-code#1299]).
9+
10+
[pybricks-code#1299]: https://github.com/pybricks/pybricks-code/issues/1299
11+
712
## [2.0.0-beta.10] - 2022-11-11
813

914
### Added

src/alerts.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { ToastProps } from '@blueprintjs/core';
55
import alerts from './alerts/alerts';
6+
import app from './app/alerts';
67
import ble from './ble/alerts';
78
import explorer from './explorer/alerts';
89
import firmware from './firmware/alerts';
@@ -12,6 +13,7 @@ import type { CreateToast } from './toasterTypes';
1213
/** This collects alerts from all of the subsystems of the app */
1314
const alertDomains = {
1415
alerts,
16+
app,
1517
ble,
1618
explorer,
1719
firmware,

src/app/actions.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ export const appDidCheckForUpdate = createAction((updateFound: boolean) => ({
2121
updateFound,
2222
}));
2323

24+
/** Action that indicates that checking for an update failed. */
25+
export const appDidFailToCheckForUpdate = createAction(() => ({
26+
type: 'app.action.didFailToCheckForUpdate',
27+
}));
28+
2429
/* Action that indicates the browser wants to prompt the use to install the app. */
2530
export const appDidReceiveBeforeInstallPrompt = createAction(() => ({
2631
type: 'app.action.didBeforeInstallPrompt',
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// SPDX-License-Identifier: MIT
2+
// Copyright (c) 2022 The Pybricks Authors
3+
4+
import { Intent } from '@blueprintjs/core';
5+
import React from 'react';
6+
import type { CreateToast } from '../../toasterTypes';
7+
import { useI18n } from './i18n';
8+
9+
const UpdateServerFailure: React.VoidFunctionComponent = () => {
10+
const i18n = useI18n();
11+
return <p>{i18n.translate('updateServerFailure.message')}</p>;
12+
};
13+
14+
export const updateServerFailure: CreateToast = (onAction) => {
15+
return {
16+
message: <UpdateServerFailure />,
17+
icon: 'error',
18+
intent: Intent.DANGER,
19+
onDismiss: () => onAction('dismiss'),
20+
};
21+
};

src/app/alerts/i18n.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// SPDX-License-Identifier: MIT
2+
// Copyright (c) 2022 The Pybricks Authors
3+
4+
import { useI18n as useShopifyI18n } from '@shopify/react-i18n';
5+
import type { TypedI18n } from '../../i18n';
6+
import type translations from './translations/en.json';
7+
8+
export function useI18n(): TypedI18n<typeof translations> {
9+
// istanbul ignore next: babel-loader rewrites this line
10+
const [i18n] = useShopifyI18n();
11+
return i18n;
12+
}

src/app/alerts/index.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// SPDX-License-Identifier: MIT
2+
// Copyright (c) 2022 The Pybricks Authors
3+
4+
import { updateServerFailure } from './UpdateServerFailure';
5+
6+
// gathers all of the alert creation functions for passing up to the top level
7+
export default {
8+
updateServerFailure,
9+
};
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"updateServerFailure": {
3+
"message": "Failed to connect to update server. The Internet connection or the server may be down. Try again later."
4+
}
5+
}

src/app/reducers.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import {
1010
appCheckForUpdate,
1111
appDidCheckForUpdate,
12+
appDidFailToCheckForUpdate,
1213
appDidReceiveBeforeInstallPrompt,
1314
appDidResolveInstallPrompt,
1415
appShowInstallPrompt,
@@ -59,6 +60,10 @@ test('checkingForUpdate', () => {
5960
reducers({ checkingForUpdate: true } as State, appDidCheckForUpdate(false))
6061
.checkingForUpdate,
6162
).toBe(false);
63+
expect(
64+
reducers({ checkingForUpdate: true } as State, appDidFailToCheckForUpdate())
65+
.checkingForUpdate,
66+
).toBe(false);
6267
expect(
6368
reducers({ checkingForUpdate: true } as State, serviceWorkerDidUpdate())
6469
.checkingForUpdate,

src/app/reducers.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
import {
1212
appCheckForUpdate,
1313
appDidCheckForUpdate,
14+
appDidFailToCheckForUpdate,
1415
appDidReceiveBeforeInstallPrompt,
1516
appDidResolveInstallPrompt,
1617
appShowInstallPrompt,
@@ -39,6 +40,10 @@ const checkingForUpdate: Reducer<boolean> = (state = false, action) => {
3940
return state;
4041
}
4142

43+
if (appDidFailToCheckForUpdate.matches(action)) {
44+
return false;
45+
}
46+
4247
if (serviceWorkerDidUpdate.matches(action)) {
4348
return false;
4449
}

src/app/sagas.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@
33

44
import { eventChannel } from 'redux-saga';
55
import { call, delay, fork, put, take, takeEvery } from 'typed-redux-saga/macro';
6+
import { alertsShowAlert } from '../alerts/actions';
67
import {
78
serviceWorkerDidSucceed,
89
serviceWorkerDidUpdate,
910
} from '../service-worker/actions';
1011
import * as serviceWorkerRegistration from '../serviceWorkerRegistration';
12+
import { ensureError } from '../utils';
1113
import { BeforeInstallPromptEvent } from '../utils/dom';
1214
import {
1315
appCheckForUpdate,
1416
appDidCheckForUpdate,
17+
appDidFailToCheckForUpdate,
1518
appDidReceiveBeforeInstallPrompt,
1619
appDidResolveInstallPrompt,
1720
appReload,
@@ -36,9 +39,23 @@ function* handleAppReload(registration: ServiceWorkerRegistration): Generator {
3639
* Must be called (forked) with serviceWorkerRegistration context set.
3740
*/
3841
function* handleAppCheckForUpdate(registration: ServiceWorkerRegistration): Generator {
39-
yield* call(() => registration.update());
40-
const updateFound = registration.installing !== null;
41-
yield* put(appDidCheckForUpdate(updateFound));
42+
try {
43+
yield* call(() => registration.update());
44+
const updateFound = registration.installing !== null;
45+
yield* put(appDidCheckForUpdate(updateFound));
46+
} catch (err) {
47+
if (err instanceof TypeError) {
48+
yield* put(alertsShowAlert('app', 'updateServerFailure'));
49+
} else {
50+
yield* put(
51+
alertsShowAlert('alerts', 'unexpectedError', {
52+
error: ensureError(err),
53+
}),
54+
);
55+
}
56+
57+
yield* put(appDidFailToCheckForUpdate());
58+
}
4259
}
4360

4461
/**

0 commit comments

Comments
 (0)