fix(giellie): guard non-JSON MCP responses and surface error.data
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
133fa626fa
commit
e7281c30f4
2 changed files with 48 additions and 3 deletions
|
|
@ -33,6 +33,18 @@ describe("listMcpTools", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("listMcpTools non-JSON", () => {
|
||||
it("rejects on a non-JSON response and does not poison the cache", async () => {
|
||||
mcpPost.mockResolvedValueOnce(new Response("<html>bad gateway</html>", { status: 502 }));
|
||||
await expect(listMcpTools(ORIGIN)).rejects.toThrow(/tools\/list failed/);
|
||||
mcpPost.mockResolvedValueOnce(
|
||||
NextResponse.json({ jsonrpc: "2.0", id: 1, result: { tools: [{ name: "orders.list" }] } }),
|
||||
);
|
||||
expect(await listMcpTools(ORIGIN)).toEqual([{ name: "orders.list" }]);
|
||||
expect(mcpPost).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("callMcpTool", () => {
|
||||
it("sends name + args + actor and unwraps the text envelope", async () => {
|
||||
mcpPost.mockResolvedValue(
|
||||
|
|
@ -68,6 +80,30 @@ describe("callMcpTool", () => {
|
|||
expect(out.text).toContain("Order not found");
|
||||
});
|
||||
|
||||
it("returns isError for a non-JSON response", async () => {
|
||||
mcpPost.mockResolvedValue(new Response("<html>bad gateway</html>", { status: 502 }));
|
||||
const out = await callMcpTool(ORIGIN, "orders.get", {}, { id: "u1", name: "Dave" });
|
||||
expect(out.isError).toBe(true);
|
||||
expect(out.text).toContain("502");
|
||||
});
|
||||
|
||||
it("includes JSON-RPC error.data (hints) in the error text", async () => {
|
||||
mcpPost.mockResolvedValue(
|
||||
NextResponse.json(
|
||||
{
|
||||
jsonrpc: "2.0",
|
||||
id: 2,
|
||||
error: { code: -32000, message: "Upstream 404", data: { hint: "use orders.find_by_order_number" } },
|
||||
},
|
||||
{ status: 500 },
|
||||
),
|
||||
);
|
||||
const out = await callMcpTool(ORIGIN, "orders.get", { id: "x" }, { id: "u1", name: "Dave" });
|
||||
expect(out.isError).toBe(true);
|
||||
expect(out.text).toContain("Upstream 404");
|
||||
expect(out.text).toContain("use orders.find_by_order_number");
|
||||
});
|
||||
|
||||
it("throws a clear error when AGENT_API_KEY is missing", async () => {
|
||||
delete process.env.AGENT_API_KEY;
|
||||
await expect(callMcpTool(ORIGIN, "orders.get", {}, { id: "u1", name: "Dave" })).rejects.toThrow(
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ async function rpc(
|
|||
params: Record<string, unknown>,
|
||||
id: number,
|
||||
): Promise<{
|
||||
error?: { code?: number; message?: string };
|
||||
error?: { code?: number; message?: string; data?: unknown };
|
||||
result?: { tools?: McpTool[]; content?: Array<{ type: string; text?: string }> };
|
||||
}> {
|
||||
const key = process.env.AGENT_API_KEY;
|
||||
|
|
@ -43,7 +43,11 @@ async function rpc(
|
|||
body: JSON.stringify({ jsonrpc: "2.0", id, method, params }),
|
||||
});
|
||||
const res = await mcpPost(req);
|
||||
return (await res.json()) as Awaited<ReturnType<typeof rpc>>;
|
||||
try {
|
||||
return (await res.json()) as Awaited<ReturnType<typeof rpc>>;
|
||||
} catch {
|
||||
return { error: { message: `MCP ${res.status} non-JSON response` } };
|
||||
}
|
||||
}
|
||||
|
||||
export async function listMcpTools(origin: string): Promise<McpTool[]> {
|
||||
|
|
@ -64,7 +68,12 @@ export async function callMcpTool(
|
|||
// `actor` goes last so the model cannot spoof another user.
|
||||
const json = await rpc(origin, "tools/call", { name, arguments: { ...args, actor } }, 2);
|
||||
if (json.error) {
|
||||
return { text: `Error: ${json.error.message ?? "tool call failed"}`, isError: true };
|
||||
const base = `Error: ${json.error.message ?? "tool call failed"}`;
|
||||
const data =
|
||||
json.error.data !== undefined && json.error.data !== null
|
||||
? `\n${(JSON.stringify(json.error.data) ?? "").slice(0, 4000)}`
|
||||
: "";
|
||||
return { text: base + data, isError: true };
|
||||
}
|
||||
const text = json.result?.content?.find((c) => c.type === "text")?.text ?? "";
|
||||
return { text, isError: false };
|
||||
|
|
|
|||
Loading…
Reference in a new issue