Merge pull request #1954 from modelcontextprotocol/v2/fix/surface-list-fetch-errors
fix: surface list-fetch failures instead of swallowing them
This commit is contained in:
commit
8b3d2bce32
39 changed files with 1268 additions and 48 deletions
|
|
@ -994,24 +994,33 @@ function App() {
|
|||
} = useInspectorClient(inspectorClient);
|
||||
const {
|
||||
tools: managedTools,
|
||||
error: toolsLoadError,
|
||||
listChanged: toolsListChanged,
|
||||
refresh: refreshTools,
|
||||
clearListChanged: clearToolsListChanged,
|
||||
} = useManagedTools(inspectorClient, managedToolsState);
|
||||
const {
|
||||
prompts: managedPrompts,
|
||||
error: promptsLoadError,
|
||||
listChanged: promptsListChanged,
|
||||
refresh: refreshPrompts,
|
||||
clearListChanged: clearPromptsListChanged,
|
||||
} = useManagedPrompts(inspectorClient, managedPromptsState);
|
||||
const {
|
||||
resources: managedResources,
|
||||
error: resourcesLoadError,
|
||||
listChanged: resourcesListChanged,
|
||||
refresh: refreshResources,
|
||||
clearListChanged: clearResourcesListChanged,
|
||||
} = useManagedResources(inspectorClient, managedResourcesState);
|
||||
const { resourceTemplates, refresh: refreshResourceTemplates } =
|
||||
useManagedResourceTemplates(inspectorClient, managedResourceTemplatesState);
|
||||
const {
|
||||
resourceTemplates,
|
||||
error: resourceTemplatesLoadError,
|
||||
refresh: refreshResourceTemplates,
|
||||
} = useManagedResourceTemplates(
|
||||
inspectorClient,
|
||||
managedResourceTemplatesState,
|
||||
);
|
||||
// Paged (paginated) list sources. When `paginatedLists` is on the managed
|
||||
// states skip their all-page walk and these drive the sidebar instead (#1721).
|
||||
const {
|
||||
|
|
@ -4373,6 +4382,9 @@ function App() {
|
|||
toolsListChanged={toolsListChanged}
|
||||
promptsListChanged={promptsListChanged}
|
||||
resourcesListChanged={resourcesListChanged}
|
||||
toolsLoadError={toolsLoadError}
|
||||
promptsLoadError={promptsLoadError}
|
||||
resourcesLoadError={resourcesLoadError ?? resourceTemplatesLoadError}
|
||||
subscriptions={subscriptions}
|
||||
subscriptionStreamState={subscriptionStreamState}
|
||||
logs={logs}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,48 @@
|
|||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { fn } from "storybook/test";
|
||||
import { ListLoadError } from "./ListLoadError";
|
||||
|
||||
const meta: Meta<typeof ListLoadError> = {
|
||||
title: "Elements/ListLoadError",
|
||||
component: ListLoadError,
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof ListLoadError>;
|
||||
|
||||
/** The failure this was built for: a modern server returning a list result the
|
||||
* SDK codec rejects (#1953). */
|
||||
export const CodecRejection: Story = {
|
||||
args: {
|
||||
what: "tools",
|
||||
error: new Error(
|
||||
'Invalid result for tools/list: [\n {\n "expected": "number",\n "code": "invalid_type",\n "path": [\n "ttlMs"\n ]\n }\n]',
|
||||
),
|
||||
onRetry: fn(),
|
||||
},
|
||||
};
|
||||
|
||||
export const TransportFailure: Story = {
|
||||
args: {
|
||||
what: "prompts",
|
||||
error: new Error("fetch failed: ECONNREFUSED 127.0.0.1:3100"),
|
||||
onRetry: fn(),
|
||||
},
|
||||
};
|
||||
|
||||
/** No retry handler — the alert renders without the affordance. */
|
||||
export const WithoutRetry: Story = {
|
||||
args: {
|
||||
what: "resources",
|
||||
error: new Error("Request timed out"),
|
||||
},
|
||||
};
|
||||
|
||||
/** The resting state: no error, nothing rendered. */
|
||||
export const NoError: Story = {
|
||||
args: {
|
||||
what: "tools",
|
||||
error: null,
|
||||
onRetry: fn(),
|
||||
},
|
||||
};
|
||||
|
|
@ -0,0 +1,51 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { renderWithMantine, screen } from "../../../test/renderWithMantine";
|
||||
import { ListLoadError } from "./ListLoadError";
|
||||
|
||||
describe("ListLoadError", () => {
|
||||
it("renders nothing when there is no error", () => {
|
||||
renderWithMantine(<ListLoadError error={null} what="tools" />);
|
||||
expect(screen.queryByText(/Couldn't load/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders nothing when the error prop is omitted", () => {
|
||||
renderWithMantine(<ListLoadError what="tools" />);
|
||||
expect(screen.queryByText(/Couldn't load/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("names what failed to load and shows the reason verbatim", () => {
|
||||
// The exact text is the diagnostic — a validation failure names the JSON
|
||||
// path and what was expected, so it is rendered unabridged (#1953).
|
||||
const message = 'Invalid result for tools/list: [{"path":["ttlMs"]}]';
|
||||
renderWithMantine(
|
||||
<ListLoadError error={new Error(message)} what="tools" />,
|
||||
);
|
||||
|
||||
expect(screen.getByText("Couldn't load tools")).toBeInTheDocument();
|
||||
expect(screen.getByText(message)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("uses the given noun so each list names itself", () => {
|
||||
renderWithMantine(<ListLoadError error={new Error("x")} what="prompts" />);
|
||||
expect(screen.getByText("Couldn't load prompts")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("invokes onRetry when Retry is clicked", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onRetry = vi.fn();
|
||||
renderWithMantine(
|
||||
<ListLoadError error={new Error("x")} what="tools" onRetry={onRetry} />,
|
||||
);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Retry" }));
|
||||
expect(onRetry).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("omits the retry affordance when no handler is given", () => {
|
||||
renderWithMantine(<ListLoadError error={new Error("x")} what="tools" />);
|
||||
expect(
|
||||
screen.queryByRole("button", { name: "Retry" }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,65 @@
|
|||
import { Alert, Button, Code, ScrollArea, Stack } from "@mantine/core";
|
||||
|
||||
export interface ListLoadErrorProps {
|
||||
/**
|
||||
* The failed load's error, or `null`/`undefined` when the last load
|
||||
* succeeded (renders nothing).
|
||||
*/
|
||||
error?: Error | null;
|
||||
/** What failed to load, for the alert title — e.g. "tools", "prompts". */
|
||||
what: string;
|
||||
/** Retry the load. Omit to render the alert without a retry affordance. */
|
||||
onRetry?: () => void;
|
||||
}
|
||||
|
||||
// `variant="light"` + red: an error the user can act on (retry), not a fatal
|
||||
// one. Sits above the list rather than replacing it — a stale list plus a
|
||||
// visible "this didn't reload" beats an empty panel that looks like an answer.
|
||||
const ErrorAlert = Alert.withProps({
|
||||
color: "red",
|
||||
variant: "light",
|
||||
});
|
||||
|
||||
// The raw message, monospaced and wrapping: these are validation failures
|
||||
// (JSON paths, schema expectations) where the exact text is the diagnostic.
|
||||
const ErrorMessage = Code.withProps({
|
||||
block: true,
|
||||
variant: "wrapping",
|
||||
});
|
||||
|
||||
// Caps the message: a schema-validation failure serializes to a dozen-plus
|
||||
// lines, which would otherwise push the list itself off the sidebar.
|
||||
const MessageScroll = ScrollArea.withProps({
|
||||
mah: 180,
|
||||
type: "auto",
|
||||
});
|
||||
|
||||
const RetryButton = Button.withProps({
|
||||
size: "xs",
|
||||
variant: "light",
|
||||
color: "red",
|
||||
w: "fit-content",
|
||||
});
|
||||
|
||||
/**
|
||||
* The list panel's "couldn't load" state (#1953).
|
||||
*
|
||||
* A list fetch that fails — a transport error, or a result the SDK codec
|
||||
* rejects as invalid for the negotiated protocol era — used to leave the panel
|
||||
* empty, which is indistinguishable from a server that legitimately has no
|
||||
* tools/prompts/resources. This says what happened and offers a retry.
|
||||
*/
|
||||
export function ListLoadError({ error, what, onRetry }: ListLoadErrorProps) {
|
||||
if (!error) return null;
|
||||
|
||||
return (
|
||||
<ErrorAlert title={`Couldn't load ${what}`}>
|
||||
<Stack gap="xs">
|
||||
<MessageScroll>
|
||||
<ErrorMessage>{error.message}</ErrorMessage>
|
||||
</MessageScroll>
|
||||
{onRetry && <RetryButton onClick={onRetry}>Retry</RetryButton>}
|
||||
</Stack>
|
||||
</ErrorAlert>
|
||||
);
|
||||
}
|
||||
|
|
@ -160,4 +160,29 @@ describe("PromptControls", () => {
|
|||
expect(screen.getByText("Prompts")).toBeInTheDocument();
|
||||
expect(screen.queryByText("translate")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
// A failed load is rendered above the list instead of leaving the panel
|
||||
// empty, which is indistinguishable from a server that has none (#1953).
|
||||
it("renders a failed load above the list and retries via onRefreshList", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onRefreshList = vi.fn();
|
||||
renderWithMantine(
|
||||
<PromptControls
|
||||
{...baseProps}
|
||||
loadError={new Error("codec said no")}
|
||||
onRefreshList={onRefreshList}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText("Couldn't load prompts")).toBeInTheDocument();
|
||||
expect(screen.getByText("codec said no")).toBeInTheDocument();
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Retry" }));
|
||||
expect(onRefreshList).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("renders no load error by default", () => {
|
||||
renderWithMantine(<PromptControls {...baseProps} />);
|
||||
expect(screen.queryByText(/Couldn't load/)).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { Group, ScrollArea, Stack, TextInput, Title } from "@mantine/core";
|
|||
import { ClearButton } from "../../elements/ClearButton/ClearButton";
|
||||
import type { Prompt } from "@modelcontextprotocol/client";
|
||||
import { ListChangedIndicator } from "../../elements/ListChangedIndicator/ListChangedIndicator";
|
||||
import { ListLoadError } from "../../elements/ListLoadError/ListLoadError";
|
||||
import {
|
||||
ListPaginationControls,
|
||||
type ListPaginationControlsProps,
|
||||
|
|
@ -36,6 +37,11 @@ export interface PromptControlsProps {
|
|||
searchText?: string;
|
||||
listChanged: boolean;
|
||||
onRefreshList: () => void;
|
||||
/**
|
||||
* A failed list load, surfaced above the list instead of leaving the panel
|
||||
* empty (which reads as "this server has none") (#1953).
|
||||
*/
|
||||
loadError?: Error | null;
|
||||
/** Pagination controls (#1721). */
|
||||
pagination: ListPaginationControlsProps;
|
||||
onSearchChange: (value: string) => void;
|
||||
|
|
@ -48,6 +54,7 @@ export function PromptControls({
|
|||
searchText = "",
|
||||
listChanged,
|
||||
onRefreshList,
|
||||
loadError,
|
||||
pagination,
|
||||
onSearchChange,
|
||||
onSelectPrompt,
|
||||
|
|
@ -75,6 +82,7 @@ export function PromptControls({
|
|||
}
|
||||
/>
|
||||
<ListPaginationControls {...pagination} />
|
||||
<ListLoadError error={loadError} what="prompts" onRetry={onRefreshList} />
|
||||
<ListScroll viewportRef={viewportRef}>
|
||||
<Stack gap="xs">
|
||||
{filteredPrompts.map((prompt) => (
|
||||
|
|
|
|||
|
|
@ -711,3 +711,92 @@ describe("ProtocolEntry — modern vocabulary", () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
// A response the server answered successfully but the CLIENT then refused —
|
||||
// e.g. the SDK's era codec rejecting a 2026-07-28 list result missing
|
||||
// `ttlMs`/`cacheScope`. The wire frame is a valid JSON-RPC result, so before
|
||||
// #1953 this rendered as a clean success.
|
||||
describe("ProtocolEntry — client-rejected response", () => {
|
||||
const REASON = "Invalid result for tools/list: ttlMs required";
|
||||
|
||||
const rejectedEntry: MessageEntry = {
|
||||
id: "rej-1",
|
||||
timestamp: new Date("2026-07-28T10:00:00Z"),
|
||||
direction: "request",
|
||||
origin: "client",
|
||||
message: { jsonrpc: "2.0", id: 9, method: "tools/list", params: {} },
|
||||
response: {
|
||||
jsonrpc: "2.0",
|
||||
id: 9,
|
||||
result: { resultType: "complete", tools: [] },
|
||||
},
|
||||
clientError: REASON,
|
||||
};
|
||||
|
||||
it("renders Error rather than a success status", () => {
|
||||
renderWithMantine(<ProtocolEntry {...baseProps} entry={rejectedEntry} />);
|
||||
expect(screen.getByText("Error")).toBeInTheDocument();
|
||||
expect(screen.queryByText("OK")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows the Error badge even though a resultType badge is present", () => {
|
||||
// The resultType normally suppresses the status badge (a modern success
|
||||
// says "complete"). A rejected result must not hide behind it — the wire
|
||||
// said complete, the client disagreed, and both are worth seeing.
|
||||
renderWithMantine(<ProtocolEntry {...baseProps} entry={rejectedEntry} />);
|
||||
expect(screen.getByText("complete")).toBeInTheDocument();
|
||||
expect(screen.getByText("Error")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("names the Inspector as the rejecter and gives the reason when expanded", () => {
|
||||
renderWithMantine(
|
||||
<ProtocolEntry {...baseProps} entry={rejectedEntry} isListExpanded />,
|
||||
);
|
||||
expect(screen.getByText("Rejected by the Inspector")).toBeInTheDocument();
|
||||
expect(screen.getByText(REASON)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// messageLogState falls back to the standalone response frame when no request
|
||||
// entry was there to fold into (a trimmed log, or a reconnect boundary). That
|
||||
// entry is not `direction: "request"`, so the request-only status lifecycle
|
||||
// would have rendered it without any badge at all.
|
||||
const rejectedStandaloneResponse: MessageEntry = {
|
||||
id: "rej-standalone",
|
||||
timestamp: new Date("2026-07-28T10:00:00Z"),
|
||||
direction: "response",
|
||||
origin: "server",
|
||||
message: {
|
||||
jsonrpc: "2.0",
|
||||
id: 11,
|
||||
result: { resultType: "complete", tools: [] },
|
||||
},
|
||||
clientError: REASON,
|
||||
};
|
||||
|
||||
it("renders Error on a rejected standalone response entry", () => {
|
||||
renderWithMantine(
|
||||
<ProtocolEntry {...baseProps} entry={rejectedStandaloneResponse} />,
|
||||
);
|
||||
expect(screen.getByText("Error")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("still renders no status badge on an unrejected standalone response", () => {
|
||||
const clean: MessageEntry = {
|
||||
...rejectedStandaloneResponse,
|
||||
clientError: undefined,
|
||||
};
|
||||
renderWithMantine(<ProtocolEntry {...baseProps} entry={clean} />);
|
||||
expect(screen.queryByText("Error")).not.toBeInTheDocument();
|
||||
expect(screen.queryByText("OK")).not.toBeInTheDocument();
|
||||
expect(screen.queryByText("Pending")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows no rejection alert on an ordinary success", () => {
|
||||
renderWithMantine(
|
||||
<ProtocolEntry {...baseProps} entry={successEntry} isListExpanded />,
|
||||
);
|
||||
expect(
|
||||
screen.queryByText("Rejected by the Inspector"),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -135,6 +135,16 @@ const SpecErrorAlert = Alert.withProps({
|
|||
icon: <RiErrorWarningLine />,
|
||||
});
|
||||
|
||||
// The client rejected an otherwise well-formed response (#1953). Distinct from
|
||||
// SpecErrorAlert: nothing is wrong with the server's JSON-RPC frame — the
|
||||
// Inspector's own decoding refused the result — so the title says who rejected it.
|
||||
const ClientErrorAlert = Alert.withProps({
|
||||
variant: "light",
|
||||
color: "red",
|
||||
icon: <RiErrorWarningLine />,
|
||||
title: "Rejected by the Inspector",
|
||||
});
|
||||
|
||||
// Link (button-styled) that jumps to the correlated HTTP entry in the Network tab.
|
||||
const RevealLink = Anchor.withProps({
|
||||
component: "button",
|
||||
|
|
@ -210,6 +220,13 @@ function extractResourceUri(entry: MessageEntry): string | undefined {
|
|||
function extractStatus(
|
||||
entry: MessageEntry,
|
||||
): "success" | "error" | "pending" | "none" {
|
||||
// A response the CLIENT refused is an error whichever entry carries it, so
|
||||
// this is checked BEFORE the request-only lifecycle below (#1953).
|
||||
// messageLogState annotates the request entry when the response was folded
|
||||
// into one, but falls back to the standalone response frame when there was
|
||||
// no matching request (a trimmed log, or a reconnect boundary) — and that
|
||||
// entry would otherwise fall straight through to "none" and render no badge.
|
||||
if (entry.clientError) return "error";
|
||||
if (entry.direction !== "request") return "none";
|
||||
if (!entry.response) return "pending";
|
||||
if ("error" in entry.response) return "error";
|
||||
|
|
@ -330,11 +347,12 @@ export function ProtocolEntry({
|
|||
// Suppress the redundant green "OK" when a `resultType` badge already conveys
|
||||
// the outcome (a modern success is `complete`/`input required`); errors and
|
||||
// pending have no `resultType`, so their status badge still shows.
|
||||
const statusBadge = status !== "none" && !resultType && (
|
||||
<Badge color={statusColor(status)} variant="status">
|
||||
{statusLabel(status)}
|
||||
</Badge>
|
||||
);
|
||||
const statusBadge = status !== "none" &&
|
||||
(!resultType || entry.clientError) && (
|
||||
<Badge color={statusColor(status)} variant="status">
|
||||
{statusLabel(status)}
|
||||
</Badge>
|
||||
);
|
||||
const subscriptionBadge = subscriptionId && (
|
||||
<SubscriptionCluster>
|
||||
<SubscriptionLabel>sub</SubscriptionLabel>
|
||||
|
|
@ -431,6 +449,11 @@ export function ProtocolEntry({
|
|||
<Collapse in={isExpanded}>
|
||||
<Stack gap="sm">
|
||||
<Divider />
|
||||
{entry.clientError && (
|
||||
<ClientErrorAlert>
|
||||
<Text size="xs">{entry.clientError}</Text>
|
||||
</ClientErrorAlert>
|
||||
)}
|
||||
{specError && (
|
||||
<McpSpecErrorAlert
|
||||
error={specError}
|
||||
|
|
|
|||
|
|
@ -395,4 +395,29 @@ describe("ResourceControls", () => {
|
|||
expect(screen.queryByText("Listening")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// A failed load is rendered above the list instead of leaving the panel
|
||||
// empty, which is indistinguishable from a server that has none (#1953).
|
||||
it("renders a failed load above the list and retries via onRefreshList", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onRefreshList = vi.fn();
|
||||
renderWithMantine(
|
||||
<ResourceControls
|
||||
{...baseProps}
|
||||
loadError={new Error("codec said no")}
|
||||
onRefreshList={onRefreshList}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText("Couldn't load resources")).toBeInTheDocument();
|
||||
expect(screen.getByText("codec said no")).toBeInTheDocument();
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Retry" }));
|
||||
expect(onRefreshList).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("renders no load error by default", () => {
|
||||
renderWithMantine(<ResourceControls {...baseProps} />);
|
||||
expect(screen.queryByText(/Couldn't load/)).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import type {
|
|||
import { isModernEra } from "../../elements/EraBadge/eraUtils";
|
||||
import { SubscriptionStreamBadge } from "../../elements/SubscriptionStreamBadge/SubscriptionStreamBadge";
|
||||
import { ListChangedIndicator } from "../../elements/ListChangedIndicator/ListChangedIndicator";
|
||||
import { ListLoadError } from "../../elements/ListLoadError/ListLoadError";
|
||||
import {
|
||||
ListPaginationControls,
|
||||
type ListPaginationControlsProps,
|
||||
|
|
@ -66,6 +67,11 @@ export interface ResourceControlsProps {
|
|||
openSections?: string[];
|
||||
listChanged: boolean;
|
||||
onRefreshList: () => void;
|
||||
/**
|
||||
* A failed list load, surfaced above the list instead of leaving the panel
|
||||
* empty (which reads as "this server has none") (#1953).
|
||||
*/
|
||||
loadError?: Error | null;
|
||||
/** Pagination controls for the Resources list (#1721). */
|
||||
pagination: ListPaginationControlsProps;
|
||||
onSearchChange: (value: string) => void;
|
||||
|
|
@ -109,6 +115,7 @@ export function ResourceControls({
|
|||
openSections: controlledOpenSections,
|
||||
listChanged,
|
||||
onRefreshList,
|
||||
loadError,
|
||||
pagination,
|
||||
onSearchChange,
|
||||
onOpenSectionsChange,
|
||||
|
|
@ -234,6 +241,11 @@ export function ResourceControls({
|
|||
<ListToggle compact={!allExpanded} onToggle={handleToggleList} />
|
||||
</TightRow>
|
||||
<ListPaginationControls {...pagination} />
|
||||
<ListLoadError
|
||||
error={loadError}
|
||||
what="resources"
|
||||
onRetry={onRefreshList}
|
||||
/>
|
||||
{/* Stays inline: Accordion is a compound, `multiple`-discriminated generic,
|
||||
so `.withProps({ multiple: true, ... })` loses its JSX call signature
|
||||
(same tooling limit as Box). */}
|
||||
|
|
|
|||
|
|
@ -183,4 +183,29 @@ describe("ToolControls", () => {
|
|||
);
|
||||
expect(screen.getByText("invalid_header_tool")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// A failed load is rendered above the list instead of leaving the panel
|
||||
// empty, which is indistinguishable from a server that has none (#1953).
|
||||
it("renders a failed load above the list and retries via onRefreshList", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onRefreshList = vi.fn();
|
||||
renderWithMantine(
|
||||
<ToolControls
|
||||
{...baseProps}
|
||||
loadError={new Error("codec said no")}
|
||||
onRefreshList={onRefreshList}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText("Couldn't load tools")).toBeInTheDocument();
|
||||
expect(screen.getByText("codec said no")).toBeInTheDocument();
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Retry" }));
|
||||
expect(onRefreshList).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("renders no load error by default", () => {
|
||||
renderWithMantine(<ToolControls {...baseProps} />);
|
||||
expect(screen.queryByText(/Couldn't load/)).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import { ClearButton } from "../../elements/ClearButton/ClearButton";
|
|||
import type { Tool } from "@modelcontextprotocol/client";
|
||||
import type { ExcludedTool } from "@inspector/core/mcp/types.js";
|
||||
import { ListChangedIndicator } from "../../elements/ListChangedIndicator/ListChangedIndicator";
|
||||
import { ListLoadError } from "../../elements/ListLoadError/ListLoadError";
|
||||
import {
|
||||
ListPaginationControls,
|
||||
type ListPaginationControlsProps,
|
||||
|
|
@ -32,6 +33,11 @@ export interface ToolControlsProps {
|
|||
searchText?: string;
|
||||
listChanged: boolean;
|
||||
onRefreshList: () => void;
|
||||
/**
|
||||
* A failed list load, surfaced above the list instead of leaving the panel
|
||||
* empty (which reads as "this server has none") (#1953).
|
||||
*/
|
||||
loadError?: Error | null;
|
||||
/** Pagination controls (#1721). */
|
||||
pagination: ListPaginationControlsProps;
|
||||
onSearchChange: (value: string) => void;
|
||||
|
|
@ -109,6 +115,7 @@ export function ToolControls({
|
|||
searchText = "",
|
||||
listChanged,
|
||||
onRefreshList,
|
||||
loadError,
|
||||
pagination,
|
||||
onSearchChange,
|
||||
onSelectTool,
|
||||
|
|
@ -146,6 +153,7 @@ export function ToolControls({
|
|||
}
|
||||
/>
|
||||
<ListPaginationControls {...pagination} />
|
||||
<ListLoadError error={loadError} what="tools" onRetry={onRefreshList} />
|
||||
<SidebarScroll viewportRef={viewportRef}>
|
||||
<Stack gap="xs">
|
||||
{filteredTools.map((tool) => (
|
||||
|
|
|
|||
|
|
@ -33,6 +33,8 @@ export interface PromptsScreenProps {
|
|||
completionsSupported?: boolean;
|
||||
onUiChange: (next: PromptsUiState) => void;
|
||||
onRefreshList: () => void;
|
||||
/** A failed list load, rendered above the sidebar list (#1953). */
|
||||
loadError?: Error | null;
|
||||
/** Pagination controls rendered in the sidebar (#1721). */
|
||||
pagination: ListPaginationControlsProps;
|
||||
onGetPrompt: (name: string, args: Record<string, string>) => void;
|
||||
|
|
@ -136,6 +138,7 @@ export function PromptsScreen({
|
|||
completionsSupported,
|
||||
onUiChange,
|
||||
onRefreshList,
|
||||
loadError,
|
||||
pagination,
|
||||
onGetPrompt,
|
||||
onCopyMessages,
|
||||
|
|
@ -273,6 +276,7 @@ export function PromptsScreen({
|
|||
searchText={search}
|
||||
listChanged={listChanged}
|
||||
onRefreshList={onRefreshList}
|
||||
loadError={loadError}
|
||||
pagination={pagination}
|
||||
onSearchChange={(value) => onUiChange({ ...ui, search: value })}
|
||||
onSelectPrompt={handleSelectPrompt}
|
||||
|
|
|
|||
|
|
@ -57,6 +57,8 @@ export interface ResourcesScreenProps {
|
|||
subscriptionsSupported?: boolean;
|
||||
onUiChange: (next: ResourcesUiState) => void;
|
||||
onRefreshList: () => void;
|
||||
/** A failed list load, rendered above the sidebar list (#1953). */
|
||||
loadError?: Error | null;
|
||||
/** Pagination controls rendered in the sidebar (#1721). */
|
||||
pagination: ListPaginationControlsProps;
|
||||
onReadResource: (uri: string) => void;
|
||||
|
|
@ -170,6 +172,7 @@ export function ResourcesScreen({
|
|||
subscriptionsSupported = true,
|
||||
onUiChange,
|
||||
onRefreshList,
|
||||
loadError,
|
||||
pagination,
|
||||
onReadResource,
|
||||
onSubscribeResource,
|
||||
|
|
@ -325,6 +328,7 @@ export function ResourcesScreen({
|
|||
openSections={openSections}
|
||||
listChanged={listChanged}
|
||||
onRefreshList={onRefreshList}
|
||||
loadError={loadError}
|
||||
pagination={pagination}
|
||||
onSearchChange={(value) => onUiChange({ ...ui, search: value })}
|
||||
onOpenSectionsChange={(value) =>
|
||||
|
|
|
|||
|
|
@ -60,6 +60,8 @@ export interface ToolsScreenProps {
|
|||
modernTasks?: boolean;
|
||||
onUiChange: (next: ToolsUiState) => void;
|
||||
onRefreshList: () => void;
|
||||
/** A failed list load, rendered above the sidebar list (#1953). */
|
||||
loadError?: Error | null;
|
||||
/** Pagination controls rendered in the sidebar (#1721). */
|
||||
pagination: ListPaginationControlsProps;
|
||||
onCallTool: (
|
||||
|
|
@ -152,6 +154,7 @@ export function ToolsScreen({
|
|||
modernTasks = false,
|
||||
onUiChange,
|
||||
onRefreshList,
|
||||
loadError,
|
||||
pagination,
|
||||
onCallTool,
|
||||
onCancelCall,
|
||||
|
|
@ -192,6 +195,7 @@ export function ToolsScreen({
|
|||
searchText={search}
|
||||
listChanged={listChanged}
|
||||
onRefreshList={onRefreshList}
|
||||
loadError={loadError}
|
||||
pagination={pagination}
|
||||
onSearchChange={(value) => onUiChange({ ...ui, search: value })}
|
||||
onSelectTool={handleSelectTool}
|
||||
|
|
|
|||
|
|
@ -435,6 +435,17 @@ export interface InspectorViewProps {
|
|||
toolsListChanged: boolean;
|
||||
promptsListChanged: boolean;
|
||||
resourcesListChanged: boolean;
|
||||
|
||||
// Last list-load failure per screen, sourced from the same managed-state
|
||||
// layer. Rendered above the sidebar list so a failed load (including the
|
||||
// connect-time one) can't read as "this server has none" (#1953).
|
||||
toolsLoadError?: Error | null;
|
||||
promptsLoadError?: Error | null;
|
||||
/**
|
||||
* The Resources sidebar lists resources AND templates behind a single
|
||||
* Refresh, so App passes whichever of the two loads failed.
|
||||
*/
|
||||
resourcesLoadError?: Error | null;
|
||||
logs: LogEntryData[];
|
||||
tasks: Task[];
|
||||
progressByTaskId?: Record<string, TaskProgress>;
|
||||
|
|
@ -627,6 +638,9 @@ export function InspectorView({
|
|||
toolsListChanged,
|
||||
promptsListChanged,
|
||||
resourcesListChanged,
|
||||
toolsLoadError,
|
||||
promptsLoadError,
|
||||
resourcesLoadError,
|
||||
subscriptions,
|
||||
subscriptionStreamState,
|
||||
logs,
|
||||
|
|
@ -1354,6 +1368,7 @@ export function InspectorView({
|
|||
callState={toolCallState}
|
||||
ui={toolsUi}
|
||||
listChanged={toolsListChanged}
|
||||
loadError={toolsLoadError}
|
||||
serverSupportsTaskToolCalls={serverSupportsTaskToolCalls}
|
||||
modernTasks={
|
||||
protocolEra === "modern" &&
|
||||
|
|
@ -1393,6 +1408,7 @@ export function InspectorView({
|
|||
getPromptState={getPromptState}
|
||||
ui={promptsUi}
|
||||
listChanged={promptsListChanged}
|
||||
loadError={promptsLoadError}
|
||||
completionsSupported={completionsSupported}
|
||||
onUiChange={onPromptsUiChange}
|
||||
onRefreshList={onRefreshPrompts}
|
||||
|
|
@ -1412,6 +1428,7 @@ export function InspectorView({
|
|||
readState={readResourceState}
|
||||
ui={resourcesUi}
|
||||
listChanged={resourcesListChanged}
|
||||
loadError={resourcesLoadError}
|
||||
completionsSupported={completionsSupported}
|
||||
subscriptionsSupported={subscriptionsSupported}
|
||||
onUiChange={onResourcesUiChange}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client";
|
||||
import type { Prompt } from "@modelcontextprotocol/client";
|
||||
import type { InspectorServerSettings } from "@inspector/core/mcp/types.js";
|
||||
import { ManagedPromptsState } from "@inspector/core/mcp/state/managedPromptsState";
|
||||
|
|
@ -285,6 +286,22 @@ describe("ManagedPromptsState", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// The base class owns the error plumbing (covered in managedToolsState); this
|
||||
// pins THIS list's method string, which is what attributes a failure to the
|
||||
// right Protocol entry (#1953).
|
||||
it("records a failed load and attributes it to prompts/list", async () => {
|
||||
const boom = new SdkError(SdkErrorCode.InvalidResult, "nope");
|
||||
client.setStatus("connected");
|
||||
client.listAllPrompts.mockRejectedValueOnce(boom);
|
||||
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(state.getError()).toBe(boom);
|
||||
expect(client.markResponseRejected).toHaveBeenCalledWith(
|
||||
"prompts/list",
|
||||
"nope",
|
||||
);
|
||||
});
|
||||
|
||||
it("destroy is idempotent", () => {
|
||||
state.destroy();
|
||||
expect(() => state.destroy()).not.toThrow();
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client";
|
||||
import type { ResourceTemplateType as ResourceTemplate } from "@modelcontextprotocol/client";
|
||||
import type { InspectorServerSettings } from "@inspector/core/mcp/types.js";
|
||||
import { ManagedResourceTemplatesState } from "@inspector/core/mcp/state/managedResourceTemplatesState";
|
||||
|
|
@ -266,6 +267,22 @@ describe("ManagedResourceTemplatesState", () => {
|
|||
expect(state.getResourceTemplates()).toEqual([]);
|
||||
});
|
||||
|
||||
// The base class owns the error plumbing (covered in managedToolsState); this
|
||||
// pins THIS list's method string, which is what attributes a failure to the
|
||||
// right Protocol entry (#1953).
|
||||
it("records a failed load and attributes it to resources/templates/list", async () => {
|
||||
const boom = new SdkError(SdkErrorCode.InvalidResult, "nope");
|
||||
client.setStatus("connected");
|
||||
client.listAllResourceTemplates.mockRejectedValueOnce(boom);
|
||||
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(state.getError()).toBe(boom);
|
||||
expect(client.markResponseRejected).toHaveBeenCalledWith(
|
||||
"resources/templates/list",
|
||||
"nope",
|
||||
);
|
||||
});
|
||||
|
||||
it("destroy is idempotent", () => {
|
||||
state.destroy();
|
||||
expect(() => state.destroy()).not.toThrow();
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client";
|
||||
import type { Resource } from "@modelcontextprotocol/client";
|
||||
import type { InspectorServerSettings } from "@inspector/core/mcp/types.js";
|
||||
import { ManagedResourcesState } from "@inspector/core/mcp/state/managedResourcesState";
|
||||
|
|
@ -292,6 +293,22 @@ describe("ManagedResourcesState", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// The base class owns the error plumbing (covered in managedToolsState); this
|
||||
// pins THIS list's method string, which is what attributes a failure to the
|
||||
// right Protocol entry (#1953).
|
||||
it("records a failed load and attributes it to resources/list", async () => {
|
||||
const boom = new SdkError(SdkErrorCode.InvalidResult, "nope");
|
||||
client.setStatus("connected");
|
||||
client.listAllResources.mockRejectedValueOnce(boom);
|
||||
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(state.getError()).toBe(boom);
|
||||
expect(client.markResponseRejected).toHaveBeenCalledWith(
|
||||
"resources/list",
|
||||
"nope",
|
||||
);
|
||||
});
|
||||
|
||||
it("destroy is idempotent", () => {
|
||||
state.destroy();
|
||||
expect(() => state.destroy()).not.toThrow();
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client";
|
||||
import type { Tool } from "@modelcontextprotocol/client";
|
||||
import type { InspectorServerSettings } from "@inspector/core/mcp/types.js";
|
||||
import { ManagedToolsState } from "@inspector/core/mcp/state/managedToolsState";
|
||||
|
|
@ -412,6 +413,172 @@ describe("ManagedToolsState", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// A failed load used to vanish: the connect-time refresh is fired with no
|
||||
// caller to await it, so its rejection became an unhandled rejection and the
|
||||
// panel just rendered an empty list — indistinguishable from a server with no
|
||||
// tools (#1953).
|
||||
describe("load errors", () => {
|
||||
const boom = new Error("Invalid result for tools/list: ttlMs required");
|
||||
|
||||
function waitForError(state: ManagedToolsState): Promise<Error | null> {
|
||||
return waitForChangeEvent(state, "errorChange");
|
||||
}
|
||||
|
||||
it("starts with no error", () => {
|
||||
expect(state.getError()).toBeNull();
|
||||
});
|
||||
|
||||
it("records a failed refresh as state AND re-throws", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
|
||||
// The rejection still propagates: App's auth-recovery wrapper keys off it
|
||||
// to detect a 401 and start a re-authorization.
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(state.getError()).toBe(boom);
|
||||
});
|
||||
|
||||
it("dispatches errorChange with the error", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
const changed = waitForError(state);
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(await changed).toBe(boom);
|
||||
});
|
||||
|
||||
it("wraps a non-Error rejection", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce("just a string");
|
||||
await expect(state.refresh()).rejects.toBe("just a string");
|
||||
expect(state.getError()).toBeInstanceOf(Error);
|
||||
expect(state.getError()?.message).toBe("just a string");
|
||||
});
|
||||
|
||||
// Only a decode rejection may be attributed to a Protocol entry. The id is
|
||||
// recovered as "the last response for this method", which is the failing
|
||||
// exchange ONLY when a response actually arrived and was refused — see
|
||||
// isClientDecodeRejection.
|
||||
describe("Protocol-entry attribution", () => {
|
||||
const decodeRejection = new SdkError(
|
||||
SdkErrorCode.InvalidResult,
|
||||
"Invalid result for tools/list: ttlMs required",
|
||||
);
|
||||
|
||||
it("attributes a decode rejection to its Protocol entry", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(decodeRejection);
|
||||
await expect(state.refresh()).rejects.toThrow(decodeRejection);
|
||||
expect(client.markResponseRejected).toHaveBeenCalledWith(
|
||||
"tools/list",
|
||||
decodeRejection.message,
|
||||
);
|
||||
});
|
||||
|
||||
it("attributes an unsupported resultType too", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(
|
||||
new SdkError(SdkErrorCode.UnsupportedResultType, "unknown type"),
|
||||
);
|
||||
await expect(state.refresh()).rejects.toThrow();
|
||||
expect(client.markResponseRejected).toHaveBeenCalledWith(
|
||||
"tools/list",
|
||||
"unknown type",
|
||||
);
|
||||
});
|
||||
|
||||
// The regression this guard exists for: no response frame arrived, so the
|
||||
// last-answered id still points at an EARLIER successful call. Marking it
|
||||
// would stamp "Rejected by the Inspector" onto an exchange that worked.
|
||||
it("does NOT attribute a transport failure", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(
|
||||
new SdkError(SdkErrorCode.ConnectionClosed, "Connection closed"),
|
||||
);
|
||||
await expect(state.refresh()).rejects.toThrow();
|
||||
expect(client.markResponseRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT attribute a request timeout", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(
|
||||
new SdkError(SdkErrorCode.RequestTimeout, "Request timed out"),
|
||||
);
|
||||
await expect(state.refresh()).rejects.toThrow();
|
||||
expect(client.markResponseRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// A real response, so the id would be right — but the failure is the
|
||||
// server's, and its entry already renders as an error from the error
|
||||
// frame. Blaming the Inspector would misattribute the cause.
|
||||
it("does NOT attribute a plain (non-SDK) error", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(client.markResponseRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still records every failure as state, attributed or not", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(state.getError()).toBe(boom);
|
||||
expect(client.markResponseRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("clears the error once a refresh succeeds", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
|
||||
const cleared = waitForError(state);
|
||||
client.queueToolPages({ tools: [tool("a")] });
|
||||
await state.refresh();
|
||||
expect(await cleared).toBeNull();
|
||||
expect(state.getError()).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps the connect-time failure in state instead of rejecting unobserved", async () => {
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
const changed = waitForError(state);
|
||||
await client.connect();
|
||||
expect(await changed).toBe(boom);
|
||||
});
|
||||
|
||||
it("keeps the auto-refresh failure in state instead of rejecting unobserved", async () => {
|
||||
client.setServerSettings(AUTO_REFRESH_SETTINGS);
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
const changed = waitForError(state);
|
||||
client.dispatchTypedEvent("toolsListChanged");
|
||||
expect(await changed).toBe(boom);
|
||||
});
|
||||
|
||||
it("clears the error on disconnect so it can't outlive its session", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValueOnce(boom);
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
|
||||
const cleared = waitForError(state);
|
||||
client.setStatus("disconnected");
|
||||
expect(await cleared).toBeNull();
|
||||
expect(state.getError()).toBeNull();
|
||||
});
|
||||
|
||||
it("does not re-dispatch when the same error is recorded twice", async () => {
|
||||
client.setStatus("connected");
|
||||
client.listAllTools.mockRejectedValue(boom);
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
|
||||
let fired = 0;
|
||||
state.addEventListener("errorChange", () => {
|
||||
fired++;
|
||||
});
|
||||
await expect(state.refresh()).rejects.toThrow(boom);
|
||||
expect(fired).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
it("destroy is idempotent", () => {
|
||||
state.destroy();
|
||||
expect(() => state.destroy()).not.toThrow();
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue