From 06babea9ad928816e52fc7e778f841d4b8b46b43 Mon Sep 17 00:00:00 2001 From: mtoohey Date: Mon, 9 Nov 2026 15:20:46 +0000 Subject: [PATCH 1/2] fix(messages): map upload failures to plain copy and explain a blocked Send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed attachment upload showed the Error's own message, such as "Failed to fetch" or "signal timed out". attachment-draft.ts now maps it by type with userCopy's failureKind: - UploadError keeps its message, which is already written for people - a lost connection: "Upload didn’t finish. Check your connection, then retry." - the five-minute timeout: "Upload took too long. Retry, or remove this file." - anything else: "Upload failed. Retry or remove this file.", now the one fallback shared with ComposerAttachments (which said "Upload failed. Retry or remove it to send.") While a failed upload blocks it, Send stays focusable (aria-disabled) with the tooltip "Retry or remove the failed file to send.", and Send and Enter still send nothing. The failed tile keeps Retry, then Remove, as on base. The error and the actions each take a full row from the tile's inner edge; the error sits 12px below the file row and the actions 12px below the error, 8px apart. The actions never wrap: the tile's min-content width keeps room for both, so a strip too narrow for them scrolls instead. Co-Authored-By: Claude Fable 5.1 Signed-off-by: mtoohey --- .../NewMessage.recovery.test.tsx | 3 +- .../messages/ComposerAttachments.module.css | 13 ++- .../messages/ComposerAttachments.test.tsx | 9 +- src/features/messages/ComposerAttachments.tsx | 4 +- .../messages/MessageComposer.test.tsx | 54 ++++++++++-- src/features/messages/MessageComposer.tsx | 8 +- .../messages/attachment-draft.test.tsx | 71 ++++++++++++++-- src/features/messages/attachment-draft.ts | 21 ++++- src/features/solo/SoloDraft.test.tsx | 6 +- tests/browser/composer-attachments.spec.mjs | 84 ++++++++++++++----- 10 files changed, 228 insertions(+), 45 deletions(-) diff --git a/src/features/direct-messages/NewMessage.recovery.test.tsx b/src/features/direct-messages/NewMessage.recovery.test.tsx index e98034a6c4..3b2cedb5b1 100644 --- a/src/features/direct-messages/NewMessage.recovery.test.tsx +++ b/src/features/direct-messages/NewMessage.recovery.test.tsx @@ -1328,7 +1328,8 @@ it("waits for uploads before opening a DM and reports failure only on the chip", ), ).toHaveAttribute("role", "status"); expect(screen.getAllByRole("alert")).toHaveLength(1); - expect(send()).toBeDisabled(); + // Focusable while blocked, so its tooltip can say why. + expect(send()).toHaveAttribute("aria-disabled", "true"); expect(t.openDirectMessage).not.toHaveBeenCalled(); expect(t.publish).not.toHaveBeenCalled(); await t.user.click(screen.getByRole("button", { name: "Retry photo.png" })); diff --git a/src/features/messages/ComposerAttachments.module.css b/src/features/messages/ComposerAttachments.module.css index 5680e4ea40..f8b8482d8a 100644 --- a/src/features/messages/ComposerAttachments.module.css +++ b/src/features/messages/ComposerAttachments.module.css @@ -168,24 +168,31 @@ html[data-keyboard-navigation] .preview:focus-visible { } .item[data-upload-error] { - /* Fit the strip when possible, but never squeeze the recovery controls. */ + /* The error and its actions each take a full row from the tile's inner + edge, and the icon gap stays the resting tile's. Fit the strip when + possible, but never squeeze the recovery controls. */ min-width: min-content; flex-wrap: wrap; - column-gap: var(--space-2); row-gap: var(--space-1); padding-inline-end: calc(var(--space-5) / 2); } .item[data-upload-error] .error { + /* With the row gap, 12px below the file row, as the actions sit 12px + below the error. */ flex-basis: 100%; + margin-block-start: var(--space-2); } .item[data-upload-error] .details { /* Ellipsized filenames must not determine the card's minimum width. */ contain: inline-size; } .item[data-upload-error] .actions { + /* As in a toast: 12px from the message, 8px between actions. The actions + never wrap; the tile's min-content width keeps room for both. */ position: static; flex-basis: 100%; - flex-wrap: wrap; + gap: var(--space-2); + margin-block-start: var(--space-2); min-width: min-content; } diff --git a/src/features/messages/ComposerAttachments.test.tsx b/src/features/messages/ComposerAttachments.test.tsx index 5ec95446a4..d86da7bcf4 100644 --- a/src/features/messages/ComposerAttachments.test.tsx +++ b/src/features/messages/ComposerAttachments.test.tsx @@ -355,6 +355,11 @@ it.each([ const removeButton = screen.getByRole("button", { name: `Remove ${name}` }); expect(retryButton.textContent).toBe("Retry"); expect(removeButton.textContent).toBe("Remove"); + // Retry first, then Remove. + expect([...(retryButton.parentElement?.children ?? [])]).toEqual([ + retryButton, + removeButton, + ]); act(() => retryButton.focus()); expect(retryButton.getAttribute("title")).toBeNull(); fireEvent.click(retryButton); @@ -399,7 +404,7 @@ it.each([ expect(screen.getByText(error, { selector: "p" })).toBe(alert); expect(alert.classList.contains("sr-only")).toBe(false); expect( - screen.queryByText("Upload failed. Retry or remove it to send."), + screen.queryByText("Upload failed. Retry or remove this file."), ).toBeNull(); }); @@ -414,7 +419,7 @@ it("explains errors without service details and disables both recovery actions", , ); expect(screen.getByRole("alert").textContent).toBe( - "photo.png: Upload failed. Retry or remove it to send.", + "photo.png: Upload failed. Retry or remove this file.", ); for (const action of ["Retry", "Remove"]) { const button = screen.getByRole("button", { name: `${action} photo.png` }); diff --git a/src/features/messages/ComposerAttachments.tsx b/src/features/messages/ComposerAttachments.tsx index c59266874c..0c73f2e8ef 100644 --- a/src/features/messages/ComposerAttachments.tsx +++ b/src/features/messages/ComposerAttachments.tsx @@ -23,7 +23,7 @@ import { useDraftAttachmentSource, } from "./use-draft-attachment-source"; import { XIcon } from "../../shared/design-system/icons"; -import type { DraftAttachment } from "./attachment-draft"; +import { UPLOAD_FAILED_TEXT, type DraftAttachment } from "./attachment-draft"; import styles from "./ComposerAttachments.module.css"; import { useMediaElementSource } from "./use-media-element-source"; @@ -417,7 +417,7 @@ function AttachmentItem({ {uploadFailed && (

