Skip to content

fix: set isError on failed query-docs results - #2989

Open
AlexRixten wants to merge 1 commit into
upstash:masterfrom
AlexRixten:fix/query-docs-iserror
Open

fix: set isError on failed query-docs results#2989
AlexRixten wants to merge 1 commit into
upstash:masterfrom
AlexRixten:fix/query-docs-iserror

Conversation

@AlexRixten

@AlexRixten AlexRixten commented Aug 3, 2026

Copy link
Copy Markdown

Problem

query-docs returns its result without setting isError, even when the call failed. Per the MCP schema an unset isError is "assumed to be false (the call was successful)", so a client that branches on isError treats 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 isError sees 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 the catch) returns { data: errorMessage } with no way for the caller to tell it apart from a real document, and the query-docs handler always builds a result without isError.

Fix

  • ContextResponse gains an optional isError flag.
  • fetchLibraryContext sets isError: true on its failure returns (non-ok response, thrown fetch). The data text is unchanged.
  • The query-docs handler 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.ts covering all three paths (non-ok → flagged, thrown fetch → flagged, success → not flagged). Full packages/mcp suite passes (49
tests), lint and format clean.

Built the server and re-probed both cases end to end:

before after
query-docs with a bad / nonexistent id isError unset (reads as resolve-library-id has the same shape (returns its error text withoutup in a separate PR to keep this one focused.

@enesgules

Copy link
Copy Markdown
Collaborator

On which clients can we test the effect of this change?

@AlexRixten

Copy link
Copy Markdown
Author

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 npx confuses its arg parsing):

# 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=react

Before:

{
  "content": [
    { "type": "text", "text": "Library \"/nonexistent/does-not-exist-xyz\" not found. ..." }
  ]
}

No isError — per the schema, that is a successful call.

After (same call against this branch, built with pnpm --filter @upstash/context7-mcp build):

{
  "content": [
    { "type": "text", "text": "Library \"/nonexistent/does-not-exist-xyz\" not found. ..." }
  ],
  "isError": true
}

2. Any MCP client, programmatically

This 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:

before → client sees: SUCCESS → passes this to the model as documentation:
             Library "/nonexistent/does-not-exist-xyz" not found. Please check the ...
after  → client sees: ERROR → can retry / tell the model the call failed

So the affected clients are any that branch on isError: the Inspector shows it in the raw result, and agent hosts (Claude Code, Cursor, Claude Desktop, custom SDK clients) use it to tell a failed tool call from a successful one.

On risk

content is byte-for-byte unchanged, so clients that only render the text see exactly what they see today. The only difference is the flag going from unset to true on failures — the documented signal. Clients that ignore it are unaffected; clients that respect it stop mistaking errors for docs.

@AlexRixten
AlexRixten force-pushed the fix/query-docs-iserror branch from f34b01e to 46f7680 Compare August 8, 2026 21:36
@AlexRixten

Copy link
Copy Markdown
Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants