Skip to content

Commit 82ddd29

Browse files
Apply review feedback
- Add Content-Type headers to test scopes to fix CI - Add public is_terminated property to avoid accessing private _terminated - Remove unnecessary try-catch from test - Change logging from warning to error for session crashes
1 parent 208f13b commit 82ddd29

File tree

3 files changed

+27
-22
lines changed

3 files changed

+27
-22
lines changed

src/mcp/server/streamable_http.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,11 @@ def __init__(
175175
] = {}
176176
self._terminated = False
177177

178+
@property
179+
def is_terminated(self) -> bool:
180+
"""Check if this transport has been explicitly terminated."""
181+
return self._terminated
182+
178183
def _create_error_response(
179184
self,
180185
error_message: str,
@@ -461,7 +466,7 @@ async def sse_writer():
461466
):
462467
break
463468
except Exception as e:
464-
logger.warning(f"Error in SSE writer: {e}", exc_info=True)
469+
logger.exception(f"Error in SSE writer: {e}")
465470
finally:
466471
logger.debug("Closing SSE writer")
467472
await self._clean_up_memory_streams(request_id)
@@ -491,13 +496,13 @@ async def sse_writer():
491496
session_message = SessionMessage(message, metadata=metadata)
492497
await writer.send(session_message)
493498
except Exception:
494-
logger.warning("SSE response error", exc_info=True)
499+
logger.exception("SSE response error")
495500
await sse_stream_writer.aclose()
496501
await sse_stream_reader.aclose()
497502
await self._clean_up_memory_streams(request_id)
498503

499504
except Exception as err:
500-
logger.warning("Error handling POST request", exc_info=True)
505+
logger.exception("Error handling POST request")
501506
response = self._create_error_response(
502507
f"Error handling POST request: {err}",
503508
HTTPStatus.INTERNAL_SERVER_ERROR,
@@ -579,7 +584,7 @@ async def standalone_sse_writer():
579584
event_data = self._create_event_data(event_message)
580585
await sse_stream_writer.send(event_data)
581586
except Exception as e:
582-
logger.warning(f"Error in standalone SSE writer: {e}", exc_info=True)
587+
logger.exception(f"Error in standalone SSE writer: {e}")
583588
finally:
584589
logger.debug("Closing standalone SSE writer")
585590
await self._clean_up_memory_streams(GET_STREAM_KEY)
@@ -595,7 +600,7 @@ async def standalone_sse_writer():
595600
# This will send headers immediately and establish the SSE connection
596601
await response(request.scope, request.receive, send)
597602
except Exception as e:
598-
logger.warning(f"Error in standalone SSE response: {e}", exc_info=True)
603+
logger.exception(f"Error in standalone SSE response: {e}")
599604
await sse_stream_writer.aclose()
600605
await sse_stream_reader.aclose()
601606
await self._clean_up_memory_streams(GET_STREAM_KEY)

src/mcp/server/streamable_http_manager.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ async def run_stateless_server(*, task_status: TaskStatus[None] = anyio.TASK_STA
180180
stateless=True,
181181
)
182182
except Exception as e:
183-
logger.warning(f"Stateless session crashed: {e}", exc_info=True)
183+
logger.error(f"Stateless session crashed: {e}", exc_info=True)
184184

185185
# Assert task group is not None for type checking
186186
assert self._task_group is not None
@@ -243,7 +243,7 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
243243
stateless=False, # Stateful mode
244244
)
245245
except Exception as e:
246-
logger.warning(
246+
logger.error(
247247
f"Session {http_transport.mcp_session_id} crashed: {e}",
248248
exc_info=True,
249249
)
@@ -252,9 +252,7 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
252252
if (
253253
http_transport.mcp_session_id
254254
and http_transport.mcp_session_id in self._server_instances
255-
and not (
256-
hasattr(http_transport, "_terminated") and http_transport._terminated # pyright: ignore
257-
)
255+
and not http_transport.is_terminated
258256
):
259257
logger.info(
260258
"Cleaning up crashed session "

tests/server/test_streamable_http_manager.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,12 @@ async def test_stateful_session_cleanup_on_graceful_exit(running_manager):
104104
async def mock_send(message):
105105
sent_messages.append(message)
106106

107-
scope = {"type": "http", "method": "POST", "path": "/mcp", "headers": []}
107+
scope = {
108+
"type": "http",
109+
"method": "POST",
110+
"path": "/mcp",
111+
"headers": [(b"content-type", b"application/json")],
112+
}
108113

109114
async def mock_receive():
110115
return {"type": "http.request", "body": b"", "more_body": False}
@@ -158,21 +163,18 @@ async def mock_send(message):
158163
if message["type"] == "http.response.start" and message["status"] >= 500:
159164
pass # Expected if TestException propagates that far up the transport
160165

161-
scope = {"type": "http", "method": "POST", "path": "/mcp", "headers": []}
166+
scope = {
167+
"type": "http",
168+
"method": "POST",
169+
"path": "/mcp",
170+
"headers": [(b"content-type", b"application/json")],
171+
}
162172

163173
async def mock_receive():
164174
return {"type": "http.request", "body": b"", "more_body": False}
165175

166-
# It's possible handle_request itself might raise an error if the TestException
167-
# isn't caught by the transport layer before propagating.
168-
# The key is that the session manager's internal task for MCPServer.run
169-
# encounters the exception.
170-
try:
171-
await manager.handle_request(scope, mock_receive, mock_send)
172-
except TestException:
173-
# This might be caught here if not handled by StreamableHTTPServerTransport's
174-
# error handling
175-
pass
176+
# Trigger session creation
177+
await manager.handle_request(scope, mock_receive, mock_send)
176178

177179
session_id = None
178180
for msg in sent_messages:

0 commit comments

Comments
 (0)