Skip to content

Commit 24cc85c

Browse files
authored
Merge branch 'modelcontextprotocol:main' into oauth-proxy
2 parents 3b33de5 + 7b1078b commit 24cc85c

28 files changed

+1637
-279
lines changed

.pre-commit-config.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ repos:
2727
- id: pyright
2828
name: pyright
2929
entry: uv run pyright
30-
args: [src]
3130
language: system
3231
types: [python]
3332
pass_filenames: false

CLAUDE.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ This document contains critical information about working with this codebase. Fo
1616
- Public APIs must have docstrings
1717
- Functions must be focused and small
1818
- Follow existing patterns exactly
19-
- Line length: 88 chars maximum
19+
- Line length: 120 chars maximum
2020

2121
3. Testing Requirements
2222
- Framework: `uv run --frozen pytest`
@@ -116,3 +116,15 @@ This document contains critical information about working with this codebase. Fo
116116
- Follow existing patterns
117117
- Document public APIs
118118
- Test thoroughly
119+
120+
## Exception Handling
121+
122+
- **Always use `logger.exception()` instead of `logger.error()` when catching exceptions**
123+
- Don't include the exception in the message: `logger.exception("Failed")` not `logger.exception(f"Failed: {e}")`
124+
- **Catch specific exceptions** where possible:
125+
- File ops: `except (OSError, PermissionError):`
126+
- JSON: `except json.JSONDecodeError:`
127+
- Network: `except (ConnectionError, TimeoutError):`
128+
- **Only catch `Exception` for**:
129+
- Top-level handlers that must not crash
130+
- Cleanup blocks (log at debug level)

