From 368bfe19bfa88155fd9b1788b2ee3d61884fc0fe Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Mon, 20 Jul 2026 17:37:24 -0700 Subject: [PATCH] fix(ui): surface add/update credential failures with an error toast The add and update handlers had no try/catch (carried over from the legacy panel), so a failed credentialCreateCall / credentialUpdateCall became an unhandled rejection: no error notification and the modal left open with no feedback. Bring them in line with the co-located delete handler by catching and calling NotificationsManager.error, keeping the modal open on failure so the user can retry. Add panel tests for the success (modal closes, refetch, success toast) and failure (error toast, modal stays open) paths. --- .../model_add/CredentialsPanel.test.tsx | 95 +++++++++++++++++-- .../components/model_add/CredentialsPanel.tsx | 28 ++++-- 2 files changed, 106 insertions(+), 17 deletions(-) diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.test.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.test.tsx index 7124cd9ab09..af38645b00d 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.test.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.test.tsx @@ -1,9 +1,11 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { UploadProps } from "antd/es/upload"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; -import { CredentialItem } from "@/components/networking"; +import { CredentialItem, credentialCreateCall } from "@/components/networking"; +import NotificationsManager from "@/components/molecules/notifications_manager"; import CredentialsPanel from "./CredentialsPanel"; @@ -20,6 +22,46 @@ vi.mock("@/app/(dashboard)/hooks/credentials/useCredentials", () => ({ useCredentials: () => mockUseCredentials(), })); +vi.mock("@/components/molecules/notifications_manager", () => ({ + default: { success: vi.fn(), error: vi.fn(), fromBackend: vi.fn() }, +})); + +vi.mock("@/components/networking", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + credentialCreateCall: vi.fn(), + credentialUpdateCall: vi.fn(), + credentialDeleteCall: vi.fn(), + }; +}); + +// Stub the modal so the panel's submit handlers can be driven directly: the +// button fires onSubmit with form-shaped values, and it only renders when open. +vi.mock("./CredentialModal", () => ({ + default: function CredentialModalMock({ + mode, + open, + onSubmit, + }: { + mode: "add" | "edit"; + open: boolean; + onSubmit: (values: Record) => void; + }) { + if (!open) { + return null; + } + return ( + + ); + }, +})); + const credentials: CredentialItem[] = [ { credential_name: "openai-key", @@ -46,6 +88,10 @@ const renderPanel = () => ); describe("CredentialsPanel", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + it("renders the Add Credential button for an admin", () => { mockUseAuthorized.mockReturnValue({ accessToken: "test-token", userRole: "Admin" }); mockUseCredentials.mockReturnValue({ data: { credentials: [] }, isLoading: false, refetch: vi.fn() }); @@ -84,18 +130,53 @@ describe("CredentialsPanel", () => { }); it("opens the add modal when the add button is clicked", async () => { + const user = userEvent.setup(); mockUseAuthorized.mockReturnValue({ accessToken: "test-token", userRole: "Admin" }); mockUseCredentials.mockReturnValue({ data: { credentials: [] }, isLoading: false, refetch: vi.fn() }); renderPanel(); - act(() => { - fireEvent.click(screen.getByRole("button", { name: /add credential/i })); - }); + expect(screen.queryByTestId("credential-modal-add-submit")).not.toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: /add credential/i })); + expect(screen.getByTestId("credential-modal-add-submit")).toBeInTheDocument(); + }); + + it("closes the add modal and refetches after a successful add", async () => { + const user = userEvent.setup(); + const refetch = vi.fn(); + mockUseAuthorized.mockReturnValue({ accessToken: "test-token", userRole: "Admin" }); + mockUseCredentials.mockReturnValue({ data: { credentials: [] }, isLoading: false, refetch }); + vi.mocked(credentialCreateCall).mockResolvedValueOnce(undefined as never); + + renderPanel(); + + await user.click(screen.getByRole("button", { name: /add credential/i })); + await user.click(screen.getByTestId("credential-modal-add-submit")); await waitFor(() => { - expect(screen.getByText("Add New Credential")).toBeInTheDocument(); + expect(NotificationsManager.success).toHaveBeenCalledWith("Credential added successfully"); }); + expect(refetch).toHaveBeenCalled(); + expect(screen.queryByTestId("credential-modal-add-submit")).not.toBeInTheDocument(); + }); + + it("surfaces an error and keeps the add modal open when the create call fails", async () => { + const user = userEvent.setup(); + mockUseAuthorized.mockReturnValue({ accessToken: "test-token", userRole: "Admin" }); + mockUseCredentials.mockReturnValue({ data: { credentials: [] }, isLoading: false, refetch: vi.fn() }); + vi.mocked(credentialCreateCall).mockRejectedValueOnce(new Error("network down")); + + renderPanel(); + + await user.click(screen.getByRole("button", { name: /add credential/i })); + await user.click(screen.getByTestId("credential-modal-add-submit")); + + await waitFor(() => { + expect(NotificationsManager.error).toHaveBeenCalledWith("Failed to add credential"); + }); + // The modal stays open so the user can retry, and no success toast fired. + expect(screen.getByTestId("credential-modal-add-submit")).toBeInTheDocument(); + expect(NotificationsManager.success).not.toHaveBeenCalled(); }); describe("Admin Viewer write-action gating", () => { diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.tsx index f9754210851..99ab9525966 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialsPanel.tsx @@ -56,22 +56,30 @@ export default function CredentialsPanel({ uploadProps }: CredentialsPanelProps) if (!accessToken) { return; } - const newCredential = buildCredential(values, stripMaskedSecrets(withoutRestrictedFields(values))); - await credentialUpdateCall(accessToken, values.credential_name as string, newCredential); - NotificationsManager.success("Credential updated successfully"); - setIsUpdateModalOpen(false); - await refetchCredentials(); + try { + const newCredential = buildCredential(values, stripMaskedSecrets(withoutRestrictedFields(values))); + await credentialUpdateCall(accessToken, values.credential_name as string, newCredential); + NotificationsManager.success("Credential updated successfully"); + setIsUpdateModalOpen(false); + await refetchCredentials(); + } catch (error) { + NotificationsManager.error("Failed to update credential"); + } }; const handleAddCredential = async (values: Record) => { if (!accessToken) { return; } - const newCredential = buildCredential(values, withoutRestrictedFields(values)); - await credentialCreateCall(accessToken, newCredential); - NotificationsManager.success("Credential added successfully"); - setIsAddModalOpen(false); - await refetchCredentials(); + try { + const newCredential = buildCredential(values, withoutRestrictedFields(values)); + await credentialCreateCall(accessToken, newCredential); + NotificationsManager.success("Credential added successfully"); + setIsAddModalOpen(false); + await refetchCredentials(); + } catch (error) { + NotificationsManager.error("Failed to add credential"); + } }; const handleDeleteCredential = async () => {