-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix MCPServerStreamableHTTP closing user-provided HTTP clients with MCP SDK 1.25+
#3854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…-user-provided-HTTP-clients-with-MCP-SDK-1.25+
|
hey @eekstunt thank you for your contribution! please note that we currently have a couple PRs reviewing and updating dependencies, see here https://github.com/pydantic/pydantic-ai/pull/3778/files#r2648478356 If the problem were to be solved by that then we would avoid the extra logic and tests from this PR, otherwise we'll revisit this. |
|
hey @eekstunt we've closed the PR I referenced, so we can continue with this one. I would prefer though if we just require the higher version of the mcp package so we don't have to maintain both branches of this logic, do you think you could course-correct your PR? |
MCPServerStreamableHTTP closing user-provided HTTP clients with MCP SDK 1.25+
dsfaccini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my last comment
I will do it. Thank you! |
| yield self.http_client | ||
|
|
||
| async with transport_client_partial(httpx_client_factory=httpx_client_factory) as ( | ||
| async with streamable_http_client(self.url, http_client=self.http_client) as ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still need to use _transport_client
| @property | ||
| def _transport_client(self): | ||
| return streamablehttp_client | ||
| return self.http_client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be reverted
| assert self.http_client is not None | ||
| yield self.http_client | ||
|
|
||
| async with transport_client_partial(httpx_client_factory=httpx_client_factory) as ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the transport_client_partial method can now be deleted
| read_stream, | ||
| write_stream, | ||
| *_, | ||
| _, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if this is right
Summary
Fix
AttributeError: '_AsyncGeneratorContextManager' object has no attribute 'stream'when using customhttp_clientwithMCPServerStreamableHTTPafter MCP SDK update.Problem
After MCP SDK PR modelcontextprotocol/python-sdk#1177 (which FastMCP 2.14.1 depends on), the
streamable_http_clientfunction changed its signature. The new function acceptshttp_client: httpx.AsyncClientdirectly, while the deprecatedstreamablehttp_clientwrapper expects a factory function (httpx_client_factory).When
pydantic-aipassed a customhttp_client, the deprecated wrapper's internalasync with client:block would close the user's HTTP client prematurely, breaking reusability.Solution
Added conditional logic to use the new
streamable_http_clientAPI directly when:http_clientis providedFor older MCP SDK versions or SSE transport, falls back to the factory approach.
Changes
streamable_http_clientwith graceful fallbackclient_streams()inMCPServerStreamableHTTPto use new API when available