-
Notifications
You must be signed in to change notification settings - Fork 5k
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
fix: component MCP Tools (SSE): 'Timeout' #5638
base: main
Are you sure you want to change the base?
Conversation
Uses Python's built-in asyncio.timeout() context manager Properly handles timeout exceptions Maintains the same functionality but with correct async context management
+clean up comment
Missing args_schema inside cause that tools are generated without input schema and are not able to be properly executed as agent know tool, but dost know what input field tool have. Same problem looks to be in MCP STDIO.
Line 56: Error: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling TRY003 Avoid specifying long messages outside the exception class EM102 Exception must not use an f-string literal, assign to variable first
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.
Thanks, @azdolinski !
try: | ||
async with asyncio.timeout(timeout_seconds): | ||
sse_transport = await self.exit_stack.enter_async_context( | ||
sse_client(url, headers, timeout_seconds, sse_read_timeout_seconds) | ||
) | ||
self.sse, self.write = sse_transport | ||
self.session = await self.exit_stack.enter_async_context(ClientSession(self.sse, self.write)) |
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 don't think Python 3.10 has that function.
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 am not quite sure which function you are talking about? Can you be more precise?
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.
Python 3.10 doesn't have asyncio.timeout(). I think asyncio.wait_for() along with minor modifications to your code would serve the purpose.
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.
That's why @phact created the custom context manager.
Remove asyncio.timeout() (not valid for Py3.10) and replace it by asyncio.wait_for()
Move return response.tools inside an else block. This makes it clearer that tools are returned only if the connection is successful, and not if a TimeoutError occurs.
Propose for resolve: #5637
Tested: