mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-06-11 01:45:58 +00:00
fix(frontend): surface backend detail when agent name check fails (#3048)
* fix(frontend): surface backend detail when agent name check fails The new-agent page caught AgentNameCheckError but only branched on reason === "backend_unreachable". Everything else (notably the 422 "Invalid agent name '...'. Must match ^[A-Za-z0-9-]+$" response from GET /api/agents/check when the user submits a name with disallowed characters — trailing space, dot, Chinese, invisible whitespace from copy-paste) fell through to the generic fallback "Could not verify name availability — please try again", swallowing the detail that already told the user exactly what to fix. Add a request_failed branch that surfaces err.message (which checkAgentName already populates from the backend's detail at core/agents/api.ts). The disabled / backend_unreachable / unknown- error paths are unchanged. Pin the contract with unit tests covering: 200 success, fetch rejection, 502/503/504 network errors, agents_api disabled detail, 422 validation detail carried verbatim, statusText fallback when detail is absent, and a regression guard against misclassifying a 422 as agents_api disabled. Closes #3041 * fix(frontend): localise the error prefix when surfacing backend detail The previous commit surfaced the backend's raw `err.message` on the new-agent page when the name check failed. The detail itself is English (backend's `_validate_agent_name` text, any 5xx business message, etc.) and dropping it bare into a zh-CN page produced a jarring English-among-Chinese line that didn't match neighbouring strings like "已存在同名智能体" / "无法验证名称可用性". Add `nameStepCheckErrorWithDetail` as a templated string ("Name check failed: {detail}" / "名称校验失败:{detail}"), mirroring the existing `nameStepBootstrapMessage` `{name}` template pattern. The page wraps `err.message` in it when present and falls back to the plain `nameStepCheckError` when the detail is empty. Rendered output (verified locally with a Console fetch mock that returns 500 + detail): zh-CN: 名称校验失败:Database connection lost: SQLAlchemy connection pool exhausted (max 5 connections, all in use) en-US: Name check failed: Database connection lost: SQLAlchemy connection pool exhausted (max 5 connections, all in use) The localised prefix tells the user *what operation* failed; the raw detail tells them *why*. Translating the detail itself would be lossy (any unbounded backend string would need a translation table) and would break the debuggability the previous commit delivered. Refs #3041 * fix(frontend): distinguish backend detail from generated fallback in AgentNameCheckError Addresses Copilot's review on #3048: the previous commits keyed off `err.message`, but `checkAgentName` substitutes a generated fallback string ("Failed to check agent name: ${statusText}") when the backend sent no detail. That guaranteed `err.message` was always truthy, made the `nameStepCheckError` fallback branch unreachable in practice, and could surface awkward strings like "名称校验失败:Failed to check agent name: Bad Gateway" in the UI. Add an explicit `detail: string | null` field to AgentNameCheckError. `checkAgentName` populates it only when the backend response actually carried a string `detail` (defensive guard against the dict-shaped detail that other deer-flow endpoints use for typed error codes). The new-agent page now selects on `err.detail` instead of `err.message` so the localised fallback wins when no real detail exists. Also fix the prettier formatting that broke lint-frontend CI on the previous push. Test changes: - The 422 carry-through test now asserts both `detail` and `message` hold the backend string verbatim. - A new "falls back to statusText in message but leaves detail null" test pins the contract that no real detail ⇒ no UI surface leak. - A new "treats non-string detail as null" test guards against future backend schema drift toward dict-shaped detail. Refs #3041 #3048
This commit is contained in:
@@ -146,6 +146,27 @@ export default function NewAgentPage() {
|
|||||||
err.reason === "backend_unreachable"
|
err.reason === "backend_unreachable"
|
||||||
) {
|
) {
|
||||||
setNameError(t.agents.nameStepNetworkError);
|
setNameError(t.agents.nameStepNetworkError);
|
||||||
|
} else if (
|
||||||
|
err instanceof AgentNameCheckError &&
|
||||||
|
err.reason === "request_failed"
|
||||||
|
) {
|
||||||
|
// Surface the backend-provided detail (e.g. validation error) when
|
||||||
|
// one is present, wrapped in a localised prefix so zh-CN users
|
||||||
|
// don't see a bare English string next to the surrounding Chinese
|
||||||
|
// UI. Falls back to the generic localised fallback when the backend
|
||||||
|
// sent no detail — `err.message` is unreliable for this branch
|
||||||
|
// because `checkAgentName` substitutes a generated fallback string
|
||||||
|
// ("Failed to check agent name: ${statusText}") when `detail` is
|
||||||
|
// missing, so testing `err.message` would always be truthy and the
|
||||||
|
// generated fallback would leak through.
|
||||||
|
setNameError(
|
||||||
|
err.detail
|
||||||
|
? t.agents.nameStepCheckErrorWithDetail.replace(
|
||||||
|
"{detail}",
|
||||||
|
err.detail,
|
||||||
|
)
|
||||||
|
: t.agents.nameStepCheckError,
|
||||||
|
);
|
||||||
} else {
|
} else {
|
||||||
setNameError(t.agents.nameStepCheckError);
|
setNameError(t.agents.nameStepCheckError);
|
||||||
}
|
}
|
||||||
@@ -172,6 +193,7 @@ export default function NewAgentPage() {
|
|||||||
t.agents.nameStepNetworkError,
|
t.agents.nameStepNetworkError,
|
||||||
t.agents.nameStepBootstrapMessage,
|
t.agents.nameStepBootstrapMessage,
|
||||||
t.agents.nameStepCheckError,
|
t.agents.nameStepCheckError,
|
||||||
|
t.agents.nameStepCheckErrorWithDetail,
|
||||||
t.agents.nameStepInvalidError,
|
t.agents.nameStepInvalidError,
|
||||||
threadId,
|
threadId,
|
||||||
]);
|
]);
|
||||||
|
|||||||
@@ -9,6 +9,15 @@ export class AgentNameCheckError extends Error {
|
|||||||
constructor(
|
constructor(
|
||||||
message: string,
|
message: string,
|
||||||
public readonly reason: "backend_unreachable" | "request_failed",
|
public readonly reason: "backend_unreachable" | "request_failed",
|
||||||
|
/**
|
||||||
|
* Raw backend `detail` string when the failure came from a backend
|
||||||
|
* response carrying one. `null` when no detail was provided (e.g.
|
||||||
|
* network-layer failure, empty response body, unparseable body) — in
|
||||||
|
* which case `message` is a generated fallback like "Failed to check
|
||||||
|
* agent name: Bad Gateway" and the UI should prefer its own localized
|
||||||
|
* fallback instead of surfacing the generated string.
|
||||||
|
*/
|
||||||
|
public readonly detail: string | null = null,
|
||||||
) {
|
) {
|
||||||
super(message);
|
super(message);
|
||||||
this.name = "AgentNameCheckError";
|
this.name = "AgentNameCheckError";
|
||||||
@@ -104,9 +113,11 @@ export async function checkAgentName(
|
|||||||
"backend_unreachable",
|
"backend_unreachable",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
const backendDetail = typeof err.detail === "string" ? err.detail : null;
|
||||||
throw new AgentNameCheckError(
|
throw new AgentNameCheckError(
|
||||||
err.detail ?? `Failed to check agent name: ${res.statusText}`,
|
backendDetail ?? `Failed to check agent name: ${res.statusText}`,
|
||||||
"request_failed",
|
"request_failed",
|
||||||
|
backendDetail,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
return res.json() as Promise<{ available: boolean; name: string }>;
|
return res.json() as Promise<{ available: boolean; name: string }>;
|
||||||
|
|||||||
@@ -204,6 +204,7 @@ export const enUS: Translations = {
|
|||||||
nameStepNetworkError:
|
nameStepNetworkError:
|
||||||
"Network request failed — check your network or backend connection",
|
"Network request failed — check your network or backend connection",
|
||||||
nameStepCheckError: "Could not verify name availability — please try again",
|
nameStepCheckError: "Could not verify name availability — please try again",
|
||||||
|
nameStepCheckErrorWithDetail: "Name check failed: {detail}",
|
||||||
nameStepApiDisabledError:
|
nameStepApiDisabledError:
|
||||||
"Custom agent management is not enabled on this server. Please contact your administrator.",
|
"Custom agent management is not enabled on this server. Please contact your administrator.",
|
||||||
nameStepBootstrapMessage:
|
nameStepBootstrapMessage:
|
||||||
|
|||||||
@@ -141,6 +141,7 @@ export interface Translations {
|
|||||||
nameStepAlreadyExistsError: string;
|
nameStepAlreadyExistsError: string;
|
||||||
nameStepNetworkError: string;
|
nameStepNetworkError: string;
|
||||||
nameStepCheckError: string;
|
nameStepCheckError: string;
|
||||||
|
nameStepCheckErrorWithDetail: string;
|
||||||
nameStepApiDisabledError: string;
|
nameStepApiDisabledError: string;
|
||||||
nameStepBootstrapMessage: string;
|
nameStepBootstrapMessage: string;
|
||||||
save: string;
|
save: string;
|
||||||
|
|||||||
@@ -192,6 +192,7 @@ export const zhCN: Translations = {
|
|||||||
nameStepAlreadyExistsError: "已存在同名智能体",
|
nameStepAlreadyExistsError: "已存在同名智能体",
|
||||||
nameStepNetworkError: "网络请求失败,请检查网络或后端连接",
|
nameStepNetworkError: "网络请求失败,请检查网络或后端连接",
|
||||||
nameStepCheckError: "无法验证名称可用性,请稍后重试",
|
nameStepCheckError: "无法验证名称可用性,请稍后重试",
|
||||||
|
nameStepCheckErrorWithDetail: "名称校验失败:{detail}",
|
||||||
nameStepApiDisabledError:
|
nameStepApiDisabledError:
|
||||||
"服务器未开启自定义智能体管理功能,请联系管理员。",
|
"服务器未开启自定义智能体管理功能,请联系管理员。",
|
||||||
nameStepBootstrapMessage:
|
nameStepBootstrapMessage:
|
||||||
|
|||||||
@@ -0,0 +1,149 @@
|
|||||||
|
/**
|
||||||
|
* Tests for the error-classification behaviour of `checkAgentName`.
|
||||||
|
*
|
||||||
|
* Issue #3041: when the backend returns a non-200 response (e.g. a 500 with
|
||||||
|
* a database error, a 422 from misbehaving routing, or any other 4xx/5xx
|
||||||
|
* not in the 502/503/504 set), the UI used to swallow the backend detail
|
||||||
|
* into a generic "Could not verify name availability" fallback because the
|
||||||
|
* page-level catch block only handled `reason === "backend_unreachable"`.
|
||||||
|
*
|
||||||
|
* The fix carries the raw backend detail as `AgentNameCheckError.detail`
|
||||||
|
* (distinct from `message`, which always has a non-empty value because
|
||||||
|
* `checkAgentName` substitutes a generated fallback when the backend sent
|
||||||
|
* no detail). The UI uses `detail` to decide whether to surface a real
|
||||||
|
* backend string or fall back to the localised "could not verify" copy.
|
||||||
|
*
|
||||||
|
* These tests pin both halves of the contract so a future refactor doesn't
|
||||||
|
* silently drop the detail or leak the generated fallback into the UI.
|
||||||
|
*/
|
||||||
|
import { beforeEach, describe, expect, test, vi } from "vitest";
|
||||||
|
|
||||||
|
vi.mock("@/core/api/fetcher", () => ({
|
||||||
|
fetch: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/core/config", () => ({
|
||||||
|
getBackendBaseURL: () => "",
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { AgentsApiDisabledError, checkAgentName } from "@/core/agents/api";
|
||||||
|
import { fetch as fetcher } from "@/core/api/fetcher";
|
||||||
|
|
||||||
|
const mockedFetch = vi.mocked(fetcher);
|
||||||
|
|
||||||
|
function jsonResponse(status: number, body: unknown): Response {
|
||||||
|
return new Response(JSON.stringify(body), {
|
||||||
|
status,
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
mockedFetch.mockReset();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("checkAgentName", () => {
|
||||||
|
test("returns availability payload on 200", async () => {
|
||||||
|
mockedFetch.mockResolvedValueOnce(
|
||||||
|
jsonResponse(200, { available: true, name: "dealagent" }),
|
||||||
|
);
|
||||||
|
const result = await checkAgentName("dealagent");
|
||||||
|
expect(result).toEqual({ available: true, name: "dealagent" });
|
||||||
|
});
|
||||||
|
|
||||||
|
test("treats network-layer fetch rejection as backend_unreachable", async () => {
|
||||||
|
mockedFetch.mockRejectedValueOnce(new TypeError("Failed to fetch"));
|
||||||
|
await expect(checkAgentName("dealagent")).rejects.toMatchObject({
|
||||||
|
name: "AgentNameCheckError",
|
||||||
|
reason: "backend_unreachable",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test.each([502, 503, 504])(
|
||||||
|
"treats HTTP %i as backend_unreachable",
|
||||||
|
async (status) => {
|
||||||
|
mockedFetch.mockResolvedValueOnce(
|
||||||
|
jsonResponse(status, { detail: "Bad Gateway" }),
|
||||||
|
);
|
||||||
|
await expect(checkAgentName("dealagent")).rejects.toMatchObject({
|
||||||
|
name: "AgentNameCheckError",
|
||||||
|
reason: "backend_unreachable",
|
||||||
|
});
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
test("recognises agents_api disabled detail and throws AgentsApiDisabledError", async () => {
|
||||||
|
const detail =
|
||||||
|
"Custom-agent management API is disabled. Set agents_api.enabled=true to expose agent and user-profile routes over HTTP.";
|
||||||
|
mockedFetch.mockResolvedValueOnce(jsonResponse(403, { detail }));
|
||||||
|
await expect(checkAgentName("dealagent")).rejects.toBeInstanceOf(
|
||||||
|
AgentsApiDisabledError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("carries backend 422 detail through AgentNameCheckError.detail (issue #3041)", async () => {
|
||||||
|
// This is the exact response shape produced by `_validate_agent_name`
|
||||||
|
// when the user submits a name with disallowed characters — e.g. a
|
||||||
|
// trailing space, a dot, a Chinese character, or invisible whitespace
|
||||||
|
// pasted in from another window.
|
||||||
|
const detail =
|
||||||
|
"Invalid agent name 'deal agent'. Must match ^[A-Za-z0-9-]+$ (letters, digits, and hyphens only).";
|
||||||
|
mockedFetch.mockResolvedValueOnce(jsonResponse(422, { detail }));
|
||||||
|
|
||||||
|
await expect(checkAgentName("deal agent")).rejects.toMatchObject({
|
||||||
|
name: "AgentNameCheckError",
|
||||||
|
reason: "request_failed",
|
||||||
|
// The full detail is preserved on both `detail` (for the UI to
|
||||||
|
// recognise "real backend detail vs generated fallback") and
|
||||||
|
// `message` (for stack traces / logs).
|
||||||
|
detail,
|
||||||
|
message: detail,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("falls back to statusText in message but leaves detail null when backend returns no detail", async () => {
|
||||||
|
// The fallback message must NOT mask the absence of a real backend
|
||||||
|
// detail — the page-level catch relies on `detail === null` to choose
|
||||||
|
// the localised generic fallback rather than rendering the bare
|
||||||
|
// "Failed to check agent name: Internal Server Error" string.
|
||||||
|
mockedFetch.mockResolvedValueOnce(
|
||||||
|
new Response("", { status: 500, statusText: "Internal Server Error" }),
|
||||||
|
);
|
||||||
|
await expect(checkAgentName("dealagent")).rejects.toMatchObject({
|
||||||
|
name: "AgentNameCheckError",
|
||||||
|
reason: "request_failed",
|
||||||
|
detail: null,
|
||||||
|
message: expect.stringContaining("Internal Server Error"),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("treats non-string detail as null (defence against future schema drift)", async () => {
|
||||||
|
// If the backend ever returns `{detail: {code, message}}` (the shape
|
||||||
|
// used by auth errors today) on this endpoint, we must not surface a
|
||||||
|
// `[object Object]` string. `detail` should fall back to null so the
|
||||||
|
// page uses its localised fallback.
|
||||||
|
mockedFetch.mockResolvedValueOnce(
|
||||||
|
jsonResponse(500, { detail: { code: "x", message: "y" } }),
|
||||||
|
);
|
||||||
|
await expect(checkAgentName("dealagent")).rejects.toMatchObject({
|
||||||
|
name: "AgentNameCheckError",
|
||||||
|
reason: "request_failed",
|
||||||
|
detail: null,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("does not misclassify a 422 with unrelated detail as agents_api disabled", async () => {
|
||||||
|
// Defence-in-depth: the disabled detector matches on the substring
|
||||||
|
// "agents_api.enabled", so a 422 whose detail accidentally contains
|
||||||
|
// the same substring would be misclassified. The validation detail
|
||||||
|
// produced by `_validate_agent_name` never contains it; this test
|
||||||
|
// simply asserts that "Invalid agent name ..." stays in the
|
||||||
|
// request_failed branch, which is where the page now surfaces it.
|
||||||
|
const detail =
|
||||||
|
"Invalid agent name 'deal.agent'. Must match ^[A-Za-z0-9-]+$ (letters, digits, and hyphens only).";
|
||||||
|
mockedFetch.mockResolvedValueOnce(jsonResponse(422, { detail }));
|
||||||
|
await expect(checkAgentName("deal.agent")).rejects.not.toBeInstanceOf(
|
||||||
|
AgentsApiDisabledError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user