From 7caa847737696429ce82750e40b1136945878569 Mon Sep 17 00:00:00 2001 From: John Ellis Date: Sun, 23 Aug 2026 13:51:02 -0700 Subject: [PATCH 1/2] Say which property a JMAP SetError rejected "Send failed: Invalid property or value." is Stalwart's description for invalidProperties, and on its own it says nothing about what to fix. The SetError also carries a `properties` array naming the offending fields, which every call site was discarding. setErrorMessage appends them, and the 35 places that surfaced a SetError - send, save draft, mailboxes, calendars, contacts, sieve, files, signature images, sharing - now go through it. --- web/src/jmap/client.ts | 14 ++++++++++++++ web/src/lib/__tests__/seterror.test.ts | 18 ++++++++++++++++++ web/src/lib/signatureImages.ts | 8 ++++---- web/src/store/calendar.ts | 14 +++++++------- web/src/store/compose.ts | 12 ++++++------ web/src/store/contacts.ts | 14 +++++++------- web/src/store/files.ts | 12 ++++++------ web/src/store/mail.ts | 16 ++++++++-------- web/src/store/sieve.ts | 6 +++--- web/src/views/settings/ShareDialog.tsx | 4 ++-- 10 files changed, 75 insertions(+), 43 deletions(-) create mode 100644 web/src/lib/__tests__/seterror.test.ts diff --git a/web/src/jmap/client.ts b/web/src/jmap/client.ts index ca35623..ba07fdb 100644 --- a/web/src/jmap/client.ts +++ b/web/src/jmap/client.ts @@ -346,3 +346,17 @@ export function chunk(arr: T[], size: number): T[][] { for (let i = 0; i < arr.length; i += size) out.push(arr.slice(i, i + size)); return out; } + +/** + * A readable message for a JMAP SetError. + * + * Servers name the offending field in `properties`, which is usually the whole + * answer to "why was this rejected" — Stalwart's description alone is often + * just "Invalid property or value." Keep both. + */ +export function setErrorMessage(err: { type: string; description?: string; properties?: string[] } | null | undefined): string { + if (!err) return "Unknown error"; + const base = err.description ?? err.type; + const props = err.properties?.length ? ` (${err.properties.join(", ")})` : ""; + return `${base}${props}`; +} diff --git a/web/src/lib/__tests__/seterror.test.ts b/web/src/lib/__tests__/seterror.test.ts new file mode 100644 index 0000000..75c6219 --- /dev/null +++ b/web/src/lib/__tests__/seterror.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from "vitest"; +import { setErrorMessage } from "@/jmap/client"; + +describe("setErrorMessage", () => { + it("names the offending properties, which is usually the whole answer", () => { + expect(setErrorMessage({ type: "invalidProperties", description: "Invalid property or value.", properties: ["sentAt"] })) + .toBe("Invalid property or value. (sentAt)"); + expect(setErrorMessage({ type: "invalidProperties", properties: ["to", "cc"] })) + .toBe("invalidProperties (to, cc)"); + }); + + it("falls back cleanly when the server says less", () => { + expect(setErrorMessage({ type: "forbidden" })).toBe("forbidden"); + expect(setErrorMessage({ type: "forbidden", description: "Not allowed" })).toBe("Not allowed"); + expect(setErrorMessage({ type: "x", properties: [] })).toBe("x"); + expect(setErrorMessage(null)).toBe("Unknown error"); + }); +}); diff --git a/web/src/lib/signatureImages.ts b/web/src/lib/signatureImages.ts index 06f1a16..16eaef5 100644 --- a/web/src/lib/signatureImages.ts +++ b/web/src/lib/signatureImages.ts @@ -4,7 +4,7 @@ * blobs) under an "ihasmail" folder and reference them by blob URL; the composer * turns such references into inline cid: parts when sending. */ -import { CAP, client } from "@/jmap/client"; +import { CAP, client, setErrorMessage } from "@/jmap/client"; import type { FileNode, GetResponse, QueryResponse, SetResponse } from "@/jmap/types"; import { useSession } from "@/store/session"; import { toast } from "@/ui/toast"; @@ -31,7 +31,7 @@ async function ensureFolder(accountId: string): Promise { if (existing) return existing.id; const set = await client.call>("FileNode/set", { accountId, create: { d: { parentId: null, name: FOLDER, nodeType: "directory" } } }); const err = set.notCreated?.d; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); return set.created!.d!.id; } @@ -53,7 +53,7 @@ export async function uploadSignatureImage(file: File): Promise { const name = `${Date.now()}-${file.name.replace(/[^\w.-]+/g, "_")}`; const res = await client.call>("FileNode/set", { accountId, create: { f: { parentId: folderId, name, nodeType: "file", blobId: up.blobId, type } } }); const err = res.notCreated?.f; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); const created = res.created?.f as Partial | undefined; // Prefer the node's (persistent) blobId if the server returned one. const blobId = created?.blobId ?? (await nodeBlobId(accountId, created?.id)) ?? up.blobId; @@ -73,7 +73,7 @@ export async function storeSignatureHtml(html: string): Promise { const name = `signature-${Date.now()}.html`; const res = await client.call>("FileNode/set", { accountId, create: { f: { parentId: folderId, name, nodeType: "file", blobId: up.blobId, type: "text/html" } } }); const err = res.notCreated?.f; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); const created = res.created?.f as Partial | undefined; return created?.blobId ?? (await nodeBlobId(accountId, created?.id)) ?? up.blobId; } diff --git a/web/src/store/calendar.ts b/web/src/store/calendar.ts index dd18e46..9513180 100644 --- a/web/src/store/calendar.ts +++ b/web/src/store/calendar.ts @@ -1,5 +1,5 @@ import { create } from "zustand"; -import { CAP, client } from "@/jmap/client"; +import { CAP, client, setErrorMessage } from "@/jmap/client"; import type { BusyPeriod, Calendar, CalendarEvent, GetResponse, Id, ParticipantIdentity, QueryResponse, SetResponse } from "@/jmap/types"; import { toUTCDate, toLocalDateTime, zonedToDate, parseDuration, DAY_MS, browserTimeZone } from "@/lib/dates"; import { settings } from "./settings"; @@ -168,7 +168,7 @@ export const useCalendar = create((set, get) => ({ const obj = { "@type": "Event", uid: crypto.randomUUID(), ...event, calendarIds: { [calendarId]: true } }; const res = await client.call>("CalendarEvent/set", { accountId, create: { e: obj }, sendSchedulingMessages: sendInvites }); const err = res.notCreated?.e; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); get().invalidate(); return res.created!.e!.id; }, @@ -177,7 +177,7 @@ export const useCalendar = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("CalendarEvent/set", { accountId, update: { [id]: patch }, sendSchedulingMessages: sendInvites }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); get().invalidate(); }, @@ -185,7 +185,7 @@ export const useCalendar = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("CalendarEvent/set", { accountId, destroy: [id], sendSchedulingMessages: sendInvites }); const err = res.notDestroyed?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); set((s) => { const events = { ...s.events }; delete events[id]; @@ -212,7 +212,7 @@ export const useCalendar = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call>("Calendar/set", { accountId, create: { c: { name: "Calendar", ...data } } }); const err = res.notCreated?.c; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadCalendars(); return res.created!.c!.id; }, @@ -221,7 +221,7 @@ export const useCalendar = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("Calendar/set", { accountId, update: { [id]: patch } }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadCalendars(); }, @@ -229,7 +229,7 @@ export const useCalendar = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("Calendar/set", { accountId, destroy: [id], onDestroyRemoveEvents: true }); const err = res.notDestroyed?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadCalendars(); get().invalidate(); }, diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 3a1e23c..4e18300 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -1,5 +1,5 @@ import { create } from "zustand"; -import { client } from "@/jmap/client"; +import { 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"; @@ -611,7 +611,7 @@ async function saveDraftInternal(d: Draft, get: () => ComposeState, set: (fn: (s if (d.draftId) args.destroy = [d.draftId]; const res = await client.call>("Email/set", args); const err = res.notCreated?.draft; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); const newId = res.created?.draft?.id ?? null; if (!opts.final) set((s) => ({ drafts: s.drafts.map((x) => (x.key === d.key ? { ...x, draftId: newId, saving: false, dirty: false, savedAt: Date.now(), error: null } : x)) })); void mail.loadMailboxes(); @@ -654,16 +654,16 @@ async function sendInternal(d: Draft, _get: () => ComposeState): Promise { } const res = await client.chain(calls, { allowErrors: true }); const e = res.get("e")?.[0] as unknown as SetResponse & { __error?: { type: string; description?: string } }; - if (e.__error) throw new Error(e.__error.description ?? e.__error.type); - if (e.notCreated?.m) throw new Error(e.notCreated.m.description ?? e.notCreated.m.type); + if (e.__error) throw new Error(setErrorMessage(e.__error)); + if (e.notCreated?.m) throw new Error(setErrorMessage(e.notCreated.m)); const s = res.get("s")?.[0] as unknown as SetResponse & { __error?: { type: string; description?: string } }; - if (s.__error) throw new Error(s.__error.description ?? s.__error.type); + if (s.__error) throw new Error(setErrorMessage(s.__error)); if (s.notCreated?.s) { const err = s.notCreated.s; // Clean up the created (unsent) email so it doesn't linger in Sent. const created = e.created?.m?.id; if (created) void client.call("Email/set", { accountId, destroy: [created] }); - throw new Error(err.description ?? err.type); + throw new Error(setErrorMessage(err)); } if (d.relatedEmailId && d.relatedKeyword) { useMail.setState((st) => { diff --git a/web/src/store/contacts.ts b/web/src/store/contacts.ts index 1c36e3a..a0c1447 100644 --- a/web/src/store/contacts.ts +++ b/web/src/store/contacts.ts @@ -1,5 +1,5 @@ import { create } from "zustand"; -import { CAP, client } from "@/jmap/client"; +import { CAP, client, setErrorMessage } from "@/jmap/client"; import type { AddressBook, ContactCard, EmailAddress, GetResponse, Id, Principal, QueryResponse, SetResponse } from "@/jmap/types"; import { contactDisplayName, contactEmails, sortKey } from "@/lib/contacts"; import { useSession } from "./session"; @@ -133,7 +133,7 @@ export const useContacts = create((set, get) => ({ const obj = { "@type": "Card", version: "1.0", uid: crypto.randomUUID(), kind: "individual", ...card, addressBookIds: { [addressBookId]: true } }; const res = await client.call>("ContactCard/set", { accountId, create: { c: obj } }); const err = res.notCreated?.c; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); const id = res.created!.c!.id; await get().getCard(id); return id; @@ -143,7 +143,7 @@ export const useContacts = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("ContactCard/set", { accountId, update: { [id]: patch } }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().getCard(id); }, @@ -151,7 +151,7 @@ export const useContacts = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("ContactCard/set", { accountId, destroy: ids }); const failed = Object.values(res.notDestroyed ?? {})[0]; - if (failed) throw new Error(failed.description ?? failed.type); + if (failed) throw new Error(setErrorMessage(failed)); set((s) => { const cards = { ...s.cards }; for (const id of ids) delete cards[id]; @@ -163,7 +163,7 @@ export const useContacts = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call>("AddressBook/set", { accountId, create: { b: { name } } }); const err = res.notCreated?.b; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadBooks(); return res.created!.b!.id; }, @@ -172,7 +172,7 @@ export const useContacts = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("AddressBook/set", { accountId, update: { [id]: patch } }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadBooks(); }, @@ -180,7 +180,7 @@ export const useContacts = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("AddressBook/set", { accountId, destroy: [id], onDestroyRemoveContents: true }); const err = res.notDestroyed?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadBooks(); await get().loadAll(); }, diff --git a/web/src/store/files.ts b/web/src/store/files.ts index d01f435..f5cb530 100644 --- a/web/src/store/files.ts +++ b/web/src/store/files.ts @@ -1,5 +1,5 @@ import { create } from "zustand"; -import { CAP, client, JmapMethodError } from "@/jmap/client"; +import { CAP, JmapMethodError, client, setErrorMessage } from "@/jmap/client"; import type { FileNode, GetResponse, Id, QueryResponse, SetResponse } from "@/jmap/types"; import { useSession } from "./session"; @@ -114,7 +114,7 @@ export const useFiles = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call>("FileNode/set", { accountId, create: { d: { parentId, name, nodeType: "directory" } } }); const err = res.notCreated?.d; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadChildren(parentId); return res.created!.d!.id; }, @@ -134,7 +134,7 @@ export const useFiles = create((set, get) => ({ create: { f: { parentId, name: f.name, nodeType: "file", blobId: up.blobId, type: f.type || "application/octet-stream" } }, }); const err = res.notCreated?.f; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); set((s) => ({ uploads: s.uploads.filter((u) => u.id !== id) })); } catch (err) { set((s) => ({ uploads: s.uploads.map((u) => (u.id === id ? { ...u, error: (err as Error).message } : u)) })); @@ -147,7 +147,7 @@ export const useFiles = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("FileNode/set", { accountId, update: { [id]: { name } } }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadChildren(get().nodes[id]?.parentId ?? null); }, @@ -156,7 +156,7 @@ export const useFiles = create((set, get) => ({ const from = get().nodes[id]?.parentId ?? null; const res = await client.call("FileNode/set", { accountId, update: { [id]: { parentId } } }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await Promise.all([get().loadChildren(from), get().loadChildren(parentId)]); }, @@ -165,7 +165,7 @@ export const useFiles = create((set, get) => ({ const parents = new Set(ids.map((id) => get().nodes[id]?.parentId ?? null)); const res = await client.call("FileNode/set", { accountId, destroy: ids, onDestroyRemoveChildren: true }); const failed = Object.values(res.notDestroyed ?? {})[0]; - if (failed) throw new Error(failed.description ?? failed.type); + if (failed) throw new Error(setErrorMessage(failed)); for (const p of parents) await get().loadChildren(p); }, diff --git a/web/src/store/mail.ts b/web/src/store/mail.ts index 2b8bf78..5955768 100644 --- a/web/src/store/mail.ts +++ b/web/src/store/mail.ts @@ -1,5 +1,5 @@ import { create } from "zustand"; -import { client, chunk, JmapMethodError } from "@/jmap/client"; +import { JmapMethodError, chunk, client, setErrorMessage } from "@/jmap/client"; import type { Comparator, Email, @@ -626,7 +626,7 @@ export const useMail = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call>("Mailbox/set", { accountId, create: { n: { name, parentId, isSubscribed: true } } }); const err = res.notCreated?.n; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadMailboxes(); return res.created!.n!.id; }, @@ -635,7 +635,7 @@ export const useMail = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("Mailbox/set", { accountId, update: { [id]: patch } }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadMailboxes(); }, @@ -643,7 +643,7 @@ export const useMail = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("Mailbox/set", { accountId, destroy: [id], onDestroyRemoveEmails: removeEmails }); const err = res.notDestroyed?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadMailboxes(); }, @@ -684,7 +684,7 @@ export const useMail = create((set, get) => ({ ? await client.call>("Identity/set", { accountId, update: { [id]: patch } }) : await client.call>("Identity/set", { accountId, create: { n: patch } }); const err = id ? res.notUpdated?.[id] : res.notCreated?.n; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadIdentities(); }, @@ -692,7 +692,7 @@ export const useMail = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("Identity/set", { accountId, destroy: [id] }); const err = res.notDestroyed?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadIdentities(); }, @@ -711,7 +711,7 @@ export const useMail = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("VacationResponse/set", { accountId, update: { singleton: patch } }); const err = res.notUpdated?.singleton; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().loadVacation(); }, @@ -823,7 +823,7 @@ export const useMail = create((set, get) => ({ accountId, emails: { i: { blobId, mailboxIds: { [mailboxId]: true }, keywords } }, }); - if (res.notCreated?.i) throw new Error(res.notCreated.i.description ?? res.notCreated.i.type); + if (res.notCreated?.i) throw new Error(setErrorMessage(res.notCreated.i)); void get().refreshList(); void get().loadMailboxes(); return res.created?.i?.id ?? null; diff --git a/web/src/store/sieve.ts b/web/src/store/sieve.ts index 0f0294c..b5b6631 100644 --- a/web/src/store/sieve.ts +++ b/web/src/store/sieve.ts @@ -1,5 +1,5 @@ import { create } from "zustand"; -import { CAP, client } from "@/jmap/client"; +import { CAP, client, setErrorMessage } from "@/jmap/client"; import type { GetResponse, Id, SetResponse, SieveScript } from "@/jmap/types"; import { rulesToSieve, sieveToRules, type SieveRule } from "@/lib/sieve"; import { useSession } from "./session"; @@ -97,7 +97,7 @@ export const useSieve = create((set, get) => ({ if (activate) args.onSuccessActivateScript = id ?? "#s"; const res = await client.call>("SieveScript/set", args); const err = id ? res.notUpdated?.[id] : res.notCreated?.s; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); const newId = id ?? res.created!.s!.id; set((s) => ({ contents: { ...s.contents, [newId]: content } })); await get().load(); @@ -118,7 +118,7 @@ export const useSieve = create((set, get) => ({ const accountId = get().accountId!; const res = await client.call("SieveScript/set", { accountId, destroy: [id] }); const err = res.notDestroyed?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); await get().load(); }, diff --git a/web/src/views/settings/ShareDialog.tsx b/web/src/views/settings/ShareDialog.tsx index 7395010..9408079 100644 --- a/web/src/views/settings/ShareDialog.tsx +++ b/web/src/views/settings/ShareDialog.tsx @@ -4,7 +4,7 @@ import { Dialog } from "@/ui/dialog"; import { useContacts } from "@/store/contacts"; import { useMail } from "@/store/mail"; import { useCalendar } from "@/store/calendar"; -import { client } from "@/jmap/client"; +import { client, setErrorMessage } from "@/jmap/client"; import { toast } from "@/ui/toast"; import type { Id, Principal } from "@/jmap/types"; @@ -70,7 +70,7 @@ export function ShareDialog({ kind, id, name, shareWith, onClose }: { kind: Kind const accountId = kind === "Mailbox" ? useMail.getState().accountId : kind === "Calendar" ? useCalendar.getState().accountId : useContacts.getState().accountId; const res = await client.call<{ notUpdated?: Record }>(`${kind}/set`, { accountId, update: { [id]: { shareWith: Object.keys(rights).length ? rights : null } } }); const err = res.notUpdated?.[id]; - if (err) throw new Error(err.description ?? err.type); + if (err) throw new Error(setErrorMessage(err)); toast.success("Sharing updated"); if (kind === "Mailbox") void useMail.getState().loadMailboxes(); if (kind === "Calendar") void useCalendar.getState().loadCalendars(); From eadec49b4f0dd2d8203f56337e1051d2f8effd4f Mon Sep 17 00:00:00 2001 From: John Ellis Date: Sun, 23 Aug 2026 13:59:33 -0700 Subject: [PATCH 2/2] Fix sending: never send null for an empty header property Every message ihasmail sent set cc, bcc and replyTo to null when unused, and inReplyTo/references likewise on a new message. Stalwart parses those properties with try_into_address_list, which returns None for null, and the create is rejected outright: if let Some(addresses) = value.try_into_address_list() { ... } else { response.invalid_property_create(id, header); continue 'create; } So every send failed with "Invalid property or value.", new messages and replies alike, regardless of attachments or signature. The mock server accepts anything, which is why this only showed up against a real server. Empty header properties are now omitted. On a create there is no previous value to clear, so null was never needed - only the properties actually being set belong in the object. buildEmailObject is exported so the shape can be tested directly, with a regression test that no property is ever null. --- web/src/store/__tests__/compose-email.test.ts | 76 +++++++++++++++++++ web/src/store/compose.ts | 19 +++-- 2 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 web/src/store/__tests__/compose-email.test.ts diff --git a/web/src/store/__tests__/compose-email.test.ts b/web/src/store/__tests__/compose-email.test.ts new file mode 100644 index 0000000..1424bcd --- /dev/null +++ b/web/src/store/__tests__/compose-email.test.ts @@ -0,0 +1,76 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { buildEmailObject, type Draft } from "@/store/compose"; +import { useMail } from "@/store/mail"; + +/** + * JMAP servers may reject `null` for a header property — Stalwart parses these + * as address lists and fails the whole create, which took down every send. + * Empty header fields must be omitted, not nulled. + */ +function draft(over: Partial = {}): Draft { + return { + key: "k", draftId: null, identityId: "i1", + to: [{ name: null, email: "ann@example.com" }], + cc: [], bcc: [], replyTo: [], + subject: "Hello", html: "", text: "Hi there", format: "text", + attachments: [], inReplyTo: null, references: null, + relatedEmailId: null, relatedKeyword: null, + requestReceipt: false, priority: "normal", + showCc: false, showBcc: false, showReplyTo: false, + minimized: false, maximized: false, dirty: false, savedAt: null, + saving: false, sending: false, error: null, signatureHtml: "", replyMode: null, + ...over, + }; +} + +beforeEach(() => { + useMail.setState({ + accountId: "a1", + identities: [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as never, + mailboxes: { mb1: { id: "mb1", role: "sent", name: "Sent" }, mb2: { id: "mb2", role: "drafts", name: "Drafts" } } as never, + }); +}); + +describe("buildEmailObject", () => { + it("omits empty header properties rather than sending null", async () => { + const obj = await buildEmailObject(draft(), { forSend: true }); + expect(obj).not.toHaveProperty("cc"); + expect(obj).not.toHaveProperty("bcc"); + expect(obj).not.toHaveProperty("replyTo"); + expect(obj).not.toHaveProperty("inReplyTo"); + expect(obj).not.toHaveProperty("references"); + expect(obj.to).toEqual([{ name: null, email: "ann@example.com" }]); + }); + + it("includes header properties that have a value", async () => { + const obj = await buildEmailObject( + draft({ + cc: [{ name: null, email: "c@x.io" }], + bcc: [{ name: null, email: "d@x.io" }], + replyTo: [{ name: null, email: "r@x.io" }], + inReplyTo: [""], + references: [""], + }), + { forSend: true }, + ); + expect(obj.cc).toHaveLength(1); + expect(obj.bcc).toHaveLength(1); + expect(obj.replyTo).toHaveLength(1); + expect(obj.inReplyTo).toEqual([""]); + expect(obj.references).toEqual([""]); + }); + + it("never emits a null value for any property", async () => { + for (const forSend of [true, false]) { + const obj = await buildEmailObject(draft({ subject: "" }), { forSend }); + for (const [k, v] of Object.entries(obj)) { + expect(v, `${k} is null`).not.toBeNull(); + } + } + }); + + it("files a sent message in Sent and a draft in Drafts", async () => { + expect((await buildEmailObject(draft(), { forSend: true })).mailboxIds).toEqual({ mb1: true }); + expect((await buildEmailObject(draft(), { forSend: false })).mailboxIds).toEqual({ mb2: true }); + }); +}); diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 4e18300..1702573 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -481,7 +481,7 @@ function scheduleAutosave(key: string, get: () => ComposeState) { } /** Build the JMAP Email creation object from a draft. */ -async function buildEmailObject(d: Draft, opts: { forSend: boolean }): Promise> { +export async function buildEmailObject(d: Draft, opts: { forSend: boolean }): Promise> { const mail = useMail.getState(); const accountId = mail.accountId!; const ident = mail.identities.find((i) => i.id === d.identityId) ?? mail.identities[0]; @@ -561,18 +561,23 @@ async function buildEmailObject(d: Draft, opts: { forSend: boolean }): Promise = { from: [from], - to: d.to.length ? d.to : null, - cc: d.cc.length ? d.cc : null, - bcc: d.bcc.length ? d.bcc : null, - replyTo: d.replyTo.length ? d.replyTo : ident.replyTo?.length ? ident.replyTo : null, subject: d.subject, sentAt: new Date().toISOString().replace(/\.\d{3}Z$/, "Z"), - inReplyTo: d.inReplyTo, - references: d.references, bodyStructure, bodyValues, "header:User-Agent:asText": "ihasmail/2.0", }; + // Header properties are omitted when empty, never sent as null: JMAP servers + // are entitled to reject null for a header field (Stalwart parses these as + // address lists and fails the whole create), and on a create there is no + // previous value that would need clearing. + const replyTo = d.replyTo.length ? d.replyTo : (ident.replyTo ?? []); + if (d.to.length) obj.to = d.to; + if (d.cc.length) obj.cc = d.cc; + if (d.bcc.length) obj.bcc = d.bcc; + if (replyTo.length) obj.replyTo = replyTo; + if (d.inReplyTo?.length) obj.inReplyTo = d.inReplyTo; + if (d.references?.length) obj.references = d.references; if (d.priority === "high") { obj["header:X-Priority:asText"] = "1 (Highest)"; obj["header:Importance:asText"] = "High";