fix: page large folder lists on stalwart 1.0 and retry when mail arrives mid-load #1502

lib/__tests__/jmap-mailbox-pagination.test.ts

@@ -68,7 +68,6 @@ describe('JMAP mailbox pagination', () => {
     ['missing ids', undefined, 's', []],
     ['mixed ids', ['third', 4], 's', []],
     ['duplicate ids', ['parent'], 's', [{ id: 'parent', name: 'Parent' }]],
-    ['changed state', ['third'], 's2', [{ id: 'third', name: 'Third' }]],
     ['early empty page', [], 's', []],
     ['missing object', ['third'], 's', []],
   ])('rejects %s instead of publishing a partial hierarchy', async (_label, ids, queryState, list) => {
@@ -143,16 +142,67 @@ describe('JMAP mailbox pagination', () => {
     await expect(client.getAllMailboxesWithState()).rejects.toThrow('complete mailbox list');
   });
 
-  it('pages when an unbounded get is rejected as tooManyObjects', async () => {
+  it('pages when an unbounded get is rejected as requestTooLarge (Stalwart 1.0)', async () => {
     const client = createClient();
     const list = [{ id: 'a', name: 'A' }, { id: 'b', name: 'B' }, { id: 'c', name: 'C' }];
     serve(client, { primary: list });
     vi.mocked(client['request']).mockResolvedValueOnce({ methodResponses: [
-      ['error', { type: 'tooManyObjects' }, '0'],
+      ['error', { type: 'requestTooLarge' }, '0'],
     ] });
     expect((await client.getMailboxes()).map(({ id }) => id)).toEqual(['a', 'b', 'c']);
   });
 
+  it('pages a shared account whose packed get is rejected as requestTooLarge', async () => {
+    const client = createClient(2, { primary: { name: 'Self' }, shared: { name: 'Shared' } });
+    const shared = [{ id: 'a', name: 'A' }, { id: 'b', name: 'B', parentId: 'a' }, { id: 'c', name: 'C', parentId: 'b' }];
+    serve(client, { primary: [{ id: 'inbox', name: 'Inbox' }], shared });
+    vi.mocked(client['request']).mockResolvedValueOnce({ methodResponses: [
+      ['Mailbox/get', { list: [{ id: 'inbox', name: 'Inbox' }], state: 'm' }, '0'],
+      ['error', { type: 'requestTooLarge' }, '1'],
+    ] });
+    const result = await client.getAllMailboxesWithState();
+    expect(result.mailboxes.map(({ id }) => id)).toEqual(['inbox', 'shared:a', 'shared:b', 'shared:c']);
+    expect(result.states).toEqual({ primary: 'm', shared: 'm' });
+  });
+
+  it('starts over when the mailbox state moves between pages', async () => {
+    const client = createClient();
+    const page = (ids: string[], queryState: string, total?: number) => ({ methodResponses: [
+      ['Mailbox/query', { ids, queryState, ...(total ? { total } : {}) }, 'query'],
+      ['Mailbox/get', { list: ids.map((id) => ({ id, name: id })), state: queryState }, 'get'],
+    ] });
+    const request = vi.spyOn(client as unknown as RequestTarget, 'request')
+      .mockResolvedValueOnce({ methodResponses: [['Mailbox/get', { list: [{ id: 'a' }, { id: 'b' }] }, '0']] })
+      .mockResolvedValueOnce(page(['a', 'b'], 's1', 3))
+      .mockResolvedValueOnce(page(['c'], 's2'))
+      .mockResolvedValueOnce(page(['a', 'b'], 's2', 3))
+      .mockResolvedValueOnce(page(['c'], 's2'));
+
+    const result = await client.getAllMailboxesWithState();
+    expect(result.mailboxes.map(({ id }) => id)).toEqual(['a', 'b', 'c']);
+    expect(result.states).toEqual({ primary: 's2' });
+    expect(request).toHaveBeenCalledTimes(5);
+  });
+
+  it('gives up when the mailbox state keeps moving', async () => {
+    const client = createClient();
+    vi.spyOn(console, 'error').mockImplementation(() => {});
+    let n = 0;
+    const request = vi.spyOn(client as unknown as RequestTarget, 'request').mockImplementation(async (calls) => {
+      if (calls[0][0] === 'Mailbox/get') return { methodResponses: [['Mailbox/get', { list: [{ id: 'a' }, { id: 'b' }] }, '0']] };
+      const position = calls[0][1].position as number;
+      const ids = position === 0 ? ['a', 'b'] : ['c'];
+      const state = `s${n++}`;
+      return { methodResponses: [
+        ['Mailbox/query', { ids, queryState: state, ...(position === 0 ? { total: 3 } : {}) }, 'query'],
+        ['Mailbox/get', { list: ids.map((id) => ({ id, name: id })), state }, 'get'],
+      ] };
+    });
+
+    await expect(client.getMailboxes()).rejects.toThrow('changed while it was being loaded');
+    expect(request).toHaveBeenCalledTimes(1 + 3 * 2);
+  });
+
   it.each([false, true])('respects maxCallsInRequest=1 (shared: %s)', async (shared) => {
     const client = createClient(2, shared ? { primary: { name: 'Self' }, shared: { name: 'Shared' } } : undefined);
     Object.assign(client, { capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInGet: 2, maxCallsInRequest: 1 } } });

lib/jmap/client.ts

@@ -603,6 +603,23 @@ const MAILBOX_PROPERTIES = [
   "isSubscribed",
 ] as const;
 
+// A Mailbox/get without ids over maxObjectsInGet: Stalwart 0.16 cuts the list
+// at the limit without saying so, 1.0 answers requestTooLarge (RFC 8620 §5.1).
+function mailboxListCapped(response: JMAPResponse["methodResponses"][number] | undefined, maxObjects: number): boolean {
+  if (response?.[0] === "Mailbox/get") return Array.isArray(response[1].list) && response[1].list.length >= maxObjects;
+  return response?.[0] === "error" && response[1].type === "requestTooLarge";
+}
+
+// Every new message or flag change moves the Mailbox state (the counts), so a
+// paged load is retried a few times before the refresh gives up.
+const MAILBOX_TREE_ATTEMPTS = 3;
+
+class MailboxListChangedError extends Error {
+  constructor() {
+    super("Mailbox list changed while it was being loaded");
+  }
+}
+
 // SieveScript properties (RFC 9661 §2), explicit for the same reason.
 const SIEVE_SCRIPT_PROPERTIES = ["id", "name", "blobId", "isActive"] as const;
 
@@ -2277,6 +2294,16 @@ export class JMAPClient implements IJMAPClient {
    * children without their parents. Querying all ids keeps the tree complete.
    */
   private async fetchMailboxTree(accountId: string): Promise<JMAPResponse> {
+    for (let attempt = 1; ; attempt++) {
+      try {
+        return await this.fetchMailboxTreeOnce(accountId);
+      } catch (error) {
+        if (!(error instanceof MailboxListChangedError) || attempt >= MAILBOX_TREE_ATTEMPTS) throw error;
+      }
+    }
+  }
+
+  private async fetchMailboxTreeOnce(accountId: string): Promise<JMAPResponse> {
     const mailboxes: JMAPMailbox[] = [];
     const seenIds = new Set<string>();
     let position = 0;
@@ -2335,10 +2362,10 @@ export class JMAPClient implements IJMAPClient {
       }
 
       if (queryState !== undefined && pageQueryState !== queryState) {
-        throw new Error("Mailbox list changed while it was being loaded");
+        throw new MailboxListChangedError();
       }
       if (mailboxState !== undefined && pageMailboxState !== undefined && pageMailboxState !== mailboxState) {
-        throw new Error("Mailbox list changed while it was being loaded");
+        throw new MailboxListChangedError();
       }
       queryState = pageQueryState;
       if (typeof pageMailboxState === "string") mailboxState = pageMailboxState;
@@ -2367,8 +2394,7 @@ export class JMAPClient implements IJMAPClient {
       ];
       let response = await this.request(calls, undefined, { skipOfflineRecord: true });
       const first = response.methodResponses?.[0];
-      if ((first?.[0] === "Mailbox/get" && first[1].list?.length >= this.getMaxObjectsInGet()) ||
-          (first?.[0] === "error" && first[1].type === "tooManyObjects")) {
+      if (mailboxListCapped(first, this.getMaxObjectsInGet())) {
         response = await this.fetchMailboxTree(acctId);
       }
       getOfflineHooks()?.record(this, calls, response);
@@ -2454,8 +2480,7 @@ export class JMAPClient implements IJMAPClient {
             let match = response.methodResponses?.find((r) => r[2] === String(i));
             // Keep the packed fast path for small accounts. A full-sized result
             // may be capped, so reload that account through query/get pages.
-            if ((match?.[0] === "Mailbox/get" && Array.isArray(match[1].list) && match[1].list.length >= maxObjects) ||
-                (match?.[0] === "error" && match[1].type === "tooManyObjects")) {
+            if (mailboxListCapped(match, maxObjects)) {
               try {
                 const complete = await this.fetchMailboxTree(accountId);
                 match = complete.methodResponses[0];