fix(client): tolerate invalid UTF-8 from server stdout in stdio_client#2873
fix(client): tolerate invalid UTF-8 from server stdout in stdio_client#2873Bartok9 wants to merge 1 commit into
Conversation
Closes modelcontextprotocol#2454. stdio_client decoded child stdout with encoding_error_handler="strict", so a server emitting malformed bytes mid-session raised UnicodeDecodeError inside the decode loop, escaping both except clauses and tearing down the transport task group (surfacing as an ExceptionGroup out of the context manager) instead of surfacing the bad line as an in-stream parse error. Default encoding_error_handler to "replace" so invalid bytes become U+FFFD; the malformed line then fails JSON validation and is delivered as an Exception via _parse_line, keeping the transport alive for subsequent valid messages. This mirrors the server-side stdin hardening (errors="replace") referenced in the issue (modelcontextprotocol#2302). Verified: repro that crashed the task group on main now surfaces a parse error then reads the following valid message; new regression test fails (5s task-group hang) without the fix, passes with it. ruff + pyright clean, 238 client/stdio tests pass.
|
CI note: the one red cell (`test (3.13, lowest-direct, windows-latest)`) is a flake unrelated to this change. It failed on `tests/interaction/transports/test_stdio.py::test_tool_call_and_notification_round_trip_over_a_stdio_subprocess` with `assert captured_stderr == "stdio-echo: clean exit\n"` getting `` — i.e. the server child was escalation-killed on a loaded Windows runner before it could flush its clean-exit line. That is a server-side shutdown-timing assertion in a different test file; this PR only changes client-side stdout decoding in `src/mcp/client/stdio.py` (+ `tests/client/test_stdio.py`). The same `lowest-direct/windows` config passed on 3.10/3.11/3.12/3.14, and the aggregate `all-green` check is green. Local run on this branch: `uv run pytest tests/client/test_stdio.py` → 34 passed, 1 skipped. A re-run of the failed job should clear it (I don't have repo admin to trigger it). |
Summary
StdioServerParameters.encoding_error_handlerto"replace"so a server emitting malformed UTF-8 no longer crashes the client transport.Motivation
Closes #2454.
stdio_client()decoded child stdout withencoding_error_handler="strict". When a spawned server writes invalid UTF-8 bytes to stdout, theUnicodeDecodeErrorraised duringTextReceiveStreamiteration escapes the decode loop'sexceptclauses and tears down the transport task group — it surfaces as anExceptionGroupout of the context manager instead of being delivered as a normal in-stream parse error.The issue itself points at the analogous server-side hardening (#2302,
errors="replace"on stdin), which deliberately chose to: replace invalid bytes with U+FFFD, let JSON validation fail on the malformed line, and keep the transport alive. This change brings the client to parity: defaultingencoding_error_handlerto"replace"means the bad line fails JSON-RPC validation and is delivered as anExceptionvia_parse_line, while later valid messages still come through.Verification
uv run pytest tests/client/test_stdio.py -q— 34 passed, 1 skippeduv run pytest tests/client/ tests/interaction/transports/test_stdio.py -q— 238 passed, 1 skipped, 1 xfailed (no regression from the default change)test_invalid_utf8_mid_session_surfaces_as_an_in_stream_exceptionfails without the fix (5s task-group hang) and passes with it.\xff\xfe\nfollowed by a validping) crashed the task group onmainwith anExceptionGroup(UnicodeDecodeError, ...); with the fix it surfaces aValidationErrorthen reads the following valid message.ruff format --check+ruff check+pyrightclean on both changed files.Notes
mainafter the transport was refactored. The decode/drain plumbing moved, so this is a fresh implementation on the new code path with a mid-session regression test (distinct from the existingtest_invalid_utf8_flushed_by_a_dying_server_does_not_break_shutdown, which only covers the raw-bytes shutdown drain).