From 6428c2b8800fc56b788eedcd6cd7243bd58fce0b Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:07:30 +0100 Subject: [PATCH] fix: answer from the device only when the mail server is really gone, and finish the first offline sync #523 --- lib/admin/outgoing-hosts-registry.ts | 2 +- lib/jmap/client.ts | 7 +- lib/jmap/offline-hooks.ts | 6 + lib/offline/__tests__/client-fallback.test.ts | 124 ++++++++++++++++++ lib/offline/offline-mode.ts | 43 ++++-- lib/offline/sync.ts | 37 ++++-- 6 files changed, 193 insertions(+), 26 deletions(-) create mode 100644 lib/offline/__tests__/client-fallback.test.ts diff --git a/lib/admin/outgoing-hosts-registry.ts b/lib/admin/outgoing-hosts-registry.ts index f6c6f0819..7f3a8f8de 100644 --- a/lib/admin/outgoing-hosts-registry.ts +++ b/lib/admin/outgoing-hosts-registry.ts @@ -158,7 +158,7 @@ export const OUTGOING_HOSTS: readonly OutgoingHost[] = [ */ export const NON_REQUEST_HOSTS: readonly { host: string; files: readonly string[]; why: string }[] = [ { host: 'bulwarkmail.org', why: 'docs links, comments and the contact URL in User-Agent headers', files: ['app/(main)/admin/_pages/telemetry.tsx', 'app/api/fetch-ical/route.ts', 'app/api/translate/route.ts','components/admin/org/sections/recovery-section.tsx', 'components/admin/org/sections/company-calendars-section.tsx', 'components/admin/org/sections/free-busy-section.tsx', 'components/admin/org/sections/labels-section.tsx', 'components/admin/org/sections/out-of-office-calendar-section.tsx', 'components/admin/org/sections/phishing-report-section.tsx', 'instrumentation.node.ts', 'lib/telemetry/state.ts', 'lib/telemetry/types.ts'] }, - { host: 'bulwarkmail.org', why: 'docs links of the Notices, Devices and out-of-office template pages', files: ['components/admin/org/sections/announcement-section.tsx', 'components/admin/org/sections/devices-section.tsx', 'components/admin/org/sections/retention-section.tsx', 'components/admin/org/sections/vacation-templates-section.tsx'] }, + { host: 'bulwarkmail.org', why: 'docs links of the Notices, Devices (offline mode too) and out-of-office template pages', files: ['components/admin/org/sections/announcement-section.tsx', 'components/admin/org/sections/devices-section.tsx', 'components/admin/org/sections/offline-section.tsx', 'components/admin/org/sections/retention-section.tsx', 'components/admin/org/sections/vacation-templates-section.tsx'] }, { host: 'bulwarkmail.com', why: 'JMAP capability URI', files: ['lib/jmap/client.ts'] }, { host: 'extensions.bulwarkmail.org', why: 'signed statement type URIs', files: ['lib/admin/directory-keys.ts'] }, { host: 'www.w3.org', why: 'XML namespaces', files: ['components/ui/flag-icons.tsx', 'lib/favicon-badge.ts'] }, diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index c295e8219..43c26de38 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -2155,8 +2155,9 @@ export class JMAPClient implements IJMAPClient { try { answer = await this.networkRequest(methodCalls, using, opts); } catch (error) { - if (offline && isNetworkFailure(error)) { - offline.noteNetwork(this, false); + // Only when the server is really gone: a refusal without CORS + // headers looks the same to the browser and must stay an error. + if (offline && isNetworkFailure(error) && await offline.serverGone(this)) { const local = await offline.answer(this, methodCalls); if (local) return local as JMAPResponse; } @@ -6272,7 +6273,7 @@ export class JMAPClient implements IJMAPClient { try { return await this.authenticatedFetch(url, {}, { timeoutMs: JMAPClient.TRANSFER_TIMEOUT_MS }); } catch (error) { - if (offline && isNetworkFailure(error)) { + if (offline && isNetworkFailure(error) && await offline.serverGone(this)) { const local = await offline.blob(this, blobId, account); if (local) return local; } diff --git a/lib/jmap/offline-hooks.ts b/lib/jmap/offline-hooks.ts index 46c585397..78cd2a869 100644 --- a/lib/jmap/offline-hooks.ts +++ b/lib/jmap/offline-hooks.ts @@ -33,6 +33,12 @@ export interface OfflineHooks { record(client: OfflineClientRef, methodCalls: OfflineMethodCall[], response: OfflineResponse): void; /** A request reached the server (true) or failed on the network (false). */ noteNetwork(client: OfflineClientRef, reachable: boolean): void; + /** + * After a request failed on the network: whether the server can't be + * reached at all (then the device answers), or it only refused that + * request (then it stays an error). + */ + serverGone(client: OfflineClientRef): Promise; /** A blob kept on the device, or null. */ blob(client: OfflineClientRef, blobId: string, accountId: string): Promise; /** A blob the server sent, for the cache. */ diff --git a/lib/offline/__tests__/client-fallback.test.ts b/lib/offline/__tests__/client-fallback.test.ts new file mode 100644 index 000000000..4829e2c91 --- /dev/null +++ b/lib/offline/__tests__/client-fallback.test.ts @@ -0,0 +1,124 @@ +import 'fake-indexeddb/auto'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { JMAPClient } from '@/lib/jmap/client'; +import { useAuthStore } from '@/stores/auth-store'; +import { useAccountStore } from '@/stores/account-store'; +import { usePolicyStore } from '@/stores/policy-store'; +import { DEFAULT_ORGANISATION_POLICY } from '@/lib/admin/organisation-policy'; +import { startOfflineMode, wipeEverything } from '../offline-mode'; +import { forgetIndexes, putEmails, putMisc, emailIndex } from '../cache'; +import { forgetDeviceKey } from '../crypto'; +import { deleteOfflineDb } from '../db'; +import { useOfflineStore, writeEnabled } from '../state'; +import { syncAccount } from '../sync'; + +const SERVER = 'https://mail.example.org'; +const SESSION = { + apiUrl: `${SERVER}/jmap/`, + downloadUrl: `${SERVER}/jmap/download/{accountId}/{blobId}/{name}?accept={type}`, + uploadUrl: `${SERVER}/jmap/upload/{accountId}/`, + accounts: { j1: { name: 'alice', isPersonal: true, accountCapabilities: {} } }, + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'j1' }, + capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInGet: 500 } }, + username: 'alice@example.org', +}; + +let probeOk = true; +let apiFails = false; + +beforeEach(async () => { + forgetIndexes(); + forgetDeviceKey(); + await deleteOfflineDb(); + localStorage.clear(); + probeOk = true; + apiFails = false; + useOfflineStore.setState({ networkDown: false, browserOffline: false }); + usePolicyStore.setState({ loaded: true, subject: null, policy: { ...usePolicyStore.getState().policy, organisation: DEFAULT_ORGANISATION_POLICY } }); + useAccountStore.setState({ accounts: [{ id: 'acc', serverUrl: SERVER, username: 'alice@example.org' } as never] }); + vi.stubGlobal('fetch', vi.fn(async (url: string, init?: RequestInit) => { + if (init?.mode === 'no-cors') { + if (!probeOk) throw new TypeError('Failed to fetch'); + return new Response(null, { status: 200 }); + } + if (String(url).endsWith('/.well-known/jmap')) return new Response(JSON.stringify(SESSION), { status: 200 }); + if (apiFails) throw new TypeError('Failed to fetch'); + return new Response(JSON.stringify({ methodResponses: [['Email/get', { accountId: 'j1', list: [], notFound: ['m1'] }, '0']] }), { status: 200 }); + })); +}); + +afterEach(async () => { + vi.unstubAllGlobals(); + await wipeEverything(); +}); + +async function connectedClient(): Promise { + const client = new JMAPClient(SERVER, 'alice@example.org', 'pw'); + await client.connect(); + useAuthStore.setState({ client: client as never, activeAccountId: 'acc' }); + return client; +} + +describe('falling back to the device', () => { + it('answers from the device when the server is gone', async () => { + const client = await connectedClient(); + writeEnabled(['acc']); + const stop = startOfflineMode(); + await putEmails('acc', 'j1', [{ id: 'm1', mailboxIds: { inbox: true }, receivedAt: new Date().toISOString(), subject: 'Kept', bodyValues: { 1: { value: 'x' } } }]); + apiFails = true; + probeOk = false; + const email = await client.getEmail('m1'); + expect(email?.subject).toBe('Kept'); + expect(useOfflineStore.getState().networkDown).toBe(true); + stop(); + }); + + it('keeps a refused request an error while the server answers', async () => { + const client = await connectedClient(); + writeEnabled(['acc']); + const stop = startOfflineMode(); + await putEmails('acc', 'j1', [{ id: 'm1', mailboxIds: { inbox: true }, receivedAt: new Date().toISOString(), subject: 'Kept', bodyValues: { 1: { value: 'x' } } }]); + apiFails = true; + probeOk = true; + // getEmail swallows errors into null: the device's copy isn't passed off as the server's answer. + expect(await client.getEmail('m1')).toBeNull(); + expect(useOfflineStore.getState().networkDown).toBe(false); + stop(); + }); +}); + +describe('the sync', () => { + function fakeClient(opts: { failQuery?: boolean }) { + return { + getSessionSnapshot: () => SESSION, + getAccountId: () => 'j1', + getAllMailboxes: vi.fn(async () => { await putMisc('acc', 'mailboxes', { j1: [{ id: 'inbox', role: 'inbox' }] }); return []; }), + getMailboxes: vi.fn(async () => []), + getIdentities: vi.fn(async () => []), + getRecentEmailIds: vi.fn(async (mailboxId: string) => { if (opts.failQuery && mailboxId === 'archive') throw new Error('refused'); return mailboxId === 'inbox' ? ['new'] : []; }), + getSomeEmails: vi.fn(async () => []), + getFullEmails: vi.fn(async (ids: string[]) => { + await putEmails('acc', 'j1', ids.map((id) => ({ id, mailboxIds: { inbox: true }, receivedAt: new Date().toISOString(), bodyValues: {} }))); + return []; + }), + fetchBlob: vi.fn(), + }; + } + const deps = { stillActive: () => true, recorded: async () => {}, scopeMailboxes: async () => [{ jmap: 'j1', mailboxId: 'inbox', isInbox: true }] }; + + it('fetches what is missing and drops what left the Inbox', async () => { + await putEmails('acc', 'j1', [{ id: 'gone', mailboxIds: { inbox: true }, receivedAt: new Date().toISOString() }]); + const client = fakeClient({}); + await syncAccount('acc', client as never, deps); + expect(client.getFullEmails).toHaveBeenCalledWith(['new'], 'j1'); + const index = await emailIndex('acc'); + expect([...index.values()].map((e) => e.id)).toEqual(['new']); + }); + + it('keeps everything when a folder couldn\'t be asked', async () => { + await putEmails('acc', 'j1', [{ id: 'kept', mailboxIds: { archive: true }, receivedAt: new Date().toISOString() }]); + const both = { ...deps, scopeMailboxes: async () => [{ jmap: 'j1', mailboxId: 'inbox', isInbox: true }, { jmap: 'j1', mailboxId: 'archive', isInbox: false }] }; + await syncAccount('acc', fakeClient({ failQuery: true }) as never, both); + expect([...(await emailIndex('acc')).values()].map((e) => e.id).sort()).toEqual(['kept', 'new']); + }); +}); diff --git a/lib/offline/offline-mode.ts b/lib/offline/offline-mode.ts index a331e96e5..2b833e2b8 100644 --- a/lib/offline/offline-mode.ts +++ b/lib/offline/offline-mode.ts @@ -234,7 +234,7 @@ function enqueueRecord(account: string, task: () => Promise): void { // ---- Hooks for the JMAP client ---- let networkWasDown = false; -let probing = false; +let probing: Promise | null = null; /** * A request failed on the network. Browsers report a refusal without CORS @@ -243,17 +243,19 @@ let probing = false; * when that fails too is the server counted as gone, and sends go to the * outbox. */ -async function probeServer(client: OfflineClientRef): Promise { - if (probing) return; - probing = true; - try { - await fetch(`${client.getServerUrl().replace(/\/+$/, '')}/.well-known/jmap`, { mode: 'no-cors', cache: 'no-store', credentials: 'omit' }); - } catch { - networkWasDown = true; - if (!useOfflineStore.getState().networkDown) useOfflineStore.setState({ networkDown: true }); - } finally { - probing = false; - } +function probeServer(client: OfflineClientRef): Promise { + if (probing) return probing; + probing = (async () => { + try { + await fetch(`${client.getServerUrl().replace(/\/+$/, '')}/.well-known/jmap`, { mode: 'no-cors', cache: 'no-store', credentials: 'omit' }); + return false; + } catch { + networkWasDown = true; + if (!useOfflineStore.getState().networkDown) useOfflineStore.setState({ networkDown: true }); + return true; + } + })().finally(() => { probing = null; }); + return probing; } const hooks: OfflineHooks = { @@ -294,6 +296,11 @@ const hooks: OfflineHooks = { void probeServer(client); } }, + async serverGone(client) { + if (!offlineActive(accountForClient(client))) return false; + if (typeof navigator !== 'undefined' && navigator.onLine === false) return true; + return probeServer(client); + }, async blob(client, blobId, jmap) { const account = accountForClient(client); if (!account || !offlineActive(account)) return null; @@ -445,7 +452,11 @@ export async function runSync(account: string): Promise { syncsRunning.add(account); useOfflineStore.setState((s) => ({ syncing: [...new Set([...s.syncing, account])] })); try { - await syncAccount(account, client, { stillActive: () => offlineActive(account), scopeMailboxes }); + await syncAccount(account, client, { + stillActive: () => offlineActive(account), + recorded: async () => { await recording.get(account); }, + scopeMailboxes, + }); useOfflineStore.setState((s) => ({ lastSyncedAt: { ...s.lastSyncedAt, [account]: Date.now() } })); } catch { // Tried again at the next change or in 15 minutes. @@ -493,6 +504,12 @@ export async function enableOffline(account: string): Promise { if (!offlineAllowed(account)) return; const enabled = useOfflineStore.getState().enabled; if (!enabled.includes(account)) writeEnabled([...enabled, account]); + // Ask the browser not to clear it when space runs low (it may say no). + try { + void navigator.storage?.persist?.().catch(() => false); + } catch { + /* no Storage API */ + } const client = onlineClientFor(account); if (client) await putMisc(account, 'session', client.getSessionSnapshot()); await tellServiceWorker(true); diff --git a/lib/offline/sync.ts b/lib/offline/sync.ts index 616626119..0d4a918d5 100644 --- a/lib/offline/sync.ts +++ b/lib/offline/sync.ts @@ -21,6 +21,8 @@ const ATTACHMENT_BUDGET_BYTES = 100 * 1024 * 1024; interface SyncDeps { stillActive: () => boolean; + /** Resolves once the cache has written every answer the client recorded so far. */ + recorded: () => Promise; scopeMailboxes: (account: string) => Promise>; } @@ -31,21 +33,35 @@ interface AttachmentPart { size?: number; } +/** Once more after a second: Stalwart refuses requests beyond its limit of parallel ones. */ +async function twice(run: () => Promise): Promise { + try { + return await run(); + } catch { + await new Promise((r) => setTimeout(r, 1000)); + return run(); + } +} + export async function syncAccount(account: string, client: JMAPClient, deps: SyncDeps): Promise { await putMisc(account, 'session', client.getSessionSnapshot()); // Folders and identities are recorded from these answers. - await client.getAllMailboxes().catch(() => client.getMailboxes()); - await client.getIdentities().catch(() => []); + await twice(() => client.getAllMailboxes()).catch(() => client.getMailboxes()); + await twice(() => client.getIdentities()).catch(() => []); + // The folder list says which folder is the Inbox. + await deps.recorded(); if (!deps.stillActive()) return; const since = new Date(Date.now() - CACHE_WINDOW_MS).toISOString(); const wanted = new Map>(); const scopes = await deps.scopeMailboxes(account); + let complete = scopes.length > 0; for (const { jmap, mailboxId, isInbox } of scopes) { let ids: string[]; try { - ids = await client.getRecentEmailIds(mailboxId, jmap, since, isInbox ? INBOX_LIMIT : FOLDER_LIMIT); + ids = await twice(() => client.getRecentEmailIds(mailboxId, jmap, since, isInbox ? INBOX_LIMIT : FOLDER_LIMIT)); } catch { + complete = false; continue; } const set = wanted.get(jmap) ?? new Set(); @@ -57,13 +73,16 @@ export async function syncAccount(account: string, client: JMAPClient, deps: Syn for (const [jmap, idSet] of wanted) { const ids = [...idSet]; // Fresh keywords and folders for all of them (cheap: list properties). - if (ids.length) await client.getSomeEmails(ids, jmap === client.getAccountId() ? undefined : jmap).catch(() => []); - // Bodies for the ones that don't have one yet. + if (ids.length) await twice(() => client.getSomeEmails(ids, jmap === client.getAccountId() ? undefined : jmap)).catch(() => []); + // Bodies for the ones that don't have one yet, a batch at a time. const missing: string[] = []; for (const id of ids) if (!(await hasBody(account, jmap, id))) missing.push(id); - if (missing.length) await client.getFullEmails(missing, jmap); - if (!deps.stillActive()) return; + for (let i = 0; i < missing.length; i += 20) { + await twice(() => client.getFullEmails(missing.slice(i, i + 20), jmap)).catch(() => { complete = false; }); + if (!deps.stillActive()) return; + } } + await deps.recorded(); // What left the window or the folders goes. const index = await emailIndex(account); @@ -72,8 +91,8 @@ export async function syncAccount(account: string, client: JMAPClient, deps: Syn const jmap = key.split('\u0001')[1]; if (!wanted.get(jmap)?.has(email.id)) stale.push(key); } - // A folder that couldn't be asked keeps what it had. - if (scopes.length > 0 && wanted.size > 0) await removeEmails(account, stale); + // Only after a complete pass: a folder that couldn't be asked keeps what it had. + if (complete) await removeEmails(account, stale); // Attachments and inline images, newest messages first, within the budget. let budget = ATTACHMENT_BUDGET_BYTES;