mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-01 00:07:01 +00:00
fix: keep a working account when a switch fails during sign-in or start-up
A switch that began while an account was being added, or while a cold
start still brought the last account up, made that sign-in or restore
give up (efd31de). When the switch then failed as well, what it put
back was no working account:
- during an add-account sign-in, the switch fell back on a snapshot of
the client, which served no account before the new session was up
and the new account after. The app stayed on the account in use while
every call failed with "Not connected", the session retry saw a
session and returned, and tapping the account did nothing; or the
client served the new account under the one in use, whose calendars,
contacts and identities then came from it, and whose sends went
through it.
- a cold start shows the last account's cached mail, drawer and all,
while its session loads. A switch from there that failed left the
app on the sign-in screen, with every account still registered.
A switch now falls back on the connection an add-account sign-in under
way saved, and resumes the queue of offline edits the sign-in held, as
a failed sign-in does itself. It waits for a restore under way and
switches from the account restored. A sign-in waits for switches under
way and a restore for neither, so none of them waits on another.
This commit is contained in:
@@ -50,6 +50,8 @@ const { client, SessionSupersededError } = vi.hoisted(() => {
|
||||
connectGate: Promise.resolve() as Promise<unknown>,
|
||||
connectError: null as Error | null,
|
||||
loadGate: Promise.resolve() as Promise<unknown>,
|
||||
// loadAccount() fails for these, as for a server that does not answer.
|
||||
loadErrors: {} as Record<string, Error>,
|
||||
requestGate: null as Promise<unknown> | null,
|
||||
emailSets: [] as Array<{ served: string; args: Record<string, unknown> }>,
|
||||
},
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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']]]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -192,6 +192,21 @@ let pendingSwitch: { origin: SwitchOrigin; settled: Promise<void> } | 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<void> | null = null;
|
||||
|
||||
async function switchesSettled(): Promise<void> {
|
||||
while (pendingSwitch) await pendingSwitch.settled;
|
||||
}
|
||||
@@ -217,12 +232,14 @@ async function signIn(
|
||||
connect: () => Promise<SignedIn>,
|
||||
): Promise<SignedIn> {
|
||||
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<AuthState>((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<AuthState>((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<void>((resolve) => { settle = resolve; }) };
|
||||
@@ -776,6 +800,9 @@ export const useAuthStore = create<AuthState>((set, get) => ({
|
||||
|
||||
restoreSession: async () => {
|
||||
set({ isLoading: true });
|
||||
let restored!: () => void;
|
||||
const restoring = new Promise<void>((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<AuthState>((set, get) => ({
|
||||
} catch {
|
||||
set({ isLoading: false, hasRestoredSession: true });
|
||||
return false;
|
||||
} finally {
|
||||
if (restoreUnderWay === restoring) restoreUnderWay = null;
|
||||
restored();
|
||||
}
|
||||
},
|
||||
|
||||
|
||||
Reference in New Issue
Block a user