From 7e4996eef34d037adcb9786527d0aa2ebe140ba7 Mon Sep 17 00:00:00 2001 From: starslittle <110606805+starslittle@users.noreply.github.com> Date: Mon, 17 Aug 2026 08:23:07 +0800 Subject: [PATCH] fix(frontend): surface model loading failures (#4840) * fix(frontend): surface model loading failures * refactor(frontend): reuse model error UI primitives * fix(frontend): address model banner review feedback * refactor(frontend): remove unused model fetch state --- .../src/app/workspace/workspace-content.tsx | 2 + .../workspace/model-load-error-banner.tsx | 73 ++++++ frontend/src/core/api/errors.ts | 13 ++ frontend/src/core/api/fetcher.ts | 4 +- frontend/src/core/i18n/locales/en-US.ts | 4 + frontend/src/core/i18n/locales/types.ts | 3 + frontend/src/core/i18n/locales/zh-CN.ts | 4 + frontend/src/core/models/hooks.ts | 7 +- .../model-load-error-banner.dom.test.tsx | 221 ++++++++++++++++++ frontend/tests/unit/core/models/api.test.ts | 16 ++ 10 files changed, 344 insertions(+), 3 deletions(-) create mode 100644 frontend/src/components/workspace/model-load-error-banner.tsx create mode 100644 frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx diff --git a/frontend/src/app/workspace/workspace-content.tsx b/frontend/src/app/workspace/workspace-content.tsx index 3e712eb29..8d14d5763 100644 --- a/frontend/src/app/workspace/workspace-content.tsx +++ b/frontend/src/app/workspace/workspace-content.tsx @@ -5,6 +5,7 @@ import { QueryClientProvider } from "@/components/query-client-provider"; import { SidebarInset, SidebarProvider } from "@/components/ui/sidebar"; import { CommandPalette } from "@/components/workspace/command-palette"; import { GatewayOfflineBanner } from "@/components/workspace/gateway-offline-banner"; +import { ModelLoadErrorBanner } from "@/components/workspace/model-load-error-banner"; import { SettingsDialogHost } from "@/components/workspace/settings"; import { WorkspaceSettingsDeepLink } from "@/components/workspace/workspace-settings-deep-link"; import { WorkspaceSidebar } from "@/components/workspace/workspace-sidebar"; @@ -35,6 +36,7 @@ export async function WorkspaceContent({ + {children} diff --git a/frontend/src/components/workspace/model-load-error-banner.tsx b/frontend/src/components/workspace/model-load-error-banner.tsx new file mode 100644 index 000000000..129294a04 --- /dev/null +++ b/frontend/src/components/workspace/model-load-error-banner.tsx @@ -0,0 +1,73 @@ +"use client"; + +import { useState } from "react"; + +import { Alert, AlertDescription } from "@/components/ui/alert"; +import { Button } from "@/components/ui/button"; +import { UnauthorizedError } from "@/core/api/errors"; +import { useAuth } from "@/core/auth/AuthProvider"; +import { useI18n } from "@/core/i18n/hooks"; +import { useModels } from "@/core/models/hooks"; + +import { shouldShowOfflineBanner } from "./gateway-offline-banner-helpers"; + +interface ModelLoadErrorBannerProps { + gatewayUnavailable?: boolean; +} + +export function ModelLoadErrorBanner({ + gatewayUnavailable = false, +}: ModelLoadErrorBannerProps) { + const { t } = useI18n(); + const { user } = useAuth(); + const [isRetrying, setIsRetrying] = useState(false); + // Observe the shared query without starting it. Model consumers remain in + // charge of loading; this single observer only centralizes their feedback. + const { error, refetch } = useModels({ enabled: false }); + + const retry = async () => { + setIsRetrying(true); + try { + await refetch(); + } finally { + setIsRetrying(false); + } + }; + + // The shared fetcher has already started a login redirect for this error. + // Rendering a model-specific warning during navigation would be duplicate + // and misleading feedback. + if ( + (!error && !isRetrying) || + error instanceof UnauthorizedError || + shouldShowOfflineBanner(user, gatewayUnavailable) + ) { + return null; + } + + return ( + + + {t.workspace.modelLoadFailed} + + + + ); +} diff --git a/frontend/src/core/api/errors.ts b/frontend/src/core/api/errors.ts index e8b22755c..db55d346c 100644 --- a/frontend/src/core/api/errors.ts +++ b/frontend/src/core/api/errors.ts @@ -1,3 +1,16 @@ +/** + * Raised after the shared fetcher has started a login redirect for a 401. + * + * Callers may use this type to avoid showing a second, misleading API error + * while the browser is already navigating to the authentication flow. + */ +export class UnauthorizedError extends Error { + constructor() { + super("Unauthorized"); + this.name = "UnauthorizedError"; + } +} + /** * Throw an Error from a failed Gateway REST response. * diff --git a/frontend/src/core/api/fetcher.ts b/frontend/src/core/api/fetcher.ts index ca13f425e..6dc3f3ad5 100644 --- a/frontend/src/core/api/fetcher.ts +++ b/frontend/src/core/api/fetcher.ts @@ -1,5 +1,7 @@ import { buildLoginUrl } from "@/core/auth/types"; +import { UnauthorizedError } from "./errors"; + /** HTTP methods that the gateway's CSRFMiddleware checks. */ export type StateChangingMethod = "POST" | "PUT" | "DELETE" | "PATCH"; @@ -82,7 +84,7 @@ export async function fetch( if (res.status === 401) { window.location.href = buildLoginUrl(window.location.pathname); - throw new Error("Unauthorized"); + throw new UnauthorizedError(); } return res; diff --git a/frontend/src/core/i18n/locales/en-US.ts b/frontend/src/core/i18n/locales/en-US.ts index 878f54b1d..eb8f6ef6a 100644 --- a/frontend/src/core/i18n/locales/en-US.ts +++ b/frontend/src/core/i18n/locales/en-US.ts @@ -528,6 +528,10 @@ export const enUS: Translations = { logout: "Log out", gatewayUnavailable: "Gateway is temporarily unavailable.", gatewayUnavailableRetrying: "Retrying in the background…", + modelLoadFailed: + "Models couldn't be loaded. Model selection and token usage may be unavailable.", + modelLoadRetry: "Retry", + modelLoadRetrying: "Retrying…", }, // Conversation diff --git a/frontend/src/core/i18n/locales/types.ts b/frontend/src/core/i18n/locales/types.ts index fdcf728fe..1cb1c0878 100644 --- a/frontend/src/core/i18n/locales/types.ts +++ b/frontend/src/core/i18n/locales/types.ts @@ -421,6 +421,9 @@ export interface Translations { logout: string; gatewayUnavailable: string; gatewayUnavailableRetrying: string; + modelLoadFailed: string; + modelLoadRetry: string; + modelLoadRetrying: string; }; // Conversation diff --git a/frontend/src/core/i18n/locales/zh-CN.ts b/frontend/src/core/i18n/locales/zh-CN.ts index e8b1f9838..652be31b1 100644 --- a/frontend/src/core/i18n/locales/zh-CN.ts +++ b/frontend/src/core/i18n/locales/zh-CN.ts @@ -503,6 +503,10 @@ export const zhCN: Translations = { logout: "退出登录", gatewayUnavailable: "网关暂时不可用。", gatewayUnavailableRetrying: "正在后台重试…", + modelLoadFailed: + "模型列表加载失败，模型选择和 Token 用量信息可能暂时不可用。", + modelLoadRetry: "重试", + modelLoadRetrying: "正在重试…", }, // Conversation diff --git a/frontend/src/core/models/hooks.ts b/frontend/src/core/models/hooks.ts index 53bee9b1a..636316464 100644 --- a/frontend/src/core/models/hooks.ts +++ b/frontend/src/core/models/hooks.ts @@ -2,9 +2,11 @@ import { useQuery } from "@tanstack/react-query"; import { loadModels } from "./api"; +export const MODELS_QUERY_KEY = ["models"] as const; + export function useModels({ enabled = true }: { enabled?: boolean } = {}) { - const { data, isLoading, error } = useQuery({ - queryKey: ["models"], + const { data, isLoading, error, refetch } = useQuery({ + queryKey: MODELS_QUERY_KEY, queryFn: () => loadModels(), enabled, refetchOnWindowFocus: false, @@ -20,5 +22,6 @@ export function useModels({ enabled = true }: { enabled?: boolean } = {}) { tokenUsageEnabled: data?.token_usage.enabled ?? false, isLoading, error, + refetch, }; } diff --git a/frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx b/frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx new file mode 100644 index 000000000..c6c9bb898 --- /dev/null +++ b/frontend/tests/unit/components/workspace/model-load-error-banner.dom.test.tsx @@ -0,0 +1,221 @@ +import { afterEach, beforeEach, describe, expect, it, rs } from "@rstest/core"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import type { PropsWithChildren } from "react"; + +rs.mock("@/core/models/api", () => ({ + loadModels: rs.fn(), +})); + +rs.mock("@/core/i18n/hooks", () => ({ + useI18n: () => ({ + locale: "en-US", + t: { + workspace: { + modelLoadFailed: + "Models couldn't be loaded. Model selection and token usage may be unavailable.", + modelLoadRetry: "Retry", + modelLoadRetrying: "Retrying…", + }, + }, + changeLocale: rs.fn(), + }), +})); + +rs.mock("@/core/auth/AuthProvider", () => ({ + useAuth: rs.fn(), +})); + +import { ModelLoadErrorBanner } from "@/components/workspace/model-load-error-banner"; +import { UnauthorizedError } from "@/core/api/errors"; +import { useAuth } from "@/core/auth/AuthProvider"; +import type { User } from "@/core/auth/types"; +import { loadModels } from "@/core/models/api"; +import { MODELS_QUERY_KEY, useModels } from "@/core/models/hooks"; +import type { ModelsResponse } from "@/core/models/types"; + +const mockedLoadModels = rs.mocked(loadModels); +const mockedUseAuth = rs.mocked(useAuth); +const fakeUser = {} as User; + +function createAuthState(user: User | null): ReturnType { + return { + user, + isAuthenticated: user !== null, + isLoading: false, + logout: rs.fn(), + refreshUser: rs.fn(), + applyUser: rs.fn(), + }; +} + +function createDeferred() { + let resolve!: (value: T) => void; + const promise = new Promise((next) => { + resolve = next; + }); + return { promise, resolve }; +} + +beforeEach(() => { + mockedUseAuth.mockReturnValue(createAuthState(fakeUser)); +}); + +afterEach(() => { + cleanup(); + mockedLoadModels.mockReset(); + mockedUseAuth.mockReset(); +}); + +function createWrapper() { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + }, + }); + + function QueryWrapper({ children }: PropsWithChildren) { + return ( + {children} + ); + } + + return { queryClient, QueryWrapper }; +} + +function ModelConsumer() { + useModels(); + return null; +} + +describe("ModelLoadErrorBanner", () => { + it("observes model failures without starting an extra request", async () => { + const { QueryWrapper } = createWrapper(); + render(, { wrapper: QueryWrapper }); + + await Promise.resolve(); + + expect(mockedLoadModels).not.toHaveBeenCalled(); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("shows one actionable error for all model consumers and clears after retry", async () => { + const retryResult = createDeferred(); + mockedLoadModels + .mockRejectedValueOnce(new Error("Gateway returned 503")) + .mockImplementationOnce(() => retryResult.promise); + const { QueryWrapper } = createWrapper(); + + render( + <> + + + + , + { wrapper: QueryWrapper }, + ); + + const alert = await screen.findByRole("alert"); + expect(alert.textContent).toContain("Models couldn't be loaded"); + expect(alert.textContent).not.toContain("Gateway returned 503"); + expect(screen.getAllByRole("alert")).toHaveLength(1); + expect(mockedLoadModels).toHaveBeenCalledTimes(1); + + fireEvent.click(screen.getByRole("button", { name: "Retry" })); + + const retryingButton = await screen.findByRole("button", { + name: "Retrying…", + }); + expect((retryingButton as HTMLButtonElement).disabled).toBe(true); + + retryResult.resolve({ + models: [], + token_usage: { enabled: false }, + }); + + await waitFor(() => { + expect(screen.queryByRole("alert")).toBeNull(); + }); + expect(mockedLoadModels).toHaveBeenCalledTimes(2); + }); + + it("does not duplicate the login redirect with a model warning", async () => { + mockedLoadModels.mockRejectedValueOnce(new UnauthorizedError()); + const { queryClient, QueryWrapper } = createWrapper(); + + render( + <> + + + , + { wrapper: QueryWrapper }, + ); + + await waitFor(() => { + expect(queryClient.getQueryState(MODELS_QUERY_KEY)?.status).toBe("error"); + }); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("suppresses a model symptom only while the gateway banner is visible", async () => { + mockedUseAuth.mockReturnValue(createAuthState(null)); + mockedLoadModels.mockRejectedValueOnce(new Error("Gateway returned 503")); + const { queryClient, QueryWrapper } = createWrapper(); + + const renderView = () => ( + <> + + + + ); + const { rerender } = render(renderView(), { wrapper: QueryWrapper }); + + await waitFor(() => { + expect(queryClient.getQueryState(MODELS_QUERY_KEY)?.status).toBe("error"); + }); + expect(screen.queryByRole("alert")).toBeNull(); + + mockedUseAuth.mockReturnValue(createAuthState(fakeUser)); + rerender(renderView()); + + expect(await screen.findByRole("alert")).not.toBeNull(); + }); + + it("does not show manual retry progress for a shared background refetch", async () => { + const backgroundResult = createDeferred(); + mockedLoadModels + .mockRejectedValueOnce(new Error("Gateway returned 503")) + .mockImplementationOnce(() => backgroundResult.promise); + const { queryClient, QueryWrapper } = createWrapper(); + + render( + <> + + + , + { wrapper: QueryWrapper }, + ); + + await screen.findByRole("alert"); + const backgroundRefetch = queryClient.refetchQueries({ + queryKey: MODELS_QUERY_KEY, + }); + await waitFor(() => { + expect(mockedLoadModels).toHaveBeenCalledTimes(2); + }); + + expect(screen.queryByRole("button", { name: "Retrying…" })).toBeNull(); + + backgroundResult.resolve({ + models: [], + token_usage: { enabled: false }, + }); + await backgroundRefetch; + }); +}); diff --git a/frontend/tests/unit/core/models/api.test.ts b/frontend/tests/unit/core/models/api.test.ts index ca1ca28cf..95d13e944 100644 --- a/frontend/tests/unit/core/models/api.test.ts +++ b/frontend/tests/unit/core/models/api.test.ts @@ -1,5 +1,7 @@ import { afterEach, expect, test, rs } from "@rstest/core"; +import { UnauthorizedError } from "@/core/api/errors"; + afterEach(() => { rs.unstubAllGlobals(); }); @@ -53,6 +55,20 @@ test("loadModels rejects unsuccessful gateway responses", async () => { await expect(loadModels()).rejects.toThrow("Model registry unavailable"); }); +test("loadModels exposes the typed 401 redirect error", async () => { + const location = { href: "", pathname: "/workspace/chats" }; + rs.stubGlobal("window", { location }); + rs.stubGlobal( + "fetch", + rs.fn(async () => new Response(null, { status: 401 })), + ); + + const { loadModels } = await import("@/core/models/api"); + + await expect(loadModels()).rejects.toBeInstanceOf(UnauthorizedError); + expect(location.href).toBe("/login?next=%2Fworkspace%2Fchats"); +}); + test("loadModels includes the status code when statusText is empty", async () => { rs.stubGlobal( "fetch",