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).