Support Streamable HTTP MCP endpoints#1300
Conversation
Add direct POST-based MCP transport for Streamable HTTP servers while keeping the existing SSE endpoint-discovery path as a fallback. Parse both JSON and text/event-stream responses so servers like DeepWiki can be validated and used.
There was a problem hiding this comment.
Code Review
This pull request introduces HttpTransport, a new transport layer for the Model Context Protocol (MCP) that supports streamable HTTP responses with a fallback to Server-Sent Events (SSE). It includes logic for parsing SSE messages and handling various HTTP status codes. Feedback highlights a potential issue where an empty message queue in StreamableHttpTransport::recv could lead to unintended connection closures. Additionally, the SSE parser's memory efficiency could be improved, and the global reqwest::Client timeout might prematurely terminate long-running tool executions if it is set lower than the application-level execution timeout.
| async fn recv(&mut self) -> Result<serde_json::Value> { | ||
| self.pending_messages | ||
| .pop_front() | ||
| .context("MCP Streamable HTTP response queue is empty") | ||
| } |
There was a problem hiding this comment.
The recv implementation for StreamableHttpTransport returns an error immediately if the pending_messages queue is empty. While this transport is primarily request-response, McpConnection::recv (line 1307) treats any error from transport.recv() as a fatal disconnection. If a server returns a 202 Accepted or 204 No Content status (handled at line 683), the queue will be empty and subsequent calls to recv will cause the connection to be marked as Disconnected. Ensure that recv behavior aligns with the expected lifecycle of the connection, perhaps by returning an empty result or waiting if appropriate.
| } | ||
|
|
||
| fn parse_sse_json_messages(body: &str) -> Result<Vec<serde_json::Value>> { | ||
| let normalized = body.replace("\r\n", "\n"); |
There was a problem hiding this comment.
The SSE parser is quite naive and may fail on large responses or non-standard line endings. Using body.replace("\r\n", "\n") creates a full copy of the response body in memory, which could be inefficient for large payloads. Consider using a more robust SSE parsing library or an incremental parser that operates on the byte stream directly.
| let client = reqwest::Client::builder() | ||
| .timeout(Duration::from_secs(connect_timeout_secs)) | ||
| .build()?; |
There was a problem hiding this comment.
The reqwest::Client is built with a global timeout set to connect_timeout_secs. This timeout applies to all requests made by the client, including tool executions. If connect_timeout (default 10s) is significantly shorter than execute_timeout (default 60s), tool calls will time out prematurely at the HTTP layer before the application-level timeout at line 1232 is reached. Consider configuring the client without a global timeout and applying specific timeouts per request, or using the maximum of the configured timeouts.
Summary
This fixes MCP servers that use Streamable HTTP endpoints, such as DeepWiki at
https://mcp.deepwiki.com/mcp.Previously, URL-based MCP connections only used the older SSE endpoint discovery
flow: open the URL with
GET, wait for anendpointevent, then send requeststhere. Streamable HTTP servers expect JSON-RPC requests to be POSTed directly to
the MCP URL and may return responses as
text/event-stream, so validation failedbefore any tools could be discovered.
This change adds Streamable HTTP support for URL MCP servers and preserves the
existing SSE discovery behavior as a fallback for older servers.
fixes: #1266
Testing
cargo test --all-featurescargo fmt --all -- --checkcargo clippy --all-targets --all-featuresChecklist