Fix RetryMiddleware not retrying tool errors (#3858)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com>

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."""

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 <original>,
+        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(