From 15740fa2868a2a5c0b644a6de8002edd25065937 Mon Sep 17 00:00:00 2001 From: lucamzanon Date: Thu, 1 Oct 2026 12:12:46 +0200 Subject: [PATCH] fix: keep the draft when a send is not confirmed When the response carries no EmailSubmission - a method error, or no answer for it at all - nothing confirms the message left, yet the send went on as if it had: the old draft was replaced and the composer closed. It now fails with "Send confirmation was not received" and keeps the draft, so the user can check Sent before sending again. An empty notCreated map is a success some servers spell out; it is no longer read as a failure. --- lib/__tests__/jmap-draft-lifecycle.test.ts | 18 +++++++++++++++++- lib/jmap/client.ts | 12 ++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/__tests__/jmap-draft-lifecycle.test.ts b/lib/__tests__/jmap-draft-lifecycle.test.ts index a01fc2cae..f876368a0 100644 --- a/lib/__tests__/jmap-draft-lifecycle.test.ts +++ b/lib/__tests__/jmap-draft-lifecycle.test.ts @@ -29,7 +29,7 @@ interface CapturedRequest { methodCalls: JMAPMethodCall[] } * Email/set create come back as notCreated; `failSubmission` does the same * for EmailSubmission/set. */ -function mockFlow({ failCreate = false, failSubmission = false } = {}) { +function mockFlow({ failCreate = false, failSubmission = false, methodError = false, missingSubmission = false, emptyErrors = false } = {}) { const captured: CapturedRequest[] = []; vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { const body = JSON.parse((init as { body: string }).body) as CapturedRequest; @@ -56,11 +56,14 @@ function mockFlow({ failCreate = false, failSubmission = false } = {}) { result.created = { [key]: { id: 'email-new-1' } }; } } + if (emptyErrors) result.notCreated = {}; if (args.destroy) { result.destroyed = args.destroy; } methodResponses.push(['Email/set', result, callId]); } else if (method === 'EmailSubmission/set') { + if (missingSubmission) continue; + if (methodError) { methodResponses.push(['error', { type: 'serverFail', description: 'Google unavailable; draft retained' }, callId]); continue; } methodResponses.push(['EmailSubmission/set', failSubmission ? { notCreated: { '1': { type: 'forbiddenFrom', description: 'not allowed' } } } @@ -230,3 +233,16 @@ describe('draft replace lifecycle (#849)', () => { ]); }); }); + +describe('compose failure recovery', () => { + beforeEach(() => { vi.restoreAllMocks(); vi.spyOn(console, 'error').mockImplementation(() => {}); }); + it.each([{ methodError: true }, { missingSubmission: true }])('does not claim success or delete the old draft without confirmation: %j', async options => { + const client = createClient(); const captured = mockFlow(options); + await expect(client.sendEmail(['bob@example.com'], 'Subject', 'body', undefined, undefined, 'identity-1', 'user@example.com', 'old-draft')).rejects.toThrow(); + expect(emailSetCalls(captured).some(call => call.destroy)).toBe(false); + }); + it('accepts empty notCreated maps as success', async () => { + const client = createClient(); mockFlow({ emptyErrors: true }); + expect(await client.createDraft(['bob@example.com'], 'Subject', 'body')).toBe('email-new-1'); + }); +}); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 1ce01fa06..f7a64de5c 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -3868,7 +3868,8 @@ export class JMAPClient implements IJMAPClient { if (response.methodResponses?.[0]?.[0] === "Email/set") { const result = response.methodResponses[0][1]; - if (result.notCreated) { + // An empty notCreated map is a success some servers spell out. + if (result.notCreated && Object.keys(result.notCreated).length) { const errors = result.notCreated; const firstError = Object.values(errors)[0] as { description?: string; type?: string }; console.error('Draft save error:', firstError); @@ -4134,7 +4135,7 @@ export class JMAPClient implements IJMAPClient { if (response.methodResponses) { for (const [methodName, result] of response.methodResponses) { - if (result.notCreated) { + if (result.notCreated && Object.keys(result.notCreated).length) { // Include method name + full error object so it's clear whether the // failure came from Email/set (draft create) or EmailSubmission/set // (actual send) and which JMAP error type/properties were returned. @@ -4188,6 +4189,13 @@ export class JMAPClient implements IJMAPClient { } } + // No EmailSubmission came back - a method error, or no answer for it at + // all. Nothing confirms the message left: report a failure and keep the + // old draft rather than replacing it as if the send had worked. + if (!emailSubmissionId) { + throw new Error('Send confirmation was not received. Check Sent before sending again. Your draft has been kept.'); + } + // With every recipient refused nothing was queued, yet the submission // exists and its onSuccessUpdateEmail filed the copy into Sent (#1123). // Drop that copy and keep the old draft: the send failed.