Fix MCP connection timeouts and disable tool call retries by default - #6744
Open
lorenzejay wants to merge 4 commits into
Open
Fix MCP connection timeouts and disable tool call retries by default#6744lorenzejay wants to merge 4 commits into
lorenzejay wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughMCP transport connections now use configurable, Python-version-compatible timeouts. MCPClient initialization, cleanup, and tool-call retries were adjusted, with retries disabled by default. New tests cover lifecycle behavior, and MCP subprocess tests use the active Python interpreter. ChangesMCP lifecycle updates
Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant Transport
participant MCPSession
MCPClient->>Transport: connect within configured timeout
Transport-->>MCPClient: transport context and streams
MCPClient->>MCPSession: initialize within remaining timeout
MCPSession-->>MCPClient: initialization result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/mcp/transports/http.py`:
- Around line 90-93: Preserve transport timeout exceptions so
MCPClient.connect() reports them as "timeout" rather than "network": in
lib/crewai/src/crewai/mcp/transports/http.py lines 90-93, retain TimeoutError or
use a dedicated timeout exception recognized by MCPClient; in
lib/crewai/src/crewai/mcp/transports/sse.py lines 72-73, handle TimeoutError
before broad Exception/ConnectionError handling; and in
lib/crewai/src/crewai/mcp/transports/stdio.py lines 101-102, preserve
TimeoutError through both the timeout wrapper and transport exception branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e78cd7ee-6ff5-44ea-836d-289b2e450b69
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
lib/crewai-tools/tests/adapters/mcp_adapter_test.pylib/crewai/pyproject.tomllib/crewai/src/crewai/mcp/_utils.pylib/crewai/src/crewai/mcp/client.pylib/crewai/src/crewai/mcp/transports/base.pylib/crewai/src/crewai/mcp/transports/http.pylib/crewai/src/crewai/mcp/transports/sse.pylib/crewai/src/crewai/mcp/transports/stdio.pylib/crewai/tests/mcp/test_client.pylib/crewai/tests/mcp/test_tool_resolver_native.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/mcp/transports/http.py`:
- Around line 90-93: Preserve transport timeout exceptions so
MCPClient.connect() reports them as "timeout" rather than "network": in
lib/crewai/src/crewai/mcp/transports/http.py lines 90-93, retain TimeoutError or
use a dedicated timeout exception recognized by MCPClient; in
lib/crewai/src/crewai/mcp/transports/sse.py lines 72-73, handle TimeoutError
before broad Exception/ConnectionError handling; and in
lib/crewai/src/crewai/mcp/transports/stdio.py lines 101-102, preserve
TimeoutError through both the timeout wrapper and transport exception branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e78cd7ee-6ff5-44ea-836d-289b2e450b69
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
lib/crewai-tools/tests/adapters/mcp_adapter_test.pylib/crewai/pyproject.tomllib/crewai/src/crewai/mcp/_utils.pylib/crewai/src/crewai/mcp/client.pylib/crewai/src/crewai/mcp/transports/base.pylib/crewai/src/crewai/mcp/transports/http.pylib/crewai/src/crewai/mcp/transports/sse.pylib/crewai/src/crewai/mcp/transports/stdio.pylib/crewai/tests/mcp/test_client.pylib/crewai/tests/mcp/test_tool_resolver_native.py
🛑 Comments failed to post (1)
lib/crewai/src/crewai/mcp/transports/http.py (1)
90-93: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash # Run under each supported project Python version. python - <<'PY' import asyncio try: from asyncio import timeout except ImportError: from async_timeout import timeout async def main(): try: async with timeout(0): await asyncio.sleep(0) except TimeoutError as error: print(type(error).__name__, repr(str(error))) asyncio.run(main()) PYRepository: crewAIInc/crewAI
Length of output: 170
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail printf 'Repository files matching mcp transports/client:\n' git ls-files | rg 'lib/crewai/src/crewai/mcp/(transports|.*client.*)' || true printf '\nRelevant file excerpts:\n' for f in lib/crewai/src/crewai/mcp/transports/http.py lib/crewai/src/crewai/mcp/transports/sse.py lib/crewai/src/crewai/mcp/transports/stdio.py; do [ -f "$f" ] && { printf '\n--- %s ---\n' "$f"; sed -n '1,140p' "$f" | nl -ba; } done printf '\nSearch for timeout handling and MCPClient connect:\n' rg -n "class MCPClient|def connect|timeout|TimeoutError|ConnectionError|async_timeout|asyncio.timeout|timeout_event|emit|network|connect_timeout" lib/crewai/src/crewai/mcp -SRepository: crewAIInc/crewAI
Length of output: 587
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail for f in lib/crewai/src/crewai/mcp/transports/http.py lib/crewai/src/crewai/mcp/transports/sse.py lib/crewai/src/crewai/mcp/transports/stdio.py lib/crewai/src/crewai/mcp/client.py; do if [ -f "$f" ]; then echo "=== $f ===" cat -n "$f" | sed -n '1,160p' fi done echo "=== focused searches ===" rg -n "class MCPClient|async def connect|def connect|connect_timeout|async_timeout|asyncio.timeout|TimeoutError|ErrorType|ErrorType|emit|ConnectionError" lib/crewai/src/crewai/mcp -SRepository: crewAIInc/crewAI
Length of output: 31729
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail echo "=== lib/crewai/src/crewai/mcp/client.py lines 148-275 ===" cat -n lib/crewai/src/crewai/mcp/client.py | sed -n '148,275p' echo "=== lib/crewai/src/crewai/mcp/client.py lines 334-360 ===" cat -n lib/crewai/src/crewai/mcp/client.py | sed -n '334,360p' echo "=== MCPConnectionFailedEvent definition ===" fd -a 'mcp_events.py$|mcp_events' . | while read -r f; do echo "--- $f ---" cat -n "$f" doneRepository: crewAIInc/crewAI
Length of output: 11403
Preserve transport-level timeouts for
MCPClient.
MCPClient.connect()emits"timeout"only forasyncio.TimeoutError; wrapping transport timeout failures asConnectionErrorfirst makes them emit as"network"failures.
lib/crewai/src/crewai/mcp/transports/http.py#L90-L95: preserve a timeout-specific exception, or add a dedicated transport timeout error thatMCPClientrecognizes.lib/crewai/src/crewai/mcp/transports/sse.py#L72-L85: catch and preserveTimeoutErrorbeforeException/ConnectionError.lib/crewai/src/crewai/mcp/transports/stdio.py#L101-L108: preserveTimeoutErrorthrough both the timeout wrapper and the transportExceptionbranch.📍 Affects 3 files
lib/crewai/src/crewai/mcp/transports/http.py#L90-L93(this comment)lib/crewai/src/crewai/mcp/transports/sse.py#L72-L73lib/crewai/src/crewai/mcp/transports/stdio.py#L101-L102🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/mcp/transports/http.py` around lines 90 - 93, Preserve transport timeout exceptions so MCPClient.connect() reports them as "timeout" rather than "network": in lib/crewai/src/crewai/mcp/transports/http.py lines 90-93, retain TimeoutError or use a dedicated timeout exception recognized by MCPClient; in lib/crewai/src/crewai/mcp/transports/sse.py lines 72-73, handle TimeoutError before broad Exception/ConnectionError handling; and in lib/crewai/src/crewai/mcp/transports/stdio.py lines 101-102, preserve TimeoutError through both the timeout wrapper and transport exception branch.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cf4d891. Configure here.
joaomdmoura
approved these changes
Jul 31, 2026
| max_retries: int = MCP_MAX_RETRIES, | ||
| cache_tools_list: bool = False, | ||
| logger: logging.Logger | None = None, | ||
| retry_tool_calls: bool = False, |
Collaborator
There was a problem hiding this comment.
is this intended? jsut asking because disabling tool-call retries by default is a behavior change, so want to point it out
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Medium Risk
Changes default MCP tool-call behavior (no automatic retries) and rework async connection/cleanup paths that agents depend on; misclassification or teardown bugs could affect production MCP integrations.
Overview
Hardens MCP connect/teardown so
connect_timeoutapplies to transport SDK startup (HTTP/SSE/stdio) and sessioninitialize(), using in-taskasync_timeoutinstead ofasyncio.wait_forto avoid anyio “cancel scope in a different task” failures. Addsasync-timeouton Python < 3.11 and wires timeout throughBaseTransport.Connection errors and cleanup are clearer: shared helpers classify failures (timeout, TLS, auth, network) for events, unwind partial connects via
AsyncExitStack.__aexit__, and surface nested TLS/cancel cases without masking the root error. HTTP disconnect passes the triggering exception into the SDK context exit; optional best-effort logging on cleanup.Tool execution no longer retries by default (
retry_tool_calls=False); retries are opt-in to avoid replaying mutating MCP calls after a lost response.Tests add lifecycle coverage in
test_client.py;crewai-toolsMCP adapter integration tests spawn servers withsys.executableinstead ofuv run python.Reviewed by Cursor Bugbot for commit d0d2547. Bugbot is set up for automated code reviews on this repo. Configure here.