Don't cache error results in ResponseCachingMiddleware (#4705)

fastmcp_slim/fastmcp/server/middleware/caching.py

@@ -483,6 +483,14 @@ class ResponseCachingMiddleware(Middleware):
         if not isinstance(tool_result, ToolResult):
             return tool_result
 
+        # Never cache an error result. A tool that reports failure by returning
+        # is_error=True is describing this attempt, not a stable answer — the
+        # upstream 503 or bad gateway it is reporting is exactly the kind of
+        # thing that clears on retry. Caching it would pin the failure in place
+        # for the full TTL and stop the tool from ever being retried.
+        if tool_result.is_error:
+            return tool_result
+
         cacheable_tool_result: CacheableToolResult = CacheableToolResult.wrap(
             value=tool_result
         )

tests/server/middleware/test_caching.py

@@ -658,6 +658,51 @@ class TestCacheableToolResult:
         assert cached_tool_result.is_error is True
 
 
+class TestErrorResultsAreNotCached:
+    """Regression tests for issue #4395: an error result was cached for the full
+    TTL, so a transient failure permanently shadowed the tool until it expired."""
+
+    async def test_error_result_is_not_cached(self):
+        mcp = FastMCP("ErrorCachingTestServer")
+        mcp.add_middleware(ResponseCachingMiddleware(cache_storage=MemoryStore()))
+
+        call_count = 0
+
+        @mcp.tool
+        def flakey() -> ToolResult:
+            nonlocal call_count
+            call_count += 1
+            if call_count == 1:
+                return ToolResult("upstream 503", is_error=True)
+            return ToolResult("recovered")
+
+        async with Client(mcp) as client:
+            first = await client.call_tool("flakey", {}, raise_on_error=False)
+            assert first.is_error is True
+
+            # The tool must actually run again rather than replay the error.
+            second = await client.call_tool("flakey", {}, raise_on_error=False)
+            assert second.is_error is False
+            assert call_count == 2
+
+    async def test_successful_result_is_still_cached(self):
+        mcp = FastMCP("SuccessCachingTestServer")
+        mcp.add_middleware(ResponseCachingMiddleware(cache_storage=MemoryStore()))
+
+        call_count = 0
+
+        @mcp.tool
+        def stable() -> str:
+            nonlocal call_count
+            call_count += 1
+            return "ok"
+
+        async with Client(mcp) as client:
+            await client.call_tool("stable", {})
+            await client.call_tool("stable", {})
+            assert call_count == 1
+
+
 class TestCachingWithImportedServerPrefixes:
     """Test that caching preserves prefixes from imported servers.