diff --git a/src/fastmcp/server/middleware/error_handling.py b/src/fastmcp/server/middleware/error_handling.py index 5b235e804..81a13bb84 100644 --- a/src/fastmcp/server/middleware/error_handling.py +++ b/src/fastmcp/server/middleware/error_handling.py @@ -181,8 +181,18 @@ class RetryMiddleware(Middleware): self.logger = logger or logging.getLogger("fastmcp.retry") def _should_retry(self, error: Exception) -> bool: - """Determine if an error should trigger a retry.""" - return isinstance(error, self.retry_exceptions) + """Determine if an error should trigger a retry. + + Checks both the error itself and its ``__cause__``, since FastMCP + wraps tool exceptions as ``ToolError(...) from original``. Only one + level of cause is inspected — middleware below this one must not + re-wrap errors with a new ``from`` clause, or the real type will be + hidden from the retry decision. + """ + if isinstance(error, self.retry_exceptions): + return True + cause = error.__cause__ + return cause is not None and isinstance(cause, self.retry_exceptions) def _calculate_delay(self, attempt: int) -> float: """Calculate delay for the given attempt number.""" diff --git a/tests/server/middleware/test_error_handling.py b/tests/server/middleware/test_error_handling.py index 3bac578d7..dfc551fbf 100644 --- a/tests/server/middleware/test_error_handling.py +++ b/tests/server/middleware/test_error_handling.py @@ -6,6 +6,8 @@ from unittest.mock import AsyncMock, MagicMock import pytest from mcp import McpError +from fastmcp import FastMCP +from fastmcp.client import Client from fastmcp.exceptions import NotFoundError, ToolError from fastmcp.server.middleware.error_handling import ( ErrorHandlingMiddleware, @@ -295,6 +297,32 @@ class TestRetryMiddleware: assert middleware._should_retry(ValueError()) is False assert middleware._should_retry(RuntimeError()) is False + def test_should_retry_checks_cause_chain(self): + """Retry should match on __cause__ since FastMCP wraps tool errors. + + When a tool raises ConnectionError, FastMCP catches it and raises + ToolError(...) from ConnectionError. The middleware must check + __cause__ to detect the retryable original exception. + """ + middleware = RetryMiddleware(retry_exceptions=(ConnectionError,)) + + # Direct ConnectionError — should retry + assert middleware._should_retry(ConnectionError()) is True + + # ToolError wrapping ConnectionError — should also retry + wrapped = ToolError("Error calling tool") + wrapped.__cause__ = ConnectionError("conn refused") + assert middleware._should_retry(wrapped) is True + + # ToolError wrapping ValueError — should NOT retry + wrong_cause = ToolError("Error calling tool") + wrong_cause.__cause__ = ValueError("bad input") + assert middleware._should_retry(wrong_cause) is False + + # ToolError with no cause — should NOT retry + no_cause = ToolError("Error calling tool") + assert middleware._should_retry(no_cause) is False + def test_calculate_delay(self): """Test delay calculation.""" middleware = RetryMiddleware( @@ -364,8 +392,6 @@ class TestRetryMiddleware: @pytest.fixture def error_handling_server(): """Create a FastMCP server specifically for error handling middleware tests.""" - from fastmcp import FastMCP - mcp = FastMCP("ErrorHandlingTestServer") @mcp.tool @@ -417,8 +443,6 @@ class TestErrorHandlingMiddlewareIntegration: self, error_handling_server, caplog ): """Test that error handling middleware logs real errors from tools.""" - from fastmcp.client import Client - error_handling_server.add_middleware(ErrorHandlingMiddleware()) with caplog.at_level(logging.ERROR): @@ -442,8 +466,6 @@ class TestErrorHandlingMiddlewareIntegration: self, error_handling_server ): """Test that error handling middleware accurately tracks error statistics.""" - from fastmcp.client import Client - error_middleware = ErrorHandlingMiddleware() error_handling_server.add_middleware(error_middleware) @@ -475,8 +497,6 @@ class TestErrorHandlingMiddlewareIntegration: self, error_handling_server, caplog ): """Test error handling middleware with mix of successful and failed operations.""" - from fastmcp.client import Client - error_handling_server.add_middleware(ErrorHandlingMiddleware()) with caplog.at_level(logging.ERROR): @@ -501,8 +521,6 @@ class TestErrorHandlingMiddlewareIntegration: self, error_handling_server ): """Test error handling middleware with custom error callback.""" - from fastmcp.client import Client - captured_errors = [] def error_callback(error, context): @@ -536,8 +554,6 @@ class TestErrorHandlingMiddlewareIntegration: self, error_handling_server ): """Test error transformation functionality.""" - from fastmcp.client import Client - error_handling_server.add_middleware( ErrorHandlingMiddleware(transform_errors=True) ) @@ -554,55 +570,60 @@ class TestErrorHandlingMiddlewareIntegration: class TestRetryMiddlewareIntegration: """Integration tests for retry middleware with real FastMCP server.""" - async def test_retry_middleware_with_transient_failures( - self, error_handling_server, caplog - ): - """Test retry middleware with operations that have transient failures.""" - from fastmcp.client import Client + async def test_retry_actually_retries_through_server_pipeline(self): + """Retry middleware should retry tool calls that raise retryable errors. - # Configure retry middleware to retry connection errors - error_handling_server.add_middleware( + FastMCP wraps tool exceptions as ToolError(...) from , + so the middleware must check __cause__ to detect retryable errors. + This test verifies the full pipeline works by counting call attempts. + """ + call_count = 0 + server = FastMCP("RetryTest") + server.add_middleware( RetryMiddleware( max_retries=3, - base_delay=0.01, # Very short delay for testing + base_delay=0.01, retry_exceptions=(ConnectionError,), ) ) - with caplog.at_level(logging.WARNING): - async with Client(error_handling_server) as client: - # This operation fails intermittently - try several times - success_count = 0 - for _ in range(5): - try: - await client.call_tool( - "intermittent_operation", {"fail_rate": 0.7} - ) - success_count += 1 - except Exception: - pass # Some failures expected even with retries + @server.tool + def fails_then_succeeds() -> str: + nonlocal call_count + call_count += 1 + if call_count < 3: + raise ConnectionError("transient failure") + return "success" - # Should have some retry log messages - # Note: Retry logs might not appear if the underlying errors are wrapped by FastMCP - # The key is that some operations should succeed due to retries + async with Client(server) as client: + result = await client.call_tool("fails_then_succeeds") + assert result.data == "success" - async def test_retry_middleware_with_permanent_failures( - self, error_handling_server - ): - """Test that retry middleware doesn't retry non-retryable errors.""" - from fastmcp.client import Client + # Tool should have been called 3 times: 2 failures + 1 success + assert call_count == 3 - # Configure retry middleware for connection errors only - error_handling_server.add_middleware( + async def test_retry_middleware_with_permanent_failures(self): + """A tool error whose cause is not in ``retry_exceptions`` should + fail on the first attempt — no retries.""" + call_count = 0 + server = FastMCP("RetryPermanentFailuresTest") + server.add_middleware( RetryMiddleware( max_retries=3, base_delay=0.01, retry_exceptions=(ConnectionError,) ) ) - async with Client(error_handling_server) as client: - # Value errors should not be retried + @server.tool + def always_fails() -> str: + nonlocal call_count + call_count += 1 + raise ValueError("permanent failure") + + async with Client(server) as client: with pytest.raises(Exception): - await client.call_tool("failing_operation", {"error_type": "value"}) + await client.call_tool("always_fails", {}) + + assert call_count == 1 # Should fail immediately without retries @@ -610,8 +631,6 @@ class TestRetryMiddlewareIntegration: self, error_handling_server, caplog ): """Test error handling and retry middleware working together.""" - from fastmcp.client import Client - # Add both middleware error_handling_server.add_middleware(ErrorHandlingMiddleware()) error_handling_server.add_middleware(