From f52c7639fdb6491817e756abcfba6f9d05d9ab57 Mon Sep 17 00:00:00 2001 From: ivanmilevtues Date: Wed, 11 Nov 2026 03:05:19 +0100 Subject: [PATCH] feat(mcp): retry timed-out server checks from settings --- .../base/langflow/api/utils/mcp/discovery.py | 16 +++++-- src/backend/base/langflow/api/v2/mcp.py | 4 +- .../unit/api/utils/mcp/test_discovery.py | 1 + .../tests/unit/api/v2/test_mcp_discovery.py | 1 + .../mcp/__tests__/use-get-mcp-servers.test.ts | 26 ++++++---- .../API/queries/mcp/use-get-mcp-servers.ts | 5 +- .../__tests__/serverErrorText.test.ts | 21 ++++++++- .../pages/MCPServersPage/index.tsx | 47 ++++++++++++++----- .../pages/MCPServersPage/serverErrorText.ts | 10 ++++ src/frontend/src/types/mcp/index.ts | 1 + 10 files changed, 104 insertions(+), 28 deletions(-) diff --git a/src/backend/base/langflow/api/utils/mcp/discovery.py b/src/backend/base/langflow/api/utils/mcp/discovery.py index e0648b1143ea..31ed2ea5be48 100644 --- a/src/backend/base/langflow/api/utils/mcp/discovery.py +++ b/src/backend/base/langflow/api/utils/mcp/discovery.py @@ -16,6 +16,17 @@ ServerInfo = dict[str, str | int | None] +def server_check_timeout(server_name: str) -> ServerInfo: + """Keep individual and batch deadline results on the same API contract.""" + return { + "name": server_name, + "mode": None, + "toolsCount": None, + "error": "Timeout when checking server tools", + "errorCode": "timeout", + } + + async def _finish_cleanup(future: asyncio.Future) -> bool: """Drain owned work even under repeated cancellation; report deferred cancellation.""" cancelled = False @@ -95,10 +106,7 @@ async def run_server_checks( headers={"Retry-After": "1"}, ) - results: list[ServerInfo] = [ - {"name": name, "mode": None, "toolsCount": None, "error": "Timeout when checking server tools"} - for name in server_names - ] + results = [server_check_timeout(name) for name in server_names] pending = iter(enumerate(server_names)) async def worker() -> None: diff --git a/src/backend/base/langflow/api/v2/mcp.py b/src/backend/base/langflow/api/v2/mcp.py index 4efc9dde7f57..d9a7a5e1d130 100644 --- a/src/backend/base/langflow/api/v2/mcp.py +++ b/src/backend/base/langflow/api/v2/mcp.py @@ -11,7 +11,7 @@ from sqlmodel import select from langflow.api.utils import CurrentActiveUser, DbSession -from langflow.api.utils.mcp.discovery import discovery_clients, run_server_checks +from langflow.api.utils.mcp.discovery import discovery_clients, run_server_checks, server_check_timeout from langflow.api.v2.files import ( MCP_SERVERS_FILE, download_file, @@ -336,7 +336,7 @@ async def check_server(server_name: str) -> dict: except (TimeoutError, asyncio.TimeoutError) as e: # Timeout errors await logger.aerror(f"Timeout error for server {server_name}: {e}") - server_info["error"] = "Timeout when checking server tools" + return server_check_timeout(server_name) except OSError as e: # System-level errors (process execution, file access) await logger.aerror(f"System error for server {server_name}: {e}") diff --git a/src/backend/tests/unit/api/utils/mcp/test_discovery.py b/src/backend/tests/unit/api/utils/mcp/test_discovery.py index 8aace8d605cd..bb1817de6ffa 100644 --- a/src/backend/tests/unit/api/utils/mcp/test_discovery.py +++ b/src/backend/tests/unit/api/utils/mcp/test_discovery.py @@ -84,6 +84,7 @@ async def check(name): assert results[0] == {"name": "fast", "mode": "stdio", "toolsCount": 2} assert all(result["error"] == "Timeout when checking server tools" for result in results[1:]) + assert all(result["errorCode"] == "timeout" for result in results[1:]) assert len(started) == 1 + MAX_CONCURRENT_CHECKS assert sorted(finished) == sorted(started[1:]) diff --git a/src/backend/tests/unit/api/v2/test_mcp_discovery.py b/src/backend/tests/unit/api/v2/test_mcp_discovery.py index 5e209da6fe75..adf393e0dc19 100644 --- a/src/backend/tests/unit/api/v2/test_mcp_discovery.py +++ b/src/backend/tests/unit/api/v2/test_mcp_discovery.py @@ -129,6 +129,7 @@ async def test_timeout_limits_real_processes_and_reaps_uninitialized_sessions( assert len(results) == 12 assert all(result["toolsCount"] is None and "error" in result for result in results) + assert all(result["errorCode"] == "timeout" for result in results) assert len(processes[0]) == MAX_CONCURRENT_CHECKS assert all(process.returncode is not None for process in processes[0]) assert _manager_tasks() <= before diff --git a/src/frontend/src/controllers/API/queries/mcp/__tests__/use-get-mcp-servers.test.ts b/src/frontend/src/controllers/API/queries/mcp/__tests__/use-get-mcp-servers.test.ts index 2dcdb3802c06..bb6098d41912 100644 --- a/src/frontend/src/controllers/API/queries/mcp/__tests__/use-get-mcp-servers.test.ts +++ b/src/frontend/src/controllers/API/queries/mcp/__tests__/use-get-mcp-servers.test.ts @@ -17,14 +17,15 @@ jest.mock("@/controllers/API/helpers/constants", () => ({ import { mergeMCPServerCounts, useGetMCPServers } from "../use-get-mcp-servers"; describe("mergeMCPServerCounts", () => { - it("clears a cached error when refreshed counts omit an error", () => { + it("clears the cached timeout after a successful retry", () => { const merged = mergeMCPServerCounts( [ { name: "server", mode: null, toolsCount: null, - error: "Connection refused", + error: "Timeout when checking server tools", + errorCode: "timeout", }, ], [ @@ -42,6 +43,7 @@ describe("mergeMCPServerCounts", () => { mode: "streamable_http", toolsCount: 2, error: undefined, + errorCode: undefined, }, ]); }); @@ -64,16 +66,20 @@ describe("useGetMCPServers", () => { queryClient.clear(); }); - it("fetches counts once without retriggering from its own cache update", async () => { - let toolsCount = 2; + it("retries a timed-out check without retriggering from its own cache update", async () => { + let toolsCount: number | null = null; let servers = [{ name: "server", mode: null, toolsCount: null }]; mockApiGet.mockImplementation((url: string) => Promise.resolve({ data: url.endsWith("action_count=true") ? servers.map((server) => ({ ...server, - mode: "stdio", + mode: toolsCount === null ? null : "stdio", toolsCount, + ...(toolsCount === null && { + error: "Timeout when checking server tools", + errorCode: "timeout", + }), })) : servers, }), @@ -93,8 +99,9 @@ describe("useGetMCPServers", () => { await waitFor(() => { expect(result.current.data?.[0]).toMatchObject({ name: "server", - mode: "stdio", - toolsCount: 2, + mode: null, + toolsCount: null, + errorCode: "timeout", }); }); @@ -114,7 +121,10 @@ describe("useGetMCPServers", () => { }); await waitFor(() => { - expect(result.current.data?.[0]?.toolsCount).toBe(3); + expect(result.current.data?.[0]).toMatchObject({ + toolsCount: 3, + errorCode: undefined, + }); }); expect( mockApiGet.mock.calls.filter(([url]) => diff --git a/src/frontend/src/controllers/API/queries/mcp/use-get-mcp-servers.ts b/src/frontend/src/controllers/API/queries/mcp/use-get-mcp-servers.ts index 97a587aebd22..5dbfa1ae0bf4 100644 --- a/src/frontend/src/controllers/API/queries/mcp/use-get-mcp-servers.ts +++ b/src/frontend/src/controllers/API/queries/mcp/use-get-mcp-servers.ts @@ -22,6 +22,7 @@ export const mergeMCPServerCounts = ( ...server, ...updated, error: updated.error ?? undefined, + errorCode: updated.errorCode, }; }); @@ -49,12 +50,14 @@ export const useGetMCPServers: useQueryFunctionType< return cached && (cached.toolsCount !== null || cached.mode !== null || - cached.error !== null) + cached.error !== undefined || + cached.errorCode !== undefined) ? { ...server, toolsCount: cached.toolsCount, mode: cached.mode, error: cached.error, + errorCode: cached.errorCode, } : server; }); diff --git a/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/__tests__/serverErrorText.test.ts b/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/__tests__/serverErrorText.test.ts index 393fb1906b48..84f3e7c25fdc 100644 --- a/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/__tests__/serverErrorText.test.ts +++ b/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/__tests__/serverErrorText.test.ts @@ -1,9 +1,28 @@ -import { serverErrorText, type Translate } from "../serverErrorText"; +import { + isServerCheckTimeout, + serverErrorText, + type Translate, +} from "../serverErrorText"; // Echo the key and its detail so each test sees which copy was chosen. const t: Translate = (key, options) => options?.detail ? `${key}(${options.detail})` : key; +describe("isServerCheckTimeout", () => { + it("recognizes the API code even if the message changes", () => { + expect( + isServerCheckTimeout({ errorCode: "timeout", error: "Deadline reached" }), + ).toBe(true); + }); + + it("still recognizes responses from older servers", () => { + expect( + isServerCheckTimeout({ error: "Timeout when checking server tools" }), + ).toBe(true); + expect(isServerCheckTimeout({ error: "Connection failed" })).toBe(false); + }); +}); + describe("serverErrorText", () => { it.each([ ["No tools found", "mcp.servers.statusNoTools"], diff --git a/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/index.tsx b/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/index.tsx index 4e3f40f31de5..975275b833f1 100644 --- a/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/index.tsx +++ b/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/index.tsx @@ -20,11 +20,12 @@ import SettingsPageHeader from "@/pages/SettingsPage/components/SettingsPageHead import useAlertStore from "@/stores/alertStore"; import type { MCPServerInfoType } from "@/types/mcp"; import { cn } from "@/utils/utils"; -import { serverErrorText } from "./serverErrorText"; +import { isServerCheckTimeout, serverErrorText } from "./serverErrorText"; export default function MCPServersPage() { const { t } = useTranslation(); - const { data: servers } = useGetMCPServers({ withCounts: true }); + const { data: servers, refetch } = useGetMCPServers({ withCounts: true }); + const [isRetrying, setIsRetrying] = useState(false); const { mutate: deleteServer } = useDeleteMCPServer(); const setErrorData = useAlertStore((state) => state.setErrorData); const [addOpen, setAddOpen] = useState(false); @@ -36,6 +37,15 @@ export default function MCPServersPage() { const [serverToDelete, setServerToDelete] = useState(null); + const retryServerChecks = async () => { + setIsRetrying(true); + try { + await refetch(); + } finally { + setIsRetrying(false); + } + }; + const handleEdit = async (name: string) => { try { const data = await getServer({ name }); @@ -97,11 +107,24 @@ export default function MCPServersPage() { )} {servers.length > 0 && ( -
- {t("mcp.servers.addedServers")} +
+ + {t("mcp.servers.addedServers")} + + {servers.some(isServerCheckTimeout) && ( + + )}
)} {/* Rendered only when populated: a list role owning no listitem @@ -139,11 +162,11 @@ export default function MCPServersPage() { )} > {server.toolsCount === null - ? server.error - ? server.error.startsWith("Timeout") - ? t("mcp.servers.statusTimeout") - : t("mcp.servers.statusError") - : t("mcp.servers.statusLoading") + ? isServerCheckTimeout(server) + ? t("mcp.servers.statusTimeout") + : server.error + ? t("mcp.servers.statusError") + : t("mcp.servers.statusLoading") : !server.toolsCount ? t("mcp.servers.statusNoTools") : t("mcp.servers.toolsCount", { diff --git a/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/serverErrorText.ts b/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/serverErrorText.ts index fd4b896c8a3f..1725892e8117 100644 --- a/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/serverErrorText.ts +++ b/src/frontend/src/pages/SettingsPage/pages/MCPServersPage/serverErrorText.ts @@ -1,3 +1,5 @@ +import type { MCPServerInfoType } from "@/types/mcp"; + // The MCP server check (api/v2/mcp.py `check_server`) reports failures as // English sentences. Translate the fixed ones and the sentence around the // prefixed ones; the underlying exception text stays as the server sent it. @@ -19,6 +21,14 @@ const PREFIXED: ReadonlyArray = [ /** The slice of i18next's `t` this helper needs. */ export type Translate = (key: string, options?: { detail: string }) => string; +export function isServerCheckTimeout( + server: Pick, +): boolean { + return server.errorCode + ? server.errorCode === "timeout" + : (server.error?.startsWith("Timeout") ?? false); +} + export function serverErrorText(error: string, t: Translate): string { const exact = EXACT[error]; if (exact) return t(exact); diff --git a/src/frontend/src/types/mcp/index.ts b/src/frontend/src/types/mcp/index.ts index d168915602ec..5468bdebdf4f 100644 --- a/src/frontend/src/types/mcp/index.ts +++ b/src/frontend/src/types/mcp/index.ts @@ -37,6 +37,7 @@ export type MCPServerInfoType = { mode: string | null; toolsCount: number | null; error?: string; + errorCode?: string; }; export type MCPServerType = {