Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion src/strands/tools/mcp/mcp_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import asyncio
import base64
import contextvars
import json
import logging
import threading
import uuid
Expand All @@ -24,8 +25,10 @@
import anyio
from mcp import ClientSession, ListToolsResult
from mcp.client.session import ElicitationFnT
from mcp.shared.exceptions import McpError
from mcp.types import (
BlobResourceContents,
ElicitationRequiredErrorData,
GetPromptResult,
ListPromptsResult,
ListResourcesResult,
Expand Down Expand Up @@ -668,7 +671,31 @@ async def call_tool_async(
return self._handle_tool_execution_error(tool_use_id, e)

def _handle_tool_execution_error(self, tool_use_id: str, exception: Exception) -> MCPToolResult:
"""Create error ToolResult with consistent logging."""
"""Create error ToolResult with consistent logging and elicitation callback support.

Args:
tool_use_id: Unique identifier for this tool use.
exception: The exception that occurred during tool execution.

Returns:
MCPToolResult: Error result containing either the elicitation data or the
original exception message.
"""
if isinstance(exception, McpError) and exception.error.code == -32042:
try:
error_data = ElicitationRequiredErrorData.model_validate(exception.error.data)
elicitations = [e.model_dump(exclude_none=True) for e in error_data.elicitations]

return MCPToolResult(
status="error",
toolUseId=tool_use_id,
Comment thread
Christian-kam marked this conversation as resolved.
content=[
{"text": (f"MCP Elicitation required: [{str(exception)}] with data {json.dumps(elicitations)}")}
],
)
except Exception:
logger.debug("Failed to parse ElicitationRequiredErrorData from -32042 error", exc_info=True)

return MCPToolResult(
status="error",
toolUseId=tool_use_id,
Expand Down
149 changes: 149 additions & 0 deletions tests/strands/tools/mcp/test_mcp_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -928,3 +928,152 @@ async def test_handle_error_message_with_percent_in_message():

# This should not raise TypeError and should not raise the exception (since it's non-fatal)
await client._handle_error_message(error_with_percent)


def test_call_tool_sync_elicitation_error(mock_transport, mock_session):
"""Test that call_tool_sync correctly handles elicitation required errors."""
from mcp.shared.exceptions import McpError
from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams

elicitation_data = ElicitationRequiredErrorData(
elicitations=[
ElicitRequestURLParams(
url="https://example.com/auth", message="Please authorize the application", elicitationId="elicit-123"
)
]
)

error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump()))
mock_session.call_tool.side_effect = error

with MCPClient(mock_transport["transport_callable"]) as client:
result = client.call_tool_sync(tool_use_id="test-123", name="test_tool", arguments={"param": "value"})

assert result["status"] == "error"
assert result["toolUseId"] == "test-123"
assert len(result["content"]) == 1
assert "MCP Elicitation required" in result["content"][0]["text"]
assert "https://example.com/auth" in result["content"][0]["text"]
assert "Please authorize the application" in result["content"][0]["text"]
assert "elicit-123" in result["content"][0]["text"]


def test_call_tool_sync_elicitation_error_multiple_urls(mock_transport, mock_session):
"""Test that call_tool_sync correctly handles elicitation errors with multiple elicitations."""
from mcp.shared.exceptions import McpError
from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams

elicitation_data = ElicitationRequiredErrorData(
elicitations=[
ElicitRequestURLParams(
url="https://example.com/auth1", message="First authorization", elicitationId="elicit-1"
),
ElicitRequestURLParams(
url="https://example.com/auth2", message="Second authorization", elicitationId="elicit-2"
),
]
)

error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump()))
mock_session.call_tool.side_effect = error

with MCPClient(mock_transport["transport_callable"]) as client:
result = client.call_tool_sync(tool_use_id="test-123", name="test_tool", arguments={"param": "value"})

assert result["status"] == "error"
assert result["toolUseId"] == "test-123"
assert len(result["content"]) == 1
assert "MCP Elicitation required" in result["content"][0]["text"]
assert "https://example.com/auth1" in result["content"][0]["text"]
assert "https://example.com/auth2" in result["content"][0]["text"]
assert "First authorization" in result["content"][0]["text"]
assert "Second authorization" in result["content"][0]["text"]
assert "elicit-1" in result["content"][0]["text"]
assert "elicit-2" in result["content"][0]["text"]


def test_call_tool_sync_elicitation_error_no_urls(mock_transport, mock_session):
"""Test that -32042 error with empty URL still returns generic elicitation result."""
from mcp.shared.exceptions import McpError
from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams

elicitation_data = ElicitationRequiredErrorData(
elicitations=[ElicitRequestURLParams(url="", message="No URL provided", elicitationId="elicit-1")]
)
error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump()))
mock_session.call_tool.side_effect = error

with MCPClient(mock_transport["transport_callable"]) as client:
result = client.call_tool_sync(tool_use_id="test-123", name="test_tool", arguments={})
assert result["status"] == "error"
assert "MCP Elicitation required" in result["content"][0]["text"]
assert "elicit-1" in result["content"][0]["text"]
assert "No URL provided" in result["content"][0]["text"]


def test_call_tool_sync_other_mcp_error_code(mock_transport, mock_session):
"""Test that non-32042 McpError falls through to generic error."""
from mcp.shared.exceptions import McpError

error = McpError(error=MagicMock(code=-32600, message="Invalid request"))
mock_session.call_tool.side_effect = error

with MCPClient(mock_transport["transport_callable"]) as client:
result = client.call_tool_sync(tool_use_id="test-123", name="test_tool", arguments={})
assert result["status"] == "error"
assert "Tool execution failed" in result["content"][0]["text"]


def test_call_tool_sync_elicitation_error_malformed_data(mock_transport, mock_session):
"""Test that -32042 with unparseable data falls through to generic error."""
from mcp.shared.exceptions import McpError

error = McpError(error=MagicMock(code=-32042, data={"garbage": True}))
mock_session.call_tool.side_effect = error

with MCPClient(mock_transport["transport_callable"]) as client:
result = client.call_tool_sync(tool_use_id="test-123", name="test_tool", arguments={})
assert result["status"] == "error"
assert "Tool execution failed" in result["content"][0]["text"]


@pytest.mark.asyncio
async def test_call_tool_async_elicitation_error(mock_transport, mock_session):
"""Test that call_tool_async correctly handles elicitation required errors."""
from mcp.shared.exceptions import McpError
from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams

elicitation_data = ElicitationRequiredErrorData(
elicitations=[
ElicitRequestURLParams(
url="https://example.com/auth", message="Please authorize the application", elicitationId="elicit-123"
)
]
)

error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump()))

with MCPClient(mock_transport["transport_callable"]) as client:
with (
patch("asyncio.run_coroutine_threadsafe") as mock_run_coroutine_threadsafe,
patch("asyncio.wrap_future") as mock_wrap_future,
):
mock_future = MagicMock()
mock_run_coroutine_threadsafe.return_value = mock_future

async def mock_awaitable():
raise error

mock_wrap_future.return_value = mock_awaitable()

result = await client.call_tool_async(
tool_use_id="test-123", name="test_tool", arguments={"param": "value"}
)

assert result["status"] == "error"
assert result["toolUseId"] == "test-123"
assert len(result["content"]) == 1
assert "MCP Elicitation required" in result["content"][0]["text"]
assert "https://example.com/auth" in result["content"][0]["text"]
assert "Please authorize the application" in result["content"][0]["text"]
assert "elicit-123" in result["content"][0]["text"]
Loading