Skip to content

Commit 7c23857

Browse files
authored
[OSS-137] Report MCP HTTP auth failures instead of cancelled connections (#7067)
* feat(mcp): add shared error classifier for HTTP auth failures When an MCP server refuses a streamable-HTTP connection, the HTTP status is observed by the client but often buried inside anyio teardown. Add typed connection exceptions and helpers to recover the status from exception groups, CancelledError context chains, and httpx errors so later call sites can report authentication failures instead of guessing. Groundwork only; no call sites wired yet. * feat(mcp): raise typed errors from HTTPTransport.connect on HTTP status When streamable-HTTP connect fails with an httpx HTTPStatusError, classify the status via the shared MCP exception helpers and raise MCPAuthenticationError for 401/403 or MCPHTTPError for other refused statuses instead of a generic ConnectionError that hides the credential problem. * refactor(mcp): centralize raise_connection_failure and simplify connect Move connection failure classification into exceptions.py so transports and clients share one helper. Flatten HTTPTransport.connect to a single except path that cleans up once and raises, avoiding the outer handler re-classifying errors the inner handler already typed. * fix(mcp): classify auth failures in MCPClient.connect before reporting cancelled When a streamable-HTTP server refuses the connection, the awaiting coroutine often sees only CancelledError while the HTTP status surfaces during transport unwind. Inspect cleanup for the status before emitting error_type=cancelled, and fix HTTPTransport.disconnect so it raises typed errors instead of suppressing exception groups that carry the refusal. * fix(mcp): replace speculative tool resolver errors with classifier Use raise_connection_failure for native MCP discovery instead of hedged cancel-scope wording, preserve typed MCPConnectionError from setup, and detect event-loop presence explicitly so ConnectionError is not mistaken for a missing running loop. Update HTTPS discovery to classify HTTP status codes via find_http_status. * refactor(mcp): collapse native tool resolver failure handlers CancelledError is not an Exception subclass, so handle it alongside Exception in one except clause and delegate to a shared helper. * refactor(mcp): call raise_connection_failure directly in tool resolver * fix(mcp): classify tool execution auth failures in events Add tool_execution_error_type so call_tool_result emits authentication instead of server_error for MCPAuthenticationError and HTTP 401/403. Preserve typed MCPConnectionError in _retry_operation instead of flattening them into a generic ConnectionError first. * feat(mcp): add status_code to MCPConnectionFailedEvent Surface the HTTP status observed during connection failures on the event payload and in verbose console output, so executions and checkpoints record 401/403 alongside error_type=authentication instead of only the message text. * fix(mcp): handle cancellation and exception groups in auth paths Ensure discovery cleanup runs on CancelledError, classify mixed BaseExceptionGroups during HTTP connect, and fix ExceptionGroup imports on Python 3.10 with regression tests. * fix(mcp): preserve auth errors from discovery disconnect cleanup Re-raise MCPConnectionError from disconnect during cancellation cleanup instead of logging and swallowing it, with a regression test. * fix(mcp): unwind transport context to recover auth on cancel Always exit pending streamable-HTTP contexts before classifying failures, handle CancelledError during client cleanup, propagate typed HTTPS discovery errors, and add regression tests for the teardown recovery path. * fix(mcp): classify auth from groups and timeout teardown Handle BaseExceptionGroup in HTTPS discovery and recover HTTP 401 from streamable-HTTP context exit after connect timeouts, with regression tests. * refactor(mcp): consolidate client connection failure reporting Extract _report_connection_failure and delegate _http_failure and _connection_failure to it without changing connect error behavior. * refactor(mcp): drop redundant client failure helper wrappers Call _report_connection_failure directly from connect() instead of _http_failure and _connection_failure delegators. * fix(mcp): propagate CancelledError after HTTP transport teardown Re-raise cancellation from disconnect when no HTTP auth status is recovered during context unwind, with a regression test. * fix(mcp): preserve typed errors from MCPClient.disconnect Re-raise MCPConnectionError and CancelledError from exit-stack teardown instead of wrapping auth failures in RuntimeError, with a regression test.
1 parent 3df34d9 commit 7c23857

13 files changed

Lines changed: 1337 additions & 201 deletions

lib/crewai/src/crewai/events/event_listener.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,7 @@ def on_mcp_connection_failed(_: Any, event: MCPConnectionFailedEvent) -> None:
919919
event.transport_type,
920920
event.error,
921921
event.error_type,
922+
event.status_code,
922923
)
923924
self._telemetry.feature_usage_span("mcp:connection_failed")
924925

