From d57bdfaace6da36aa038edd740d622b0ed18176a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:12:41 +0100 Subject: [PATCH] fix: send writes only once after a dropped connection, retry only batches of reads --- .../__tests__/jmap-client-hardening.test.ts | 34 +++++++++++++++++++ src/api/jmap-client.ts | 16 ++++----- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/api/__tests__/jmap-client-hardening.test.ts b/src/api/__tests__/jmap-client-hardening.test.ts index a3c732b9..cda342eb 100644 --- a/src/api/__tests__/jmap-client-hardening.test.ts +++ b/src/api/__tests__/jmap-client-hardening.test.ts @@ -318,6 +318,40 @@ describe('JMAPClient hardening', () => { expect(fetch).toHaveBeenCalledTimes(2); }); + // A write the server may already have applied must not run twice. + it.each([ + [[['Email/set', { accountId: 'acc-1', destroy: ['e1'] }, '0']]], + [[['ContactCard/set', { accountId: 'acc-1', create: { c: {} } }, '0']]], + [[['Mailbox/get', { accountId: 'acc-1' }, '0'], ['Email/copy', { accountId: 'acc-1' }, '1']]], + [[['SieveScript/set', { accountId: 'acc-1' }, '0']]], + ])('does not replay a batch with a write after a network error (%j)', async (calls) => { + const client = await connected(); + const fetch = mockFetch([ + { status: 200, throw: new TypeError('Network request failed') }, + { status: 200, json: { methodResponses: [] } }, + ]); + const err = await client.request(calls as never).catch((e) => e); + expect(err).toBeInstanceOf(NetworkError); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('replays a batch made only of reads', async () => { + vi.useFakeTimers(); + const client = await connected(); + const fetch = mockFetch([ + { status: 200, throw: new TypeError('Network request failed') }, + { status: 200, json: { methodResponses: [['Email/query', { ids: [] }, '0'], ['Email/get', { list: [] }, '1']] } }, + ]); + const p = client.request([ + ['Email/query', { accountId: 'acc-1' }, '0'], + ['Email/get', { accountId: 'acc-1', '#ids': { resultOf: '0', name: 'Email/query', path: '/ids' } }, '1'], + ['Email/changes', { accountId: 'acc-1', sinceState: 's' }, '2'], + ]); + await vi.advanceTimersByTimeAsync(1100); + await p; + expect(fetch).toHaveBeenCalledTimes(2); + }); + it('does not retry a submission after a network error and wraps it as NetworkError', async () => { const client = await connected(); const fetch = mockFetch([{ status: 200, throw: new TypeError('Network request failed') }]); diff --git a/src/api/jmap-client.ts b/src/api/jmap-client.ts index 09457c31..bdf27a16 100644 --- a/src/api/jmap-client.ts +++ b/src/api/jmap-client.ts @@ -98,15 +98,15 @@ export function parseRetryAfter(header: string | null): number { return 60_000; } -// Methods whose bodies must never be replayed automatically: a retried -// EmailSubmission/set would send the mail twice. +const READ_ONLY_METHOD = /\/(?:get|query|changes|queryChanges|parse)$|^Core\/echo$/; + +// Whether a batch must not be sent again after the connection failed without +// an answer. The server may already have acted on it, so only batches made of +// read methods are replayed (webmail isReplaySafeRequest): a retried /set, +// /copy, /import or submission would be done twice - one Send delivering two +// messages, one move or delete applied on top of its own result. function hasNonIdempotentMethod(methodCalls: ReadonlyArray): boolean { - return methodCalls.some( - ([name]) => - name === 'EmailSubmission/set' || - name === 'Email/import' || - name === 'Blob/upload', - ); + return methodCalls.length === 0 || methodCalls.some(([name]) => !READ_ONLY_METHOD.test(name)); } function fetchSessionDescribes(session: unknown): session is JMAPSession {