From 2c5d95184e00c22979890857de53e22f3018e7e5 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:39:34 +0100 Subject: [PATCH] fix: wait for the saved session check instead of loading the account beside it when the network comes back at start --- src/stores/__tests__/auth-store.test.ts | 49 ++++++++++++++++++++++++ src/stores/auth-store.ts | 50 +++++++++++++++++++++++-- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/src/stores/__tests__/auth-store.test.ts b/src/stores/__tests__/auth-store.test.ts index 192d9f38..32f33420 100644 --- a/src/stores/__tests__/auth-store.test.ts +++ b/src/stores/__tests__/auth-store.test.ts @@ -391,6 +391,55 @@ describe('auth-store', () => { expect(useAccountStore.getState().getAccountById('acc-1')?.needsSignIn).toBeFalsy(); }); + // The network coming back (App.tsx) retries the session while the + // start from the saved session is still checking it: the retry used to + // load the account beside the check, and check the server while the + // start's reads were still landing. + it('waits for the check of the saved session when the network comes back during it', async () => { + registerAccount(); + mockLoadFromCache.mockResolvedValueOnce(true); + const check = deferred(); + mockRevalidate.mockReturnValueOnce(check.promise); + await useAuthStore.getState().restoreSession(); + + let retried: boolean | undefined; + const retry = useAuthStore.getState().retrySession().then((ok) => { retried = ok; }); + await new Promise((r) => setTimeout(r, 10)); + expect(mockLoadAccount).not.toHaveBeenCalled(); + expect(retried).toBeUndefined(); + + check.resolve({ apiUrl: 'https://mail.example.com/jmap/' }); + await retry; + expect(retried).toBe(true); + expect(useAuthStore.getState().session).not.toBeNull(); + expect(mockLoadAccount).not.toHaveBeenCalled(); + expect(mockRevalidate).toHaveBeenCalledTimes(1); + expect((resume.mock.calls as unknown[][]).filter(([id]) => id === 'acc-1')).toHaveLength(1); + }); + + it('loads the account once the check of the saved session failed without network', async () => { + registerAccount(); + mockLoadFromCache.mockResolvedValueOnce(true); + const check = deferred(); + mockRevalidate.mockReturnValueOnce(check.promise); + await useAuthStore.getState().restoreSession(); + + mockLoadAccount.mockResolvedValueOnce(true); + const retry = useAuthStore.getState().retrySession(); + await new Promise((r) => setTimeout(r, 10)); + expect(mockLoadAccount).not.toHaveBeenCalled(); + check.reject(new NetworkError('offline')); + + expect(await retry).toBe(true); + expect(mockLoadAccount).toHaveBeenCalledTimes(1); + expect(useAuthStore.getState().session).not.toBeNull(); + // The start is confirmed: a failed switch away does not check it again. + mockLoadAccount.mockRejectedValueOnce(new NetworkError('offline')); + useAccountStore.setState({ accounts: [...useAccountStore.getState().accounts, account('acc-2', false)] }); + await useAuthStore.getState().switchAccount('acc-2'); + expect(mockRevalidate).toHaveBeenCalledTimes(1); + }); + // A switch that fails puts the account back. Its saved session was // never confirmed (the switch overtook the check), so the check runs // again, and the outbox waits for it. diff --git a/src/stores/auth-store.ts b/src/stores/auth-store.ts index 38d50fad..cc31ab97 100644 --- a/src/stores/auth-store.ts +++ b/src/stores/auth-store.ts @@ -323,6 +323,26 @@ let restoreUnderWay: Promise | null = null; // connect, until the server confirms it (revalidateRestoredSession). Its // session is not live yet: its server was not checked and its outbox waits. let unconfirmedStart: string | null = null; +/** The reads that start sent with the saved session. */ +let unconfirmedStartReads: { accountId: string; promise: Promise } | null = null; +/** + * The check of the saved session under way. A session retry (the network + * coming back) during a start from the saved session waits for it instead of + * loading the account beside it. + */ +let revalidation: { accountId: string; promise: Promise } | null = null; + +function readsOfStart(accountId: string): Promise | undefined { + return unconfirmedStartReads?.accountId === accountId ? unconfirmedStartReads.promise : undefined; +} + +function startRevalidation(accountId: string, reads?: Promise): Promise { + const promise: Promise = revalidateRestoredSession(accountId, reads).finally(() => { + if (revalidation?.promise === promise) revalidation = null; + }); + revalidation = { accountId, promise }; + return promise; +} async function switchesSettled(): Promise { while (pendingSwitch) await pendingSwitch.settled; @@ -627,6 +647,7 @@ function applyConnectedState( // A session from the server, or a start from the saved one, which sets // this again right after. unconfirmedStart = null; + unconfirmedStartReads = null; } export const useAuthStore = create((set, get) => ({ @@ -1057,7 +1078,7 @@ export const useAuthStore = create((set, get) => ({ // still in the snapshot taken then, and its server was checked. jmapClient.restoreSnapshot(origin.client); if (origin.live) refetchFeatureStores(accountId); - else if (unconfirmedStart === accountId) void revalidateRestoredSession(accountId); + else if (unconfirmedStart === accountId) void startRevalidation(accountId); set({ isLoading: false }); return; } @@ -1072,7 +1093,7 @@ export const useAuthStore = create((set, get) => ({ if (origin.accountId && origin.live) resumeOutbox(origin.accountId); // A start from the saved session whose check this switch overtook. else if (origin.accountId && unconfirmedStart === origin.accountId) { - void revalidateRestoredSession(origin.accountId); + void startRevalidation(origin.accountId); } }; try { @@ -1245,7 +1266,8 @@ export const useAuthStore = create((set, get) => ({ unconfirmedStart = target.id; perfMark('restore-done', { cached: true }); const reads = reloadFeatureStores(); - void revalidateRestoredSession(target.id, reads); + unconfirmedStartReads = { accountId: target.id, promise: reads }; + void startRevalidation(target.id, reads); void syncAccountDisplayName(target.id); return true; } @@ -1358,6 +1380,20 @@ export const useAuthStore = create((set, get) => ({ if (!target) return false; // The server refused its sign-in: only a new one brings it back. if (target.needsSignIn) return false; + // A start from the saved session: its check brings the session up, after + // the reads that start sent. Loading the account beside it checked the + // server while those reads were still landing. Only a check that ended + // without network leaves the load below to try. + if (unconfirmedStart === activeAccountId) { + const running = revalidation?.accountId === activeAccountId + ? revalidation.promise + : startRevalidation(activeAccountId, readsOfStart(activeAccountId)); + await running; + if (!isCurrent() || get().activeAccountId !== activeAccountId) return false; + if (get().session) return true; + if (get().reauth || unconfirmedStart === activeAccountId) return false; + } + const startReads = readsOfStart(activeAccountId); try { const ok = await jmapClient.loadAccount(activeAccountId); if (!ok || !isCurrent()) return false; @@ -1369,6 +1405,13 @@ export const useAuthStore = create((set, get) => ({ }); const fresh = jmapClient.currentSession!; followStoredServer(activeAccountId); + if (startReads) { + // What the start read from the saved session lands before a server + // that was replaced drops the caches. + const recorded = await readServerFingerprint(activeAccountId).catch(() => null); + if (recorded !== sessionFingerprint(fresh)) await startReads; + if (!isCurrent()) return false; + } await checkServerChange(activeAccountId, fresh); if (!isCurrent()) return false; applyConnectedState(set, fresh, target.serverUrl, target.username, activeAccountId); @@ -1449,6 +1492,7 @@ async function revalidateRestoredSession(accountId: string, reads?: Promise