SSE-safe HTTP timeouts in SamClient, self peer_id in get_mesh_info - #326
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the local peer ID to the mesh information handler in the Go node implementation and updates the Python client to use an SSE-friendly HTTP client from the MCP library. The review feedback advises against importing from the private mcp.shared._httpx_utils module, suggesting instead to configure a standard httpx.AsyncClient with custom timeouts to avoid potential breaking changes in future library updates.
| from typing import Any, Dict, List, Optional | ||
| from mcp import ClientSession | ||
| from mcp.client.streamable_http import streamable_http_client | ||
| from mcp.shared._httpx_utils import create_mcp_http_client |
There was a problem hiding this comment.
Importing from a private module (mcp.shared._httpx_utils) is risky because private implementation details (indicated by the leading underscore) are not part of the public API and can change or be removed in future releases of the mcp library without warning.
Instead, import httpx directly and configure the client with custom timeouts to prevent the SSE stream from being dropped.
| from mcp.shared._httpx_utils import create_mcp_http_client | |
| import httpx |
There was a problem hiding this comment.
Agreed on not reaching into _httpx_utils. Since there's no public re-export of create_mcp_http_client I switched to a plain httpx.AsyncClient. Kept the SDK's actual defaults rather than the suggested values: Timeout(30.0, read=300.0) and follow_redirects=True, since read=None would make a dead connection hang forever instead of failing.
| self._http_client = httpx.AsyncClient(headers=headers) | ||
|
|
||
| # Applies MCP's SSE-friendly timeouts; a plain httpx client reads for 5s and drops the stream. | ||
| self._http_client = create_mcp_http_client(headers=headers) |
There was a problem hiding this comment.
Instead of using the private create_mcp_http_client helper, configure httpx.AsyncClient directly with a custom timeout. This achieves the same SSE-friendly behavior (disabling the read timeout) while relying entirely on public, stable APIs.
| self._http_client = create_mcp_http_client(headers=headers) | |
| self._http_client = httpx.AsyncClient(headers=headers, timeout=httpx.Timeout(timeout=60.0, read=None)) |
There was a problem hiding this comment.
Agreed on not reaching into _httpx_utils: since there's no public re-export of create_mcp_http_client, switched to a plain httpx.AsyncClient. Kept the SDK's actual defaults rather than the suggested values: Timeout(30.0, read=300.0) and follow_redirects=True, since read=None would make a dead connection hang forever instead of failing.
A plain httpx.AsyncClient applies its default 5s read timeout to the MCP Streamable HTTP SSE stream, killing the session whenever it sits quiet longer than that. Use the SDK's create_mcp_http_client, which sets SSE-friendly timeouts.
32c9bf7 to
3f56055
Compare
|
LGTM |
2 participants
The two changes from #314 requested for immediate merge (#314 (comment)):