examples/servers/simple-auth/mcp_simple_auth/server.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,8 @@ def main(port: int, auth_server: str, transport: Literal["sse", "streamable-http
160160
mcp_server.run(transport=transport)
161161
logger.info("Server stopped")
162162
return 0
163-
except Exception as e:
164-
logger.error(f"Server error: {e}")
165-
logger.exception("Exception details:")
163+
except Exception:
164+
logger.exception("Server error")
166165
return 1
167166

168167

pyproject.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ dependencies = [
3232
"pydantic-settings>=2.5.2",
3333
"uvicorn>=0.23.1; sys_platform != 'emscripten'",
3434
"jsonschema>=4.20.0",
35+
"pywin32>=310; sys_platform == 'win32'",
3536
]
3637

3738
[project.optional-dependencies]
@@ -125,4 +126,6 @@ filterwarnings = [
125126
"ignore::DeprecationWarning:websockets",
126127
"ignore:websockets.server.WebSocketServerProtocol is deprecated:DeprecationWarning",
127128
"ignore:Returning str or bytes.*:DeprecationWarning:mcp.server.lowlevel",
129+
# pywin32 internal deprecation warning
130+
"ignore:getargs.*The 'u' format is deprecated:DeprecationWarning"
128131
]

src/mcp/cli/claude.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,10 @@ def update_claude_config(
7575
if not config_file.exists():
7676
try:
7777
config_file.write_text("{}")
78-
except Exception as e:
79-
logger.error(
78+
except Exception:
79+
logger.exception(
8080
"Failed to create Claude config file",
8181
extra={
82-
"error": str(e),
8382
"config_file": str(config_file),
8483
},
8584
)
@@ -139,11 +138,10 @@ def update_claude_config(
139138
extra={"config_file": str(config_file)},
140139
)
141140
return True
142-
except Exception as e:
143-
logger.error(
141+
except Exception:
142+
logger.exception(
144143
"Failed to update Claude config",
145144
extra={
146-
"error": str(e),
147145
"config_file": str(config_file),
148146
},
149147
)

src/mcp/cli/cli.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -349,12 +349,11 @@ def run(
349349

350350
server.run(**kwargs)
351351

352-
except Exception as e:
353-
logger.error(
354-
f"Failed to run server: {e}",
352+
except Exception:
353+
logger.exception(
354+
"Failed to run server",
355355
extra={
356356
"file": str(file),
357-
"error": str(e),
358357
},
359358
)
360359
sys.exit(1)
@@ -464,8 +463,8 @@ def install(
464463
if dotenv:
465464
try:
466465
env_dict |= {k: v for k, v in dotenv.dotenv_values(env_file).items() if v is not None}
467-
except Exception as e:
468-
logger.error(f"Failed to load .env file: {e}")
466+
except (OSError, ValueError):
467+
logger.exception("Failed to load .env file")
469468
sys.exit(1)
470469
else:
471470
logger.error("python-dotenv is not installed. Cannot load .env file.")

src/mcp/client/auth.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -464,8 +464,8 @@ async def _handle_refresh_response(self, response: httpx.Response) -> bool:
464464
await self.context.storage.set_tokens(token_response)
465465

466466
return True
467-
except ValidationError as e:
468-
logger.error(f"Invalid refresh response: {e}")
467+
except ValidationError:
468+
logger.exception("Invalid refresh response")
469469
self.context.clear_tokens()
470470
return False
471471

@@ -522,8 +522,8 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
522522
token_request = await self._exchange_token(auth_code, code_verifier)
523523
token_response = yield token_request
524524
await self._handle_token_response(token_response)
525-
except Exception as e:
526-
logger.error(f"OAuth flow error: {e}")
525+
except Exception:
526+
logger.exception("OAuth flow error")
527527
raise
528528

529529
# Add authorization header and make request

src/mcp/client/sse.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ async def sse_reader(
9797
)
9898
logger.debug(f"Received server message: {message}")
9999
except Exception as exc:
100-
logger.error(f"Error parsing server message: {exc}")
100+
logger.exception("Error parsing server message")
101101
await read_stream_writer.send(exc)
102102
continue
103103

@@ -106,7 +106,7 @@ async def sse_reader(
106106
case _:
107107
logger.warning(f"Unknown SSE event: {sse.event}")
108108
except Exception as exc:
109-
logger.error(f"Error in sse_reader: {exc}")
109+
logger.exception("Error in sse_reader")
110110
await read_stream_writer.send(exc)
111111
finally:
112112
await read_stream_writer.aclose()
@@ -126,8 +126,8 @@ async def post_writer(endpoint_url: str):
126126
)
127127
response.raise_for_status()
128128
logger.debug(f"Client message sent successfully: {response.status_code}")
129-
except Exception as exc:
130-
logger.error(f"Error in post_writer: {exc}")
129+
except Exception:
130+
logger.exception("Error in post_writer")
131131
finally:
132132
await write_stream.aclose()
133133

src/mcp/client/stdio/__init__.py

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
import os
23
import sys
34
from contextlib import asynccontextmanager
@@ -6,18 +7,22 @@
67

78
import anyio
89
import anyio.lowlevel
10+
from anyio.abc import Process
911
from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream
1012
from anyio.streams.text import TextReceiveStream
1113
from pydantic import BaseModel, Field
1214

1315
import mcp.types as types
14-
from mcp.shared.message import SessionMessage
15-
16-
from .win32 import (
16+
from mcp.os.posix.utilities import terminate_posix_process_tree
17+
from mcp.os.win32.utilities import (
18+
FallbackProcess,
1719
create_windows_process,
1820
get_windows_executable_command,
19-
terminate_windows_process,
21+
terminate_windows_process_tree,
2022
)
23+
from mcp.shared.message import SessionMessage
24+
25+
logger = logging.getLogger(__name__)
2126

2227
# Environment variables to inherit by default
2328
DEFAULT_INHERITED_ENV_VARS = (
@@ -38,6 +43,9 @@
3843
else ["HOME", "LOGNAME", "PATH", "SHELL", "TERM", "USER"]
3944
)
4045

46+
# Timeout for process termination before falling back to force kill
47+
PROCESS_TERMINATION_TIMEOUT = 2.0
48+
4149

4250
def get_default_environment() -> dict[str, str]:
4351
"""
@@ -178,12 +186,25 @@ async def stdin_writer():
178186
try:
179187
yield read_stream, write_stream
180188
finally:
181-
# Clean up process to prevent any dangling orphaned processes
189+
# MCP spec: stdio shutdown sequence
190+
# 1. Close input stream to server
191+
# 2. Wait for server to exit, or send SIGTERM if it doesn't exit in time
192+
# 3. Send SIGKILL if still not exited
193+
if process.stdin:
194+
try:
195+
await process.stdin.aclose()
196+
except Exception:
197+
# stdin might already be closed, which is fine
198+
pass
199+
182200
try:
183-
if sys.platform == "win32":
184-
await terminate_windows_process(process)
185-
else:
186-
process.terminate()
201+
# Give the process time to exit gracefully after stdin closes
202+
with anyio.fail_after(PROCESS_TERMINATION_TIMEOUT):
203+
await process.wait()
204+
except TimeoutError:
205+
# Process didn't exit from stdin closure, use platform-specific termination
206+
# which handles SIGTERM -> SIGKILL escalation
207+
await _terminate_process_tree(process)
187208
except ProcessLookupError:
188209
# Process already exited, which is fine
189210
pass
@@ -218,11 +239,38 @@ async def _create_platform_compatible_process(
218239
):
219240
"""
220241
Creates a subprocess in a platform-compatible way.
221-
Returns a process handle.
242+
243+
Unix: Creates process in a new session/process group for killpg support
244+
Windows: Creates process in a Job Object for reliable child termination
222245
"""
223246
if sys.platform == "win32":
224247
process = await create_windows_process(command, args, env, errlog, cwd)
225248
else:
226-
process = await anyio.open_process([command, *args], env=env, stderr=errlog, cwd=cwd)
249+
process = await anyio.open_process(
250+
[command, *args],
251+
env=env,
252+
stderr=errlog,
253+
cwd=cwd,
254+
start_new_session=True,
255+
)
227256

228257
return process
258+
259+
260+
async def _terminate_process_tree(process: Process | FallbackProcess, timeout_seconds: float = 2.0) -> None:
261+
"""
262+
Terminate a process and all its children using platform-specific methods.
263+
264+
Unix: Uses os.killpg() for atomic process group termination
265+
Windows: Uses Job Objects via pywin32 for reliable child process cleanup
266+
267+
Args:
268+
process: The process to terminate
269+
timeout_seconds: Timeout in seconds before force killing (default: 2.0)
270+
"""
271+
if sys.platform == "win32":
272+
await terminate_windows_process_tree(process, timeout_seconds)
273+
else:
274+
# FallbackProcess should only be used for Windows compatibility
275+
assert isinstance(process, Process)
276+
await terminate_posix_process_tree(process, timeout_seconds)

0 commit comments

Comments
 (0)