Skip to content

fix(server): answer unparseable stdio lines with a JSON-RPC error - #3742

Closed
akshatshahh wants to merge 1 commit into
modelcontextprotocol:mainfrom
akshatshahh:fix/2751-stdio-parse-error-response
Closed

akshatshahh wants to merge 1 commit into
modelcontextprotocol:mainfrom
akshatshahh:fix/2751-stdio-parse-error-response

Conversation

@akshatshahh

Copy link
Copy Markdown

Fixes #2751

Summary

When a stdio line fails message validation, the server now answers it on the wire with a JSON-RPC error frame instead of forwarding a bare exception on the read stream, which the server dispatcher drops. A request that cannot be parsed now completes instead of staying pending until the client times out.

Motivation and Context

Reproduced on main (2af9e30) with stdio_server() over injected streams: a ping request with params nested to depth 256 fails validation with "Invalid JSON: recursion limit exceeded", arrives on the read stream as a bare exception, and nothing is written to stdout, so the request id never completes. A shallow ping right after it arrives fine, matching the report and the 2.3.0 confirmation on the issue. The Streamable HTTP transport already returns a -32700 parse error for the same input; stdio did not.

The handling stays in the stdio transport, following the review direction on #2755 and #2766, rather than moving into the session or dispatcher layer:

  • Invalid JSON gets -32700 / "Parse error"; a line that parses but is not a valid message gets -32600 / "Invalid request".
  • The request id is recovered from the raw line where possible (a full parse first, then a pattern scan for lines too malformed to parse) and falls back to id: null, as JSON-RPC prescribes when the id cannot be determined.
  • The error message is kept bare on purpose: the pydantic validation text embeds the offending input, which can be arbitrarily large.

One deliberate behaviour change: code reading stdio_server() streams directly no longer receives failed lines as in-stream exceptions. That is the path that made the bug possible, and the existing test for it has been updated to assert the error frame instead.

How Has This Been Tested?

  • New regression tests in tests/server/test_stdio.py, all failing on the unmodified code and passing with this change: the depth-256 request completes with a parse error carrying id 900256 while a following shallow ping still arrives; an end-to-end test through MCPServer.run("stdio") asserting the same on the real process streams; parametrized cases for id recovery (truncated JSON with string and numeric ids, no id, an unrecoverable id token) and for the invalid-request split.
  • tests/server/test_stdio.py, tests/client/test_stdio.py, tests/interaction/transports/test_stdio.py, and tests/server/test_lowlevel_exception_handling.py: 64 passed.
  • src/mcp/server/stdio.py coverage under those suites: 100%.
  • ruff check, ruff format --check, and pyright on both changed files: clean.
  • Full suite via ./scripts/test: 6211 passed; the 12 failures (httpx InvalidURL cases, docs, a file-permission test) reproduce identically on unmodified main in this environment and are unrelated to this change.

Breaking Changes

None for protocol clients: a request that previously hung now receives an error response. Direct consumers of the stdio_server() read stream no longer see failed lines as in-stream Exception items, as noted above.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I am assigned to the linked issue (or it is labeled help wanted, or I'm a maintainer)
  • I have disclosed any AI assistance and can explain the change in my own words
  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Disclosure: I used AI tooling (Claude) to help investigate and draft this change. I verified the reproduction and the tests myself and I am accountable for the result.

On the unchecked boxes: I am not yet assigned to #2751 and it is not labeled help wanted, so I understand this PR may be closed automatically until a maintainer assigns the issue, and that it reopens automatically when that happens; I will push any updates as new commits rather than force-pushing in the meantime. I posted the reproduction and this approach on the issue before opening this. No documentation changes are included because no public API or documented behaviour changes.

A stdio line that fails validation (e.g. a request nested past the
recursion limit) was forwarded as a bare exception on the read stream,
which the server dispatcher drops, leaving the request id pending
until the client timed out.

Send a JSONRPCError on the write stream instead: -32700 for invalid
JSON, -32600 for a parsed but invalid message, recovering the request
id from the raw line where possible and falling back to null.
@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Oct 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This PR has been closed automatically. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and you aren't currently assigned to #2751.

If a maintainer assigns you to #2751, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take.

You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way.

CONTRIBUTING.md has the full reasoning, but in short:

  • We're a small team with very little capacity to review community PRs right now.
  • Many recent PRs are AI-generated with little human review, and reviewing one carefully still costs a maintainer as much time as it ever did. A well-described issue is usually more useful to us than the code.

Maintainers: reopen, remove missing-issue-link, or add bypass-issue-check to override.

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

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deep JSON-RPC request over stdio logs a validation error and leaves the original request pending

1 participant