fix: surface Email/set failures on delete and move #956

lib/__tests__/jmap-email-set-failures.test.ts

@@ -0,0 +1,104 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { JMAPClient } from '../jmap/client';
+
+// #956: Email/set failures were swallowed by the delete/move helpers because
+// request() only rejects on HTTP errors. Method-level errors and per-id
+// notUpdated/notDestroyed entries must reject so the store keeps the rows.
+
+function makeSession() {
+  return {
+    capabilities: { 'urn:ietf:params:jmap:core': {} },
+    accounts: { 'acct-1': { name: 'test', isPersonal: true, accountCapabilities: {} } },
+    primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acct-1' },
+    apiUrl: 'https://mail.example.com/jmap/api',
+    downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}',
+    uploadUrl: 'https://mail.example.com/jmap/upload/{accountId}/',
+    eventSourceUrl: 'https://mail.example.com/jmap/eventsource',
+  };
+}
+
+function jsonResponse(body: unknown): Response {
+  return new Response(JSON.stringify(body), {
+    status: 200,
+    headers: { 'Content-Type': 'application/json' },
+  });
+}
+
+const notUpdated = () =>
+  jsonResponse({
+    methodResponses: [
+      ['Email/set', { updated: null, notUpdated: { e1: { type: 'invalidProperties', description: 'Mailbox does not exist' } } }, '0'],
+    ],
+  });
+const notDestroyed = () =>
+  jsonResponse({
+    methodResponses: [
+      ['Email/set', { destroyed: null, notDestroyed: { e1: { type: 'forbidden', description: 'No permission' } } }, '0'],
+    ],
+  });
+const methodError = () =>
+  jsonResponse({
+    methodResponses: [['error', { type: 'accountNotFound', description: 'Account not found' }, '0']],
+  });
+const success = () =>
+  jsonResponse({
+    methodResponses: [['Email/set', { updated: { e1: null }, destroyed: ['e1'] }, '0']],
+  });
+
+describe('JMAPClient Email/set failure surfacing (#956)', () => {
+  let fetchSpy: ReturnType<typeof vi.spyOn>;
+
+  beforeEach(() => {
+    fetchSpy = vi.spyOn(globalThis, 'fetch');
+  });
+
+  afterEach(() => {
+    fetchSpy.mockRestore();
+  });
+
+  async function connectedClient(): Promise<JMAPClient> {
+    fetchSpy.mockResolvedValueOnce(jsonResponse(makeSession()));
+    const client = JMAPClient.withBearer('https://mail.example.com', 'token123', 'user@test.com');
+    await client.connect();
+    fetchSpy.mockReset();
+    return client;
+  }
+
+  it('deleteEmail rejects on notDestroyed', async () => {
+    const client = await connectedClient();
+    fetchSpy.mockResolvedValueOnce(notDestroyed());
+    await expect(client.deleteEmail('e1')).rejects.toThrow(/No permission/);
+  });
+
+  it('moveToTrash rejects on notUpdated', async () => {
+    const client = await connectedClient();
+    fetchSpy.mockResolvedValueOnce(notUpdated());
+    await expect(client.moveToTrash('e1', 'trash')).rejects.toThrow(/Mailbox does not exist/);
+  });
+
+  it('batchDeleteEmails rejects on notDestroyed', async () => {
+    const client = await connectedClient();
+    fetchSpy.mockResolvedValueOnce(notDestroyed());
+    await expect(client.batchDeleteEmails(['e1', 'e2'])).rejects.toThrow(/No permission/);
+  });
+
+  it('batchMoveEmails rejects on notUpdated', async () => {
+    const client = await connectedClient();
+    fetchSpy.mockResolvedValueOnce(notUpdated());
+    await expect(client.batchMoveEmails(['e1', 'e2'], 'archive')).rejects.toThrow(/Mailbox does not exist/);
+  });
+
+  it('rejects on a method-level error response', async () => {
+    const client = await connectedClient();
+    fetchSpy.mockResolvedValueOnce(methodError());
+    await expect(client.batchMoveEmails(['e1'], 'archive')).rejects.toThrow(/Account not found/);
+  });
+
+  it('still resolves when the server accepted the change', async () => {
+    const client = await connectedClient();
+    fetchSpy.mockResolvedValueOnce(success());
+    await expect(client.deleteEmail('e1')).resolves.toBeUndefined();
+    fetchSpy.mockResolvedValueOnce(success());
+    await expect(client.batchMoveEmails(['e1'], 'archive')).resolves.toBeUndefined();
+  });
+});

lib/jmap/client.ts

@@ -2308,37 +2308,65 @@ export class JMAPClient implements IJMAPClient {
     return { migrated, refused: refusals };
   }
 
+  /**
+   * Reject an Email/set response that reports a method-level error or per-id
+   * `notUpdated`/`notDestroyed` failures. `request()` only throws on HTTP
+   * errors, so without this the store removes rows the server refused. (#956)
+   */
+  private assertEmailSetSucceeded(response: JMAPResponse, action: string): void {
+    const [name, result] = response.methodResponses?.[0] ?? [];
+    if (name === "error") {
+      throw new Error(`Failed to ${action}: ${result?.description || result?.type || 'unknown error'}`);
+    }
+    const failed: Record<string, { type?: string; description?: string }> = {
+      ...(result?.notUpdated ?? {}),
+      ...(result?.notDestroyed ?? {}),
+    };
+    const failedIds = Object.keys(failed);
+    if (failedIds.length === 0) return;
+    const first = failed[failedIds[0]];
+    const reason = first?.description || first?.type || 'unknown error';
+    throw new Error(
+      failedIds.length === 1
+        ? `Failed to ${action}: ${reason}`
+        : `Failed to ${action} ${failedIds.length} email(s); first error: ${reason}`,
+    );
+  }
+
   async deleteEmail(emailId: string, accountId?: string): Promise<void> {
-    await this.request([
+    const response = await this.request([
       ["Email/set", {
         accountId: accountId || this.accountId,
         destroy: [emailId],
       }, "0"],
     ]);
+    this.assertEmailSetSucceeded(response, 'delete email');
   }
 
   async moveToTrash(emailId: string, trashMailboxId: string, accountId?: string, markAsRead?: boolean): Promise<void> {
     const targetAccountId = accountId || this.accountId;
     const patch: Record<string, unknown> = { mailboxIds: { [trashMailboxId]: true } };
     if (markAsRead) patch["keywords/$seen"] = true;
-    await this.request([
+    const response = await this.request([
       ["Email/set", {
         accountId: targetAccountId,
         update: { [emailId]: patch },
       }, "0"],
     ]);
+    this.assertEmailSetSucceeded(response, 'move email to trash');
   }
 
   async batchDeleteEmails(emailIds: string[], accountId?: string): Promise<void> {
     if (emailIds.length === 0) return;
 
     for (const batch of batched(emailIds, this.getMaxObjectsInSet())) {
-      await this.request([
+      const response = await this.request([
         ["Email/set", {
           accountId: accountId || this.accountId,
           destroy: batch,
         }, "0"],
       ]);
+      this.assertEmailSetSucceeded(response, 'delete emails');
     }
   }
 
@@ -2352,9 +2380,10 @@ export class JMAPClient implements IJMAPClient {
     };
     for (const batch of batched(emailIds, this.getMaxObjectsInSet())) {
       const updates = Object.fromEntries(batch.map(id => [id, buildPatch()]));
-      await this.request([
+      const response = await this.request([
         ["Email/set", { accountId: accountId || this.accountId, update: updates }, "0"],
       ]);
+      this.assertEmailSetSucceeded(response, 'move emails');
     }
   }
 

stores/email-store.ts

@@ -2020,8 +2020,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
         throw new Error('Trash mailbox not found - cannot move email to trash');
       }
 
-      // Permanent delete
-      await effectiveClient.deleteEmail(emailId);
+      // Permanent delete - in the email's own account, like the batch path.
+      await effectiveClient.deleteEmail(emailId, accountId);
 
       // Remove from local state and update mailbox counters (in the email's own
       // account list). Unread emails also decrement the unread counters. (#281)