fix: say when a start stays on the local copy, queue its sends, paint it only with folders kept and fill the From list after the swap

This commit is contained in:
Linus Rath
2026-11-03 08:43:35 +01:00
parent 706550daf4
commit 55ca12adbd
9 changed files with 160 additions and 8 deletions
+2
View File
@@ -22,6 +22,8 @@ export interface OfflineClientRef {
getServerUrl(): string;
getUsername(): string;
getAccountId(): string;
/** Answers from this device only: a local copy on screen at a start. */
isOfflineOnly?(): boolean;
}
export interface OfflineHooks {
@@ -28,6 +28,7 @@ import { flushQueue, settleStaleSends, SENDING_STALE_MS } from '../flush';
import { claimForSending, holdFile, queueSend, readQueue } from '../queue';
import { forgetLegacyMove } from '../migrate-legacy';
import { consumeLost } from '../lost';
import { getOutboxHooks } from '../outbox-hooks';
import { shellFiles } from '../shell';
import { isOfflineNow, OFFLINE_ACCOUNTS_KEY, setServerDown, useOfflineStore, writeEnabled } from '../state';
@@ -283,6 +284,28 @@ describe('11: starting from the device', () => {
writeEnabled(['acc']);
expect(await offlineSessionFor('acc')).toMatchObject({ primaryAccounts: { 'urn:ietf:params:jmap:mail': JMAP_ACCOUNT } });
});
it('doesn\'t offer a quick start before the store has its folders', async () => {
store = new InProcessStore(new FakeJmapMail());
setStoreBackend(store);
writeEnabled(['acc']);
expect(await offlineSessionFor('acc', { withFolders: true })).toBeNull();
await store.sync('acc');
expect(await offlineSessionFor('acc', { withFolders: true })).toMatchObject({ primaryAccounts: { 'urn:ietf:params:jmap:mail': JMAP_ACCOUNT } });
});
});
describe('a send on the local copy', () => {
it('queues, before the swap too, while the probes say the server is up', () => {
writeEnabled(['acc']);
stop = startOfflineMode();
useAuthStore.setState({ client: fakeClient('https://mail.example.org', { isOfflineOnly: () => true }) as never });
const client = useAuthStore.getState().client as never;
expect(isOfflineNow('https://mail.example.org')).toBe(false);
expect(getOutboxHooks()?.shouldQueue(client)).toBe(true);
useAuthStore.setState({ client: fakeClient() as never });
expect(getOutboxHooks()?.shouldQueue(useAuthStore.getState().client as never)).toBe(false);
});
});
describe('12: one server down doesn’t hold another’s mail', () => {
+9 -3
View File
@@ -54,7 +54,7 @@ import {
type PolicyFingerprint,
} from './queue';
import { clearHandoffs } from '@/lib/launch-handoff';
import { clearEvicted, EVICTED_KEEP_MS, isEvicted, isOfflineNow, OFFLINE_ACCOUNTS_KEY, readEnabledFrom, readEvicted, visibleAccounts, serverKey, setServerDown, useOfflineStore, writeEnabled, type OutboxRow } from './state';
import { clearEvicted, EVICTED_KEEP_MS, forgetMailSnapshot, isEvicted, isOfflineNow, OFFLINE_ACCOUNTS_KEY, readEnabledFrom, readEvicted, visibleAccounts, serverKey, setServerDown, useOfflineStore, writeEnabled, type OutboxRow } from './state';
import { flushQueue, settleStaleSends } from './flush';
import { LEGACY_DB_NAME, moveLegacyOutbox } from './migrate-legacy';
import { deleteShellCaches, tellServiceWorker } from './shell';
@@ -160,6 +160,7 @@ const storeHooks: StorePageHooks = {
onEvent(event) {
if (event.type === 'synced') {
useOfflineStore.setState((s) => ({ lastSyncedAt: { ...s.lastSyncedAt, [event.account]: event.at } }));
forgetMailSnapshot(event.account);
void refreshUsage();
}
if (event.type === 'error' && event.area === STORAGE_ERROR_AREA) {
@@ -392,6 +393,8 @@ const outbox: OutboxHooks = {
shouldQueue(client) {
const account = accountForClient(client);
if (!account || !offlineActive(account)) return false;
// A local copy has no way to the server, whatever the probes last said.
if (client.isOfflineOnly?.()) return true;
return isOfflineNow(client.getServerUrl()) || (typeof navigator !== 'undefined' && navigator.onLine === false);
},
async queueEmail(client, args) {
@@ -981,7 +984,7 @@ export async function serverUnreachable(serverUrl: string): Promise<boolean> {
const SESSION_READ_TIMEOUT_MS = 5_000;
/** The session kept for an account, for starting from the local copy. */
export async function offlineSessionFor(account: string): Promise<unknown | null> {
export async function offlineSessionFor(account: string, options: { withFolders?: boolean } = {}): Promise<unknown | null> {
// The organisation's last word: the policy kept from the last fetch (the
// fetch itself fails without a network), never the defaults.
const policy = usePolicyStore.getState();
@@ -992,7 +995,10 @@ export async function offlineSessionFor(account: string): Promise<unknown | null
try {
await moveLegacyOutbox();
return await Promise.race([
getStore().area<unknown>(account, 'mail.session'),
(async () => {
if (options.withFolders && !(await getStore().area<boolean>(account, 'mail.hasFolders'))) return null;
return getStore().area<unknown>(account, 'mail.session');
})(),
new Promise<null>((resolve) => setTimeout(() => resolve(null), SESSION_READ_TIMEOUT_MS)),
]);
} catch {
+15
View File
@@ -119,6 +119,21 @@ export function visibleAccounts(): string[] {
return useOfflineStore.getState().enabled.filter((account) => !(account in evicted));
}
/**
* Drops the plain-text list kept in localStorage for a quick start (email-store)
* once the store can do that job. Until an account's first sync finishes it's
* what stays on screen, so turning the store on alone doesn't remove it.
*/
export function forgetMailSnapshot(account: string): void {
try {
const raw = localStorage.getItem('email-snapshot');
if (raw && (JSON.parse(raw) as { account?: unknown } | null)?.account !== account) return;
localStorage.removeItem('email-snapshot');
} catch {
localStorage.removeItem('email-snapshot');
}
}
export function serverKey(url: string): string {
return url.replace(/\/+$/, '').toLowerCase();
}
+26
View File
@@ -263,6 +263,32 @@ describe('Email/query filters and sorts', () => {
});
});
describe('a store without its mail yet', () => {
it('says so rather than answering an empty list, until the first sync has finished', async () => {
const server = new FakeJmapMail();
server.addEmail({ id: 'one' });
const { engine } = engineFor(server);
const inbox = Object.keys((server.emails.get('one')!.mailboxIds) as Record<string, boolean>)[0];
const query = () => one(engine, ['Email/query', { filter: { inMailbox: inbox } }, 'q']);
// No answer: the client reports it can't tell, and the list on screen stays.
expect(await query()).toBeUndefined();
await engine.requestSync();
expect((await query())![1].ids).toEqual(['one']);
// After a finished sync, an empty folder really is empty.
server.destroyEmail('one');
await engine.requestSync();
expect((await query())![1].ids).toEqual([]);
});
it('tells a start whether there are folders to show', async () => {
const server = new FakeJmapMail();
const { engine } = engineFor(server);
expect(await engine.areaCall('mail.hasFolders', {})).toBe(false);
await engine.requestSync();
expect(await engine.areaCall('mail.hasFolders', {})).toBe(true);
});
});
describe('keyword case, as measured on both Stalwart versions', () => {
it('keeps two keywords differing only in case, and sync doesn\'t stop on them', async () => {
const server = new FakeJmapMail();
+2
View File
@@ -59,4 +59,6 @@ export const MAIL_CALLS: Readonly<Record<string, AreaCall>> = {
'mail.usage': usage,
/** The session kept with the account, for a start from the local copy. */
'mail.session': (ctx) => ctx.session,
/** Whether a start may show the store: it has a folder list from the server. */
'mail.hasFolders': (ctx) => ctx.db.get("SELECT 1 AS yes FROM sync_state WHERE type = 'Mailbox' AND state IS NOT NULL") !== undefined,
};
+3
View File
@@ -317,6 +317,9 @@ const emailQuery: MethodHandler = (ctx, args) => {
).map((row) => String(row.id));
// Stalwart's queryState is the Email state. Absent until the first sync, as allowed.
const queryState = lastState(ctx, jmap, 'Email');
// Nothing here doesn't mean nothing there: an empty folder of a first sync
// that hasn't finished mustn't paint over what's on screen.
if (ids.length === 0 && position === 0 && !queryState) throw new MethodError('serverUnavailable', 'Mail not kept yet');
const result: MethodResult = {
accountId: jmap,
canCalculateChanges: false,
+63 -3
View File
@@ -7,11 +7,15 @@ const offline = vi.hoisted(() => ({
order: [] as string[],
session: { apiUrl: 'x' } as unknown,
gone: false,
ready: true,
// A local copy with no identities kept answers an empty list.
localIdentities: Promise.resolve([]) as Promise<unknown[]>,
releaseLocal: () => {},
}));
vi.mock('@/lib/offline/offline-mode', () => ({
registerOfflineHooks: () => { offline.order.push('hooks'); },
offlineSessionFor: async () => offline.session,
offlineSessionFor: async (_account: string, options?: { withFolders?: boolean }) => (options?.withFolders && !offline.ready ? null : offline.session),
serverUnreachable: async () => offline.gone,
markStartedOffline: () => { offline.order.push('startedOffline'); },
}));
@@ -33,7 +37,9 @@ vi.mock('@/lib/jmap/client', () => {
async connect() { offline.order.push('connect'); await connectGate; }
getSessionUsername() { return this.username; }
getUsername() { return this.username; }
async getIdentities() { return [{ id: 'a', email: this.username, name: this.username, mayDelete: false }]; }
async getIdentities() {
return this.offlineOnly ? offline.localIdentities : [{ id: 'a', email: this.username, name: this.username, mayDelete: false }];
}
getAuthHeader() { return 'Basic eA=='; }
onConnectionChange() {}
onRateLimit() {}
@@ -88,11 +94,13 @@ beforeEach(async () => {
offline.order.length = 0;
offline.session = { apiUrl: 'x' };
offline.gone = false;
offline.ready = true;
offline.localIdentities = Promise.resolve([]);
connectGate = Promise.resolve();
sessionAnswer = goodSession;
useAccountStore.setState({ accounts: [ACCOUNT] as never, activeAccountId: ACCOUNT.id, defaultAccountId: ACCOUNT.id });
useOfflineStore.setState({ enabled: [ACCOUNT.id], startedOffline: false });
useAuthStore.setState({ client: null, isAuthenticated: false, activeAccountId: ACCOUNT.id });
useAuthStore.setState({ client: null, isAuthenticated: false, activeAccountId: ACCOUNT.id, identities: [], primaryIdentity: null });
});
type ClientLike = { isOfflineOnly(): boolean };
@@ -128,6 +136,23 @@ it('stays on the local copy when the server can\'t be reached', async () => {
expect(useAccountStore.getState().getAccountById(ACCOUNT.id)).toBeTruthy();
});
it('stays on the local copy, and says so, when the server answers a start with a 5xx or 429', async () => {
for (const status of [502, 429]) {
ACCOUNT = makeAccount();
offline.order.length = 0;
useAccountStore.setState({ accounts: [ACCOUNT] as never, activeAccountId: ACCOUNT.id, defaultAccountId: ACCOUNT.id });
useOfflineStore.setState({ enabled: [ACCOUNT.id] });
useAuthStore.setState({ client: null, isAuthenticated: false, activeAccountId: ACCOUNT.id });
sessionAnswer = () => new Response('{}', { status });
// The probe passes: the server itself is there.
offline.gone = false;
await useAuthStore.getState().checkAuth();
await vi.waitFor(() => expect(offline.order).toContain('startedOffline'));
expect(current()?.isOfflineOnly()).toBe(true);
expect(useAccountStore.getState().getAccountById(ACCOUNT.id)).toBeTruthy();
}
});
it('goes the usual way when the store has no copy', async () => {
offline.session = null;
await useAuthStore.getState().checkAuth();
@@ -142,6 +167,41 @@ it('doesn\'t touch the store for an account without it', async () => {
expect(current()?.isOfflineOnly()).toBe(false);
});
it('goes the usual way while the store has no folders yet', async () => {
offline.ready = false;
await useAuthStore.getState().checkAuth();
expect(offline.order).not.toContain('local client');
expect(current()?.isOfflineOnly()).toBe(false);
});
it('still starts on an empty store when the server can\'t be reached', async () => {
offline.ready = false;
offline.gone = true;
sessionAnswer = () => new Response('{}', { status: 503 });
await useAuthStore.getState().checkAuth();
await vi.waitFor(() => expect(current()?.isOfflineOnly()).toBe(true));
});
it('fills the From list from the server after the swap, when the local copy had none', async () => {
let open: () => void = () => {};
connectGate = new Promise((resolve) => { open = resolve; });
await useAuthStore.getState().checkAuth();
await vi.waitFor(() => expect(current()?.isOfflineOnly()).toBe(true));
expect(useAuthStore.getState().identities).toEqual([]);
open();
await vi.waitFor(() => expect(useAuthStore.getState().identities.map((identity) => identity.email)).toEqual([ACCOUNT.username]));
expect(useAuthStore.getState().primaryIdentity?.email).toBe(ACCOUNT.username);
});
it('doesn\'t let a late local answer overwrite the server\'s From list', async () => {
offline.localIdentities = new Promise((resolve) => { offline.releaseLocal = () => resolve([{ id: 'old', email: 'stale@x.test', name: 'Stale', mayDelete: false }]); });
await useAuthStore.getState().checkAuth();
await vi.waitFor(() => expect(useAuthStore.getState().identities.map((identity) => identity.email)).toEqual([ACCOUNT.username]));
offline.releaseLocal();
await new Promise((resolve) => setTimeout(resolve, 10));
expect(useAuthStore.getState().identities.map((identity) => identity.email)).toEqual([ACCOUNT.username]);
});
it('leaves the login path to a refused account, and keeps its data', async () => {
sessionAnswer = () => new Response('{}', { status: 401 });
await useAuthStore.getState().checkAuth();
+17 -2
View File
@@ -1368,7 +1368,8 @@ async function startLocalCopyClient(accountId: string, serverUrl: string, userna
if (IS_LITE || skipLocalCopy || !useOfflineStore.getState().enabled.includes(accountId)) return null;
try {
const offline = await import('@/lib/offline/offline-mode');
const session = await offline.offlineSessionFor(accountId);
// A store that has no folders yet has nothing to show: the snapshot stays up.
const session = await offline.offlineSessionFor(accountId, { withFolders: true });
return session ? JMAPClient.fromOfflineSession(serverUrl, username, session) : null;
} catch {
return null;
@@ -2916,7 +2917,7 @@ export const useAuthStore = create<AuthState>()(
impersonated: session.impersonated === true,
});
void syncAccountDisplayName(account.id, client);
} else if (res.status >= 500) {
} else if (res.status >= 500 || res.status === 429) {
throw new TransientAuthError('Session restore failed', res.status);
} else {
throw new Error(`Session cookie missing: ${res.status}`);
@@ -2994,6 +2995,9 @@ export const useAuthStore = create<AuthState>()(
if (!real) {
if (useAccountStore.getState().getAccountById(entry.id)) {
clients.set(entry.id, local);
// A 5xx or a rate limit, with the server up otherwise. The app stays
// on the local copy, and says so, and leaves it when the server answers.
void import('@/lib/offline/offline-mode').then((offline) => offline.markStartedOffline()).catch(() => {});
return;
}
if (get().activeAccountId !== entry.id) return;
@@ -3006,6 +3010,15 @@ export const useAuthStore = create<AuthState>()(
if (get().activeAccountId !== entry.id) return;
initializeFeatureStores(real);
set({ client: real, ...getClientRateLimitState(real), connectionLost: false, error: null });
// The local copy may have had none yet, and the server's are newer.
void real.getIdentities()
.then((raw) => {
if (get().client !== real) return;
const { identities, primaryIdentity } = loadIdentities(raw, entry.username);
set({ identities, primaryIdentity });
applyPreferredIdentity(entry.id);
})
.catch((err) => debug.error('Failed to load identities after the local copy:', err));
};
// A few at a time rather than one after the other: with many logins
@@ -3050,6 +3063,8 @@ export const useAuthStore = create<AuthState>()(
// spending a serial round trip before the UI unblocks.
const identitiesLoaded = targetClient.getIdentities()
.then((raw) => {
// The real client took over meanwhile, and asks for itself.
if (localCopy && get().client !== null && get().client !== targetClient) return;
const { identities, primaryIdentity } = loadIdentities(raw, targetAccount.username);
set({ identities, primaryIdentity });
})