mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-09-21 10:26:25 +00:00
7c23857aed
* 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.
215 lines
7.1 KiB
Python
215 lines
7.1 KiB
Python
"""Tests for MCPClient connect authentication error handling."""
|
|
|
|
import asyncio
|
|
from typing import Any
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import httpx
|
|
import pytest
|
|
|
|
from crewai.events.event_bus import crewai_event_bus
|
|
from crewai.events.types.mcp_events import MCPConnectionFailedEvent
|
|
from crewai.mcp.client import MCPClient
|
|
from crewai.mcp.exceptions import MCPAuthenticationError
|
|
from crewai.mcp.transports.base import BaseTransport, TransportType
|
|
from crewai.mcp.transports.http import HTTPTransport
|
|
|
|
|
|
class MockTransport(BaseTransport):
|
|
"""Minimal transport stub for connect() error-path tests."""
|
|
|
|
@property
|
|
def transport_type(self) -> TransportType:
|
|
return TransportType.STREAMABLE_HTTP
|
|
|
|
async def connect(self) -> "MockTransport":
|
|
self._read_stream = MagicMock()
|
|
self._write_stream = MagicMock()
|
|
self._connected = True
|
|
return self
|
|
|
|
async def disconnect(self) -> None:
|
|
self._connected = False
|
|
|
|
async def __aenter__(self) -> "MockTransport":
|
|
return await self.connect()
|
|
|
|
async def __aexit__(
|
|
self,
|
|
exc_type: type[BaseException] | None,
|
|
exc_val: BaseException | None,
|
|
exc_tb: Any,
|
|
) -> None:
|
|
await self.disconnect()
|
|
|
|
|
|
def _http_status_error(status_code: int) -> httpx.HTTPStatusError:
|
|
request = httpx.Request("POST", "https://mcp.example.com/mcp")
|
|
response = httpx.Response(status_code, text="refused", request=request)
|
|
return httpx.HTTPStatusError(
|
|
f"HTTP {status_code}",
|
|
request=request,
|
|
response=response,
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_connect_cancelled_with_auth_status_in_cleanup():
|
|
transport = MockTransport()
|
|
client = MCPClient(transport)
|
|
auth_error = _http_status_error(401)
|
|
failed_events: list[MCPConnectionFailedEvent] = []
|
|
|
|
mock_session = MagicMock()
|
|
mock_session.initialize = AsyncMock(side_effect=asyncio.CancelledError())
|
|
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
|
mock_session.__aexit__ = AsyncMock(return_value=None)
|
|
|
|
with (
|
|
patch("mcp.ClientSession", return_value=mock_session),
|
|
patch.object(
|
|
client,
|
|
"_cleanup_on_error",
|
|
AsyncMock(return_value=auth_error),
|
|
),
|
|
crewai_event_bus.scoped_handlers(),
|
|
):
|
|
@crewai_event_bus.on(MCPConnectionFailedEvent)
|
|
def _capture(_: object, event: MCPConnectionFailedEvent) -> None:
|
|
failed_events.append(event)
|
|
|
|
with pytest.raises(MCPAuthenticationError) as exc_info:
|
|
await client.connect()
|
|
|
|
assert crewai_event_bus.flush(timeout=10)
|
|
|
|
assert exc_info.value.status_code == 401
|
|
assert len(failed_events) == 1
|
|
assert failed_events[0].error_type == "authentication"
|
|
assert failed_events[0].status_code == 401
|
|
assert "401 Unauthorized" in failed_events[0].error
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_connect_cancelled_without_underlying_failure_emits_cancelled():
|
|
transport = MockTransport()
|
|
client = MCPClient(transport)
|
|
failed_events: list[MCPConnectionFailedEvent] = []
|
|
|
|
mock_session = MagicMock()
|
|
mock_session.initialize = AsyncMock(side_effect=asyncio.CancelledError())
|
|
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
|
mock_session.__aexit__ = AsyncMock(return_value=None)
|
|
|
|
with (
|
|
patch("mcp.ClientSession", return_value=mock_session),
|
|
patch.object(client, "_cleanup_on_error", AsyncMock(return_value=None)),
|
|
crewai_event_bus.scoped_handlers(),
|
|
):
|
|
@crewai_event_bus.on(MCPConnectionFailedEvent)
|
|
def _capture(_: object, event: MCPConnectionFailedEvent) -> None:
|
|
failed_events.append(event)
|
|
|
|
with pytest.raises(asyncio.CancelledError):
|
|
await client.connect()
|
|
|
|
assert crewai_event_bus.flush(timeout=10)
|
|
|
|
assert len(failed_events) == 1
|
|
assert failed_events[0].error_type == "cancelled"
|
|
assert failed_events[0].error == "Connection cancelled"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_connect_raises_authentication_error_for_typed_transport_failure():
|
|
transport = MockTransport()
|
|
client = MCPClient(transport)
|
|
failed_events: list[MCPConnectionFailedEvent] = []
|
|
|
|
mock_session = MagicMock()
|
|
mock_session.initialize = AsyncMock(
|
|
side_effect=MCPAuthenticationError(401)
|
|
)
|
|
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
|
mock_session.__aexit__ = AsyncMock(return_value=None)
|
|
|
|
with (
|
|
patch("mcp.ClientSession", return_value=mock_session),
|
|
patch.object(
|
|
client,
|
|
"_cleanup_on_error",
|
|
AsyncMock(return_value=None),
|
|
),
|
|
crewai_event_bus.scoped_handlers(),
|
|
):
|
|
@crewai_event_bus.on(MCPConnectionFailedEvent)
|
|
def _capture(_: object, event: MCPConnectionFailedEvent) -> None:
|
|
failed_events.append(event)
|
|
|
|
with pytest.raises(MCPAuthenticationError):
|
|
await client.connect()
|
|
|
|
assert crewai_event_bus.flush(timeout=10)
|
|
|
|
assert len(failed_events) == 1
|
|
assert failed_events[0].error_type == "authentication"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_connect_cancelled_during_initialize_recovers_auth_on_transport_unwind():
|
|
transport = HTTPTransport(url="https://mcp.example.com/mcp")
|
|
client = MCPClient(transport)
|
|
auth_error = _http_status_error(401)
|
|
failed_events: list[MCPConnectionFailedEvent] = []
|
|
|
|
mock_streams = (MagicMock(), MagicMock(), None)
|
|
mock_context = MagicMock()
|
|
mock_context.__aenter__ = AsyncMock(return_value=mock_streams)
|
|
mock_context.__aexit__ = AsyncMock(side_effect=auth_error)
|
|
|
|
mock_session = MagicMock()
|
|
mock_session.initialize = AsyncMock(side_effect=asyncio.CancelledError())
|
|
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
|
mock_session.__aexit__ = AsyncMock(return_value=None)
|
|
|
|
with (
|
|
patch(
|
|
"mcp.client.streamable_http.streamablehttp_client",
|
|
return_value=mock_context,
|
|
),
|
|
patch("mcp.ClientSession", return_value=mock_session),
|
|
crewai_event_bus.scoped_handlers(),
|
|
):
|
|
@crewai_event_bus.on(MCPConnectionFailedEvent)
|
|
def _capture(_: object, event: MCPConnectionFailedEvent) -> None:
|
|
failed_events.append(event)
|
|
|
|
with pytest.raises(MCPAuthenticationError) as exc_info:
|
|
await client.connect()
|
|
|
|
assert crewai_event_bus.flush(timeout=10)
|
|
|
|
mock_context.__aexit__.assert_awaited()
|
|
assert exc_info.value.status_code == 401
|
|
assert len(failed_events) == 1
|
|
assert failed_events[0].error_type == "authentication"
|
|
assert failed_events[0].status_code == 401
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_disconnect_preserves_authentication_error_from_teardown():
|
|
transport = MockTransport()
|
|
client = MCPClient(transport)
|
|
auth_error = MCPAuthenticationError(401)
|
|
transport._connected = True
|
|
client._initialized = True
|
|
|
|
with patch.object(
|
|
client._exit_stack, "aclose", AsyncMock(side_effect=auth_error)
|
|
):
|
|
with pytest.raises(MCPAuthenticationError) as exc_info:
|
|
await client.disconnect()
|
|
|
|
assert exc_info.value is auth_error
|
|
assert not client.connected
|