From c2bce09d48247ad8e37e050289f6c10f067e2db5 Mon Sep 17 00:00:00 2001 From: Zeus-Deus Date: Tue, 25 Aug 2026 21:11:49 +0200 Subject: [PATCH] fix(desktop): recover failed gateway switch setup --- apps/desktop/src/lib/with-timeout.test.ts | 24 ++++++++++ apps/desktop/src/lib/with-timeout.ts | 12 ++++- apps/desktop/src/store/gateway-switch.test.ts | 46 +++++++++++++++++++ apps/desktop/src/store/gateway-switch.ts | 34 ++++++++++++-- 4 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 apps/desktop/src/lib/with-timeout.test.ts diff --git a/apps/desktop/src/lib/with-timeout.test.ts b/apps/desktop/src/lib/with-timeout.test.ts new file mode 100644 index 0000000000..f756a2e3e3 --- /dev/null +++ b/apps/desktop/src/lib/with-timeout.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it, vi } from 'vitest' + +import { withTimeout } from './with-timeout' + +describe('withTimeout', () => { + it('rejects with an onTimeout exception instead of letting it escape the timer callback', async () => { + vi.useFakeTimers() + + try { + const callbackFailure = new Error('abort callback failed') + + const result = withTimeout(new Promise(() => undefined), 10, 'work timed out', () => { + throw callbackFailure + }) + + const rejection = expect(result).rejects.toBe(callbackFailure) + + await vi.advanceTimersByTimeAsync(10) + await rejection + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/apps/desktop/src/lib/with-timeout.ts b/apps/desktop/src/lib/with-timeout.ts index 83b3e035bc..fda453503b 100644 --- a/apps/desktop/src/lib/with-timeout.ts +++ b/apps/desktop/src/lib/with-timeout.ts @@ -13,7 +13,8 @@ export function isTimeoutError(error: unknown): error is TimeoutError { /** Settle with `promise`, or reject with a TimeoutError after `ms`. * `onTimeout` runs synchronously before the rejection is published so callers - * can revoke ownership of work that would otherwise keep running unowned. */ + * can revoke ownership of work that would otherwise keep running unowned. If + * that callback throws, its error becomes this promise's rejection. */ export function withTimeout( promise: Promise, ms: number, @@ -24,7 +25,14 @@ export function withTimeout( const timer = setTimeout(() => { const error = new TimeoutError(message) - onTimeout?.(error) + try { + onTimeout?.(error) + } catch (onTimeoutError) { + reject(onTimeoutError) + + return + } + reject(error) }, ms) diff --git a/apps/desktop/src/store/gateway-switch.test.ts b/apps/desktop/src/store/gateway-switch.test.ts index a6a67f7157..fb891a7ab2 100644 --- a/apps/desktop/src/store/gateway-switch.test.ts +++ b/apps/desktop/src/store/gateway-switch.test.ts @@ -132,6 +132,52 @@ describe('beginGatewaySwitch / endGatewaySwitch — the shared switch commit poi off() }) + it('tears down its barrier when the registered lifecycle throws before the wipe', () => { + const failure = new Error('machine-context reset failed') + + const off = registerGatewaySwitchLifecycle({ + beforeConnectionSwitch: () => { + throw failure + }, + refreshSessions: vi.fn(async () => undefined) + }) + + expect(() => beginGatewaySwitch()).toThrow(failure) + expect($gatewaySwitching.get()).toBe(false) + // No wipe started, so the still-active source remains intact and needs no + // repaint. A later switch can acquire and release barrier ownership. + expect($activeSessionId.get()).toBe('a93bb39d') + expect($sessions.get()).toHaveLength(1) + + off() + const next = beginGatewaySwitch() + expect($gatewaySwitching.get()).toBe(true) + endGatewaySwitch(next) + expect($gatewaySwitching.get()).toBe(false) + }) + + it('recovers the active source and tears down its barrier when the wipe throws partway through', async () => { + const failure = new Error('profile fetch invalidation failed') + const refreshSessions = vi.fn(async () => undefined) + const off = registerGatewaySwitchLifecycle({ beforeConnectionSwitch: () => undefined, refreshSessions }) + + vi.mocked(invalidateProfileListFetches).mockImplementationOnce(() => { + throw failure + }) + + expect(() => beginGatewaySwitch()).toThrow(failure) + expect($gatewaySwitching.get()).toBe(false) + await vi.waitFor(() => expect(refreshSessions).toHaveBeenCalledTimes(1)) + expect($sessionsLoading.get()).toBe(false) + + // The failed token cannot strand or lower ownership acquired afterwards. + const next = beginGatewaySwitch() + expect($gatewaySwitching.get()).toBe(true) + endGatewaySwitch(next) + expect($gatewaySwitching.get()).toBe(false) + off() + }) + it('the barrier belongs to the LATEST switch: an older switch ending mid-commit of a newer one is a no-op', () => { const older = beginGatewaySwitch() const newer = beginGatewaySwitch() diff --git a/apps/desktop/src/store/gateway-switch.ts b/apps/desktop/src/store/gateway-switch.ts index 3348274cc0..cd4556687d 100644 --- a/apps/desktop/src/store/gateway-switch.ts +++ b/apps/desktop/src/store/gateway-switch.ts @@ -78,11 +78,37 @@ export function registerGatewaySwitchLifecycle(lifecycle: GatewaySwitchLifecycle */ export function beginGatewaySwitch(): GatewaySwitchToken { const token = ++latestSwitchToken - $gatewaySwitching.set(true) - switchLifecycle?.beforeConnectionSwitch() - wipeSessionListsForGatewaySwitch() + let wipeStarted = false - return token + $gatewaySwitching.set(true) + + try { + switchLifecycle?.beforeConnectionSwitch() + wipeStarted = true + wipeSessionListsForGatewaySwitch() + + return token + } catch (error) { + // No caller received this token, so begin owns cleanup. Token-aware teardown + // preserves a newer recursively-started switch, if lifecycle code began one. + const stillOwnsSwitch = token === latestSwitchToken + + endGatewaySwitch(token) + + // A synchronous wipe has no rollback: once it starts, some outgoing-source + // stores may already be empty. Repaint the still-active source best-effort. + // A lifecycle failure happens before the wipe and leaves lists untouched. + // If a nested switch superseded this one, its owner is responsible instead. + if (wipeStarted && stillOwnsSwitch) { + try { + recoverActiveSourceAfterFailedGatewaySwitch() + } catch { + // Recovery must never replace the original commit failure. + } + } + + throw error + } } /**