From 9b2898e6b2c84b05aa470c2eec164d0b31f34e07 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 9 Aug 2026 12:59:26 +0200 Subject: [PATCH] fix(settings): tolerate an updater error push racing the install reply IPC ordering between the install invoke reply and status pushes is not guaranteed. installQuitPending is now armed before the round trip, so a failure push that beats the reply already finds it set and restores the unload protection; the stale 'downloaded' reply deliberately does not re-arm it, so the guards cannot end up suspended with the app open. Co-Authored-By: Claude Fable 5 --- .../settings-app-update.facade.spec.ts | 49 +++++++++++++++++++ .../settings/settings-app-update.facade.ts | 30 +++++++----- 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/apps/web/src/app/settings/settings-app-update.facade.spec.ts b/apps/web/src/app/settings/settings-app-update.facade.spec.ts index 3ce5afea3..f6e1dd3c7 100644 --- a/apps/web/src/app/settings/settings-app-update.facade.spec.ts +++ b/apps/web/src/app/settings/settings-app-update.facade.spec.ts @@ -230,6 +230,55 @@ describe('SettingsAppUpdateFacade', () => { ); }); + it('survives an error push that beats the install reply', async () => { + // IPC ordering between the invoke reply and status pushes is not + // guaranteed: the failure can arrive first, and the stale + // 'downloaded' reply must not re-suspend the protection the + // failure already restored. + const unloadGuard = TestBed.inject(SettingsUnloadGuardService); + let resolveInstall: ( + status: ElectronBridgeAppUpdateStatus + ) => void = () => undefined; + (window.electron.installAppUpdate as jest.Mock).mockImplementation( + () => + new Promise((resolve) => { + resolveInstall = resolve; + }) + ); + facade.init(); + const pushStatus = ( + window.electron.onAppUpdateStatusChange as jest.Mock + ).mock.calls[0][0] as ( + status: ElectronBridgeAppUpdateStatus + ) => void; + + const install = facade.installAppUpdate(); + + pushStatus({ + ...DEFAULT_APP_UPDATE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( + 1 + ); + + resolveInstall({ + ...DEFAULT_APP_UPDATE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + await install; + + expect(unloadGuard.suspendForAppQuit).toHaveBeenCalledTimes(1); + // A later push must find nothing left to resume. + pushStatus({ + ...DEFAULT_APP_UPDATE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes( + 1 + ); + }); + it('opens the manual release URL from unsupported update status', () => { const openSpy = jest.spyOn(window, 'open').mockReturnValue(null); facade.status.set({ diff --git a/apps/web/src/app/settings/settings-app-update.facade.ts b/apps/web/src/app/settings/settings-app-update.facade.ts index 81e09f27e..3a6cdf22c 100644 --- a/apps/web/src/app/settings/settings-app-update.facade.ts +++ b/apps/web/src/app/settings/settings-app-update.facade.ts @@ -78,30 +78,39 @@ export class SettingsAppUpdateFacade { // strand the install. A 'downloaded' reply means quitAndInstall ran // and the app is going down; anything else — including a rejected // IPC — means no quit happened, so the protection comes back. + // electron-updater can also report the failure as a later 'error' + // status push instead (bindStatusEvents handles that). this.unloadGuard.suspendForAppQuit(); + // Armed before the IPC round trip: the error push can beat the + // invoke reply, and the status listener must already see the + // pending quit to resume the guard. The 'downloaded' reply + // deliberately does not re-arm it — if the push won the race, + // re-arming would re-suspend a protection the failure just + // restored. + this.installQuitPending = true; try { const status = await window.electron.installAppUpdate(); this.status.set(status); if ( - status?.status === + status?.status !== ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded ) { - // quitAndInstall ran, but electron-updater reports its - // failures as a later 'error' status push rather than here — - // the status stream (bindStatusEvents) resumes the guard if - // that happens instead of a quit. - this.installQuitPending = true; - } else { - this.unloadGuard.resumeAfterAbortedAppQuit(); + this.abortInstallQuit(); } } catch (error) { - this.unloadGuard.resumeAfterAbortedAppQuit(); + this.abortInstallQuit(); throw error; } } + /** The promised quit is not happening: restore the unload protection. */ + private abortInstallQuit(): void { + this.installQuitPending = false; + this.unloadGuard.resumeAfterAbortedAppQuit(); + } + openManualAppUpdate(): void { const manualDownloadUrl = this.status()?.manualDownloadUrl; @@ -206,8 +215,7 @@ export class SettingsAppUpdateFacade { status.status !== ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded ) { - this.installQuitPending = false; - this.unloadGuard.resumeAfterAbortedAppQuit(); + this.abortInstallQuit(); } } );