fix(ui): dropped attachments that fail to read vanish with no visible outcome (#123274)

* fix(ui): report attachment read failures instead of dropping files silently

Unreadable drops and pastes (folder drops, permission-denied files) resolved
null in readAttachmentFile and were filtered out with no visible outcome —
the attachment simply never appeared. Name the skipped files in a toast while
still attaching the successful siblings; aborted batches stay silent.

* test: assert the read-failure toast message is a plain string (oxlint no-base-to-string)
This commit is contained in:
Peter Steinberger
2026-08-13 16:33:30 -07:00
committed by GitHub
parent c574f403e9
commit 81a80a63e7
3 changed files with 121 additions and 3 deletions
+1
View File
@@ -5389,6 +5389,7 @@ export const en: TranslationMap = {
},
attachments: {
attachedFile: "Attached file",
readFailed: "Could not attach: {names}{more}",
showInTextField: "Show in text field",
outsideAllowedFolders: "Outside allowed folders",
unavailable: "Unavailable",
@@ -0,0 +1,102 @@
// @vitest-environment jsdom
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { handleChatAttachmentPaste } from "./chat-attachments.ts";
vi.mock("../../../lib/toast.ts", () => ({ showToast: vi.fn() }));
import { showToast } from "../../../lib/toast.ts";
class StubFileReader {
static failNames = new Set<string>();
result: string | ArrayBuffer | null = null;
private listeners = new Map<string, Array<() => void>>();
addEventListener(type: string, listener: () => void) {
const existing = this.listeners.get(type) ?? [];
existing.push(listener);
this.listeners.set(type, existing);
}
removeEventListener() {}
abort() {}
readAsDataURL(file: File) {
queueMicrotask(() => {
if (StubFileReader.failNames.has(file.name)) {
this.emit("error");
return;
}
this.result = "data:image/png;base64,aGk=";
this.emit("load");
});
}
private emit(type: string) {
for (const listener of this.listeners.get(type) ?? []) {
listener();
}
}
}
function pasteEventWithFiles(files: File[]): ClipboardEvent {
return {
preventDefault: () => {},
clipboardData: {
items: files.map((file) => ({
type: file.type,
getAsFile: () => file,
})),
getData: () => "",
},
} as unknown as ClipboardEvent;
}
describe("chat attachment read failures", () => {
const realFileReader = globalThis.FileReader;
beforeEach(() => {
vi.stubGlobal("FileReader", StubFileReader as unknown as typeof FileReader);
StubFileReader.failNames = new Set();
});
afterEach(() => {
vi.stubGlobal("FileReader", realFileReader);
vi.clearAllMocks();
});
it("names files whose read failed instead of dropping them silently", async () => {
StubFileReader.failNames = new Set(["bad.png"]);
const onAttachmentsChange = vi.fn();
handleChatAttachmentPaste(
pasteEventWithFiles([
new File(["ok"], "good.png", { type: "image/png" }),
new File(["broken"], "bad.png", { type: "image/png" }),
]),
{ attachments: [], onAttachmentsChange },
);
await vi.waitFor(() => {
expect(onAttachmentsChange).toHaveBeenCalled();
});
expect(vi.mocked(showToast)).toHaveBeenCalledTimes(1);
const message = vi.mocked(showToast).mock.calls[0]?.[0]?.message;
// The read-failure toast is a plain t() string, not a template.
expect(typeof message).toBe("string");
expect(message).toContain("bad.png");
// The successful sibling still attaches.
const attached = onAttachmentsChange.mock.calls[0]?.[0] as Array<{ fileName?: string }>;
expect(attached).toHaveLength(1);
expect(attached[0]?.fileName).toBe("good.png");
});
it("does not toast when every read succeeds", async () => {
const onAttachmentsChange = vi.fn();
handleChatAttachmentPaste(
pasteEventWithFiles([new File(["ok"], "good.png", { type: "image/png" })]),
{ attachments: [], onAttachmentsChange },
);
await vi.waitFor(() => {
expect(onAttachmentsChange).toHaveBeenCalled();
});
expect(vi.mocked(showToast)).not.toHaveBeenCalled();
});
});
@@ -6,6 +6,7 @@ import "../../../components/tooltip.ts";
import "../../../components/web-awesome.ts";
import { t } from "../../../i18n/index.ts";
import type { BrowserAnnotationAttachment, ChatAttachment } from "../../../lib/chat/chat-types.ts";
import { showToast } from "../../../lib/toast.ts";
import {
generateAttachmentId,
getChatAttachmentDataUrl,
@@ -295,15 +296,29 @@ async function appendAttachmentFiles(files: readonly File[], props: ChatAttachme
}
props.onPendingReadsChange?.(1);
try {
const additions = (
await Promise.all(files.map((file) => readAttachmentFile(file, props)))
).filter((attachment): attachment is ChatAttachment => attachment !== null);
const results = await Promise.all(files.map((file) => readAttachmentFile(file, props)));
const additions = results.filter(
(attachment): attachment is ChatAttachment => attachment !== null,
);
if (props.readSignal?.aborted) {
for (const attachment of additions) {
releaseChatAttachmentPayload(attachment.id);
}
return;
}
// Unreadable drops (folders, permission-denied files) must not vanish
// silently: name what was skipped so the user knows it never attached.
const failed = results
.map((attachment, index) => (attachment === null ? files[index]?.name : undefined))
.filter((name): name is string => Boolean(name));
if (failed.length > 0) {
showToast({
message: t("chat.attachments.readFailed", {
names: failed.slice(0, 3).join(", "),
more: failed.length > 3 ? ` +${failed.length - 3}` : "",
}),
});
}
if (additions.length === 0) {
return;
}