From e05880eefc288e46819c0490d7a60f49bac7ab38 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 23 Aug 2026 13:51:02 -0700 Subject: [PATCH] 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();