diff --git a/src/stores/__tests__/auth-store-add-account-outbox.test.ts b/src/stores/__tests__/auth-store-add-account-outbox.test.ts index 38dc15b9..ffb1b8dd 100644 --- a/src/stores/__tests__/auth-store-add-account-outbox.test.ts +++ b/src/stores/__tests__/auth-store-add-account-outbox.test.ts @@ -50,6 +50,8 @@ const { client, SessionSupersededError } = vi.hoisted(() => { connectGate: Promise.resolve() as Promise, connectError: null as Error | null, loadGate: Promise.resolve() as Promise, + // loadAccount() fails for these, as for a server that does not answer. + loadErrors: {} as Record, requestGate: null as Promise | null, emailSets: [] as Array<{ served: string; args: Record }>, }, @@ -88,6 +90,7 @@ vi.mock('../../api/jmap-client', () => ({ client.served = null; await client.loadGate; if (epoch !== client.epoch) throw new SessionSupersededError(); + if (client.loadErrors[accountId]) throw client.loadErrors[accountId]; client.served = accountId; return true; }), @@ -186,6 +189,7 @@ beforeEach(async () => { client.connectGate = Promise.resolve(); client.connectError = null; client.loadGate = Promise.resolve(); + client.loadErrors = {}; client.requestGate = null; client.emailSets.length = 0; const entry = (id: string, username: string) => ({ @@ -320,4 +324,56 @@ describe('adding an account while the one in use has queued edits (Stalwart 1.0) expect(useEmailStore.getState().activeAccountId).toBe(B); expect(useOutboxStore.getState()).toMatchObject({ activeAccountId: B, resumed: true }); }); + + // The sign-in the switch overtook gives up. A switch that failed put back + // the client the sign-in held, which served no account before C's session + // was up and C after: A's screens stayed, with every call refused, or + // reaching C. + it.each([ + ['before the new session is up', 'connect'], + ['while the new account\'s server is checked', 'check'], + ] as const)('puts the account in use back to work when that switch fails %s', async (_label, phase) => { + await queueForA(queued('m1')); + client.loadErrors = { [B]: new Error('Server unreachable') }; + const connecting = deferred(); + if (phase === 'connect') client.connectGate = connecting.promise; + const checked = phase === 'check' ? holdServerCheck(C) : () => undefined; + + const login = addC(); + await settle(); + expect(client.served).toBe(phase === 'check' ? C : null); + await useAuthStore.getState().switchAccount(B); + connecting.open(); + checked(); + await expect(login).rejects.toThrow('Another account was opened'); + await settle(); + + expect(client.served).toBe(A); + expect(useAuthStore.getState()).toMatchObject({ activeAccountId: A, isAuthenticated: true }); + expect(useEmailStore.getState().activeAccountId).toBe(A); + expect(useOutboxStore.getState()).toMatchObject({ activeAccountId: A, resumed: true }); + // A's queue went to A, under A's id. + expect(client.emailSets.map((s) => [s.served, s.args.accountId])).toEqual([[A, 'd']]); + expect(await storedQueue(A)).toEqual([]); + }); + + it('falls back on the account a switch landed on once it overtook the sign-in, not on the one the sign-in saved', async () => { + client.loadErrors = { [A]: new Error('Server unreachable') }; + const connecting = deferred(); + client.connectGate = connecting.promise; + + const login = addC(); + await settle(); + await useAuthStore.getState().switchAccount(B); + // B is up; C's sign-in still waits on its server. + await useAuthStore.getState().switchAccount(A); + connecting.open(); + await expect(login).rejects.toThrow('Another account was opened'); + await settle(); + + expect(client.served).toBe(B); + expect(useAuthStore.getState().activeAccountId).toBe(B); + expect(useEmailStore.getState().activeAccountId).toBe(B); + expect(useOutboxStore.getState()).toMatchObject({ activeAccountId: B, resumed: true }); + }); }); diff --git a/src/stores/__tests__/auth-store-switch-outbox.test.ts b/src/stores/__tests__/auth-store-switch-outbox.test.ts index aa7f3e22..8b944276 100644 --- a/src/stores/__tests__/auth-store-switch-outbox.test.ts +++ b/src/stores/__tests__/auth-store-switch-outbox.test.ts @@ -400,4 +400,31 @@ describe('switches that overlap (Stalwart 1.0)', () => { expect((await editOnScreen()).queued).toBe(false); expect(sent()).toEqual([[A, 'd', ['x1']]]); }); + + // A cold start shows the last account's cached mail while its session + // loads, drawer and all. The restore a switch overtook gave up, and with + // the switch failing too, the app went to the sign-in screen. + it('lets the restore of a cold start land first, and puts it back when the switch fails', async () => { + client.served = null; + await useOutboxStore.getState().setAccount(null); + useEmailStore.setState({ activeAccountId: null, accountSnapshots: {} }); + useAuthStore.setState({ isAuthenticated: false, hasRestoredSession: false, session: null, activeAccountId: null, accountId: null }); + const restoringA = deferred(); + client.gates = { [A]: restoringA.promise }; + client.loadErrors = { [B]: new Error('Server unreachable') }; + + const restoring = useAuthStore.getState().restoreSession(); + await settle(); + const switchingToB = useAuthStore.getState().switchAccount(B); + await settle(); + restoringA.open(); + expect(await restoring).toBe(true); + await switchingToB; + await settle(); + + expect(useAuthStore.getState()).toMatchObject({ isAuthenticated: true, hasRestoredSession: true, error: 'Server unreachable' }); + expect(where()).toEqual(on(A)); + expect((await editOnScreen()).queued).toBe(false); + expect(sent()).toEqual([[A, 'd', ['x1']]]); + }); }); diff --git a/src/stores/auth-store.ts b/src/stores/auth-store.ts index 36543510..af1a2cd8 100644 --- a/src/stores/auth-store.ts +++ b/src/stores/auth-store.ts @@ -192,6 +192,21 @@ let pendingSwitch: { origin: SwitchOrigin; settled: Promise } | null = nul // client from one, nor from a switch. let signInsUnderWay = 0; +// The connection an add-account sign-in under way saved: the account in use, +// which it puts back should it fail. From the moment the sign-in begins, the +// client serves the account being added, or none, so a switch that overtakes +// the sign-in falls back on this connection too. It took a snapshot of the +// client instead, and when it failed as well, the sign-in it overtook had +// given up: the app stayed on the account in use while the client served no +// account, or the one being added, under its name. +let signInSaved: { client: ClientSnapshot; isCurrent: () => boolean } | null = null; + +// Set while a cold start brings the last account up. The main screens show +// its cached mail meanwhile, so a switch can begin; it waits for the restore. +// A restore that a switch overtook gave up, and with the switch failing too, +// the app went to the sign-in screen with every account still registered. +let restoreUnderWay: Promise | null = null; + async function switchesSettled(): Promise { while (pendingSwitch) await pendingSwitch.settled; } @@ -217,12 +232,14 @@ async function signIn( connect: () => Promise, ): Promise { signInsUnderWay += 1; + let saved: typeof signInSaved = null; try { // A switch under way lands first, so there is one account to put back. await switchesSettled(); const isCurrent = beginAccountChange(); const previous = opts?.addAccount && get().isAuthenticated ? jmapClient.snapshot() : null; const resumable = previous ? useOutboxStore.getState().suspend() : null; + if (previous) signInSaved = saved = { client: previous, isCurrent }; const putBack = () => { // Not over a newer account change, which has the client now. if (!previous || !isCurrent()) return; @@ -271,6 +288,7 @@ async function signIn( return signedIn; } finally { signInsUnderWay -= 1; + if (signInSaved === saved) signInSaved = null; } } @@ -642,6 +660,9 @@ export const useAuthStore = create((set, get) => ({ }, switchAccount: async (accountId) => { + // A cold start's restore lands first, so there is an account to put back + // (see restoreUnderWay). It waits for nothing a switch holds. + while (restoreUnderWay) await restoreUnderWay; const underWay = pendingSwitch; if (get().activeAccountId === accountId && !underWay) return; @@ -649,17 +670,20 @@ export const useAuthStore = create((set, get) => ({ const target = accountStore.getAccountById(accountId); if (!target) return; + // Read before this switch overtakes it (see signInSaved). + const signingIn = signInSaved?.isCurrent() ? signInSaved : null; // Once this one's awaits return, a newer account change may own the // client and the stores (see beginAccountChange). const isCurrent = beginAccountChange(); // loadAccount overwrites the client's credentials/session; keep the live // connection around so a failed switch can put it back instead of // leaving the previous account dead until relaunch. With switches under - // way, that is the account the first of them left. + // way, that is the account the first of them left; with an add-account + // sign-in under way, the connection it saved. const origin: SwitchOrigin = underWay?.origin ?? { accountId: get().activeAccountId, live: get().session !== null, - client: jmapClient.snapshot(), + client: signingIn?.client ?? jmapClient.snapshot(), }; let settle!: () => void; const mine = { origin, settled: new Promise((resolve) => { settle = resolve; }) }; @@ -776,6 +800,9 @@ export const useAuthStore = create((set, get) => ({ restoreSession: async () => { set({ isLoading: true }); + let restored!: () => void; + const restoring = new Promise((resolve) => { restored = resolve; }); + restoreUnderWay = restoring; try { // Wait for persisted caches to finish hydrating from AsyncStorage. // Otherwise we read empty defaults and bounce the user back to the @@ -886,6 +913,9 @@ export const useAuthStore = create((set, get) => ({ } catch { set({ isLoading: false, hasRestoredSession: true }); return false; + } finally { + if (restoreUnderWay === restoring) restoreUnderWay = null; + restored(); } },