From 4f28dceac87cc9e4610c01ffdc0499b4acd7c218 Mon Sep 17 00:00:00 2001
From: Sai Mouli <141447420+SaiMouli3@users.noreply.github.com>
Date: Wed, 5 Aug 2026 02:45:01 +0530
Subject: [PATCH] Don't cache error results in ResponseCachingMiddleware
 (#4705)

---
 .../fastmcp/server/middleware/caching.py      |  8 ++++
 tests/server/middleware/test_caching.py       | 45 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/fastmcp_slim/fastmcp/server/middleware/caching.py b/fastmcp_slim/fastmcp/server/middleware/caching.py
index 66918d1a5..a6db7eced 100644
--- a/fastmcp_slim/fastmcp/server/middleware/caching.py
+++ b/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
         )
diff --git a/tests/server/middleware/test_caching.py b/tests/server/middleware/test_caching.py
index c2d229468..fec5424c1 100644
--- a/tests/server/middleware/test_caching.py
+++ b/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.