Merge pull request #72 from redis/fix/fix-client-error-handling

Fix client error handling

Author: abrookins

agent-memory-client/agent_memory_client/__init__.py

@@ -5,7 +5,7 @@
 memory management capabilities for AI agents and applications.
 """
 
-__version__ = "0.12.2"
+__version__ = "0.12.3"
 
 from .client import MemoryAPIClient, MemoryClientConfig, create_memory_client
 from .exceptions import (

agent-memory-client/agent_memory_client/client.py

@@ -8,7 +8,7 @@
 import logging  # noqa: F401
 import re
 from collections.abc import AsyncIterator, Sequence
-from typing import TYPE_CHECKING, Any, Literal, TypedDict
+from typing import TYPE_CHECKING, Any, Literal, NoReturn, TypedDict
 
 if TYPE_CHECKING:
     from typing_extensions import Self
@@ -149,8 +149,11 @@ async def __aexit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
         """Close the client when exiting the context manager."""
         await self.close()
 
-    def _handle_http_error(self, response: httpx.Response) -> None:
-        """Handle HTTP errors and convert to appropriate exceptions."""
+    def _handle_http_error(self, response: httpx.Response) -> NoReturn:
+        """Handle HTTP errors and convert to appropriate exceptions.
+
+        This method always raises an exception and never returns normally.
+        """
         if response.status_code == 404:
             from .exceptions import MemoryNotFoundError
 
@@ -162,6 +165,10 @@ def _handle_http_error(self, response: httpx.Response) -> None:
             except Exception:
                 message = f"HTTP {response.status_code}: {response.text}"
             raise MemoryServerError(message, response.status_code)
+        # This should never be reached, but mypy needs to know this never returns
+        raise MemoryServerError(
+            f"Unexpected status code: {response.status_code}", response.status_code
+        )
 
     async def health_check(self) -> HealthCheckResponse:
         """
@@ -176,7 +183,6 @@ async def health_check(self) -> HealthCheckResponse:
             return HealthCheckResponse(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def list_sessions(
         self,
@@ -215,7 +221,6 @@ async def list_sessions(
             return SessionListResponse(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def get_working_memory(
         self,
@@ -291,7 +296,6 @@ async def get_working_memory(
             return WorkingMemoryResponse(**response_data)
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def get_or_create_working_memory(
         self,
@@ -454,7 +458,6 @@ async def put_working_memory(
             return WorkingMemoryResponse(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def delete_working_memory(
         self, session_id: str, namespace: str | None = None, user_id: str | None = None
@@ -487,7 +490,6 @@ async def delete_working_memory(
             return AckResponse(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def set_working_memory_data(
         self,
@@ -678,7 +680,6 @@ async def create_long_term_memory(
             return AckResponse(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def delete_long_term_memories(self, memory_ids: Sequence[str]) -> AckResponse:
         """
@@ -701,7 +702,6 @@ async def delete_long_term_memories(self, memory_ids: Sequence[str]) -> AckRespo
             return AckResponse(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def get_long_term_memory(self, memory_id: str) -> MemoryRecord:
         """
@@ -722,7 +722,6 @@ async def get_long_term_memory(self, memory_id: str) -> MemoryRecord:
             return MemoryRecord(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def edit_long_term_memory(
         self, memory_id: str, updates: dict[str, Any]
@@ -749,7 +748,6 @@ async def edit_long_term_memory(
             return MemoryRecord(**response.json())
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def search_long_term_memory(
         self,
@@ -900,7 +898,6 @@ async def search_long_term_memory(
             return MemoryRecordResults(**data)
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     # === LLM Tool Integration ===
 
@@ -2957,7 +2954,6 @@ async def memory_prompt(
             return {"response": result}
         except httpx.HTTPStatusError as e:
             self._handle_http_error(e.response)
-            raise
 
     async def hydrate_memory_prompt(
         self,

agent-memory-client/tests/test_client.py

@@ -702,6 +702,60 @@ def test_validation_with_none_values(self, enhanced_test_client):
         # Should not raise
         enhanced_test_client.validate_memory_record(memory)
 
+    @pytest.mark.asyncio
+    async def test_get_or_create_handles_404_correctly(self, enhanced_test_client):
+        """Test that get_or_create_working_memory properly handles 404 errors.
+
+        This test verifies the fix for a bug where _handle_http_error would raise
+        MemoryNotFoundError, but then the code would re-raise the original
+        HTTPStatusError, preventing get_or_create_working_memory from catching
+        the MemoryNotFoundError and creating a new session.
+        """
+
+        session_id = "nonexistent-session"
+
+        # Mock get_working_memory to raise MemoryNotFoundError (simulating 404)
+        async def mock_get_working_memory(*args, **kwargs):
+            # Simulate what happens when the server returns 404
+            response = Mock()
+            response.status_code = 404
+            response.url = f"http://test/v1/working-memory/{session_id}"
+            raise httpx.HTTPStatusError(
+                "404 Not Found", request=Mock(), response=response
+            )
+
+        # Mock put_working_memory to return a created session
+        async def mock_put_working_memory(*args, **kwargs):
+            return WorkingMemoryResponse(
+                session_id=session_id,
+                messages=[],
+                memories=[],
+                data={},
+                context=None,
+                user_id=None,
+            )
+
+        with (
+            patch.object(
+                enhanced_test_client,
+                "get_working_memory",
+                side_effect=mock_get_working_memory,
+            ),
+            patch.object(
+                enhanced_test_client,
+                "put_working_memory",
+                side_effect=mock_put_working_memory,
+            ),
+        ):
+            # This should NOT raise an exception - it should create a new session
+            created, memory = await enhanced_test_client.get_or_create_working_memory(
+                session_id=session_id
+            )
+
+            # Verify that a new session was created
+            assert created is True
+            assert memory.session_id == session_id
+
 
 class TestContextUsagePercentage:
     """Tests for context usage percentage functionality."""