Skip to content

fix(mcp): report non-timeout MCP errors with their own type instead of as a timeout - #9440

Open
Sergey Blekher (blurman-ai) wants to merge 1 commit into
crewAIInc:mainfrom
blurman-ai:mcp-error-not-timeout
Open

Sergey Blekher (blurman-ai) wants to merge 1 commit into
crewAIInc:mainfrom
blurman-ai:mcp-error-not-timeout

Conversation

@blurman-ai

@blurman-ai Sergey Blekher (blurman-ai) commented Nov 14, 2026 •

Copy link
Copy Markdown

Fixes #9438

What the user sees

When an MCP server rejects a request (for example HTTP 401 or 404), the error is raised inside the MCP client's task group. MCPToolWrapper._execute_tool treated any exception that had a __cause__, or whose text contained "TaskGroup" / "unhandled errors", as asyncio.TimeoutError. The wrapper then retried the call 3 times and told the model "Connection timed out after 60 seconds": the agent retried a request that can never succeed, and the real reason was lost.

Fix

Classify by exception type, not by text. _execute_tool flattens (nested) exception groups into their leaf exceptions:

  • if any leaf is a TimeoutError, it is a timeout and is retried as before;
  • otherwise the leaves are reported as "MCP execution error: : " and the call is not retried.

A plain exception that is not a group is re-raised unchanged and goes through the existing handling.

Test

lib/crewai/tests/mcp/test_tool_wrapper_errors.py stubs the MCP client and session so call_tool raises an exception group:

  • a group containing an HTTP 401-like error: the message names the error type and "401", does not say "timed out", and call_tool is awaited once (before the fix: 3 attempts and "Connection timed out");
  • a group containing a TimeoutError: still reported as a timeout and retried 3 times.

Both pass with the change; the first fails without it. ruff check is clean.

Written with an AI coding assistant (Claude Code) and reviewed and run before submitting, hence the llm-generated label.

Resubmission of #9368, which was automatically closed before the required bug report existed. #9438 now contains a reproduction against upstream main with a local HTTP endpoint returning 401. Reopening #9368 returns GitHub validation error 422. The fix is unchanged; its two focused MCP tests were rerun successfully on 2026-11-14.


Note

Low Risk
Narrows MCP error handling and retry behavior; adds tests with no auth or data-model changes.

Overview
Fixes misclassified MCP failures that were surfaced as connection timeouts (with 3 retries) when the MCP client raised nested TaskGroup errors for real HTTP/auth failures.

MCPToolWrapper._execute_tool now flattens exception groups via _leaf_exceptions and branches on leaf types: TimeoutError leaves still become a timeout and follow the existing retry path; other grouped errors raise a new MCPExecutionError with leaf type/message details. _execute_single_attempt catches that type, returns "MCP execution error: …", and does not retry. Plain non-group exceptions are re-raised unchanged.

Adds test_tool_wrapper_errors.py with stubbed MCP client/session: HTTP 401 inside a group is reported once with the real error (not "timed out"); timeout leaves still retry 3 times.

Reviewed by Cursor Bugbot for commit 434831f. Bugbot is set up for automated code reviews on this repo. Configure here.

…f as a timeout

MCPToolWrapper._execute_tool turned any exception with a __cause__, or whose text contained "TaskGroup", into asyncio.TimeoutError. A 401/404 raised inside the client task group was therefore retried 3 times and shown to the model as "Connection timed out". Classify by exception type: flatten exception groups, treat only TimeoutError leaves as a timeout, and report any other leaf by type and message without retrying.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Sergey <blurman@mail.ru>
@strix-security

strix-security Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Strix Security Review

✅ No open security findings.

PR summary: Changes how the MCP tool wrapper classifies failures from a remote MCP server: instead of treating any exception group as a timeout, it flattens the group and sorts by type, retrying only genuine timeouts and otherwise reporting the real error type without retrying. Adds two focused tests for the timeout and non-timeout shapes.

Review summary

I reviewed the rewritten MCP error classification in MCPToolWrapper — the new MCPExecutionError type, the _leaf_exceptions group flattener, the exception-type timeout check, the added handler that returns the error without retrying, and the accompanying tests — and followed each branch into its callers and out through the tool-observation path the agent sees.

The change only narrows what used to be misreported: every error whose leaf is a timeout was already retried before, and a dedicated handler now catches non-timeout failures before the generic substring-based handlers, so no failure newly escapes, hangs, or gets retried as a timeout. Error text now reaches the agent on this path where it previously collapsed to a canned "Connection timed out" sentence, but that text is composed from the operator's own server configuration plus whatever the configured server already sends verbatim as tool output, and the pre-existing handler already returned exception text the same way, so no trust boundary is crossed. Hostile exception trees are not server-constructible: group nesting comes from the client's own task groups, and groups carrying cancellation-style leaves bypass these handlers exactly as they did before. No security issues found.

Ruled out (4)
  • Can a remote MCP server drive the new group-flattening helper into unbounded recursion or a crash with a deeply nested or self-referential exception tree?
    Exception containers in this path are created only by the client's own task-group frames at one or two levels of nesting, so server payload cannot shape the tree, and a group carrying non-Exception leaves bypasses these handlers unchanged as before.
  • Does classifying by exception type cause a hard failure to be retried as a timeout, or let an error escape past the tool boundary?
    The set of errors converted to a retryable timeout only shrinks relative to the previous heuristic, and the new error type is caught by a dedicated handler placed before the generic ones, so every path still returns a string to the caller.
  • Does putting raw leaf exception text into the tool result disclose secrets to the model or to a third party?
    Any credentials in that text come from the operator's own server configuration and the string only reaches that same operator's tool observation, while the pre-existing handler already interpolated exception text into the model-facing result.
  • Does the wrapper ignoring the MCP isError flag become exploitable because of this change?
    An isError response never raises, so it is not classified here at all, and its content already reaches the model verbatim, making this unchanged framework bookkeeping rather than a capability the change grants.

Review 1 on this PR · 434831f · requested Nov 14 11:16 UTC when the PR was opened


Reviewed by Strix
Re-run review · Configure security review settings · Try a deeper review with @strix-security /deep

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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 434831f. Configure here.

if len(leaves) == 1 and leaves[0] is e:
raise
details = "; ".join(f"{type(leaf).__name__}: {leaf}" for leaf in leaves)
raise MCPExecutionError(details) from e

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed exception groups skip classifier

Medium Severity

The new group classifier runs only under except Exception, so a BaseExceptionGroup that mixes an HTTP refusal with CancelledError never reaches _leaf_exceptions. That is the shape the rest of the MCP stack already handles with (Exception, BaseExceptionGroup) and find_http_status. Those failures still hit the CancelledError branch and become a timeout, or they escape _run uncaught.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 434831f. Configure here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] MCP tool errors such as HTTP 401 are reported as a 60-second timeout and retried 3 times

1 participant