fix(frontend): format structured upload error details (#5071)

* fix(frontend): format structured upload error details

* fix(frontend): preserve generic message error details
This commit is contained in:
Jholly 2026-08-28 10:40:29 +08:00 committed by GitHub
parent 9c1dd11160
commit 2d0568a14f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 223 additions and 3 deletions

View File

@ -37,12 +37,87 @@ export interface UploadLimits {
max_total_size: number;
}
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null && !Array.isArray(value);
}
function formatValidationIssue(issue: unknown): string | null {
if (
!isRecord(issue) ||
typeof issue.msg !== "string" ||
!Array.isArray(issue.loc)
) {
return null;
}
if (issue.msg.trim().length === 0) {
return null;
}
const location = issue.loc
.filter(
(part): part is string | number =>
typeof part === "string" || typeof part === "number",
)
.map(String)
.filter((part) => part.length > 0)
.join(".");
return location.length > 0 ? `${location}: ${issue.msg}` : issue.msg;
}
function serializeStructuredDetail(
detail: Record<string, unknown> | unknown[],
): string | null {
if (
(Array.isArray(detail) && detail.length === 0) ||
(!Array.isArray(detail) && Object.keys(detail).length === 0)
) {
return null;
}
try {
return JSON.stringify(detail);
} catch {
return null;
}
}
function formatErrorDetail(detail: unknown): string | null {
if (typeof detail === "string") {
return detail.trim().length > 0 ? detail : null;
}
if (Array.isArray(detail)) {
if (detail.length === 0) {
return null;
}
const validationIssues = detail.map(formatValidationIssue);
if (validationIssues.every((issue) => issue !== null)) {
return validationIssues.join("; ");
}
return serializeStructuredDetail(detail);
}
if (isRecord(detail)) {
return formatValidationIssue(detail) ?? serializeStructuredDetail(detail);
}
return null;
}
async function readErrorDetail(
response: Response,
fallback: string,
): Promise<string> {
const error = await response.json().catch(() => ({ detail: fallback }));
return error.detail ?? fallback;
const error = (await response.json().catch(() => null)) as unknown;
if (!isRecord(error)) {
return fallback;
}
return formatErrorDetail(error.detail) ?? fallback;
}
/**

View File

@ -1060,6 +1060,45 @@ test.describe("Chat workspace", () => {
await expect(page.getByRole("tooltip")).toContainText("100 MiB");
});
test("shows structured upload errors as readable messages", async ({
page,
}) => {
await page.route("**/api/threads/*/uploads", (route) =>
route.fulfill({
status: 422,
contentType: "application/json",
body: JSON.stringify({
detail: [
{
type: "missing",
loc: ["body", "files"],
msg: "Field required",
input: null,
},
],
}),
}),
);
await page.goto("/workspace/chats/new");
const textarea = page.getByPlaceholder(/how can i assist you/i);
await expect(textarea).toBeVisible({ timeout: 15_000 });
await page.getByLabel("Upload files").setInputFiles({
name: "report.txt",
mimeType: "text/plain",
buffer: Buffer.from("report"),
});
await textarea.fill("Summarize this report");
await textarea.press("Enter");
const errorToast = page
.locator("[data-sonner-toast]")
.filter({ hasText: "body.files: Field required" });
await expect(errorToast).toBeVisible();
await expect(errorToast).not.toContainText("[object Object]");
});
test("rejects an oversized attachment before upload", async ({ page }) => {
let uploadCalled = false;
await page.route("**/api/threads/*/uploads", (route) => {

View File

@ -9,7 +9,7 @@ rs.mock("@/core/config", () => ({
}));
import { fetch as fetcher } from "@/core/api/fetcher";
import { deleteUploadedFile } from "@/core/uploads/api";
import { deleteUploadedFile, uploadFiles } from "@/core/uploads/api";
const mockedFetch = rs.mocked(fetcher);
@ -21,11 +21,117 @@ function jsonResponse(status: number, body: unknown): Response {
});
}
async function uploadError(response: Response): Promise<Error> {
mockedFetch.mockResolvedValueOnce(response);
let thrown: unknown;
try {
await uploadFiles("thread-1", []);
} catch (error) {
thrown = error;
}
expect(thrown).toBeInstanceOf(Error);
return thrown as Error;
}
beforeEach(() => {
mockedFetch.mockReset();
});
describe("uploads api", () => {
test("preserves string error details", async () => {
const error = await uploadError(
jsonResponse(413, { detail: "File exceeds the upload limit" }),
);
expect(error.message).toBe("File exceeds the upload limit");
});
test("formats FastAPI validation error details", async () => {
const error = await uploadError(
jsonResponse(422, {
detail: [
{
type: "missing",
loc: ["body", "files"],
msg: "Field required",
input: null,
},
{
type: "value_error",
loc: ["body", "files", 0],
msg: "File is empty",
input: "",
},
],
}),
);
expect(error.message).toBe(
"body.files: Field required; body.files.0: File is empty",
);
expect(error.message).not.toContain("[object Object]");
});
test("serializes object error details", async () => {
const error = await uploadError(
jsonResponse(400, {
detail: { code: "invalid_archive", reason: "Archive is corrupt" },
}),
);
expect(error.message).toBe(
'{"code":"invalid_archive","reason":"Archive is corrupt"}',
);
expect(error.message).not.toContain("[object Object]");
});
test.each([
[
"object",
{ msg: "Archive rejected", code: "invalid_archive", retryable: false },
'{"msg":"Archive rejected","code":"invalid_archive","retryable":false}',
],
[
"array",
[{ msg: "Archive rejected", code: "invalid_archive", retryable: false }],
'[{"msg":"Archive rejected","code":"invalid_archive","retryable":false}]',
],
])(
"serializes a generic %s detail containing msg without a validation loc",
async (_label, detail, expected) => {
const error = await uploadError(jsonResponse(400, { detail }));
expect(error.message).toBe(expected);
expect(error.message).not.toContain("[object Object]");
},
);
test.each([
["missing", {}],
["null", { detail: null }],
["blank string", { detail: " " }],
["empty array", { detail: [] }],
["empty object", { detail: {} }],
["unexpected scalar", { detail: 42 }],
])("uses the upload fallback for %s detail", async (_label, body) => {
const error = await uploadError(jsonResponse(400, body));
expect(error.message).toBe("Upload failed");
});
test("uses the upload fallback for non-JSON responses", async () => {
const error = await uploadError(
new Response("Bad Gateway", {
status: 502,
headers: { "Content-Type": "text/plain" },
}),
);
expect(error.message).toBe("Upload failed");
});
test("encodes uploaded filenames in delete request paths", async () => {
mockedFetch.mockResolvedValueOnce(
jsonResponse(200, {