From f8d282e714ffcd2b07382050cba45e610ec50a42 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Mon, 5 Oct 2026 12:38:06 -0700 Subject: [PATCH] Never send a message twice; two small composer and sidebar fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Never send twice after a lost reply. When a send's request went out and no answer came back, the composer said "Send failed" and offered the draft back; if Stalwart had in fact accepted it, sending the draft again delivered it twice. Each send now carries its own Message-ID (RFC 5322 §3.6.4), kept on the draft if it comes back, and an unanswered send asks the server what it did before calling it a failure: - submitted (EmailSubmission/query by emailIds, RFC 8621 §7.3): it was sent, and the composer says so; - created but never submitted: removed from Sent, and the failure stands; - not there: the failure stands; - the server can't be asked either: the composer says it couldn't confirm and to check Sent, instead of a plain "Send failed". A refusal (4xx) is not checked: the server did not run it. Sending again a draft that came back asks first, and sends nothing if a message with its Message-ID already exists. Stalwart expunges submissions after its hold period, so later on any surviving copy counts as sent. Also: - Suggestions no longer offer an address that is already in To, Cc or Bcc while typing in one of the other two. - The collapsed sidebar no longer draws a cut-off "FOLDERS": the rule hiding section headings lost to a later one of the same weight. New strings: 3, in all ten catalogs (1633 -> 1636, no new fallbacks). --- web/src/locales/de.ts | 3 + web/src/locales/es.ts | 3 + web/src/locales/fr.ts | 3 + web/src/locales/ja.ts | 3 + web/src/locales/nl.ts | 3 + web/src/locales/pt-BR.ts | 3 + web/src/locales/ru.ts | 3 + web/src/locales/tr.ts | 3 + web/src/locales/uk.ts | 3 + web/src/locales/zh-Hans.ts | 3 + .../__tests__/send-no-duplicates.test.ts | 211 ++++++++++++++++++ web/src/store/compose.ts | 118 +++++++++- web/src/styles/app.css | 3 +- web/src/views/compose/Composer.tsx | 6 +- web/src/views/compose/RecipientInput.tsx | 8 +- .../__tests__/recipient-exclude.test.tsx | 55 +++++ 16 files changed, 418 insertions(+), 13 deletions(-) create mode 100644 web/src/store/__tests__/send-no-duplicates.test.ts create mode 100644 web/src/views/compose/__tests__/recipient-exclude.test.tsx diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index 4b74859..730dee1 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -1341,6 +1341,9 @@ export const catalog: Catalog = { "Send anyway": "Trotzdem senden", "Send canceled — the message is back in Drafts": "Senden abgebrochen – die Nachricht liegt wieder in den Entwürfen", "Send failed: {error}": "Senden fehlgeschlagen: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "Es konnte nicht geprüft werden, ob diese Nachricht schon gesendet wurde. Prüfen Sie „Gesendet“, bevor Sie sie erneut senden.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "Es konnte nicht bestätigt werden, ob diese Nachricht gesendet wurde. Prüfen Sie „Gesendet“, bevor Sie sie erneut senden.", + "This message had already been sent, so it wasn't sent again.": "Diese Nachricht war bereits gesendet und wurde nicht noch einmal gesendet.", "Send invites": "Einladungen senden", "Send scheduled for {when}": "Senden geplant für {when}", "Send without a subject?": "Ohne Betreff senden?", diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index c3d9497..9d27901 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -1314,6 +1314,9 @@ export const catalog: Catalog = { "Send anyway": "Enviar de todos modos", "Send canceled — the message is back in Drafts": "Envío cancelado: el mensaje ha vuelto a Borradores", "Send failed: {error}": "No se pudo enviar: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "No se pudo comprobar si este mensaje ya se envió. Revisa Enviados antes de volver a enviarlo.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "No se pudo confirmar si este mensaje se envió. Revisa Enviados antes de volver a enviarlo.", + "This message had already been sent, so it wasn't sent again.": "Este mensaje ya se había enviado, así que no se ha vuelto a enviar.", "Send invites": "Enviar invitaciones", "Send scheduled for {when}": "Envío programado para {when}", "Send without a subject?": "¿Enviar sin asunto?", diff --git a/web/src/locales/fr.ts b/web/src/locales/fr.ts index fcf9a09..ff095e5 100644 --- a/web/src/locales/fr.ts +++ b/web/src/locales/fr.ts @@ -1319,6 +1319,9 @@ export const catalog: Catalog = { "Send anyway": "Envoyer quand même", "Send canceled — the message is back in Drafts": "Envoi annulé : le message est de retour dans les brouillons", "Send failed: {error}": "Échec de l’envoi : {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "Impossible de vérifier si ce message a déjà été envoyé. Consultez Envoyés avant de le renvoyer.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "Impossible de confirmer l’envoi de ce message. Consultez Envoyés avant de le renvoyer.", + "This message had already been sent, so it wasn't sent again.": "Ce message avait déjà été envoyé, il n’a donc pas été renvoyé.", "Send invites": "Envoyer les invitations", "Send scheduled for {when}": "Envoi programmé pour {when}", "Send without a subject?": "Envoyer sans objet ?", diff --git a/web/src/locales/ja.ts b/web/src/locales/ja.ts index 9e37096..7c0799f 100644 --- a/web/src/locales/ja.ts +++ b/web/src/locales/ja.ts @@ -1322,6 +1322,9 @@ export const catalog: Catalog = { "Send anyway": "このまま送信", "Send canceled — the message is back in Drafts": "送信を取り消しました。メールは下書きに戻っています", "Send failed: {error}": "送信できませんでした: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "このメッセージが送信済みかどうかを確認できませんでした。もう一度送信する前に送信済みフォルダーを確認してください。", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "このメッセージが送信されたかどうかを確認できませんでした。もう一度送信する前に送信済みフォルダーを確認してください。", + "This message had already been sent, so it wasn't sent again.": "このメッセージはすでに送信されていたため、再送信しませんでした。", "Send invites": "招待を送信", "Send scheduled for {when}": "{when} に送信を予約しました", "Send without a subject?": "件名なしで送信しますか？", diff --git a/web/src/locales/nl.ts b/web/src/locales/nl.ts index 26d68b7..83f8566 100644 --- a/web/src/locales/nl.ts +++ b/web/src/locales/nl.ts @@ -1314,6 +1314,9 @@ export const catalog: Catalog = { "Send anyway": "Toch verzenden", "Send canceled — the message is back in Drafts": "Verzenden geannuleerd — het bericht staat weer bij Concepten", "Send failed: {error}": "Verzenden mislukt: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "Kon niet controleren of dit bericht al is verzonden. Kijk in Verzonden voordat je het opnieuw verstuurt.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "Kon niet bevestigen of dit bericht is verzonden. Kijk in Verzonden voordat je het opnieuw verstuurt.", + "This message had already been sent, so it wasn't sent again.": "Dit bericht was al verzonden en is daarom niet opnieuw verstuurd.", "Send invites": "Uitnodigingen verzenden", "Send scheduled for {when}": "Verzenden gepland voor {when}", "Send without a subject?": "Verzenden zonder een onderwerp?", diff --git a/web/src/locales/pt-BR.ts b/web/src/locales/pt-BR.ts index 426b416..fb28e3b 100644 --- a/web/src/locales/pt-BR.ts +++ b/web/src/locales/pt-BR.ts @@ -1317,6 +1317,9 @@ export const catalog: Catalog = { "Send anyway": "Enviar mesmo assim", "Send canceled — the message is back in Drafts": "Envio cancelado — a mensagem voltou para Rascunhos", "Send failed: {error}": "Falha ao enviar: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "Não foi possível verificar se esta mensagem já foi enviada. Confira Enviados antes de enviá-la novamente.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "Não foi possível confirmar se esta mensagem foi enviada. Confira Enviados antes de enviá-la novamente.", + "This message had already been sent, so it wasn't sent again.": "Esta mensagem já tinha sido enviada, então não foi enviada de novo.", "Send invites": "Enviar convites", "Send scheduled for {when}": "Envio agendado para {when}", "Send without a subject?": "Enviar sem assunto?", diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index 80a0fe0..86b26fe 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -1316,6 +1316,9 @@ export const catalog: Catalog = { "Send anyway": "Всё равно отправить", "Send canceled — the message is back in Drafts": "Отправка отменена — письмо вернулось в черновики", "Send failed: {error}": "Не удалось отправить: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "Не удалось проверить, было ли это письмо уже отправлено. Проверьте «Отправленные», прежде чем отправлять его снова.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "Не удалось подтвердить, было ли это письмо отправлено. Проверьте «Отправленные», прежде чем отправлять его снова.", + "This message had already been sent, so it wasn't sent again.": "Это письмо уже было отправлено, поэтому повторно оно не отправлялось.", "Send invites": "Отправить приглашения", "Send scheduled for {when}": "Отправка запланирована на {when}", "Send without a subject?": "Отправить без темы?", diff --git a/web/src/locales/tr.ts b/web/src/locales/tr.ts index ec48d48..69c34a3 100644 --- a/web/src/locales/tr.ts +++ b/web/src/locales/tr.ts @@ -1067,6 +1067,9 @@ export const catalog: Catalog = { "Send at": "Şu saatte gönder", "Send canceled — the message is back in Drafts": "Gönderim iptal edildi — ileti tekrar Taslaklar içinde", "Send failed: {error}": "Gönderme başarısız: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "Bu iletinin daha önce gönderilip gönderilmediği denetlenemedi. Yeniden göndermeden önce Gönderilenler klasörüne bakın.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "Bu iletinin gönderilip gönderilmediği doğrulanamadı. Yeniden göndermeden önce Gönderilenler klasörüne bakın.", + "This message had already been sent, so it wasn't sent again.": "Bu ileti zaten gönderilmişti, bu yüzden yeniden gönderilmedi.", "Send invitation emails to guests": "Konuklara e-posta ile davetiye gönder", "Send invites": "Davetleri gönder", "Send now instead": "Bunun yerine şimdi gönder", diff --git a/web/src/locales/uk.ts b/web/src/locales/uk.ts index ee89263..da0ad9e 100644 --- a/web/src/locales/uk.ts +++ b/web/src/locales/uk.ts @@ -1310,6 +1310,9 @@ export const catalog: Catalog = { "Send anyway": "Усе одно надіслати", "Send canceled — the message is back in Drafts": "Надсилання скасовано — лист повернувся до чернеток", "Send failed: {error}": "Не вдалося надіслати: {error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "Не вдалося перевірити, чи цей лист уже надіслано. Перевірте «Надіслані», перш ніж надсилати його знову.", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "Не вдалося підтвердити, чи цей лист надіслано. Перевірте «Надіслані», перш ніж надсилати його знову.", + "This message had already been sent, so it wasn't sent again.": "Цей лист уже було надіслано, тому повторно його не надсилали.", "Send invites": "Надіслати запрошення", "Send scheduled for {when}": "Надсилання заплановано на {when}", "Send without a subject?": "Надіслати без теми?", diff --git a/web/src/locales/zh-Hans.ts b/web/src/locales/zh-Hans.ts index c2282c8..e36dd41 100644 --- a/web/src/locales/zh-Hans.ts +++ b/web/src/locales/zh-Hans.ts @@ -1321,6 +1321,9 @@ export const catalog: Catalog = { "Send anyway": "仍然发送", "Send canceled — the message is back in Drafts": "已取消发送——邮件已回到草稿箱", "Send failed: {error}": "发送失败：{error}", + "Couldn't check whether this message was already sent. Check Sent before sending it again.": "无法检查这封邮件是否已发送。请先查看“已发送”，再决定是否重新发送。", + "Couldn't confirm whether this message was sent. Check Sent before sending it again.": "无法确认这封邮件是否已发送。请先查看“已发送”，再决定是否重新发送。", + "This message had already been sent, so it wasn't sent again.": "这封邮件已经发送过，因此没有再次发送。", "Send invites": "发送邀请", "Send scheduled for {when}": "已定时于 {when} 发送", "Send without a subject?": "不填主题就发送吗？", diff --git a/web/src/store/__tests__/send-no-duplicates.test.ts b/web/src/store/__tests__/send-no-duplicates.test.ts new file mode 100644 index 0000000..09be0b7 --- /dev/null +++ b/web/src/store/__tests__/send-no-duplicates.test.ts @@ -0,0 +1,211 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError, CAP, client } from "@/jmap/client"; +import { sendOutcomeUnknown, useCompose } from "@/store/compose"; +import { useMail } from "@/store/mail"; +import { DEFAULT_SETTINGS, useSettings } from "@/store/settings"; +import { useToasts } from "@/ui/toast"; +import type { JmapSession } from "@/jmap/types"; + +/* + * A send whose reply goes missing may still have gone out. "Send failed -> + * Open draft -> Send" then delivered it twice: nothing said which message + * the first attempt was, so nothing could check. Each send now carries its + * own Message-ID, and an unanswered send asks the server what happened to it + * before calling it failed. + */ + +type Mode = "ok" | "lost-reply" | "never-arrived" | "orphan" | "gateway" | "refused" | "offline-after"; + +interface Server { + emails: { id: string; messageId: string }[]; + submissions: { id: string; emailId: string }[]; + destroyed: string[]; + creates: number; +} + +function server(mode: Mode): Server { + const st: Server = { emails: [], submissions: [], destroyed: [], creates: 0 }; + let first = true; + vi.stubGlobal("fetch", vi.fn(async (_url: string, init: RequestInit) => { + const body = JSON.parse(init.body as string) as { methodCalls: [string, Record, string][] }; + const sending = body.methodCalls.some(([n]) => n === "EmailSubmission/set"); + if (sending && first && mode !== "ok") { + first = false; + if (mode === "never-arrived") throw new TypeError("Failed to fetch"); + if (mode === "gateway") return { ok: false, status: 502, statusText: "Bad Gateway", json: async () => ({ error: "bad_gateway" }) } as Response; + if (mode === "refused") return { ok: false, status: 400, statusText: "Bad Request", json: async () => ({ type: "urn:ietf:params:jmap:error:notRequest" }) } as Response; + // The server does the work; only the answer is lost. + const [, args] = body.methodCalls.find(([n, a]) => n === "Email/set" && (a as { create?: unknown }).create)!; + const m = (args.create as { m: { messageId: string[] } }).m; + const id = `e${st.emails.length + 1}`; + st.creates++; + st.emails.push({ id, messageId: m.messageId[0]! }); + if (mode !== "orphan") st.submissions.push({ id: `s${id}`, emailId: id }); + throw new TypeError("Failed to fetch"); + } + if (mode === "offline-after" && !first) throw new TypeError("Failed to fetch"); + if (mode === "offline-after" && sending) { + first = false; + throw new TypeError("Failed to fetch"); + } + const methodResponses = body.methodCalls.map(([name, args, id]) => { + if (name === "Email/set" && (args as { create?: unknown }).create) { + const m = (args.create as { m: { messageId: string[] } }).m; + const eid = `e${st.emails.length + 1}`; + st.creates++; + st.emails.push({ id: eid, messageId: m.messageId[0]! }); + return [name, { accountId: "a1", oldState: "1", newState: "2", created: { m: { id: eid } } }, id]; + } + if (name === "EmailSubmission/set") { + const eid = st.emails[st.emails.length - 1]!.id; + st.submissions.push({ id: `s${eid}`, emailId: eid }); + return [name, { accountId: "a1", oldState: "1", newState: "2", created: { s: { id: `s${eid}` } } }, id]; + } + if (name === "Email/set" && (args as { destroy?: string[] }).destroy) { + st.destroyed.push(...((args as { destroy: string[] }).destroy)); + return [name, { accountId: "a1", oldState: "1", newState: "2", destroyed: (args as { destroy: string[] }).destroy }, id]; + } + if (name === "Email/query") { + const [, value] = ((args.filter ?? {}) as { header?: [string, string] }).header ?? []; + const ids = value ? st.emails.filter((e) => e.messageId === value && !st.destroyed.includes(e.id)).map((e) => e.id) : []; + return [name, { accountId: "a1", queryState: "q", canCalculateChanges: false, position: 0, ids, total: ids.length }, id]; + } + if (name === "EmailSubmission/query") { + const want = ((args.filter ?? {}) as { emailIds?: string[] }).emailIds ?? []; + const ids = st.submissions.filter((s) => want.includes(s.emailId)).map((s) => s.id); + return [name, { accountId: "a1", queryState: "q", canCalculateChanges: false, position: 0, ids, total: ids.length }, id]; + } + return [name, { accountId: "a1", state: "1", list: [], notFound: [], ids: [], total: 0, queryState: "q", position: 0, canCalculateChanges: false }, id]; + }); + return { ok: true, status: 200, json: async () => ({ methodResponses, sessionState: "1" }) } as Response; + })); + return st; +} + +const toastTexts = () => useToasts.getState().toasts.map((t) => t.message); + +async function sendOne(init: Record = {}) { + const key = useCompose.getState().open({ to: [{ name: null, email: "ann@example.com" }], subject: "Hi", ...init }); + await useCompose.getState().send(key); + return key; +} + +beforeEach(() => { + client.session = { + capabilities: { [CAP.core]: { maxObjectsInGet: 500, maxObjectsInSet: 500 }, [CAP.mail]: {}, [CAP.submission]: {} }, + accounts: { a1: { accountCapabilities: { [CAP.mail]: {}, [CAP.submission]: {} } } }, + primaryAccounts: {}, + state: "s1", + } as unknown as JmapSession; + useSettings.setState({ settings: { ...DEFAULT_SETTINGS, undoSendSeconds: 0 } }); + useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} }); + useMail.setState({ + accountId: "a1", + identities: [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as never, + mailboxes: { + mbSent: { id: "mbSent", role: "sent", parentId: null, name: "Sent" }, + mbDrafts: { id: "mbDrafts", role: "drafts", parentId: null, name: "Drafts" }, + } as never, + }); + useToasts.setState({ toasts: [] }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("which failures leave the outcome open", () => { + it("treats a refusal as not sent, and no answer or a gateway error as unknown", () => { + expect(sendOutcomeUnknown(new ApiError(400, "bad"))).toBe(false); + expect(sendOutcomeUnknown(new ApiError(401, "unauthenticated"))).toBe(false); + expect(sendOutcomeUnknown(new ApiError(502, "bad_gateway"))).toBe(true); + expect(sendOutcomeUnknown(new ApiError(408, "timeout"))).toBe(true); + expect(sendOutcomeUnknown(new TypeError("Failed to fetch"))).toBe(true); + }); +}); + +describe("sending once, whatever goes wrong on the way back", () => { + it("gives every send a Message-ID on the sender's domain", async () => { + const st = server("ok"); + await sendOne(); + expect(st.emails).toHaveLength(1); + expect(st.emails[0]!.messageId).toMatch(/^[0-9a-f-]{36}@example\.org$/); + expect(toastTexts()).toContain("Message sent"); + }); + + it("calls it sent when the reply was lost but the server sent it", async () => { + const st = server("lost-reply"); + await sendOne(); + expect(st.submissions).toHaveLength(1); + expect(st.destroyed).toEqual([]); + expect(toastTexts()).toContain("Message sent"); + expect(useCompose.getState().drafts).toHaveLength(0); + }); + + it("removes a message that was created but never submitted, and says it failed", async () => { + const st = server("orphan"); + await sendOne(); + expect(st.destroyed).toEqual(["e1"]); + expect(toastTexts().some((t) => t.startsWith("Send failed"))).toBe(true); + }); + + it("says it failed when the request never reached the server", async () => { + const st = server("never-arrived"); + await sendOne(); + expect(st.emails).toHaveLength(0); + expect(toastTexts().some((t) => t.startsWith("Send failed"))).toBe(true); + }); + + it("asks the server after a gateway error too, rather than assuming", async () => { + const st = server("gateway"); + await sendOne(); + expect(st.creates).toBe(0); + expect(toastTexts().some((t) => t.startsWith("Send failed"))).toBe(true); + }); + + it("does not ask after a refusal: the server did not run it", async () => { + server("refused"); + const fetchMock = vi.mocked(fetch); + await sendOne(); + const asked = fetchMock.mock.calls.some(([, init]) => (init as RequestInit).body?.toString().includes("Email/query")); + expect(asked).toBe(false); + }); + + it("says plainly when it cannot tell, instead of offering a resend that could duplicate", async () => { + server("offline-after"); + await sendOne(); + const msgs = toastTexts(); + expect(msgs.some((t) => t.includes("Couldn't confirm whether this message was sent"))).toBe(true); + expect(msgs.some((t) => t.startsWith("Send failed"))).toBe(false); + }); +}); + +describe("sending a draft again after a failure", () => { + it("keeps the Message-ID, and sends nothing when the first attempt went out", async () => { + const st = server("ok"); + // The first attempt went out; the client never heard. + st.emails.push({ id: "e1", messageId: "fixed@example.org" }); + st.submissions.push({ id: "se1", emailId: "e1" }); + await sendOne({ sendMessageId: "fixed@example.org" }); + expect(st.creates).toBe(0); + expect(toastTexts().some((t) => t.includes("had already been sent"))).toBe(true); + }); + + it("sends it, under the same Message-ID, when the first attempt did not go out", async () => { + const st = server("ok"); + await sendOne({ sendMessageId: "fixed@example.org" }); + expect(st.creates).toBe(1); + expect(st.emails[0]!.messageId).toBe("fixed@example.org"); + expect(toastTexts()).toContain("Message sent"); + }); + + it("reopens a failed draft with its Message-ID, so the next send can check", async () => { + server("orphan"); + await sendOne(); + const toast = useToasts.getState().toasts.find((t) => t.message.startsWith("Send failed"))!; + toast.action!.onClick(); + const reopened = useCompose.getState().drafts[0]!; + expect(reopened.sendMessageId).toMatch(/@example\.org$/); + }); +}); diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 04219d7..05831f4 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -1,5 +1,5 @@ import { create } from "zustand"; -import { client, setErrorMessage } from "@/jmap/client"; +import { ApiError, client, setErrorMessage } from "@/jmap/client"; import type { Email, EmailAddress, EmailBodyPart, Id, Identity, SetResponse } from "@/jmap/types"; import { formatFullDate, uid } from "@/lib/format"; import { formatAddress, parseMailto, sameAddress, uniqueAddresses } from "@/lib/address"; @@ -94,6 +94,12 @@ export interface Draft { mailboxIdOnSend?: Id | null; /** When set, hand the message to the server held until this instant. */ sendAt: number | null; + /** + * The Message-ID this draft goes out under, fixed at its first send and + * kept if the draft comes back after a failure. Sending it again asks the + * server first whether a message with this id already went out. + */ + sendMessageId?: string; } interface ComposeState { @@ -680,8 +686,12 @@ export const useCompose = create((set, get) => ({ }, async send(key) { - const d = get().drafts.find((x) => x.key === key); - if (!d) return; + const found = get().drafts.find((x) => x.key === key); + if (!found) return; + // A draft that already has a Message-ID has been sent before, and failed. + const retry = Boolean(found.sendMessageId); + const fromEmail = (useMail.getState().identities.find((i) => i.id === found.identityId) ?? useMail.getState().identities[0])?.email ?? ""; + const d: Draft = { ...found, sendMessageId: found.sendMessageId ?? newMessageId(fromEmail) }; const delay = settings().undoSendSeconds; // A schedule the user left sitting until it passed is just a send now. const scheduling = d.sendAt !== null && d.sendAt > Date.now(); @@ -696,9 +706,20 @@ export const useCompose = create((set, get) => ({ return { pendingSends: rest }; }); try { - await sendInternal(d, get); + const sent = await sendInternal(d, get, { retry }); + if (sent.alreadySent) { + toast.success(translate("This message had already been sent, so it wasn't sent again.")); + return; + } toast.success(scheduling ? translate("Send scheduled for {when}", { when: formatScheduleTime(new Date(d.sendAt!)) }) : translate("Message sent")); } catch (err) { + if (err instanceof SendOutcomeUnknown) { + toast.error(err.message, { + action: { label: translate("Open draft"), onClick: () => set((s) => ({ drafts: [...s.drafts, { ...d, sending: false, error: err.message }], activeKey: d.key })) }, + duration: 30000, + }); + return; + } toast.error(translate("Send failed: {error}", { error: (err as Error).message }), { action: { label: translate("Open draft"), onClick: () => set((s) => ({ drafts: [...s.drafts, { ...d, sending: false, error: (err as Error).message }], activeKey: d.key })) }, duration: 15000, @@ -1008,7 +1029,50 @@ export function buildSubmission(opts: { }; } -async function sendInternal(d: Draft, _get: () => ComposeState): Promise { +/** + * A send whose outcome nobody can state: the request went out, no answer came + * back, and the server could not be asked afterwards either. Offering a plain + * "send again" here is how one message is delivered twice. + */ +export class SendOutcomeUnknown extends Error {} + +/** A Message-ID for one send, on the sending identity's domain (RFC 5322 §3.6.4). */ +export function newMessageId(fromEmail: string): string { + const domain = fromEmail.split("@")[1] || "localhost"; + return `${crypto.randomUUID()}@${domain}`; +} + +/** + * Whether a failed request may still have been carried out. A refusal (4xx) + * means the server did not run it; no answer, or a 5xx from the proxy that + * lost the upstream reply, means it may have. + */ +export function sendOutcomeUnknown(err: unknown): boolean { + if (err instanceof ApiError) return err.status >= 500 || err.status === 408; + return true; +} + +/** + * What became of a send with this Message-ID. + * + * `sent`: a message carrying it exists and was submitted. `orphan`: it exists + * but no submission does, so it was created and never sent. `none`: nothing. + * + * Stalwart expunges submissions after its hold period, so "no submission" + * means unsent only right after the attempt (`justNow`). Later, a message + * still carrying the id is taken as sent: every path that fails a send + * destroys the copy it created. + */ +export async function traceSend(accountId: Id, messageId: string, justNow: boolean): Promise<{ trace: "sent" | "orphan" | "none"; emailIds: Id[] }> { + const q = await client.call<{ ids: Id[] }>("Email/query", { accountId, filter: { header: ["Message-ID", messageId] } }); + const emailIds = q.ids ?? []; + if (!emailIds.length) return { trace: "none", emailIds }; + if (!justNow) return { trace: "sent", emailIds }; + const subs = await client.call<{ ids: Id[] }>("EmailSubmission/query", { accountId, filter: { emailIds } }); + return { trace: subs.ids?.length ? "sent" : "orphan", emailIds }; +} + +async function sendInternal(d: Draft, _get: () => ComposeState, opts: { retry?: boolean } = {}): Promise<{ alreadySent?: boolean }> { const mail = useMail.getState(); const accountId = mail.accountId!; const ident = mail.identities.find((i) => i.id === d.identityId) ?? mail.identities[0]; @@ -1017,6 +1081,27 @@ async function sendInternal(d: Draft, _get: () => ComposeState): Promise { const scheduled = d.sendAt !== null && d.sendAt > Date.now(); const scheduledId = scheduled ? await ensureScheduledMailbox() : null; const email = await buildEmailObject(d, { forSend: true, mailboxId: scheduledId }); + const messageId = d.sendMessageId ?? newMessageId(ident.email); + email.messageId = [messageId]; + /* + * Sending again a draft that came back from a failed send: ask first. The + * failure may have been only the reply going missing, and a second + * submission would deliver the message twice. + */ + if (opts.retry) { + let found: Awaited>; + try { + found = await traceSend(accountId, messageId, false); + } catch { + throw new SendOutcomeUnknown(translate("Couldn't check whether this message was already sent. Check Sent before sending it again.")); + } + if (found.trace === "sent") { + if (d.draftId) void client.call("Email/set", { accountId, destroy: [d.draftId] }); + void mail.loadMailboxes(); + void mail.refreshList(); + return { alreadySent: true }; + } + } const sentId = mail.roleId("sent"); const draftsId = mail.roleId("drafts"); const rcpts = uniqueAddresses([...d.to, ...d.cc, ...d.bcc]).map((a) => ({ email: a.email })); @@ -1042,7 +1127,27 @@ async function sendInternal(d: Draft, _get: () => ComposeState): Promise { if (d.relatedEmailId && d.relatedKeyword) { calls.push(["Email/set", { accountId, update: { [d.relatedEmailId]: { [`keywords/${d.relatedKeyword}`]: true } } }, "k"]); } - const res = await client.chain(calls, { allowErrors: true }); + let res: Awaited>; + try { + res = await client.chain(calls, { allowErrors: true }); + } catch (err) { + if (!sendOutcomeUnknown(err)) throw err; + // No answer is not a no: find out what the server did with it. + let found: Awaited>; + try { + found = await traceSend(accountId, messageId, true); + } catch { + throw new SendOutcomeUnknown(translate("Couldn't confirm whether this message was sent. Check Sent before sending it again.")); + } + if (found.trace === "sent") { + void mail.loadMailboxes(); + void mail.refreshList(); + return {}; + } + // Created but never submitted: take it out of Sent, then report the failure. + if (found.trace === "orphan") void client.call("Email/set", { accountId, destroy: found.emailIds }); + throw err; + } const e = res.get("e")?.[0] as unknown as SetResponse & { __error?: { type: string; description?: string } }; if (e.__error) throw new Error(setErrorMessage(e.__error)); if (e.notCreated?.m) throw new Error(setErrorMessage(e.notCreated.m)); @@ -1074,6 +1179,7 @@ async function sendInternal(d: Draft, _get: () => ComposeState): Promise { } void mail.loadMailboxes(); void mail.refreshList(); + return {}; } export { FULL_PROPS, BODY_PROPS }; diff --git a/web/src/styles/app.css b/web/src/styles/app.css index de161b0..2ccfdf0 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -1302,7 +1302,8 @@ a.menu-item:hover { color: var(--fg); } .collapsed .compose-btn span { display: none; } .nav-section { display: flex; align-items: center; justify-content: space-between; padding: 10px 12px 4px; font-size: .75em; font-weight: 700; letter-spacing: .06em; text-transform: uppercase; color: var(--fg-faint); } .nav-section .icon-btn { width: 24px; height: 24px; } -.collapsed .nav-section { display: none; } +/* Outweighs `.sidebar .nav-section { display: flex }` further down, which otherwise draws a cut-off heading in the collapsed strip. */ +.collapsed .nav-section, .app-body.collapsed .sidebar .nav-section { display: none; } .nav-item { position: relative; display: flex; align-items: center; gap: 12px; height: 36px; padding: 0 12px; border-radius: 0 999px 999px 0; margin-right: 8px; color: var(--fg); text-decoration: none; white-space: nowrap; transition: background .12s; user-select: none; font-weight: 450; } .nav-item:hover { background: var(--bg-hover); } .nav-item.unread .nav-label, .nav-item.unread .nav-count { font-weight: 700; color: var(--fg); } diff --git a/web/src/views/compose/Composer.tsx b/web/src/views/compose/Composer.tsx index a08f4f7..a19224f 100644 --- a/web/src/views/compose/Composer.tsx +++ b/web/src/views/compose/Composer.tsx @@ -235,7 +235,7 @@ export function Composer({ draft }: { draft: Draft }) { already knows the name they are half-way through typing. */} - patch({ to })} placeholder={translate("Recipients")} autoFocus={initialFocus === "to"} /> + patch({ to })} placeholder={translate("Recipients")} autoFocus={initialFocus === "to"} /> {/* Beside Cc and Bcc, because that is where someone looks when they are thinking about who the message goes to. The label @@ -255,13 +255,13 @@ export function Composer({ draft }: { draft: Draft }) { {d.showCc && (
- patch({ cc })} /> + patch({ cc })} />
)} {d.showBcc && (
- patch({ bcc })} /> + patch({ bcc })} />
)}
diff --git a/web/src/views/compose/RecipientInput.tsx b/web/src/views/compose/RecipientInput.tsx index 9bb5acf..a7a81ac 100644 --- a/web/src/views/compose/RecipientInput.tsx +++ b/web/src/views/compose/RecipientInput.tsx @@ -12,9 +12,11 @@ interface Props { placeholder?: string; autoFocus?: boolean; id?: string; + /** Addresses already in the message's other recipient fields, not to be suggested again. */ + exclude?: EmailAddress[]; } -export function RecipientInput({ value, onChange, placeholder, autoFocus, id }: Props) { +export function RecipientInput({ value, onChange, placeholder, autoFocus, id, exclude }: Props) { const [text, setText] = useState(""); const [sugg, setSugg] = useState([]); const [active, setActive] = useState(0); @@ -34,7 +36,7 @@ export function RecipientInput({ value, onChange, placeholder, autoFocus, id }: const t = window.setTimeout(() => { void suggest(q).then((list) => { if (id !== reqId.current) return; - const existing = new Set(value.map((v) => v.email.toLowerCase())); + const existing = new Set([...value, ...(exclude ?? [])].map((v) => v.email.toLowerCase())); const filtered = list.filter((s) => !existing.has(s.email.toLowerCase())); setSugg(filtered); setActive(0); @@ -42,7 +44,7 @@ export function RecipientInput({ value, onChange, placeholder, autoFocus, id }: }); }, 120); return () => window.clearTimeout(t); - }, [text, suggest, value]); + }, [text, suggest, value, exclude]); const commit = (raw?: string) => { const s = (raw ?? text).trim().replace(/[,;]+$/, ""); diff --git a/web/src/views/compose/__tests__/recipient-exclude.test.tsx b/web/src/views/compose/__tests__/recipient-exclude.test.tsx new file mode 100644 index 0000000..b80ad1e --- /dev/null +++ b/web/src/views/compose/__tests__/recipient-exclude.test.tsx @@ -0,0 +1,55 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { RecipientInput } from "../RecipientInput"; +import { useContacts } from "@/store/contacts"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/* + * Someone already in Cc was still offered when typing in To, so it was easy + * to address the same person twice from two fields. + */ + +describe("suggestions across To, Cc and Bcc", () => { + let host: HTMLDivElement; + let root: Root; + + beforeEach(() => { + vi.useFakeTimers(); + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + useContacts.setState({ + suggest: (async () => [ + { name: "Ann Lee", email: "ann@example.org", source: "contact" }, + { name: "Ann Taylor", email: "ataylor@example.org", source: "contact" }, + ]) as never, + }); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + host.remove(); + vi.useRealTimers(); + }); + + it("leaves out an address already in another recipient field", async () => { + await act(async () => { + root.render( {}} />); + }); + const input = host.querySelector("input")!; + await act(async () => { + const set = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!; + set.call(input, "ann"); + input.dispatchEvent(new Event("input", { bubbles: true })); + }); + await act(async () => { + vi.advanceTimersByTime(200); + await Promise.resolve(); + }); + const offered = host.textContent ?? ""; + expect(offered).toContain("ataylor@example.org"); + expect(offered).not.toContain("ann@example.org"); + }); +}); -- 2.54.0