fix(settings): restore unload protection when an update install aborts

Two leak paths after a failed quitAndInstall: a rejected install IPC
returned before the renderer could re-arm its unload guard, and a
synchronous updater failure left the main-process one-shot close bypass
armed — the next genuine close then skipped the guard and the restored
beforeunload path treated it as a reload. The facade now resumes the
guard on the rejection path too, and installUpdate() revokes the
prepared bypass (cancelPreparedQuit -> revokeAllowedClose) and reports
the failure as an error status instead of letting it escape as an IPC
rejection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5 committed 2026-08-09 11:40:49 +02:00
1 parent 18093b0850
commit fd29a8ce92
7 files changed
+90 -8

No files matched your search

@@ -83,6 +83,7 @@ function createService(
platform?: NodeJS.Platform;
env?: NodeJS.ProcessEnv;
prepareQuit?: () => void;
cancelPreparedQuit?: () => void;
} = {}
) {
const updater = new FakeUpdater();
@@ -93,6 +94,7 @@ function createService(
isPackaged: overrides.isPackaged ?? true,
},
getMainWindow: () => win,
cancelPreparedQuit: overrides.cancelPreparedQuit,
platform: overrides.platform ?? 'darwin',
prepareQuit: overrides.prepareQuit,
processEnv: overrides.env ?? {},
@@ -480,4 +482,28 @@ describe('AppUpdateService', () => {
expect(prepareQuit).not.toHaveBeenCalled();
expect(updater.quitAndInstall).not.toHaveBeenCalled();
});
it('takes the close-guard bypass back when quitAndInstall fails', () => {
// A synchronous updater failure means no quit is coming: the
// prepared one-shot bypass must not leak into the next genuine
// close, and the renderer must see a non-Downloaded status so it
// restores its own unload guard.
const cancelPreparedQuit = jest.fn();
const prepareQuit = jest.fn();
const { service, updater } = createService({
cancelPreparedQuit,
prepareQuit,
});
updater.quitAndInstall.mockImplementation(() => {
throw new Error('spawn failed');
});
service.handleUpdateAvailable({ version: '0.23.0' });
service.handleUpdateDownloaded({ version: '0.23.0' });
const status = service.installUpdate();
expect(prepareQuit).toHaveBeenCalledTimes(1);
expect(cancelPreparedQuit).toHaveBeenCalledTimes(1);
expect(status.status).toBe(ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error);
});
});
@@ -117,6 +117,12 @@ export interface AppUpdateServiceOptions {
* plain close — abandoning the requested install.
*/
prepareQuit?: () => void;
/**
* Undoes {@link prepareQuit} when `quitAndInstall()` failed synchronously
* and no quit is coming — the prepared one-shot close bypass must not
* leak into the next genuine close.
*/
cancelPreparedQuit?: () => void;
}
function isSelfUpdateSupported(
@@ -409,7 +415,16 @@ export class AppUpdateService {
ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded
) {
this.options.prepareQuit?.();
this.updater.quitAndInstall();
try {
this.updater.quitAndInstall();
} catch (error) {
// No quit is coming: take back the close-guard bypass and
// report the failure as a status the renderer can read —
// non-Downloaded also tells it to restore its own guard.
this.options.cancelPreparedQuit?.();
this.handleError(error);
}
}
return this.getStatus();
@@ -230,6 +230,17 @@ describe('WindowCloseGuard', () => {
expect(win.fireClose()).toBe(true);
});
it('revokes an unused close bypass when the prepared quit never starts', () => {
const { guard, win } = createArmedGuard();
guard.allowNextClose();
guard.revokeAllowedClose();
// The stale bypass must not let the next genuine close skip the
// guard.
expect(win.fireClose()).toBe(true);
});
it('confirms as a plain close when nothing was intercepted', () => {
const { app, guard, win } = createArmedGuard();
@@ -114,6 +114,15 @@ export class WindowCloseGuard {
this.bypassClose = true;
}
/**
* Revokes an unused {@link allowNextClose} when the quit it prepared
* never started (`quitAndInstall()` failed synchronously). Left armed,
* the stale bypass would let the next genuine close skip the guard.
*/
revokeAllowedClose(): void {
this.bypassClose = false;
}
/**
* Renderer verdict: safe to leave (settings saved or discarded). Replays
* the intercepted intent with the guard bypassed for exactly one close.
+1
View File
@@ -147,6 +147,7 @@ export default class Main {
// (macOS), so without this an armed close guard would intercept
// the install's window close and strand the update.
prepareQuit: () => windowCloseGuard.allowNextClose(),
cancelPreparedQuit: () => windowCloseGuard.revokeAllowedClose(),
});
AppUpdateEvents.bootstrapAppUpdateEvents(appUpdateService);
@@ -181,6 +181,20 @@ describe('SettingsAppUpdateFacade', () => {
);
});
it('restores the unload guard when the install IPC rejects', async () => {
const unloadGuard = TestBed.inject(SettingsUnloadGuardService);
const failure = new Error('ipc failed');
(window.electron.installAppUpdate as jest.Mock).mockRejectedValue(
failure
);
await expect(facade.installAppUpdate()).rejects.toThrow(failure);
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({
@@ -74,17 +74,23 @@ export class SettingsAppUpdateFacade {
// not fight a quit the user just asked for — its `beforeunload`
// would cancel the updater's window close at the DOM layer and
// strand the install. A 'downloaded' reply means quitAndInstall ran
// and the app is going down; anything else means no quit happened,
// so the protection comes back.
// and the app is going down; anything else — including a rejected
// IPC — means no quit happened, so the protection comes back.
this.unloadGuard.suspendForAppQuit();
const status = await window.electron.installAppUpdate();
this.status.set(status);
try {
const status = await window.electron.installAppUpdate();
this.status.set(status);
if (
status?.status !== ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded
) {
if (
status?.status !==
ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded
) {
this.unloadGuard.resumeAfterAbortedAppQuit();
}
} catch (error) {
this.unloadGuard.resumeAfterAbortedAppQuit();
throw error;
}
}