Skip to content

Commit a6e1266

Browse files
committed
fix: harden update apply review follow-up
1 parent cb2be1f commit a6e1266

4 files changed

Lines changed: 64 additions & 15 deletions

File tree

src/app/features/settings/about/About.tsx

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useState } from 'react';
1+
import { useCallback, useEffect, useRef, useState } from 'react';
22
import { Box, Text, Scroll, Button, config, toRem, Spinner } from 'folds';
33
import { ArrowsClockwise, Code, Heart, menuIcon } from '$components/icons/phosphor';
44
import { PageContent } from '$components/page';
@@ -195,12 +195,22 @@ export function About({ requestBack, requestClose }: Readonly<AboutProps>) {
195195
const buildLabel = BUILD_HASH ? ` (${BUILD_HASH})` : '';
196196
const openBugReport = useOpenBugReportModal();
197197
const [updateStatusMessage, setUpdateStatusMessage] = useState<string | undefined>();
198+
const [isApplyingUpdate, setIsApplyingUpdate] = useState(false);
199+
const isApplyingUpdateRef = useRef(false);
200+
const isMountedRef = useRef(true);
198201
const [updateCheckState, runUpdateCheck] = useAsyncCallback<
199202
Awaited<ReturnType<typeof checkForAppUpdates>>,
200203
Error,
201204
[]
202205
>(checkForAppUpdates);
203206

207+
useEffect(
208+
() => () => {
209+
isMountedRef.current = false;
210+
},
211+
[]
212+
);
213+
204214
const handleCheckForUpdates = useCallback(async () => {
205215
try {
206216
const result = await runUpdateCheck();
@@ -214,12 +224,23 @@ export function About({ requestBack, requestClose }: Readonly<AboutProps>) {
214224
}, [runUpdateCheck]);
215225

216226
const handleApplyUpdate = useCallback(async () => {
227+
if (isApplyingUpdateRef.current) return;
228+
isApplyingUpdateRef.current = true;
229+
setIsApplyingUpdate(true);
230+
217231
try {
218232
await applyPendingAppUpdate();
219233
} catch (error) {
220-
setUpdateStatusMessage(
221-
error instanceof Error ? error.message : 'Failed to apply the update.'
222-
);
234+
if (isMountedRef.current) {
235+
setUpdateStatusMessage(
236+
error instanceof Error ? error.message : 'Failed to apply the update.'
237+
);
238+
}
239+
} finally {
240+
if (isMountedRef.current) {
241+
isApplyingUpdateRef.current = false;
242+
setIsApplyingUpdate(false);
243+
}
223244
}
224245
}, []);
225246

@@ -315,8 +336,10 @@ export function About({ requestBack, requestClose }: Readonly<AboutProps>) {
315336
size="300"
316337
radii="300"
317338
outlined
339+
disabled={isApplyingUpdate}
340+
before={isApplyingUpdate ? <Spinner variant="Secondary" size="100" /> : undefined}
318341
>
319-
<Text size="B300">Apply Update</Text>
342+
<Text size="B300">{isApplyingUpdate ? 'Applying…' : 'Apply Update'}</Text>
320343
</Button>
321344
) : (
322345
<Button

src/app/pages/client/ClientRoot.tsx

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ export function ClientRoot({ children }: ClientRootProps) {
209209
const setSessions = useSetAtom(sessionsAtom);
210210
const [defaultLandingScreen] = useSetting(settingsAtom, 'defaultLandingScreen');
211211
const [swUpdateError, setSwUpdateError] = useState<string | undefined>();
212+
const [isApplyingSwUpdate, setIsApplyingSwUpdate] = useState(false);
213+
const isApplyingSwUpdateRef = useRef(false);
214+
const isMountedRef = useRef(true);
212215

213216
const activeSession: Session | undefined =
214217
sessions.find((s) => s.userId === activeSessionId) ?? sessions[0];
@@ -219,6 +222,13 @@ export function ClientRoot({ children }: ClientRootProps) {
219222
const syncStartTimeRef = useRef(performance.now());
220223
const firstSyncReadyRef = useRef(false);
221224

225+
useEffect(
226+
() => () => {
227+
isMountedRef.current = false;
228+
},
229+
[]
230+
);
231+
222232
const [loading, setLoading] = useState(true);
223233

224234
const [loadState, loadMatrix, setLoadState] = useAsyncCallback<MatrixClient, Error, []>(
@@ -515,24 +525,39 @@ export function ClientRoot({ children }: ClientRootProps) {
515525
style={{
516526
padding: `${config.space.S100} 0`,
517527
width: '100%',
518-
cursor: 'pointer',
528+
cursor: isApplyingSwUpdate ? 'progress' : 'pointer',
519529
border: 'none',
520530
background: 'none',
521531
}}
532+
disabled={isApplyingSwUpdate}
522533
alignItems="Center"
523534
justifyContent="Center"
524535
onClick={() => {
536+
if (isApplyingSwUpdateRef.current) return;
537+
isApplyingSwUpdateRef.current = true;
538+
setIsApplyingSwUpdate(true);
539+
525540
void applyPendingAppUpdate()
526-
.then(() => setSwUpdateError(undefined))
541+
.then(() => {
542+
if (!isMountedRef.current) return;
543+
setSwUpdateError(undefined);
544+
isApplyingSwUpdateRef.current = false;
545+
setIsApplyingSwUpdate(false);
546+
})
527547
.catch((error) => {
548+
if (!isMountedRef.current) return;
528549
setSwUpdateError(
529550
error instanceof Error ? error.message : 'Failed to apply the update.'
530551
);
552+
isApplyingSwUpdateRef.current = false;
553+
setIsApplyingSwUpdate(false);
531554
});
532555
}}
533556
>
534557
<Box direction="Column" alignItems="Center" gap="100">
535-
<Text size="L400">Update available — tap to reload</Text>
558+
<Text size="L400">
559+
{isApplyingSwUpdate ? 'Applying update…' : 'Update available — tap to reload'}
560+
</Text>
536561
{swUpdateError && <Text size="T200">{swUpdateError}</Text>}
537562
</Box>
538563
</Box>

src/app/utils/appUpdates.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ describe('appUpdates', () => {
386386
expect((result as Error).message).toBe(
387387
'Failed to check for updates. Reload the app and try again.'
388388
);
389+
expect(fetchMock).not.toHaveBeenCalled();
389390
});
390391

391392
it('returns once any registration confirms an update', async () => {

src/app/utils/appUpdates.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,13 @@ export async function checkForAppUpdates(): Promise<AppUpdateCheckResult> {
402402
};
403403
}
404404

405+
if (registrations.length > 0 && (successfulUpdates.length === 0 || rejectedUpdates.length > 0)) {
406+
const firstError = rejectedUpdates[0]?.reason;
407+
throw firstError instanceof Error
408+
? new Error(UPDATE_CHECK_FAILURE_MESSAGE, { cause: firstError })
409+
: new Error(UPDATE_CHECK_FAILURE_MESSAGE);
410+
}
411+
405412
const hostedAppShellUpdateStatus = await getHostedAppShellUpdateStatus();
406413
if (hostedAppShellUpdateStatus === 'update-available') {
407414
hostedAppShellUpdateDetected = true;
@@ -412,13 +419,6 @@ export async function checkForAppUpdates(): Promise<AppUpdateCheckResult> {
412419
};
413420
}
414421

415-
if (registrations.length > 0 && (successfulUpdates.length === 0 || rejectedUpdates.length > 0)) {
416-
const firstError = rejectedUpdates[0]?.reason;
417-
throw firstError instanceof Error
418-
? new Error(UPDATE_CHECK_FAILURE_MESSAGE, { cause: firstError })
419-
: new Error(UPDATE_CHECK_FAILURE_MESSAGE);
420-
}
421-
422422
if (registrations.length === 0) {
423423
if (hostedAppShellUpdateStatus === 'up-to-date') {
424424
return {

0 commit comments

Comments
 (0)