mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
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.
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+24
-4
@@ -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<string, { description?: string; type?: string; properties?: string[] }> }).notCreated?.draft;
|
||||
@@ -1374,13 +1376,31 @@ export async function sendEmail(
|
||||
}
|
||||
if (methodName === 'EmailSubmission/set') {
|
||||
const notCreated = (result as { notCreated?: Record<string, { description?: string; type?: string }> }).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<string, { id?: string; sendAt?: string }> }).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).
|
||||
|
||||
Reference in New Issue
Block a user