{item.file.name}: - {item.error || "Upload failed. Retry or remove it to send."} + {item.error || UPLOAD_FAILED_TEXT}

)}
expect(trigger).toHaveFocus(); }); +it("words a failed upload and explains the disabled Send, until the file is removed", async () => { + const h = await mountUploadComposer(); + pickAttachments(h.container, [attachmentFile()]); + await waitFor(() => expect(h.uploadCalls).toHaveLength(1)); + await act(() => + h.uploadCalls[0]?.result.reject(new TypeError("Failed to fetch")), + ); + expect(screen.getByRole("alert")).toHaveTextContent( + "Upload didn’t finish. Check your connection, then retry.", + ); + expect(screen.queryByText(/Failed to fetch/)).toBeNull(); + // A disabled button gets no hover from the browser, so Send stays focusable + // and its tooltip says what to do. + const send = screen.getByRole("button", { name: "Send message" }); + expect(send).toHaveAttribute("aria-disabled", "true"); + await userEvent.hover(send); + expect(await screen.findByRole("tooltip")).toHaveTextContent( + "Retry or remove the failed file to send.", + ); + await userEvent.unhover(send); + // Focusable is not enabled: neither Send nor Enter sends or uploads again. + fireEvent.change(h.input(), { target: { value: "With the file" } }); + fireEvent.click(send); + fireEvent.keyDown(h.input(), { key: "Enter" }); + await act(async () => {}); + expect(h.sign).not.toHaveBeenCalled(); + expect(h.publish).not.toHaveBeenCalled(); + expect(h.uploadCalls).toHaveLength(1); + expect(h.input()).toHaveValue("With the file"); + expect(screen.getByRole("alert")).toHaveTextContent( + "Upload didn’t finish. Check your connection, then retry.", + ); + fireEvent.click(screen.getByRole("button", { name: "Remove notes.txt" })); + await waitFor(() => + expect( + screen.getByRole("button", { name: "Send message" }), + ).not.toHaveAttribute("aria-disabled", "true"), + ); +}); + it.each(["inactive", "hidden"])( "cancels picker focus when the conversation becomes %s, including a warm return", async (state) => { @@ -3000,21 +3041,22 @@ it("offers message preparation retry after a failed attachment send remounts", a }); it("identifies failed filenames through retry and removal when service errors match", async () => { + const NETWORK = "Upload didn’t finish. Check your connection, then retry."; const h = await mountUploadComposer(); attachByPaste(h.input(), attachmentFile("one.txt")); attachByPaste(h.input(), attachmentFile("two.txt")); await waitFor(() => expect(h.uploadCalls).toHaveLength(1)); await act(async () => { - h.uploadCalls[0]?.result.reject(new Error("HTTP 503")); + h.uploadCalls[0]?.result.reject(new TypeError("Failed to fetch")); }); await waitFor(() => expect(h.uploadCalls).toHaveLength(2)); await act(async () => { - h.uploadCalls[1]?.result.reject(new Error("HTTP 503")); + h.uploadCalls[1]?.result.reject(new TypeError("Failed to fetch")); }); await waitFor(() => expect( screen.getAllByRole("alert").map((node) => node.textContent), - ).toEqual(["one.txt: HTTP 503", "two.txt: HTTP 503"]), + ).toEqual([`one.txt: ${NETWORK}`, `two.txt: ${NETWORK}`]), ); await userEvent.click(screen.getByRole("button", { name: "Retry one.txt" })); await waitFor(() => expect(h.uploadCalls).toHaveLength(3)); @@ -3024,12 +3066,12 @@ it("identifies failed filenames through retry and removal when service errors ma await waitFor(() => expect( screen.getAllByRole("alert").map((node) => node.textContent), - ).toEqual(["one.txt: first failed again", "two.txt: HTTP 503"]), + ).toEqual([`one.txt: ${UPLOAD_FAILED_TEXT}`, `two.txt: ${NETWORK}`]), ); await userEvent.click(screen.getByRole("button", { name: "Remove one.txt" })); expect(h.input()).toHaveFocus(); expect(screen.getAllByRole("alert").map((node) => node.textContent)).toEqual([ - "two.txt: HTTP 503", + `two.txt: ${NETWORK}`, ]); await waitFor(() => expect(within(h.form()).queryByText("one.txt")).not.toBeInTheDocument(), @@ -3057,7 +3099,7 @@ it("retains successful attachment uploads after a later file fails and retries o }); await screen.findAllByRole("alert"); expect(screen.getAllByRole("alert").map((node) => node.textContent)).toEqual([ - "retry.txt: relay down", + `retry.txt: ${UPLOAD_FAILED_TEXT}`, ]); expect( screen.getByRole("dialog", { name: "Upload to #General failed" }), diff --git a/src/features/messages/MessageComposer.tsx b/src/features/messages/MessageComposer.tsx index 873def1d46..4a79ee0844 100644 --- a/src/features/messages/MessageComposer.tsx +++ b/src/features/messages/MessageComposer.tsx @@ -3855,10 +3855,14 @@ function Composer({ ? "Wait for compaction to finish, or interrupt it." : untagged ? recipientHint - : "Send message" + : attachments.blocked + ? "Retry or remove the failed file to send." + : "Send message" } focusableWhenDisabled={ - untagged || (!editing.target && compacting) || undefined + untagged || + (!editing.target && (compacting || attachments.blocked)) || + undefined } disabled={ disabled || diff --git a/src/features/messages/attachment-draft.test.tsx b/src/features/messages/attachment-draft.test.tsx index f09128aa0c..19ea0d768f 100644 --- a/src/features/messages/attachment-draft.test.tsx +++ b/src/features/messages/attachment-draft.test.tsx @@ -12,8 +12,13 @@ import { import { assert, afterEach, expect, it, vi } from "vitest"; import { ComposerAttachments } from "./ComposerAttachments"; import type { RelaySession } from "../relay/session"; -import type { UploadedAttachment } from "../relay/attachments"; import { + UPLOAD_FAILURES, + UploadError, + type UploadedAttachment, +} from "../relay/attachments"; +import { + UPLOAD_FAILED_TEXT, useAttachmentDraft, attachmentDraft, transferAttachmentDraft, @@ -144,19 +149,75 @@ it("preserves real upload errors while resuming unfinished siblings on return", first.result.current.store.add([file("failed.txt"), file("pending.txt")]), ); await waitFor(() => expect(h.calls).toHaveLength(1)); - await act(async () => h.calls[0]?.result.reject(new Error("relay down"))); + await act(async () => h.calls[0]?.result.reject(new UploadError("failed"))); await waitFor(() => expect(h.calls).toHaveLength(2)); first.unmount(); const returned = renderHook(() => useAttachmentDraft(h.session, "one", "one"), ); await waitFor(() => expect(h.calls).toHaveLength(3)); - expect(returned.result.current.items[0]?.error).toBe("relay down"); + expect(returned.result.current.items[0]?.error).toBe(UPLOAD_FAILURES.failed); expect(returned.result.current.items[0]?.status).toBe("error"); expect(returned.result.current.blocked).toBe(true); expect(h.calls[2]?.file.name).toBe("pending.txt"); }); +it.each([ + [ + "a fetch network failure", + () => new TypeError("Failed to fetch"), + "Upload didn’t finish. Check your connection, then retry.", + ], + [ + "the upload timeout", + // jsdom's DOMException is not an Error; browsers' is, with this name. + () => + Object.assign(new Error("signal timed out"), { name: "TimeoutError" }), + "Upload took too long. Retry, or remove this file.", + ], + [ + "a relay refusal the client already worded", + () => new UploadError("capacity"), + UPLOAD_FAILURES.capacity, + ], + ["an unrecognised error", () => new Error("relay down"), UPLOAD_FAILED_TEXT], + ["a native string", () => "Something native broke", UPLOAD_FAILED_TEXT], +])( + "words %s for the tile, never with the raw text", + async (_name, make, text) => { + const h = fixture(); + const draft = renderHook(() => useAttachmentDraft(h.session, "one", "one")); + act(() => draft.result.current.store.add([file()])); + await waitFor(() => expect(h.calls).toHaveLength(1)); + await act(async () => h.calls[0]?.result.reject(make())); + expect(draft.result.current.items[0]).toMatchObject({ + status: "error", + error: text, + }); + expect(draft.result.current.blocked).toBe(true); + // Retry still starts a fresh upload. + act(() => + draft.result.current.store.retry(draft.result.current.items[0]?.id ?? ""), + ); + await waitFor(() => expect(h.calls).toHaveLength(2)); + }, +); + +it("does not mark a cancelled upload as failed", async () => { + const h = fixture(); + const draft = renderHook(() => useAttachmentDraft(h.session, "one", "one")); + act(() => draft.result.current.store.add([file()])); + await waitFor(() => expect(h.calls).toHaveLength(1)); + act(() => + draft.result.current.store.remove(draft.result.current.items[0]?.id ?? ""), + ); + await act(async () => + h.calls[0]?.result.reject(new DOMException("aborted", "AbortError")), + ); + expect(draft.result.current.items).toEqual([]); + expect(draft.result.current.blocked).toBe(false); +}); + it("ignores a host progress report after an upload fails", async () => { const h = fixture(); const draft = renderHook(() => useAttachmentDraft(h.session, "one", "one")); @@ -595,13 +656,13 @@ it("rejects eager upload errors until explicit Retry, including after Send hando const draft = renderHook(() => useAttachmentDraft(h.session, "one", "one")); act(() => draft.result.current.store.add([file()])); await waitFor(() => expect(h.calls).toHaveLength(1)); - await act(async () => h.calls[0]?.result.reject(new Error("relay down"))); + await act(async () => h.calls[0]?.result.reject(new UploadError("failed"))); expect(draft.result.current.blocked).toBe(true); const target = attachmentDraft(h.session, "send", "one"); act(() => transferAttachmentDraft(draft.result.current.store, target)); await expect( target.prepareForSend(new AbortController().signal), - ).rejects.toThrow("relay down"); + ).rejects.toThrow(UPLOAD_FAILURES.failed); expect(h.calls).toHaveLength(1); expect(target.snapshot()[0]?.status).toBe("error"); target.retry(target.snapshot()[0]?.id ?? ""); diff --git a/src/features/messages/attachment-draft.ts b/src/features/messages/attachment-draft.ts index e49c4ea59c..b2b495ce91 100644 --- a/src/features/messages/attachment-draft.ts +++ b/src/features/messages/attachment-draft.ts @@ -7,8 +7,24 @@ import { type UploadedAttachment, type UploadProgress, } from "../relay/attachments"; +import { failureKind } from "../relay/user-copy"; import { prepareAttachment } from "./prepare-attachment"; +/** The tile's sentence when nothing more specific is known. */ +export const UPLOAD_FAILED_TEXT = "Upload failed. Retry or remove this file."; + +/** Plain-language copy for a failed upload; never the browser's or host's own + * text. An `UploadError`'s message is already written for people. */ +function uploadFailureText(error: unknown) { + if (error instanceof UploadError) return error.message; + const kind = failureKind(error); + if (kind === "connection") + return "Upload didn’t finish. Check your connection, then retry."; + if (kind === "timeout") + return "Upload took too long. Retry, or remove this file."; + return UPLOAD_FAILED_TEXT; +} + export type DraftAttachment = Readonly<{ id: string; file: File; @@ -247,10 +263,7 @@ export function attachmentDraft( task.update(item.id, { status: "error", transfer: undefined, - error: - error instanceof Error - ? error.message - : "Upload failed. Retry or remove this file.", + error: uploadFailureText(error), }); throw error; } finally { diff --git a/src/features/solo/SoloDraft.test.tsx b/src/features/solo/SoloDraft.test.tsx index 241f698046..dfbc49e96b 100644 --- a/src/features/solo/SoloDraft.test.tsx +++ b/src/features/solo/SoloDraft.test.tsx @@ -2227,7 +2227,11 @@ it("a failed upload blocks a new chat before anything is created", async () => { expect( await screen.findByRole("button", { name: "Retry notes.txt" }), ).toBeTruthy(); - expect(screen.getByRole("button", { name: "Send message" })).toBeDisabled(); + // Focusable while blocked, so its tooltip can say why. + expect(screen.getByRole("button", { name: "Send message" })).toHaveAttribute( + "aria-disabled", + "true", + ); expect(test.channelWrites.create).not.toHaveBeenCalled(); expect(test.messages.send).not.toHaveBeenCalled(); }); diff --git a/tests/browser/composer-attachments.spec.mjs b/tests/browser/composer-attachments.spec.mjs index 0ecf64a1c6..6c9052a32e 100644 --- a/tests/browser/composer-attachments.spec.mjs +++ b/tests/browser/composer-attachments.spec.mjs @@ -17,6 +17,37 @@ async function openPreparationDocument(page, server) { await page.goto(url); } +// Spare width of a failed upload tile, given its alert. The tile fits the +// strip, or is only as wide as its one-row Retry and Remove need; anything +// else widening it, such as a long name or an 18rem floor, makes this negative. +function failedTileRoom(alert) { + const item = alert.closest("li"); + const strip = item.closest("ul"); + const width = (el) => el.getBoundingClientRect().width; + const insets = (el) => { + const style = getComputedStyle(el); + return [ + "paddingLeft", + "paddingRight", + "borderLeftWidth", + "borderRightWidth", + ] + .map((name) => parseFloat(style[name])) + .reduce((total, inset) => total + inset); + }; + // Measure the actions' outer edges, whichever button comes first. + const actions = [ + ...item.querySelectorAll('[aria-label^="Remove "], [aria-label^="Retry "]'), + ].map((action) => action.getBoundingClientRect()); + const left = Math.min(...actions.map((action) => action.left)); + const right = Math.max(...actions.map((action) => action.right)); + const room = Math.max( + width(strip) - insets(strip), + right - left + insets(item), + ); + return Math.round(room - width(item)); +} + // Browser-only boundary: actual File/DataTransfer delivery, editor capture and page wiring. test("picker, pane drop and clipboard files use the same attachment draft and existing Send", async ({ page, @@ -1039,14 +1070,30 @@ test("local previews stay interactive during eager upload and removal collapses await expect(failure).toBeInViewport(); if (width === 640) { // A narrow pane must not inherit an 18rem floor at enlarged text sizes. - expect( - await failure.evaluate((el) => { - const item = el.closest("li").getBoundingClientRect(); - const strip = el.closest("ul").getBoundingClientRect(); - return strip.width - item.width; - }), - ).toBeGreaterThanOrEqual(0); + expect(await failure.evaluate(failedTileRoom)).toBeGreaterThanOrEqual( + 0, + ); } + // Retry and Remove never wrap, and the error spans the tile from its + // inner edge, under the thumbnail rather than the file name. + const [removeTop, retryTop] = await Promise.all( + [failedRemove, retry].map((action) => + action.evaluate((el) => el.getBoundingClientRect().top), + ), + ); + expect(retryTop).toBe(removeTop); + expect( + await failure.evaluate((el) => { + const item = el.closest("li"); + const style = getComputedStyle(item); + return ( + el.getBoundingClientRect().left - + item.getBoundingClientRect().left - + parseFloat(style.borderLeftWidth) - + parseFloat(style.paddingLeft) + ); + }), + ).toBeCloseTo(0, 1); for (const content of [failure, retry, failedRemove]) { expect( await content.evaluate((el) => { @@ -1066,15 +1113,20 @@ test("local previews stay interactive during eager upload and removal collapses await form.screenshot({ path: test.info().outputPath(`upload-failure-${mode}-${width}.png`), }); - // Error actions must stay clear of the following image, at any UI size. + // Error actions, up to the rightmost one, must stay clear of the + // following image at any UI size. expect( - await failedRemove.evaluate((el) => { + await failure.evaluate((el) => { const item = el.closest("li"); const next = item.nextElementSibling; - return ( - next.getBoundingClientRect().left - - el.getBoundingClientRect().right + const right = Math.max( + ...[ + ...item.querySelectorAll( + '[aria-label^="Remove "], [aria-label^="Retry "]', + ), + ].map((action) => action.getBoundingClientRect().right), ); + return next.getBoundingClientRect().left - right; }), ).toBeGreaterThanOrEqual(0); } @@ -1134,13 +1186,7 @@ test("local previews stay interactive during eager upload and removal collapses await expect( form.getByRole("button", { name: `Retry ${failedName}` }), ).toBeVisible(); - expect( - await failure.evaluate( - (el) => - el.closest("ul").getBoundingClientRect().width - - el.closest("li").getBoundingClientRect().width, - ), - ).toBeGreaterThanOrEqual(0); + expect(await failure.evaluate(failedTileRoom)).toBeGreaterThanOrEqual(0); expect( await form .getByText(failedName, { exact: true }) From 8f05fbe5620714e7b64c36cea7003a0cfe36994b Mon Sep 17 00:00:00 2001 From: mtoohey Date: Tue, 10 Nov 2026 11:05:33 +1100 Subject: [PATCH 2/2] fix(messages): keep image-preparation refusals on the failed tile Choosing an image over 25 million pixels showed "Upload failed. Retry or remove this file." instead of why. prepareAttachment threw plain Errors for its own refusals, and the tile's mapper words any unknown error generically. prepare-attachment.ts now throws a PreparationError for the three refusals it writes for people: the pixel limit, no canvas context, and an image format the browser cannot prepare. The draft shows that message as it does an UploadError's. Every other failure, including an image the browser cannot decode, a canvas that yields no blob, and the WebP worker's own text, keeps the generic copy. A draft-level test runs the real preparation with a stubbed bitmap and canvas context. On the previous commit it fails for the pixel limit and the missing context; it also pins a decode error to the generic text. Signed-off-by: mtoohey Co-Authored-By: Claude Opus 5.5 --- .../messages/attachment-draft.test.tsx | 57 +++++++++++++++++++ src/features/messages/attachment-draft.ts | 8 ++- src/features/messages/prepare-attachment.ts | 17 +++++- 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/features/messages/attachment-draft.test.tsx b/src/features/messages/attachment-draft.test.tsx index 19ea0d768f..0ed8ec2408 100644 --- a/src/features/messages/attachment-draft.test.tsx +++ b/src/features/messages/attachment-draft.test.tsx @@ -203,6 +203,63 @@ it.each([ }, ); +const bitmap = (width: number, height: number) => async () => ({ + width, + height, + close() {}, +}); +it.each([ + [ + "the pixel limit", + bitmap(5001, 5000), + "Choose an image with at most 25 million pixels.", + ], + [ + "a browser without a canvas context", + bitmap(10, 10), + "Image preparation is unavailable in this browser.", + ], + [ + "an image the browser cannot decode", + async () => { + throw Object.assign(new Error("The source image could not be decoded."), { + name: "InvalidStateError", + }); + }, + UPLOAD_FAILED_TEXT, + ], +])( + "words a preparation refusal from %s for the tile", + async (_name, decode, text) => { + vi.stubGlobal("createImageBitmap", decode); + const context = vi + .spyOn(HTMLCanvasElement.prototype, "getContext") + .mockReturnValue(null); + try { + // The fixture supplies no `prepare`, so the draft's own preparation runs. + const h = fixture(); + const draft = renderHook(() => + useAttachmentDraft(h.session, "one", "one"), + ); + const photo = new File( + [new Uint8Array([0xff, 0xd8, 0xff, 0xe0])], + "photo.jpg", + { type: "image/jpeg" }, + ) as unknown as globalThis.File; + act(() => draft.result.current.store.add([photo])); + await waitFor(() => + expect(draft.result.current.items[0]?.status).toBe("error"), + ); + expect(draft.result.current.items[0]?.error).toBe(text); + expect(draft.result.current.blocked).toBe(true); + expect(h.upload).not.toHaveBeenCalled(); + } finally { + context.mockRestore(); + vi.unstubAllGlobals(); + } + }, +); + it("does not mark a cancelled upload as failed", async () => { const h = fixture(); const draft = renderHook(() => useAttachmentDraft(h.session, "one", "one")); diff --git a/src/features/messages/attachment-draft.ts b/src/features/messages/attachment-draft.ts index b2b495ce91..6459a5c02b 100644 --- a/src/features/messages/attachment-draft.ts +++ b/src/features/messages/attachment-draft.ts @@ -8,15 +8,17 @@ import { type UploadProgress, } from "../relay/attachments"; import { failureKind } from "../relay/user-copy"; -import { prepareAttachment } from "./prepare-attachment"; +import { PreparationError, prepareAttachment } from "./prepare-attachment"; /** The tile's sentence when nothing more specific is known. */ export const UPLOAD_FAILED_TEXT = "Upload failed. Retry or remove this file."; /** Plain-language copy for a failed upload; never the browser's or host's own - * text. An `UploadError`'s message is already written for people. */ + * text. An `UploadError`'s or `PreparationError`'s message is already written + * for people. */ function uploadFailureText(error: unknown) { - if (error instanceof UploadError) return error.message; + if (error instanceof UploadError || error instanceof PreparationError) + return error.message; const kind = failureKind(error); if (kind === "connection") return "Upload didn’t finish. Check your connection, then retry."; diff --git a/src/features/messages/prepare-attachment.ts b/src/features/messages/prepare-attachment.ts index fd562b98a7..241c58a6c0 100644 --- a/src/features/messages/prepare-attachment.ts +++ b/src/features/messages/prepare-attachment.ts @@ -17,6 +17,10 @@ import { webpNeedsPixelTransform, } from "./image-metadata"; +/** A preparation refusal already written for people, such as the pixel limit; + * callers may show its message as is. */ +export class PreparationError extends Error {} + /** Prepare pixels before the byte-exact upload contract; the relay still validates. */ export async function prepareAttachment( file: File, @@ -57,14 +61,18 @@ export async function prepareAttachment( !image.height || image.width * image.height > 25_000_000 ) - throw new Error("Choose an image with at most 25 million pixels."); + throw new PreparationError( + "Choose an image with at most 25 million pixels.", + ); const canvas = document.createElement("canvas"); canvas.width = image.width; canvas.height = image.height; try { const context = canvas.getContext("2d"); if (!context) - throw new Error("Image preparation is unavailable in this browser."); + throw new PreparationError( + "Image preparation is unavailable in this browser.", + ); context.drawImage(image, 0, 0); // Old Buzz writes lossless WebP; WebKit has no canvas WebP encoder. const target = type; @@ -92,7 +100,10 @@ export async function prepareAttachment( else if (blob.type === "image/jpeg") output = cleanEncodedJpeg(new Uint8Array(await blob.arrayBuffer())); else if (blob.type === target) output = blob; - else throw new Error("This browser cannot prepare this image format."); + else + throw new PreparationError( + "This browser cannot prepare this image format.", + ); } finally { canvas.width = 0; canvas.height = 0;