From e98741d604ab9c4d8caccb6748c8cfafc61ac170 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:17:53 +0100 Subject: [PATCH] fix: say a store was reset also when a new owner took over the open, and slow raw downloads to one a second --- lib/offline/__tests__/drafts-flush.test.ts | 24 ++++++++++++++- lib/offline/offline-mode.ts | 16 ++++++++-- lib/store/__tests__/attachments.test.ts | 19 ++++++++++++ lib/store/__tests__/page-side.test.ts | 36 ++++++++++++++++++++++ lib/store/client/api.ts | 11 +++++-- lib/store/mail/attachments.ts | 4 ++- 6 files changed, 104 insertions(+), 6 deletions(-) diff --git a/lib/offline/__tests__/drafts-flush.test.ts b/lib/offline/__tests__/drafts-flush.test.ts index 32a3dd0ab..f9139cf7d 100644 --- a/lib/offline/__tests__/drafts-flush.test.ts +++ b/lib/offline/__tests__/drafts-flush.test.ts @@ -9,7 +9,7 @@ import { FakeJmapDrafts } from '@/lib/store/__tests__/helpers/fake-drafts'; import { JMAP_ACCOUNT } from '@/lib/store/__tests__/helpers/fake-jmap'; import { InProcessStore } from '@/lib/store/__tests__/helpers/in-process-store'; import { consumeLost } from '../lost'; -import { refreshQueueState, startOfflineMode, unsentCount, wipeAccount, wipeEverything } from '../offline-mode'; +import { offlineNotify, refreshQueueState, setOfflineNotifier, startOfflineMode, unsentCount, wipeAccount, wipeEverything } from '../offline-mode'; import { flushQueue, forgetOrphanCheck } from '../flush'; import { heldFile, holdFile, noteDraftReplacedElsewhere, queueSend, readLocalDrafts, retryLocalDraft } from '../queue'; import { forgetLegacyMove } from '../migrate-legacy'; @@ -187,6 +187,28 @@ describe('flushQueue and drafts', () => { }); }); +describe('a notice before the host is there', () => { + it('waits for it, and comes once', async () => { + vi.resetModules(); + const fresh = await import('../offline-mode'); + fresh.offlineNotify({ kind: 'store-reset' }); + const show = vi.fn(); + fresh.setOfflineNotifier(show); + expect(show).toHaveBeenCalledTimes(1); + expect(show).toHaveBeenCalledWith({ kind: 'store-reset' }); + fresh.setOfflineNotifier(vi.fn()); + expect(show).toHaveBeenCalledTimes(1); + vi.resetModules(); + }); + + it('goes straight to a host that\'s there', () => { + const show = vi.fn(); + setOfflineNotifier(show); + offlineNotify({ kind: 'store-reset' }); + expect(show).toHaveBeenCalledTimes(1); + }); +}); + describe('what a sign-out asks about', () => { it('counts a draft only on this device', async () => { await saveOffline('Only here'); diff --git a/lib/offline/offline-mode.ts b/lib/offline/offline-mode.ts index 4a71cfb0f..33b5fff76 100644 --- a/lib/offline/offline-mode.ts +++ b/lib/offline/offline-mode.ts @@ -81,11 +81,23 @@ export type OfflineNotice = | { kind: 'forbidden' } | { kind: 'back-online-reload' }; -let notify: (notice: OfflineNotice) => void = () => {}; +let notifier: ((notice: OfflineNotice) => void) | null = null; +/** + * The store opens at a start from the local copy, before the host component has + * mounted. What it has to say by then, like "this store couldn't be read", waits. + */ +const earlyNotices: OfflineNotice[] = []; +const MOST_EARLY_NOTICES = 10; + +function notify(notice: OfflineNotice): void { + if (notifier) notifier(notice); + else if (earlyNotices.length < MOST_EARLY_NOTICES) earlyNotices.push(notice); +} /** Where the notices go (the host component turns them into toasts). */ export function setOfflineNotifier(fn: (notice: OfflineNotice) => void): void { - notify = fn; + notifier = fn; + for (const notice of earlyNotices.splice(0)) fn(notice); } export function offlineNotify(notice: OfflineNotice): void { diff --git a/lib/store/__tests__/attachments.test.ts b/lib/store/__tests__/attachments.test.ts index 092c2b6dd..5899fe57c 100644 --- a/lib/store/__tests__/attachments.test.ts +++ b/lib/store/__tests__/attachments.test.ts @@ -462,6 +462,25 @@ describe('raw messages come last', () => { }); }); +describe('the pace', () => { + it('is slower for raw messages than for files', async () => { + const server = new Files(); + withFile(server, 'm', 'b-1', 3, 1, { blobId: 'raw-m', size: 3 }); + const waits: number[] = []; + const engine = new AccountEngine({ + account: 'a@fake.test', + db: openMemoryStore(), + session: FAKE_SESSION, + transport: server, + areas: [mailArea], + scope: SCOPE, + sleep: async (ms) => { waits.push(ms); }, + }); + await engine.requestSync(); + expect(waits).toEqual([200, 1000]); + }); +}); + describe('what a server error means', () => { it('doesn\'t give up on a file for a passing error, and stops for this pass', async () => { const server = new Files(); diff --git a/lib/store/__tests__/page-side.test.ts b/lib/store/__tests__/page-side.test.ts index fbd336b69..5b22c5260 100644 --- a/lib/store/__tests__/page-side.test.ts +++ b/lib/store/__tests__/page-side.test.ts @@ -331,6 +331,42 @@ describe('the page\'s store', () => { expect(lost).toHaveBeenCalledWith('lost@x'); }); + it('still tells the page when a new owner takes over the open that found the key gone', async () => { + await open(); + const before = await getAccountRecord('lost@x'); + await forgetAccountKey('lost@x'); + configureStore({ + inputs: () => ({ session: FAKE_SESSION, auth: 'Basic x', invertedKeywordSort: true }), + refreshAuth: async () => null, + onEvent: () => {}, + onStoreLost: lost, + }); + const store = getStore(); + await store.ensureOpen('lost@x'); + const worker = FakeWorker.all[FakeWorker.all.length - 1]; + const post = worker.postMessage.bind(worker); + let dropped = false; + worker.postMessage = (message) => { + if (message.kind === 'open' && !dropped) { + // The first open never gets its answer: ownership moved under it. + dropped = true; + worker.posted.push(message); + return; + } + post(message); + }; + worker.becomeOwner(); + await vi.waitFor(() => expect(dropped).toBe(true)); + worker.becomeOwner(); + await vi.waitFor(() => expect(lost).toHaveBeenCalledWith('lost@x')); + expect(lost).toHaveBeenCalledTimes(1); + const after = await getAccountRecord('lost@x'); + const prunes = worker.posted.filter((m) => m.kind === 'prune-files') as unknown as Array<{ keep: string[] }>; + expect(prunes.at(-1)?.keep).toContain(after!.file); + expect(prunes.at(-1)?.keep).not.toContain(before!.file); + store.stop(); + }); + it('stays quiet on a normal reopen', async () => { await open(); const worker = await open(); diff --git a/lib/store/client/api.ts b/lib/store/client/api.ts index 83cb83655..d339ace69 100644 --- a/lib/store/client/api.ts +++ b/lib/store/client/api.ts @@ -81,6 +81,11 @@ class PageStore implements StoreBackend { private readonly opening = new Map>(); /** The engine can't start in this browser: don't keep every caller waiting for an owner. */ private broken = false; + /** + * Accounts whose key was just made, until their store has opened. A new owner + * can start the open again, and the second try finds the key the first one made. + */ + private readonly keyJustMade = new Set(); /** What the owner's engine was last told to keep. */ private readonly sentScope = new Map(); @@ -131,8 +136,9 @@ class PageStore implements StoreBackend { const run = (async () => { const client = this.started(); const { keyHex, record, fresh } = await openAccountKey(account, this.hooks()?.keyLayers?.() ?? []); + if (fresh) this.keyJustMade.add(account); // The old file, if the key alone went, can never be read again. - if (fresh) await client.pruneFiles(await knownStoreFiles()).catch(() => {}); + if (this.keyJustMade.has(account)) await client.pruneFiles(await knownStoreFiles()).catch(() => {}); const inputs = this.hooks()?.inputs(account) ?? { session: null, auth: null, invertedKeywordSort: null }; if (inputs.scope) this.sentScope.set(account, JSON.stringify(inputs.scope)); const opened = await client.openAccount({ @@ -144,7 +150,8 @@ class PageStore implements StoreBackend { invertedKeywordSort: inputs.invertedKeywordSort, auth: inputs.auth, }); - if (fresh || opened?.created) { + // Whichever try of them gets here first says so. + if (this.keyJustMade.delete(account) || opened?.created) { if (readOpenedAccounts().includes(account)) this.hooks()?.onStoreLost?.(account); } rememberOpened(account, true); diff --git a/lib/store/mail/attachments.ts b/lib/store/mail/attachments.ts index 3b80fee66..f4975756f 100644 --- a/lib/store/mail/attachments.ts +++ b/lib/store/mail/attachments.ts @@ -23,6 +23,8 @@ import { BLOB_CHUNK_BYTES, dropBlob, putBlob } from './records'; /** Keeps well under the server's 1000 requests a minute, which the user's own clicks count against too. */ const PACE_MS = 200; +/** There's one raw message per message, and nothing waits for them. */ +const RAW_PACE_MS = 1000; const PAGE = 20; /** * Parts in a row that don't fit before a pass stops looking. Each check reads the @@ -311,7 +313,7 @@ async function fetchKept(ctx: SyncContext, jmap: string): Promise { if (outcome === 'skipped') passed += 1; done += 1; ctx.emit({ type: 'progress', account: ctx.account, area: 'mail', phase: 'attachments', done, total: null }); - await ctx.pause(PACE_MS); + await ctx.pause(part.raw ? RAW_PACE_MS : PACE_MS); } } }