lib/crewai/src/crewai/events/types/mcp_events.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class MCPConnectionFailedEvent(MCPEvent):
4949
type: Literal["mcp_connection_failed"] = "mcp_connection_failed"
5050
error: str
5151
error_type: str | None = None # "timeout", "authentication", "network", etc.
52+
status_code: int | None = None
5253
started_at: datetime | None = None
5354
failed_at: datetime | None = None
5455

lib/crewai/src/crewai/events/utils/console_formatter.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,6 +1665,7 @@ def handle_mcp_connection_failed(
16651665
transport_type: str | None = None,
16661666
error: str = "",
16671667
error_type: str | None = None,
1668+
status_code: int | None = None,
16681669
) -> None:
16691670
"""Handle MCP connection failed event."""
16701671
if not self.verbose:
@@ -1683,6 +1684,10 @@ def handle_mcp_connection_failed(
16831684
content.append("Transport: ", style="white")
16841685
content.append(f"{transport_type}\n", style="red")
16851686

1687+
if status_code is not None:
1688+
content.append("HTTP Status: ", style="white")
1689+
content.append(f"{status_code}\n", style="red")
1690+
16861691
if error_type:
16871692
content.append("Error Type: ", style="white")
16881693
content.append(f"{error_type}\n", style="red")

lib/crewai/src/crewai/mcp/client.py

Lines changed: 127 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@
2727
MCPToolExecutionFailedEvent,
2828
MCPToolExecutionStartedEvent,
2929
)
30+
from crewai.mcp.exceptions import (
31+
MCPConnectionError,
32+
error_for_status,
33+
error_type_for_status,
34+
find_http_status,
35+
find_transport_failure,
36+
tool_execution_error_type,
37+
)
3038
from crewai.mcp.transports.base import BaseTransport
3139
from crewai.mcp.transports.http import HTTPTransport
3240
from crewai.mcp.transports.sse import SSETransport
@@ -148,13 +156,18 @@ async def connect(self) -> Self:
148156
Self for method chaining.
149157
150158
Raises:
151-
ConnectionError: If connection fails.
159+
MCPAuthenticationError: If the server refused the connection with
160+
an authentication status.
161+
MCPHTTPError: If the server refused the connection with any other
162+
HTTP status.
163+
MCPConnectionError: If the connection failed for any other reason.
152164
ImportError: If MCP SDK not available.
153165
"""
154166
if self.connected:
155167
return self
156168

157-
server_name, server_url, transport_type = self._get_server_info()
169+
server_info = self._get_server_info()
170+
server_name, server_url, transport_type = server_info
158171
is_reconnect = self._was_connected
159172

160173
started_at = datetime.now()
@@ -184,40 +197,13 @@ async def connect(self) -> Self:
184197

185198
await self._exit_stack.enter_async_context(self._session)
186199

187-
# MCP protocol requires session.initialize() before any other request
188-
try:
189-
await asyncio.wait_for(
190-
self._session.initialize(),
191-
timeout=self.connect_timeout,
192-
)
193-
except asyncio.CancelledError:
194-
# If initialization was cancelled (e.g., event loop closing),
195-
# cleanup and re-raise - don't suppress cancellation
196-
await self._cleanup_on_error()
197-
raise
198-
except BaseExceptionGroup as eg:
199-
# Handle exception groups from anyio task groups
200-
# Extract the actual meaningful error (not GeneratorExit)
201-
actual_error = None
202-
for exc in eg.exceptions:
203-
if isinstance(exc, Exception) and not isinstance(
204-
exc, GeneratorExit
205-
):
206-
# Check if it's an HTTP error (like 401)
207-
error_msg = str(exc).lower()
208-
if "401" in error_msg or "unauthorized" in error_msg:
209-
actual_error = exc
210-
break
211-
if "cancel scope" not in error_msg and "task" not in error_msg:
212-
actual_error = exc
213-
break
214-
215-
await self._cleanup_on_error()
216-
if actual_error:
217-
raise ConnectionError(
218-
f"Failed to connect to MCP server: {actual_error}"
219-
) from actual_error
220-
raise ConnectionError(f"Failed to connect to MCP server: {eg}") from eg
200+
# MCP protocol requires session.initialize() before any other request.
201+
# Failures propagate to the handlers below, which unwind the transport
202+
# once and inspect what that unwinding reveals about the cause.
203+
await asyncio.wait_for(
204+
self._session.initialize(),
205+
timeout=self.connect_timeout,
206+
)
221207

222208
self._initialized = True
223209
self._was_connected = True
@@ -253,7 +239,12 @@ async def connect(self) -> Self:
253239
)
254240
raise ImportError(error_msg) from e
255241
except asyncio.TimeoutError as e:
256-
await self._cleanup_on_error()
242+
cleanup_error = await self._cleanup_on_error()
243+
status_code = find_http_status(e, cleanup_error)
244+
if status_code is not None:
245+
raise self._report_connection_failure(
246+
server_info, started_at, status_code=status_code
247+
) from e
257248
error_msg = f"MCP connection timed out after {self.connect_timeout} seconds. The server may be slow or unreachable."
258249
self._emit_connection_failed(
259250
server_name,
@@ -263,10 +254,21 @@ async def connect(self) -> Self:
263254
"timeout",
264255
started_at,
265256
)
266-
raise ConnectionError(error_msg) from e
267-
except asyncio.CancelledError:
268-
# Re-raise cancellation - don't suppress it
269-
await self._cleanup_on_error()
257+
raise MCPConnectionError(error_msg) from e
258+
except asyncio.CancelledError as e:
259+
# A failing transport cancels this coroutine, so cancellation alone
260+
# says nothing. Unwinding the transport is what reveals the cause.
261+
cleanup_error = await self._cleanup_on_error()
262+
status_code = find_http_status(e, cleanup_error)
263+
if status_code is not None:
264+
raise self._report_connection_failure(
265+
server_info, started_at, status_code=status_code
266+
) from e
267+
if (failure := find_transport_failure(cleanup_error)) is not None:
268+
raise self._report_connection_failure(
269+
server_info, started_at, error=failure
270+
) from failure
271+
# Nothing failed, so this is a real cancellation: never swallow it.
270272
self._emit_connection_failed(
271273
server_name,
272274
server_url,
@@ -276,54 +278,59 @@ async def connect(self) -> Self:
276278
started_at,
277279
)
278280
raise
279-
except BaseExceptionGroup as eg:
280-
# Handle exception groups from anyio task groups at outer level
281-
actual_error = None
282-
for exc in eg.exceptions:
283-
if isinstance(exc, Exception) and not isinstance(exc, GeneratorExit):
284-
error_msg = str(exc).lower()
285-
if "401" in error_msg or "unauthorized" in error_msg:
286-
actual_error = exc
287-
break
288-
if "cancel scope" not in error_msg and "task" not in error_msg:
289-
actual_error = exc
290-
break
291-
292-
await self._cleanup_on_error()
293-
error_type = (
294-
"authentication"
295-
if actual_error
296-
and (
297-
"401" in str(actual_error).lower()
298-
or "unauthorized" in str(actual_error).lower()
299-
)
300-
else "network"
301-
)
302-
error_msg = str(actual_error) if actual_error else str(eg)
303-
self._emit_connection_failed(
304-
server_name,
305-
server_url,
306-
transport_type,
307-
error_msg,
308-
error_type,
309-
started_at,
310-
)
311-
if actual_error:
312-
raise ConnectionError(
313-
f"Failed to connect to MCP server: {actual_error}"
314-
) from actual_error
315-
raise ConnectionError(f"Failed to connect to MCP server: {eg}") from eg
316-
except Exception as e:
317-
await self._cleanup_on_error()
281+
except (BaseExceptionGroup, Exception) as e:
282+
cleanup_error = await self._cleanup_on_error()
283+
status_code = find_http_status(e, cleanup_error)
284+
if status_code is not None:
285+
raise self._report_connection_failure(
286+
server_info, started_at, status_code=status_code
287+
) from e
288+
failure = find_transport_failure(e, cleanup_error) or e
289+
raise self._report_connection_failure(
290+
server_info, started_at, error=failure
291+
) from e
292+
293+
def _report_connection_failure(
294+
self,
295+
server_info: tuple[str, str | None, str | None],
296+
started_at: datetime,
297+
*,
298+
error: BaseException | None = None,
299+
status_code: int | None = None,
300+
) -> MCPConnectionError:
301+
"""Build a connection failure, emit the event, and return it to raise."""
302+
if status_code is not None:
303+
failure = error_for_status(status_code)
304+
error_msg = str(failure)
305+
error_type = error_type_for_status(status_code) or "network"
306+
elif error is not None and isinstance(error, MCPConnectionError):
307+
failure = error
308+
error_msg = str(error)
318309
error_type = (
319-
"authentication"
320-
if "401" in str(e).lower() or "unauthorized" in str(e).lower()
310+
error_type_for_status(error.status_code) or "network"
311+
if error.status_code is not None
321312
else "network"
322313
)
323-
self._emit_connection_failed(
324-
server_name, server_url, transport_type, str(e), error_type, started_at
325-
)
326-
raise ConnectionError(f"Failed to connect to MCP server: {e}") from e
314+
status_code = error.status_code
315+
elif error is not None:
316+
error_msg = f"Failed to connect to MCP server: {error}"
317+
error_type = "network"
318+
status_code = find_http_status(error)
319+
failure = MCPConnectionError(error_msg, status_code=status_code)
320+
else:
321+
raise ValueError("Either error or status_code must be provided")
322+
323+
server_name, server_url, transport_type = server_info
324+
self._emit_connection_failed(
325+
server_name,
326+
server_url,
327+
transport_type,
328+
error_msg,
329+
error_type,
330+
started_at,
331+
status_code=status_code,
332+
)
333+
return failure
327334

328335
def _emit_connection_failed(
329336
self,
@@ -333,6 +340,7 @@ def _emit_connection_failed(
333340
error: str,
334341
error_type: str,
335342
started_at: datetime,
343+
status_code: int | None = None,
336344
) -> None:
337345
"""Emit connection failed event."""
338346
failed_at = datetime.now()
@@ -344,19 +352,31 @@ def _emit_connection_failed(
344352
transport_type=transport_type,
345353
error=error,
346354
error_type=error_type,
355+
status_code=status_code,
347356
started_at=started_at,
348357
failed_at=failed_at,
349358
),
350359
)
351360

352-
async def _cleanup_on_error(self) -> None:
353-
"""Cleanup resources when an error occurs during connection."""
361+
async def _cleanup_on_error(self) -> BaseException | None:
362+
"""Cleanup resources when an error occurs during connection.
363+
364+
Returns:
365+
The exception raised while unwinding the transport, if any. The
366+
transport reports the server's refusal here rather than to the
367+
coroutine that was waiting, so the caller inspects this for an
368+
HTTP status instead of treating it as a cleanup problem.
369+
"""
354370
try:
355371
await self._exit_stack.aclose()
356-
357-
except Exception as e:
358-
# Best effort cleanup - ignore all other errors
359-
raise RuntimeError(f"Error during MCP client cleanup: {e}") from e
372+
except asyncio.CancelledError as e:
373+
return e
374+
except (Exception, BaseExceptionGroup) as e:
375+
# Groups are caught explicitly because one holding only BaseExceptions
376+
# is not an Exception, yet can still carry the server's refusal.
377+
return e
378+
else:
379+
return None
360380
finally:
361381
self._session = None
362382
self._initialized = False
@@ -369,7 +389,11 @@ async def disconnect(self) -> None:
369389

370390
try:
371391
await self._exit_stack.aclose()
372-
except Exception as e:
392+
except asyncio.CancelledError:
393+
raise
394+
except MCPConnectionError:
395+
raise
396+
except (Exception, BaseExceptionGroup) as e:
373397
raise RuntimeError(f"Error during MCP client disconnect: {e}") from e
374398
finally:
375399
self._session = None
@@ -520,12 +544,6 @@ async def call_tool_result(
520544
return tool_result
521545
except Exception as e:
522546
failed_at = datetime.now()
523-
error_type = (
524-
"timeout"
525-
if isinstance(e, (asyncio.TimeoutError, ConnectionError))
526-
and "timeout" in str(e).lower()
527-
else "server_error"
528-
)
529547
crewai_event_bus.emit(
530548
self,
531549
MCPToolExecutionFailedEvent(
@@ -535,7 +553,7 @@ async def call_tool_result(
535553
tool_name=tool_name,
536554
tool_args=cleaned_arguments,
537555
error=str(e),
538-
error_type=error_type,
556+
error_type=tool_execution_error_type(e),
539557
started_at=started_at,
540558
failed_at=failed_at,
541559
),
@@ -717,9 +735,17 @@ async def _retry_operation(
717735
raise ConnectionError(last_error) from e
718736

719737
except Exception as e:
738+
if isinstance(e, MCPConnectionError):
739+
raise
740+
741+
status_code = find_http_status(e)
742+
if status_code is not None and (
743+
error_type_for_status(status_code) == "authentication"
744+
):
745+
raise error_for_status(status_code, detail=str(e)) from e
746+
720747
error_str = str(e).lower()
721748

722-
# Classify errors as retryable or non-retryable
723749
if "authentication" in error_str or "unauthorized" in error_str:
724750
raise ConnectionError(f"Authentication failed: {e}") from e
725751

0 commit comments

Comments
 (0)