fix(settings): survive updater error events and cross-intent close requests

electron-updater reports quitAndInstall failures as a later 'error'
event rather than a synchronous throw (BaseUpdater catches internally;
MacUpdater fails after returning), so the revocation now rides the
error path: handleError takes back the prepared close-guard bypass, and
the renderer facade resumes the unload guard when an error status push
follows a quitting install.

A close or quit requested while the reload confirmation dialog is open
now escalates the active intent instead of being dropped: Save/Discard
completes the close the user most recently asked for, and Stay cancels
the intent the main process remembered.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5 committed 2026-08-09 11:51:47 +02:00
1 parent fd29a8ce92
commit ea12b06480
6 files changed
+178 -9

No files matched your search

@@ -506,4 +506,32 @@ describe('AppUpdateService', () => {
expect(cancelPreparedQuit).toHaveBeenCalledTimes(1);
expect(status.status).toBe(ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error);
});
it('takes the bypass back when the updater emits the failure as an error event', () => {
// electron-updater's BaseUpdater catches synchronous install
// failures internally and emits 'error' instead of throwing, and
// MacUpdater can fail after quitAndInstall already returned — the
// revocation must ride the error path.
const cancelPreparedQuit = jest.fn();
const { service } = createService({ cancelPreparedQuit });
service.handleUpdateAvailable({ version: '0.23.0' });
service.handleUpdateDownloaded({ version: '0.23.0' });
service.installUpdate();
expect(cancelPreparedQuit).not.toHaveBeenCalled();
// What attachUpdaterEvents forwards from the updater 'error' event.
service.handleError(new Error('ShipIt failed'));
expect(cancelPreparedQuit).toHaveBeenCalledTimes(1);
});
it('does not revoke a bypass for errors unrelated to an install', () => {
const cancelPreparedQuit = jest.fn();
const { service } = createService({ cancelPreparedQuit });
service.handleError(new Error('check failed'));
expect(cancelPreparedQuit).not.toHaveBeenCalled();
});
});
@@ -253,6 +253,8 @@ export class AppUpdateService {
private checkForUpdatesPromise: Promise<ElectronBridgeAppUpdateStatus> | null =
null;
private status: ElectronBridgeAppUpdateStatus;
/** True between prepareQuit() and the quit — or the error that voids it. */
private quitPreparationPending = false;
constructor(private readonly options: AppUpdateServiceOptions) {
this.currentVersion = resolveCurrentVersion(
@@ -414,15 +416,17 @@ export class AppUpdateService {
this.status.status ===
ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded
) {
// Consumed by handleError: electron-updater's BaseUpdater
// catches its own synchronous install failures and emits
// 'error' instead of throwing, and MacUpdater can return here
// and fail asynchronously — the revocation must ride the error
// path, not a try/catch around this call.
this.quitPreparationPending = true;
this.options.prepareQuit?.();
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);
}
}
@@ -471,6 +475,14 @@ export class AppUpdateService {
}
handleError(error: unknown): void {
// An error after a prepared quit means no quit is coming: take back
// the one-shot close-guard bypass, whether the failure was thrown
// synchronously or emitted later as an updater 'error' event.
if (this.quitPreparationPending) {
this.quitPreparationPending = false;
this.options.cancelPreparedQuit?.();
}
this.setStatus({
error: normalizeError(error),
status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error,
@@ -195,6 +195,41 @@ describe('SettingsAppUpdateFacade', () => {
);
});
it('restores the unload guard when an error push follows a quitting install', async () => {
// electron-updater reports install failures as a later 'error'
// status event, not as a rejection of the install IPC — the app is
// then still running with the protection suspended.
const unloadGuard = TestBed.inject(SettingsUnloadGuardService);
(window.electron.installAppUpdate as jest.Mock).mockResolvedValue({
...DEFAULT_APP_UPDATE_STATUS,
status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded,
});
facade.init();
const pushStatus = (
window.electron.onAppUpdateStatusChange as jest.Mock
).mock.calls[0][0] as (
status: ElectronBridgeAppUpdateStatus
) => void;
await facade.installAppUpdate();
expect(unloadGuard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled();
pushStatus({
...DEFAULT_APP_UPDATE_STATUS,
status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error,
});
expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(
1
);
// The resume is one-shot: later unrelated pushes change nothing.
pushStatus(DEFAULT_APP_UPDATE_STATUS);
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({
@@ -37,6 +37,8 @@ export class SettingsAppUpdateFacade {
readonly updateMessage = signal('');
private unsubscribeStatus: (() => void) | null = null;
/** True after an install reply promised a quit that has not happened. */
private installQuitPending = false;
/** Subscribes to status pushes and kicks off the initial status load */
init(): void {
@@ -83,9 +85,15 @@ export class SettingsAppUpdateFacade {
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();
}
} catch (error) {
@@ -189,6 +197,18 @@ export class SettingsAppUpdateFacade {
this.unsubscribeStatus = window.electron.onAppUpdateStatusChange(
(status) => {
this.status.set(status);
// Any post-install status other than the quitting install's
// own proves the app is still running — the promised quit is
// not coming, so the unload protection returns.
if (
this.installQuitPending &&
status.status !==
ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded
) {
this.installQuitPending = false;
this.unloadGuard.resumeAfterAbortedAppQuit();
}
}
);
}
@@ -227,6 +227,56 @@ describe('SettingsUnloadGuardService', () => {
expect(electronStub.cancelWindowClose).toHaveBeenCalledTimes(1);
});
it('escalates an open reload confirmation into the requested close', async () => {
let resolveConfirm: ((value: boolean) => void) | null = null;
host.confirmClose.mockImplementation(
() =>
new Promise<boolean>((resolve) => {
resolveConfirm = resolve;
})
);
const reloadPage = jest.fn();
activateInElectron();
service.reloadPage = reloadPage;
form.markAsDirty();
// Reload confirmation opens first...
dispatchBeforeUnload();
await flushAsyncWork();
expect(host.confirmClose).toHaveBeenCalledTimes(1);
// ...then the user closes the window while it is on screen.
closeRequestCallback?.();
resolveConfirm?.(true);
await flushAsyncWork();
// Save/Discard completes the close — not the stale reload.
expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1);
expect(reloadPage).not.toHaveBeenCalled();
});
it('cancels the escalated close when the user stays', async () => {
let resolveConfirm: ((value: boolean) => void) | null = null;
host.confirmClose.mockImplementation(
() =>
new Promise<boolean>((resolve) => {
resolveConfirm = resolve;
})
);
activateInElectron();
form.markAsDirty();
dispatchBeforeUnload();
await flushAsyncWork();
closeRequestCallback?.();
resolveConfirm?.(false);
await flushAsyncWork();
// The main process remembered a close; staying must clear it.
expect(electronStub.cancelWindowClose).toHaveBeenCalledTimes(1);
expect(electronStub.confirmWindowClose).not.toHaveBeenCalled();
});
it('ignores repeated close requests while a dialog is open', async () => {
let resolveConfirm: ((value: boolean) => void) | null = null;
host.confirmClose.mockImplementation(
@@ -41,6 +41,13 @@ export class SettingsUnloadGuardService implements OnDestroy {
private dirtySubscription: Subscription | null = null;
private unsubscribeCloseRequests: (() => void) | null = null;
private confirmationPending = false;
/**
* Intent of the confirmation currently on screen. A close request
* arriving while a reload confirmation is open escalates this to
* 'close' — the dialog is the same, only the continuation differs, and
* the user's most recent ask must be the one that completes.
*/
private activeIntent: 'close' | 'reload' | null = null;
/** Last guard state mirrored to the main process. */
private guardArmed = false;
/** True while an updater-driven app quit must pass unchallenged. */
@@ -144,15 +151,31 @@ export class SettingsUnloadGuardService implements OnDestroy {
): Promise<void> {
const host = this.host;
if (!host || this.confirmationPending) {
if (!host) {
return;
}
if (this.confirmationPending) {
// A close outranks a reload, never the other way around: the
// open dialog stays, but Save/Discard must complete the close
// the user just asked for, not the earlier reload.
if (intent === 'close') {
this.activeIntent = 'close';
}
return;
}
this.confirmationPending = true;
this.activeIntent = intent;
try {
if (!(await host.confirmClose())) {
if (intent === 'close') {
const proceed = await host.confirmClose();
// Re-read after the dialog: a cross-intent request may have
// escalated it while the user was deciding.
const finalIntent = this.activeIntent ?? intent;
if (!proceed) {
if (finalIntent === 'close') {
// Staying must clear the intent the main process
// remembered, or a later close attempt would replay a
// stale quit.
@@ -161,13 +184,14 @@ export class SettingsUnloadGuardService implements OnDestroy {
return;
}
if (intent === 'close') {
if (finalIntent === 'close') {
await window.electron?.confirmWindowClose?.();
} else {
this.reloadPage();
}
} finally {
this.confirmationPending = false;
this.activeIntent = null;
}
}