FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(client): support stderr logging without file descriptors by mikemikimike · Pull Request #3403 · modelcontextprotocol/python-sdk · GitHub

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .py  (3) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
28 changes: 26 additions & 2 deletions src/mcp/client/stdio.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
process nor hang on one.
"""

import io
import logging
import os
import subprocess
import sys
from collections.abc import AsyncGenerator
from contextlib import asynccontextmanager, suppress
Expand Down Expand Up @@ -122,11 +124,19 @@ async def stdio_client(
"""
command = _get_executable_command(server.command)

stderr = errlog
pipe_stderr = False
try:
errlog.fileno()
except (AttributeError, io.UnsupportedOperation, OSError):
stderr = subprocess.PIPE
pipe_stderr = True

process = await _create_platform_compatible_process(
command=command,
args=server.args,
env=get_default_environment() | (server.env or {}),
errlog=errlog,
errlog=stderr,
cwd=server.cwd,
)

Expand Down Expand Up @@ -181,6 +191,16 @@ async def stdin_writer() -> None:
finally:
writer_done.set()

async def stderr_reader() -> None:
process_stderr = getattr(process, "stderr", None)
if process_stderr is None or not pipe_stderr:
return
with suppress(anyio.EndOfStream, anyio.ClosedResourceError, anyio.BrokenResourceError, OSError):
while True:
chunk = await process_stderr.receive()
errlog.write(chunk.decode(server.encoding, errors=server.encoding_error_handler))
errlog.flush()

async def shutdown() -> None:
"""Winds the transport down: stop traffic, flush, stop the server, release the streams."""
# Unblock the reader into its drain: a server stuck writing stdout cannot
Expand All @@ -200,6 +220,7 @@ async def shutdown() -> None:
async with anyio.create_task_group() as tg:
tg.start_soon(stdout_reader)
tg.start_soon(stdin_writer)
tg.start_soon(stderr_reader)
try:
yield read_stream, write_stream
finally:
Expand Down Expand Up @@ -264,6 +285,9 @@ async def _stop_server_process(process: ServerProcess) -> None:
close_process_job(process)
# A kill survivor can hold the stdout pipe open; poison the reader anyway.
await _close_pipe(process.stdout)
process_stderr = getattr(process, "stderr", None)
if process_stderr is not None:
await _close_pipe(process_stderr)
_close_subprocess_transport(process)


Expand Down Expand Up @@ -329,7 +353,7 @@ async def _create_platform_compatible_process(
command: str,
args: list[str],
env: dict[str, str] | None = None,
errlog: TextIO = sys.stderr,
errlog: TextIO | int = sys.stderr,
cwd: Path | str | None = None,
) -> ServerProcess:
"""Spawns the server in its own kill scope.
Expand Down
4 changes: 2 additions & 2 deletions src/mcp/os/win32/utilities.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async def create_windows_process(
command: str,
args: list[str],
env: dict[str, str] | None = None,
errlog: TextIO | None = sys.stderr,
errlog: TextIO | int | None = sys.stderr,
cwd: Path | str | None = None,
) -> Process | FallbackProcess:
"""Creates a subprocess with Job Object support for tree termination.
Expand Down Expand Up @@ -177,7 +177,7 @@ async def _create_windows_fallback_process(
command: str,
args: list[str],
env: dict[str, str] | None = None,
errlog: TextIO | None = sys.stderr,
errlog: TextIO | int | None = sys.stderr,
cwd: Path | str | None = None,
) -> FallbackProcess:
"""Spawns via subprocess.Popen and wraps it in FallbackProcess."""
Expand Down
26 changes: 26 additions & 0 deletions tests/client/test_stdio.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@

import errno
import gc
import io
import logging
import math
import os
import signal
import subprocess
import sys
from collections.abc import Callable
from contextlib import AsyncExitStack, suppress
Expand Down Expand Up @@ -222,6 +224,30 @@ async def fake_terminate_tree(proc: FakeProcess) -> None:
FAKE_PARAMS = StdioServerParameters(command="fake-server")


@pytest.mark.anyio
async def test_stdio_client_pipes_stderr_for_non_file_errlog(monkeypatch: pytest.MonkeyPatch) -> None:
process = FakeProcess(on_stdin_close=lambda: process.exit(0))
seen_errlog: list[object] = []

async def fake_spawn(
command: str,
args: list[str],
env: dict[str, str] | None = None,
errlog: TextIO = sys.stderr,
cwd: Path | str | None = None,
) -> FakeProcess:
seen_errlog.append(errlog)
return process

monkeypatch.setattr(stdio, "_create_platform_compatible_process", fake_spawn)
monkeypatch.setattr(stdio, "_terminate_process_tree", lambda proc: proc.exit(-15))

async with stdio_client(FAKE_PARAMS, errlog=io.StringIO()):
pass

assert seen_errlog == [subprocess.PIPE]


def _line(message: JSONRPCMessage) -> bytes:
"""The wire form of `message`: one JSON document on its own line."""
return (message.model_dump_json(by_alias=True, exclude_unset=True) + "\n").encode()
Expand Down
Loading

Back | FazBrowse Home | New Git URL