From 04010ce41bca6e765a74f876ab0c44af6a6efd06 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 15:39:48 +0200 Subject: [PATCH] fix: split the state poll to fit the server's call limit The state poll puts two method calls per session account, plus calendars and filters, into one request. From seven accounts on (the server's default limit is 16 calls) Stalwart refused the whole request, and the failure was swallowed, so new mail and folder changes stopped showing up for users with many shared accounts. The poll now goes out in batches of `maxCallsInRequest`, one after the other. --- .../jmap-client-poll-batching.test.ts | 78 ++++++++++++++++ lib/jmap/client.ts | 92 ++++++++++--------- 2 files changed, 127 insertions(+), 43 deletions(-) create mode 100644 lib/__tests__/jmap-client-poll-batching.test.ts diff --git a/lib/__tests__/jmap-client-poll-batching.test.ts b/lib/__tests__/jmap-client-poll-batching.test.ts new file mode 100644 index 000000000..1cca0e0dc --- /dev/null +++ b/lib/__tests__/jmap-client-poll-batching.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { JMAPClient } from '../jmap/client'; + +// The state poll holds two calls per session account plus calendars and +// filters. With seven accounts (Stalwart's default limit is 16 calls) the +// server refused the whole request and change detection stopped silently. + +const SERVER = 'https://mail.example.test'; +const MAX_CALLS = 16; + +function session(accountCount: number) { + const accounts: Record = {}; + for (let i = 0; i < accountCount; i++) { + accounts[`A${i}`] = { + name: i === 0 ? 'alice@example.test' : `colleague${i}@example.test`, + isPersonal: i === 0, + accountCapabilities: { 'urn:ietf:params:jmap:mail': {}, 'urn:ietf:params:jmap:calendars': {}, 'urn:ietf:params:jmap:sieve': {} }, + }; + } + return { + username: 'alice@example.test', + apiUrl: `${SERVER}/jmap/`, + downloadUrl: `${SERVER}/jmap/download/{accountId}/{blobId}/{name}?accept={type}`, + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'A0' }, + accounts, + capabilities: { + 'urn:ietf:params:jmap:core': { maxCallsInRequest: MAX_CALLS }, + 'urn:ietf:params:jmap:calendars': {}, + 'urn:ietf:params:jmap:sieve': {}, + }, + }; +} + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('state poll across many accounts', () => { + it('splits the poll to fit maxCallsInRequest and still reports the change', async () => { + let emailState = 's1'; + const requestSizes: number[] = []; + vi.stubGlobal('fetch', vi.fn(async (url: string, init: RequestInit = {}) => { + if (String(url).endsWith('/.well-known/jmap')) return new Response(JSON.stringify(session(7)), { status: 200 }); + const body = JSON.parse(String(init.body)); + requestSizes.push(body.methodCalls.length); + if (body.methodCalls.length > MAX_CALLS) { + return new Response(JSON.stringify({ type: 'urn:ietf:params:jmap:error:limit', limit: 'maxCallsInRequest' }), { status: 400 }); + } + return new Response(JSON.stringify({ + methodResponses: body.methodCalls.map(([name, args, id]: [string, { accountId: string }, string]) => [ + name, + { accountId: args.accountId, state: name === 'Email/get' && args.accountId === 'A0' ? emailState : 'x', list: [] }, + id, + ]), + }), { status: 200 }); + })); + + const client = new JMAPClient(SERVER, 'alice@example.test', 'pw'); + await client.connect(); + const internals = client as unknown as { + pingInterval?: ReturnType; + fetchCurrentStates(): Promise; + checkForStateChanges(): Promise; + }; + clearInterval(internals.pingInterval); + const changes: unknown[] = []; + client.onStateChange((change) => changes.push(change)); + + requestSizes.length = 0; + await internals.fetchCurrentStates(); + emailState = 's2'; // new mail in the primary account + await internals.checkForStateChanges(); + + expect(Math.max(...requestSizes)).toBeLessThanOrEqual(MAX_CALLS); + expect(changes).toEqual([{ '@type': 'StateChange', changed: { A0: { Email: 's2' } } }]); + client.disconnect(); + }); +}); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index a58e83b9f..3ebcd38f7 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -261,6 +261,9 @@ interface JMAPEmailHeader { type JMAPMethodCall = [string, Record, string]; +/** One entry of a JMAP `methodResponses` array. */ +type JMAPMethodResponse = [string, JMAPResponseResult, string]; + const SUBMISSION_USING = [ 'urn:ietf:params:jmap:core', 'urn:ietf:params:jmap:mail', @@ -8366,27 +8369,41 @@ export class JMAPClient implements IJMAPClient { return stateKey ? { accountId: this.accountId, stateKey } : null; } + /** + * Run the state-poll calls and collect their responses. The poll holds two + * calls per account plus calendars and filters, so enough accounts (seven, + * on Stalwart's default of 16) push it past `maxCallsInRequest`, and the + * server refuses the whole request: change detection then stopped without + * a sound. The calls go out in batches the server accepts, one after the + * other to keep the socket budget (#702). A batch that fails is skipped. + */ + private async fetchPolledStateResponses(): Promise { + const { using, methodCalls } = this.buildStatePollingRequest(); + const responses: JMAPMethodResponse[] = []; + for (const group of batched(methodCalls, this.getMaxCallsInRequest())) { + const response = await this.firstTouchGate.run(group, () => + this.authenticatedFetch(this.apiUrl, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ using, methodCalls: group }), + }), + ); + if (!response.ok) continue; + const data = await response.json(); + if (Array.isArray(data?.methodResponses)) responses.push(...data.methodResponses); + } + return responses; + } + private async fetchCurrentStates(): Promise { if (this.isRateLimited()) { return; } try { - const { using, methodCalls } = this.buildStatePollingRequest(); - const response = await this.firstTouchGate.run(methodCalls, () => - this.authenticatedFetch(this.apiUrl, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ using, methodCalls }), - }), - ); - - if (response.ok) { - const data = await response.json(); - for (const [method, result, callId] of data.methodResponses) { - const resolved = this.resolvePolledState(method, callId); - if (resolved && result?.state) { - this.pollingStates[`${resolved.accountId}:${resolved.stateKey}`] = result.state; - } + for (const [method, result, callId] of await this.fetchPolledStateResponses()) { + const resolved = this.resolvePolledState(method, callId); + if (resolved && result?.state) { + this.pollingStates[`${resolved.accountId}:${resolved.stateKey}`] = result.state as string; } } } catch { @@ -8406,36 +8423,25 @@ export class JMAPClient implements IJMAPClient { } this.stateCheckInFlight = true; try { - const { using, methodCalls } = this.buildStatePollingRequest(); - const response = await this.firstTouchGate.run(methodCalls, () => - this.authenticatedFetch(this.apiUrl, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ using, methodCalls }), - }), - ); + // Build a per-account changed map so a background change in a shared + // (secondary) account is reported under its own accountId — which + // handleStateChange treats as "some mailbox changed" and refetches the + // full (own + delegated) mailbox list from. + const changedByAccount: Record> = {}; - if (response.ok) { - const data = await response.json(); - // Build a per-account changed map so a background change in a shared - // (secondary) account is reported under its own accountId — which - // handleStateChange treats as "some mailbox changed" and refetches the - // full (own + delegated) mailbox list from. - const changedByAccount: Record> = {}; - - for (const [method, result, callId] of data.methodResponses) { - const resolved = this.resolvePolledState(method, callId); - if (!resolved || !result?.state) continue; - const key = `${resolved.accountId}:${resolved.stateKey}`; - if (this.pollingStates[key] && this.pollingStates[key] !== result.state) { - (changedByAccount[resolved.accountId] ??= {})[resolved.stateKey] = result.state; - } - this.pollingStates[key] = result.state; + for (const [method, result, callId] of await this.fetchPolledStateResponses()) { + const resolved = this.resolvePolledState(method, callId); + const state = result?.state as string | undefined; + if (!resolved || !state) continue; + const key = `${resolved.accountId}:${resolved.stateKey}`; + if (this.pollingStates[key] && this.pollingStates[key] !== state) { + (changedByAccount[resolved.accountId] ??= {})[resolved.stateKey] = state; } + this.pollingStates[key] = state; + } - if (Object.keys(changedByAccount).length > 0 && this.stateChangeCallback) { - this.stateChangeCallback({ '@type': 'StateChange', changed: changedByAccount }); - } + if (Object.keys(changedByAccount).length > 0 && this.stateChangeCallback) { + this.stateChangeCallback({ '@type': 'StateChange', changed: changedByAccount }); } } catch { // Silently fail - polling will retry