fix: set isError on failed query-docs results - #2989
Conversation
|
On which clients can we test the effect of this change? |
|
Good question — here are two ways to see it, both client-agnostic. 1. Official MCP Inspector (CLI)Point the Inspector at a binary directly (nested # published 3.2.5, for comparison
mkdir -p /tmp/c7-before && cd /tmp/c7-before && npm i @upstash/context7-mcp@3.2.5
npx @modelcontextprotocol/inspector --cli \
node /tmp/c7-before/node_modules/@upstash/context7-mcp/dist/index.js \
--method tools/call --tool-name query-docs \
--tool-arg libraryId=/nonexistent/does-not-exist-xyz --tool-arg query=reactBefore: {
"content": [
{ "type": "text", "text": "Library \"/nonexistent/does-not-exist-xyz\" not found. ..." }
]
}No After (same call against this branch, built with {
"content": [
{ "type": "text", "text": "Library \"/nonexistent/does-not-exist-xyz\" not found. ..." }
],
"isError": true
}2. Any MCP client, programmaticallyThis is what actually changes for consumers: const result = await client.callTool({
name: "query-docs",
arguments: { libraryId: "/nonexistent/does-not-exist-xyz", query: "react" },
});
if (result.isError) {
// after: client knows the lookup failed — can retry, resolve the ID, or tell the model
} else {
// before: client treats "Library not found" as documentation and feeds it to the model
}Output: So the affected clients are any that branch on On risk
|
f34b01e to
46f7680
Compare
|
Rebased onto master after the MCP v2 refactor (#2843) — the query-docs handler moved to the new toolCtx signature, so I reapplied the change on top of it. Full packages/mcp suite passes (79 tests), and I re-verified end to end: a nonexistent library ID now comes back with isError: true. Anything else needed before this can go in? |
Problem
query-docsreturns its result without settingisError, even when the call failed. Per the MCP schema an unsetisErroris "assumed to be false (the call was successful)", so a client that branches onisErrortreats an error message as documentation.Two paths reproduce it (both
@upstash/context7-mcp@3.2.5, over stdio):query-docs({ libraryId: "probe", query: "..." })
→ "Invalid library ID format: "probe". Expected format: /owner/repo ..." (isError unset)
query-docs({ libraryId: "/nonexistent/does-not-exist-xyz", query: "..." })
→ "Library "/nonexistent/does-not-exist-xyz" not found. Please check the library ID ..." (isError unset)
The second case matters most: the ID is well-formed, so this is not a "bad input" artifact — a genuine not-found response is reported as success. An agent that checks
isErrorsees success and feeds "not found" back to the model as if it were docs.The cause is in
fetchLibraryContext: every failure path (!response.ok, and thecatch) returns{ data: errorMessage }with no way for the caller to tell it apart from a real document, and thequery-docshandler always builds a result withoutisError.Fix
ContextResponsegains an optionalisErrorflag.fetchLibraryContextsetsisError: trueon its failure returns (non-ok response, thrown fetch). Thedatatext is unchanged.query-docshandler forwards it:...(response.isError ? { isError: true } : {}).The empty-body branch (
"Documentation not found or not finalized ...") is deliberately left as-is — it can be a legitimate not-yet-finalized state rather than a hard error.The spec classifies API failures and invalid input as tool execution errors, reported with
isError: true, "otherwise, the LLM would not be able to see that an error occurred and self-correct."Verification
Added
packages/mcp/test/api.test.tscovering all three paths (non-ok → flagged, thrown fetch → flagged, success → not flagged). Fullpackages/mcpsuite passes (49tests), lint and format clean.
Built the server and re-probed both cases end to end:
query-docswith a bad / nonexistent idisErrorunset (reads asresolve-library-idhas the same shape (returns its error text withoutup in a separate PR to keep this one focused.