Skip to content

Commit 56d6bdc

Browse files
committed
Forward server stderr to errlog via pipe instead of inheriting its fd
Fixes #156: passing errlog (default sys.stderr) directly as the subprocess stderr target breaks in Jupyter, where sys.stderr has no fileno (UnsupportedOperation: fileno), swallowing all server diagnostics. The server's stderr is now always a pipe, and a reader task forwards it to errlog, so fileno-less streams work. Writes go through anyio.to_thread to keep blocking I/O off the loop.
1 parent c9356a2 commit 56d6bdc

4 files changed

Lines changed: 192 additions & 30 deletions

File tree

‎src/mcp/client/stdio.py‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import logging
1212
import os
13+
import subprocess
1314
import sys
1415
from collections.abc import AsyncGenerator
1516
from contextlib import asynccontextmanager, suppress
@@ -127,7 +128,6 @@ async def stdio_client(
127128
command=command,
128129
args=server.args,
129130
env=get_default_environment() | (server.env or {}),
130-
errlog=errlog,
131131
cwd=server.cwd,
132132
)
133133

@@ -182,6 +182,27 @@ async def stdin_writer() -> None:
182182
finally:
183183
writer_done.set()
184184

185+
async def stderr_reader() -> None:
186+
# The server's stderr is a pipe, never errlog's fd: errlog may be a
187+
# stream without a fileno (e.g. Jupyter's sys.stderr), which the
188+
# subprocess spawn cannot consume (gh-156). Forward it here instead.
189+
assert process.stderr, "Opened process is missing stderr"
190+
191+
def forward(chunk: str) -> None:
192+
# errlog is an arbitrary TextIO; keep the blocking write off the loop.
193+
errlog.write(chunk)
194+
errlog.flush()
195+
196+
stderr = TextReceiveStream(process.stderr, encoding=server.encoding, errors=server.encoding_error_handler)
197+
try:
198+
async for chunk in stderr:
199+
await anyio.to_thread.run_sync(forward, chunk)
200+
except (anyio.ClosedResourceError, anyio.BrokenResourceError, ConnectionError):
201+
# Teardown noise during shutdown, a real failure otherwise; stderr
202+
# carries only diagnostics, so the session is unaffected either way.
203+
if not shutting_down:
204+
logger.exception("Reading from the MCP server's stderr failed mid-session")
205+
185206
async def shutdown() -> None:
186207
"""Winds the transport down: stop traffic, flush, stop the server, release the streams."""
187208
# Unblock the reader into its drain: a server stuck writing stdout cannot
@@ -201,6 +222,7 @@ async def shutdown() -> None:
201222
async with anyio.create_task_group() as tg:
202223
tg.start_soon(stdout_reader)
203224
tg.start_soon(stdin_writer)
225+
tg.start_soon(stderr_reader)
204226
try:
205227
yield read_stream, write_stream
206228
finally:
@@ -252,7 +274,7 @@ async def _stop_server_process(process: ServerProcess) -> None:
252274
The escalation order is spec text; timeouts and tree-wide scope are SDK policy:
253275
https://modelcontextprotocol.io/specification/2025-11-25/basic/lifecycle#shutdown
254276
"""
255-
assert process.stdin and process.stdout, "server process is spawned with pipes"
277+
assert process.stdin and process.stdout and process.stderr, "server process is spawned with pipes"
256278

257279
await _close_pipe(process.stdin)
258280
if not await _wait_for_process_exit(process, PROCESS_TERMINATION_TIMEOUT):
@@ -265,6 +287,8 @@ async def _stop_server_process(process: ServerProcess) -> None:
265287
close_process_job(process)
266288
# A kill survivor can hold the stdout pipe open; poison the reader anyway.
267289
await _close_pipe(process.stdout)
290+
# Same for stderr: the reader only ever sees EOF, never a close it made.
291+
await _close_pipe(process.stderr)
268292
_close_subprocess_transport(process)
269293

270294

@@ -330,20 +354,23 @@ async def _create_platform_compatible_process(
330354
command: str,
331355
args: list[str],
332356
env: dict[str, str] | None = None,
333-
errlog: TextIO = sys.stderr,
334357
cwd: Path | str | None = None,
335358
) -> ServerProcess:
336359
"""Spawns the server in its own kill scope.
337360
338361
A new session/process group on POSIX, a Job Object on Windows.
362+
363+
The server's stderr is always a pipe: the caller's errlog stream may not
364+
have a fileno (e.g. Jupyter's sys.stderr), so it cannot be handed to the
365+
subprocess directly. `stdio_client` forwards the pipe to errlog instead.
339366
"""
340367
if sys.platform == "win32": # pragma: no cover
341-
return await create_windows_process(command, args, env, errlog, cwd)
368+
return await create_windows_process(command, args, env, cwd)
342369
else: # pragma: lax no cover
343370
return await anyio.open_process(
344371
[command, *args],
345372
env=env,
346-
stderr=errlog,
373+
stderr=subprocess.PIPE,
347374
cwd=cwd,
348375
start_new_session=True,
349376
)

‎src/mcp/os/win32/utilities.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import weakref
88
from contextlib import suppress
99
from pathlib import Path
10-
from typing import BinaryIO, TextIO, TypeAlias, cast
10+
from typing import BinaryIO, TypeAlias, cast
1111

1212
import anyio
1313
from anyio.abc import Process
@@ -92,9 +92,11 @@ def __init__(self, popen_obj: subprocess.Popen[bytes]) -> None:
9292
self.popen: subprocess.Popen[bytes] = popen_obj
9393
stdin = popen_obj.stdin
9494
stdout = popen_obj.stdout
95+
stderr = popen_obj.stderr
9596

9697
self.stdin = FileWriteStream(cast(BinaryIO, stdin)) if stdin else None
9798
self.stdout = FileReadStream(cast(BinaryIO, stdout)) if stdout else None
99+
self.stderr = FileReadStream(cast(BinaryIO, stderr)) if stderr else None
98100

99101
async def wait(self) -> int:
100102
"""Waits for exit by polling the Popen.
@@ -137,7 +139,6 @@ async def create_windows_process(
137139
command: str,
138140
args: list[str],
139141
env: dict[str, str] | None = None,
140-
errlog: TextIO | None = sys.stderr,
141142
cwd: Path | str | None = None,
142143
) -> Process | FallbackProcess:
143144
"""Creates a subprocess with Job Object support for tree termination.
@@ -149,21 +150,24 @@ async def create_windows_process(
149150
with it; children spawned before the assignment completes are not captured
150151
(see the inline note below).
151152
153+
The server's stderr is always a pipe (see `_create_platform_compatible_process`):
154+
the caller's errlog stream is forwarded it by `stdio_client` instead.
155+
152156
Returns:
153-
Process | FallbackProcess: The spawned process with async stdin/stdout streams.
157+
Process | FallbackProcess: The spawned process with async stdin/stdout/stderr streams.
154158
"""
155159
try:
156160
process = await anyio.open_process(
157161
[command, *args],
158162
env=env,
159163
# Ensure we don't create console windows for each process
160164
creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0),
161-
stderr=errlog,
165+
stderr=subprocess.PIPE,
162166
cwd=cwd,
163167
)
164168
except NotImplementedError:
165169
# Windows event loops without async subprocess support (SelectorEventLoop)
166-
process = await _create_windows_fallback_process(command, args, env, errlog, cwd)
170+
process = await _create_windows_fallback_process(command, args, env, cwd)
167171

168172
# Children spawned before the assignment completes land outside the job
169173
# (membership is inherited at CreateProcess, never acquired retroactively);
@@ -177,15 +181,14 @@ async def _create_windows_fallback_process(
177181
command: str,
178182
args: list[str],
179183
env: dict[str, str] | None = None,
180-
errlog: TextIO | None = sys.stderr,
181184
cwd: Path | str | None = None,
182185
) -> FallbackProcess:
183186
"""Spawns via subprocess.Popen and wraps it in FallbackProcess."""
184187
popen_obj = subprocess.Popen(
185188
[command, *args],
186189
stdin=subprocess.PIPE,
187190
stdout=subprocess.PIPE,
188-
stderr=errlog,
191+
stderr=subprocess.PIPE,
189192
env=env,
190193
cwd=cwd,
191194
bufsize=0, # Unbuffered output

0 commit comments

Comments
 (0)