From 2e78c9870cc4a146ce3bfa68a8919273edc0fa77 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:19:01 +0200 Subject: [PATCH] fix: remove the unsent copy when the server refuses a send A send creates the message in Drafts and submits it in the same request. When the server refused the submission, the created copy stayed in Drafts without anyone tracking it, so sending again created a second copy and left the first behind as a stray draft. The copy is now destroyed again when the submission is refused; the composer still holds the message and the previous draft version is left alone. --- src/api/__tests__/email-submission.test.ts | 59 ++++++++++++++++++++++ src/api/email.ts | 28 ++++++++-- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/api/__tests__/email-submission.test.ts b/src/api/__tests__/email-submission.test.ts index 351bd5c2..0b4baa35 100644 --- a/src/api/__tests__/email-submission.test.ts +++ b/src/api/__tests__/email-submission.test.ts @@ -247,6 +247,65 @@ describe('sendEmail', () => { }); await expect(sendEmail(OUTGOING, 'identity-1', 'sent-mb')).rejects.toThrow('Disk full'); + // Nothing was created, so there is nothing to remove. + expect(mockRequest).toHaveBeenCalledTimes(1); + }); + + describe('a refused submission', () => { + const destroyCalls = () => mockRequest.mock.calls + .map((c) => c[0][0]) + .filter(([method, args]: [string, { destroy?: string[] }]) => method === 'Email/set' && args.destroy) + .map(([, args]: [string, { accountId: string; destroy: string[] }]) => [args.accountId, args.destroy]); + + it('removes the copy it created in Drafts, so a retry leaves no stray draft', async () => { + mockRequest + .mockResolvedValueOnce({ + methodResponses: [ + ['Email/set', { created: { draft: { id: 'e-new' } } }, '0'], + ['EmailSubmission/set', { notCreated: { 'sub-1': { type: 'forbiddenToSend', description: 'Quota exceeded' } } }, '1'], + ], + }) + .mockResolvedValueOnce({ methodResponses: [['Email/set', { destroyed: ['e-new'] }, '0']] }); + + await expect(sendEmail(OUTGOING, 'identity-1', 'sent-mb', undefined, { + draftsMailboxId: 'drafts-mb', draftId: 'old-draft', accountId: 'shared-1', + })).rejects.toThrow('Quota exceeded'); + + // Only the unsent copy goes; the previous draft version stays. + expect(destroyCalls()).toEqual([['shared-1', ['e-new']]]); + }); + + it('removes the copy when the submission call failed as a whole', async () => { + mockRequest + .mockResolvedValueOnce({ + methodResponses: [ + ['Email/set', { created: { draft: { id: 'e-new' } } }, '0'], + ['error', { type: 'invalidArguments', description: 'Invalid envelope' }, '1'], + ], + }) + .mockResolvedValueOnce({ methodResponses: [['Email/set', { destroyed: ['e-new'] }, '0']] }); + + await expect(sendEmail(OUTGOING, 'identity-1', 'sent-mb', undefined, { draftsMailboxId: 'drafts-mb' })) + .rejects.toThrow('Invalid envelope'); + expect(destroyCalls()).toEqual([['acc-1', ['e-new']]]); + }); + + it('still reports the refusal when the copy cannot be removed', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + mockRequest + .mockResolvedValueOnce({ + methodResponses: [ + ['Email/set', { created: { draft: { id: 'e-new' } } }, '0'], + ['EmailSubmission/set', { notCreated: { 'sub-1': { type: 'forbiddenFrom', description: 'Not your address' } } }, '1'], + ], + }) + .mockRejectedValueOnce(new Error('Network request failed')); + + await expect(sendEmail(OUTGOING, 'identity-1', 'sent-mb', undefined, { draftsMailboxId: 'drafts-mb' })) + .rejects.toThrow('Not your address'); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); }); }); diff --git a/src/api/email.ts b/src/api/email.ts index b19bdf51..99091251 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -1264,8 +1264,8 @@ export interface SendEmailOptions { /** * Drafts folder id. When given, the message is created there with `$draft` * and moved to Sent by `onSuccessUpdateEmail` on the submission, so the - * SMTP send happens before it lands in Sent (#188) and a failed submission - * leaves it in Drafts instead of faking a sent copy. + * SMTP send happens before it lands in Sent (#188). A refused submission + * removes that copy again rather than leaving a stray draft. */ draftsMailboxId?: string; /** Previous draft version to destroy once the submission succeeded (#849). */ @@ -1338,6 +1338,7 @@ export async function sendEmail( let emailSubmissionId: string | undefined; let sendAt: string | undefined; let filingWarning: string | undefined; + let failure: Error | undefined; for (const [methodName, result] of res.methodResponses) { if (methodName === 'error' || methodName.endsWith('/error')) { // Once the submission exists the message has left (or is held), so a @@ -1349,7 +1350,8 @@ export async function sendEmail( filingWarning = filingWarning ?? (err.description || err.type || 'post-send filing failed'); continue; } - throw new Error((result as { description?: string }).description ?? 'Send failed'); + failure = new Error((result as { description?: string }).description ?? 'Send failed'); + break; } if (methodName === 'Email/set') { const notCreated = (result as { notCreated?: Record }).notCreated?.draft; @@ -1374,13 +1376,31 @@ export async function sendEmail( } if (methodName === 'EmailSubmission/set') { const notCreated = (result as { notCreated?: Record }).notCreated?.['sub-1']; - if (notCreated) throw submissionError(notCreated, 'Failed to submit message'); + if (notCreated) { + failure = submissionError(notCreated, 'Failed to submit message'); + break; + } const created = (result as { created?: Record }).created?.['sub-1']; emailSubmissionId = created?.id; sendAt = created?.sendAt; } } + if (failure) { + // Nothing went out. A message created before the submission was refused + // must not stay behind: nobody tracked that copy, so a retry left a + // second one next to it. The caller still holds the message, and a + // previous draft version is untouched. + if (emailId) { + try { + await destroyEmails([emailId], accountId); + } catch (err) { + console.warn('[email] failed to remove the unsent copy:', err); + } + } + throw failure; + } + // The message is out (or scheduled) - now it is safe to drop the old draft. // A failure here leaves an orphan in Drafts, which is a filing warning // rather than a failed send (#849).