mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-19 00:01:29 +00:00
fix(ui): coerce MCP cost values defensively before .toFixed (closes #27095)
`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) <noreply@anthropic.com>
This commit is contained in:
parent
c011a7e3ba
commit
f880faf045
4 changed files with 145 additions and 55 deletions
|
|
@ -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<MCPServerCostConfigProps> = ({
|
|||
)}
|
||||
</div>
|
||||
|
||||
{(value.default_cost_per_query ||
|
||||
(value.tool_name_to_cost_per_query && Object.keys(value.tool_name_to_cost_per_query).length > 0)) && (
|
||||
<div className="mt-6 p-4 bg-blue-50 border border-blue-200 rounded-lg">
|
||||
<Text className="text-blue-800 font-medium">Cost Summary:</Text>
|
||||
<div className="mt-2 space-y-1">
|
||||
{value.default_cost_per_query && (
|
||||
<Text className="text-blue-700">
|
||||
• Default cost: ${value.default_cost_per_query.toFixed(4)} per query
|
||||
</Text>
|
||||
)}
|
||||
{value.tool_name_to_cost_per_query &&
|
||||
Object.entries(value.tool_name_to_cost_per_query).map(
|
||||
([toolName, cost]) =>
|
||||
cost !== null &&
|
||||
cost !== undefined && (
|
||||
<Text key={toolName} className="text-blue-700">
|
||||
• {toolName}: ${cost.toFixed(4)} per query
|
||||
</Text>
|
||||
),
|
||||
{(() => {
|
||||
// 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 (
|
||||
<div className="mt-6 p-4 bg-blue-50 border border-blue-200 rounded-lg">
|
||||
<Text className="text-blue-800 font-medium">Cost Summary:</Text>
|
||||
<div className="mt-2 space-y-1">
|
||||
{summaryDefaultCost !== null && (
|
||||
<Text className="text-blue-700">• Default cost: ${summaryDefaultCost.toFixed(4)} per query</Text>
|
||||
)}
|
||||
{summaryToolCosts.map(([toolName, cost]) => (
|
||||
<Text key={toolName} className="text-blue-700">
|
||||
• {toolName}: ${cost.toFixed(4)} per query
|
||||
</Text>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
);
|
||||
})()}
|
||||
</div>
|
||||
</Card>
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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<MCPServerCostDisplayProps> = ({ 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<MCPServerCostDisplayProps> = ({ costConfig
|
|||
return (
|
||||
<div className="mt-6 pt-6 border-t border-gray-200">
|
||||
<div className="space-y-4">
|
||||
{hasDefaultCost &&
|
||||
costConfig?.default_cost_per_query !== undefined &&
|
||||
costConfig?.default_cost_per_query !== null && (
|
||||
<div>
|
||||
<Text className="font-medium">Default Cost per Query</Text>
|
||||
<div className="text-green-600 font-mono">${costConfig.default_cost_per_query.toFixed(4)}</div>
|
||||
</div>
|
||||
)}
|
||||
{hasDefaultCost && defaultCost !== null && (
|
||||
<div>
|
||||
<Text className="font-medium">Default Cost per Query</Text>
|
||||
<div className="text-green-600 font-mono">${defaultCost.toFixed(4)}</div>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{hasToolCosts && costConfig?.tool_name_to_cost_per_query && (
|
||||
{hasToolCosts && (
|
||||
<div>
|
||||
<Text className="font-medium">Tool-Specific Costs</Text>
|
||||
<div className="mt-2 space-y-2">
|
||||
{Object.entries(costConfig.tool_name_to_cost_per_query).map(
|
||||
([toolName, cost]) =>
|
||||
cost !== null &&
|
||||
cost !== undefined && (
|
||||
<div key={toolName} className="flex justify-between items-center p-3 bg-gray-50 rounded-lg">
|
||||
<Text className="font-medium">{toolName}</Text>
|
||||
<Text className="text-green-600 font-mono">${cost.toFixed(4)} per query</Text>
|
||||
</div>
|
||||
),
|
||||
)}
|
||||
{toolCosts.map(([toolName, cost]) => (
|
||||
<div key={toolName} className="flex justify-between items-center p-3 bg-gray-50 rounded-lg">
|
||||
<Text className="font-medium">{toolName}</Text>
|
||||
<Text className="text-green-600 font-mono">${cost.toFixed(4)} per query</Text>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
|
@ -60,17 +60,11 @@ const MCPServerCostDisplay: React.FC<MCPServerCostDisplayProps> = ({ costConfig
|
|||
<div className="mt-4 p-4 bg-blue-50 border border-blue-200 rounded-lg">
|
||||
<Text className="text-blue-800 font-medium">Cost Summary:</Text>
|
||||
<div className="mt-2 space-y-1">
|
||||
{hasDefaultCost &&
|
||||
costConfig?.default_cost_per_query !== undefined &&
|
||||
costConfig?.default_cost_per_query !== null && (
|
||||
<Text className="text-blue-700">
|
||||
• Default cost: ${costConfig.default_cost_per_query.toFixed(4)} per query
|
||||
</Text>
|
||||
)}
|
||||
{hasToolCosts && costConfig?.tool_name_to_cost_per_query && (
|
||||
<Text className="text-blue-700">
|
||||
• {Object.keys(costConfig.tool_name_to_cost_per_query).length} tool(s) with custom pricing
|
||||
</Text>
|
||||
{hasDefaultCost && defaultCost !== null && (
|
||||
<Text className="text-blue-700">• Default cost: ${defaultCost.toFixed(4)} per query</Text>
|
||||
)}
|
||||
{hasToolCosts && (
|
||||
<Text className="text-blue-700">• {toolCosts.length} tool(s) with custom pricing</Text>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
59
ui/litellm-dashboard/src/utils/numberUtils.test.ts
Normal file
59
ui/litellm-dashboard/src/utils/numberUtils.test.ts
Normal file
|
|
@ -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");
|
||||
});
|
||||
});
|
||||
33
ui/litellm-dashboard/src/utils/numberUtils.ts
Normal file
33
ui/litellm-dashboard/src/utils/numberUtils.ts
Normal file
|
|
@ -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;
|
||||
};
|
||||
Loading…
Add table
Reference in a new issue