From f880faf045c7841c2bb7cef7623e9944687e777f Mon Sep 17 00:00:00 2001 From: Booyaka101 Date: Mon, 4 May 2026 08:42:12 +0800 Subject: [PATCH] fix(ui): coerce MCP cost values defensively before .toFixed (closes #27095) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MCPServerCostDisplay` and `MCPServerCostConfig` typed cost values as `number | null` and called `.toFixed(4)` directly. The values arrive from JSONB-stored `mcp_info` (no runtime validation — `MCPServerCostInfo` is a TypedDict), YAML/JSON config import paths that preserve strings, and antd's `InputNumber`, whose real onChange signature is `(number | string | null)`. So `default_cost_per_query` and per-tool costs can be stringified numbers in practice, and the entire MCP settings page crashes with `e.default_cost_per_query.toFixed is not a function`. Add a shared `toFiniteNumber` helper in `src/utils/numberUtils.ts` and use it at every `.toFixed(...)` site in the two components (5 callsites total). Non-numeric / non-finite inputs coerce to `null` and the corresponding row is skipped, so one malformed value no longer blocks the whole page. A backend-side fix (e.g. validating MCPServerCostInfo as a Pydantic model so the proxy rejects non-numeric writes at the boundary) is a reasonable follow-up but out of scope for this UI crash fix. Tests: 8 new vitest cases in `numberUtils.test.ts`, including the exact regression shape from the issue (a stringified `"0.005"` cost surviving `.toFixed(4)`). All pass; `tsc --noEmit` reports no new errors in touched files. Reported by @marty-sullivan with full root-cause analysis. Co-authored-by: Claude Opus 4.7 (1M context) --- .../mcp_tools/mcp_server_cost_config.tsx | 46 +++++++------- .../mcp_tools/mcp_server_cost_display.tsx | 62 +++++++++---------- .../src/utils/numberUtils.test.ts | 59 ++++++++++++++++++ ui/litellm-dashboard/src/utils/numberUtils.ts | 33 ++++++++++ 4 files changed, 145 insertions(+), 55 deletions(-) create mode 100644 ui/litellm-dashboard/src/utils/numberUtils.test.ts create mode 100644 ui/litellm-dashboard/src/utils/numberUtils.ts diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_config.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_config.tsx index 3f3986d5a2e..074389ab04f 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_config.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_config.tsx @@ -3,6 +3,7 @@ import { Tooltip, InputNumber, Collapse, Badge } from "antd"; import { InfoCircleOutlined, DollarOutlined, ToolOutlined } from "@ant-design/icons"; import { Card, Title, Text } from "@tremor/react"; import { MCPServerCostInfo } from "./types"; +import { toFiniteNumber } from "../../utils/numberUtils"; interface MCPServerCostConfigProps { value?: MCPServerCostInfo; @@ -130,29 +131,32 @@ const MCPServerCostConfig: React.FC = ({ )} - {(value.default_cost_per_query || - (value.tool_name_to_cost_per_query && Object.keys(value.tool_name_to_cost_per_query).length > 0)) && ( -
- Cost Summary: -
- {value.default_cost_per_query && ( - - • Default cost: ${value.default_cost_per_query.toFixed(4)} per query - - )} - {value.tool_name_to_cost_per_query && - Object.entries(value.tool_name_to_cost_per_query).map( - ([toolName, cost]) => - cost !== null && - cost !== undefined && ( - - • {toolName}: ${cost.toFixed(4)} per query - - ), + {(() => { + // Coerce defensively: values may arrive as stringified numbers from + // JSONB / YAML imports / antd InputNumber emitting strings. + const summaryDefaultCost = toFiniteNumber(value.default_cost_per_query); + const summaryToolCosts: Array<[string, number]> = Object.entries( + value.tool_name_to_cost_per_query ?? {}, + ) + .map(([name, raw]): [string, number | null] => [name, toFiniteNumber(raw)]) + .filter((entry): entry is [string, number] => entry[1] !== null); + if (summaryDefaultCost === null && summaryToolCosts.length === 0) return null; + return ( +
+ Cost Summary: +
+ {summaryDefaultCost !== null && ( + • Default cost: ${summaryDefaultCost.toFixed(4)} per query )} + {summaryToolCosts.map(([toolName, cost]) => ( + + • {toolName}: ${cost.toFixed(4)} per query + + ))} +
-
- )} + ); + })()}
); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_display.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_display.tsx index e41a06b879e..05aed127677 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_display.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_cost_display.tsx @@ -1,16 +1,22 @@ import React from "react"; import { Text } from "@tremor/react"; import { MCPServerCostInfo } from "./types"; +import { toFiniteNumber } from "../../utils/numberUtils"; interface MCPServerCostDisplayProps { costConfig?: MCPServerCostInfo | null; } const MCPServerCostDisplay: React.FC = ({ costConfig }) => { - const hasDefaultCost = - costConfig?.default_cost_per_query !== undefined && costConfig?.default_cost_per_query !== null; - const hasToolCosts = - costConfig?.tool_name_to_cost_per_query && Object.keys(costConfig.tool_name_to_cost_per_query).length > 0; + // Cost values arrive from JSONB / YAML / antd InputNumber and may be + // stringified numbers — coerce defensively so `.toFixed(...)` is safe. + const defaultCost = toFiniteNumber(costConfig?.default_cost_per_query); + const toolCosts: Array<[string, number]> = Object.entries(costConfig?.tool_name_to_cost_per_query ?? {}) + .map(([name, raw]): [string, number | null] => [name, toFiniteNumber(raw)]) + .filter((entry): entry is [string, number] => entry[1] !== null); + + const hasDefaultCost = defaultCost !== null; + const hasToolCosts = toolCosts.length > 0; const hasCostConfig = hasDefaultCost || hasToolCosts; if (!hasCostConfig) { @@ -30,29 +36,23 @@ const MCPServerCostDisplay: React.FC = ({ costConfig return (
- {hasDefaultCost && - costConfig?.default_cost_per_query !== undefined && - costConfig?.default_cost_per_query !== null && ( -
- Default Cost per Query -
${costConfig.default_cost_per_query.toFixed(4)}
-
- )} + {hasDefaultCost && defaultCost !== null && ( +
+ Default Cost per Query +
${defaultCost.toFixed(4)}
+
+ )} - {hasToolCosts && costConfig?.tool_name_to_cost_per_query && ( + {hasToolCosts && (
Tool-Specific Costs
- {Object.entries(costConfig.tool_name_to_cost_per_query).map( - ([toolName, cost]) => - cost !== null && - cost !== undefined && ( -
- {toolName} - ${cost.toFixed(4)} per query -
- ), - )} + {toolCosts.map(([toolName, cost]) => ( +
+ {toolName} + ${cost.toFixed(4)} per query +
+ ))}
)} @@ -60,17 +60,11 @@ const MCPServerCostDisplay: React.FC = ({ costConfig
Cost Summary:
- {hasDefaultCost && - costConfig?.default_cost_per_query !== undefined && - costConfig?.default_cost_per_query !== null && ( - - • Default cost: ${costConfig.default_cost_per_query.toFixed(4)} per query - - )} - {hasToolCosts && costConfig?.tool_name_to_cost_per_query && ( - - • {Object.keys(costConfig.tool_name_to_cost_per_query).length} tool(s) with custom pricing - + {hasDefaultCost && defaultCost !== null && ( + • Default cost: ${defaultCost.toFixed(4)} per query + )} + {hasToolCosts && ( + • {toolCosts.length} tool(s) with custom pricing )}
diff --git a/ui/litellm-dashboard/src/utils/numberUtils.test.ts b/ui/litellm-dashboard/src/utils/numberUtils.test.ts new file mode 100644 index 00000000000..7dfa026f2f7 --- /dev/null +++ b/ui/litellm-dashboard/src/utils/numberUtils.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from "vitest"; +import { toFiniteNumber } from "./numberUtils"; + +describe("toFiniteNumber", () => { + it("returns finite numbers unchanged", () => { + expect(toFiniteNumber(0)).toBe(0); + expect(toFiniteNumber(0.005)).toBe(0.005); + expect(toFiniteNumber(-1.25)).toBe(-1.25); + expect(toFiniteNumber(1e-9)).toBe(1e-9); + }); + + it("rejects non-finite numbers", () => { + expect(toFiniteNumber(NaN)).toBeNull(); + expect(toFiniteNumber(Infinity)).toBeNull(); + expect(toFiniteNumber(-Infinity)).toBeNull(); + }); + + it("parses stringified numbers", () => { + expect(toFiniteNumber("0.005")).toBe(0.005); + expect(toFiniteNumber("42")).toBe(42); + expect(toFiniteNumber("-1.25")).toBe(-1.25); + expect(toFiniteNumber("1e-9")).toBe(1e-9); + expect(toFiniteNumber(" 3.14 ")).toBe(3.14); + }); + + it("rejects empty / whitespace-only strings", () => { + expect(toFiniteNumber("")).toBeNull(); + expect(toFiniteNumber(" ")).toBeNull(); + expect(toFiniteNumber("\t\n")).toBeNull(); + }); + + it("rejects non-numeric strings", () => { + expect(toFiniteNumber("abc")).toBeNull(); + expect(toFiniteNumber("0.005 USD")).toBeNull(); + expect(toFiniteNumber("NaN")).toBeNull(); + expect(toFiniteNumber("Infinity")).toBeNull(); + }); + + it("returns null for null / undefined", () => { + expect(toFiniteNumber(null)).toBeNull(); + expect(toFiniteNumber(undefined)).toBeNull(); + }); + + it("returns null for non-string non-number values", () => { + expect(toFiniteNumber({})).toBeNull(); + expect(toFiniteNumber([])).toBeNull(); + expect(toFiniteNumber(true)).toBeNull(); + expect(toFiniteNumber(false)).toBeNull(); + }); + + it("regression for litellm#27095: stringified cost survives without crashing", () => { + // This is the exact shape seen in mcp_info JSONB column when the value is + // serialized as a string instead of a number. + const stringifiedCost = "0.005"; + const coerced = toFiniteNumber(stringifiedCost); + expect(coerced).not.toBeNull(); + expect(coerced!.toFixed(4)).toBe("0.0050"); + }); +}); diff --git a/ui/litellm-dashboard/src/utils/numberUtils.ts b/ui/litellm-dashboard/src/utils/numberUtils.ts new file mode 100644 index 00000000000..ca99b5c8fbb --- /dev/null +++ b/ui/litellm-dashboard/src/utils/numberUtils.ts @@ -0,0 +1,33 @@ +/** + * Shared numeric coercion utility. + * + * Cost values (e.g. MCP server `default_cost_per_query` and per-tool overrides) + * are typed as `number | null` in the UI, but values arriving from the backend + * — JSONB columns, YAML/JSON config import paths, or `antd`'s `InputNumber` + * which can emit `string` for some precision/locale combinations — may be + * stringified numbers. Calling numeric methods like `.toFixed(...)` on those + * crashes the page. + * + * Use `toFiniteNumber` to coerce defensively before formatting. + */ + +/** + * Coerce an unknown value to a finite number, or `null` if it cannot be. + * + * - `number`: returns the value if finite (rejects `NaN` / `±Infinity`). + * - `string`: trimmed and parsed via `Number(...)` if non-empty; rejects + * results that aren't finite (e.g. empty, whitespace, `"abc"`). + * - everything else (`null`, `undefined`, objects, booleans): `null`. + */ +export const toFiniteNumber = (value: unknown): number | null => { + if (typeof value === "number") { + return Number.isFinite(value) ? value : null; + } + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed === "") return null; + const parsed = Number(trimmed); + return Number.isFinite(parsed) ? parsed : null; + } + return null; +};