diff --git a/mem0/memory/main.py b/mem0/memory/main.py index 30b4fa7bc..45511a973 100644 --- a/mem0/memory/main.py +++ b/mem0/memory/main.py @@ -1842,8 +1842,9 @@ class Memory(MemoryBase): try: existing_memory = self.vector_store.get(vector_id=memory_id) except Exception: + # Backing-store failure, not a bad memory_id: re-raise the original so the REST layer maps it to 5xx, not 4xx. logger.error(f"Error getting memory with ID {memory_id} during update.") - raise ValueError(f"Error getting memory with ID {memory_id}. Please provide a valid 'memory_id'") + raise if existing_memory is None: raise ValueError(f"Memory with id {memory_id} not found. Please provide a valid 'memory_id'") @@ -3406,8 +3407,9 @@ class AsyncMemory(MemoryBase): try: existing_memory = await asyncio.to_thread(self.vector_store.get, vector_id=memory_id) except Exception: + # Backing-store failure, not a bad memory_id: re-raise the original so the REST layer maps it to 5xx, not 4xx. logger.error(f"Error getting memory with ID {memory_id} during update.") - raise ValueError(f"Error getting memory with ID {memory_id}. Please provide a valid 'memory_id'") + raise if existing_memory is None: raise ValueError(f"Memory with id {memory_id} not found. Please provide a valid 'memory_id'") diff --git a/server/main.py b/server/main.py index 7476698a1..d10a9d67c 100644 --- a/server/main.py +++ b/server/main.py @@ -19,6 +19,7 @@ from errors import ( from fastapi import Depends, FastAPI, HTTPException, Request from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse, RedirectResponse +from mem0.exceptions import ValidationError as Mem0ValidationError from models import RequestLog, User from pydantic import BaseModel, Field from rate_limit import limiter @@ -206,6 +207,14 @@ class GenerateInstructionsRequest(BaseModel): use_case: str = Field(..., description="Description of what the user will use Mem0 for.") +def _client_error(exc: Exception) -> HTTPException: + """Map core validation / not-found errors to 4xx so clients can tell a bad + request from an upstream outage. 'not found' is a 404, everything else a 400.""" + detail = str(exc) + status_code = 404 if isinstance(exc, ValueError) and "not found" in detail.lower() else 400 + return HTTPException(status_code=status_code, detail=detail) + + def _redact_config(value: Any, key: str | None = None) -> Any: if isinstance(value, dict): return {item_key: _redact_config(item_value, item_key) for item_key, item_value in value.items()} @@ -363,6 +372,8 @@ def add_memory(memory_create: MemoryCreate, _auth=Depends(verify_auth)): if response.get("results"): telemetry.log_dashboard_nudge_once(DASHBOARD_URL) return JSONResponse(content=response) + except (ValueError, Mem0ValidationError) as e: + raise _client_error(e) except Exception: raise upstream_error() @@ -466,6 +477,8 @@ def update_memory(memory_id: str, updated_memory: MemoryUpdate, _auth=Depends(ve return get_memory_instance().update( memory_id=memory_id, data=updated_memory.text, metadata=updated_memory.metadata ) + except (ValueError, Mem0ValidationError) as e: + raise _client_error(e) except Exception: raise upstream_error() @@ -485,6 +498,8 @@ def delete_memory(memory_id: str, _auth=Depends(verify_auth)): try: get_memory_instance().delete(memory_id=memory_id) return MessageResponse(message="Memory deleted successfully") + except (ValueError, Mem0ValidationError) as e: + raise _client_error(e) except Exception: raise upstream_error() diff --git a/tests/test_memory.py b/tests/test_memory.py index 71a86d438..da00953ef 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -536,6 +536,59 @@ async def test_async_update_nonexistent_memory_raises_error(mock_sqlite, mock_ll mock_vector_store.update.assert_not_called() +@patch('mem0.utils.factory.EmbedderFactory.create') +@patch('mem0.utils.factory.VectorStoreFactory.create') +@patch('mem0.utils.factory.LlmFactory.create') +@patch('mem0.memory.storage.SQLiteManager') +def test_update_propagates_vector_store_failure(mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory): + """A backing-store failure while fetching the memory during update must + surface as the original error, not be masked as a 'provide a valid + memory_id' ValueError. The REST layer relies on this so an outage maps to + 5xx instead of a misleading 4xx.""" + mock_embedder_factory.return_value = MagicMock() + mock_vector_store = MagicMock() + mock_vector_factory.return_value = mock_vector_store + mock_llm_factory.return_value = MagicMock() + mock_sqlite.return_value = MagicMock() + + from mem0.memory.main import Memory as MemoryClass + config = MemoryConfig() + memory = MemoryClass(config) + + mock_vector_store.get.side_effect = ConnectionError("vector store unreachable") + + with pytest.raises(ConnectionError, match="vector store unreachable"): + memory._update_memory("mem-1", "new data", {"new data": [0.1, 0.2]}) + + mock_vector_store.update.assert_not_called() + + +@pytest.mark.asyncio +@patch('mem0.utils.factory.EmbedderFactory.create') +@patch('mem0.utils.factory.VectorStoreFactory.create') +@patch('mem0.utils.factory.LlmFactory.create') +@patch('mem0.memory.storage.SQLiteManager') +async def test_async_update_propagates_vector_store_failure(mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory): + """Async twin: a backing-store failure during update re-raises the original + error instead of masking it as a ValueError.""" + mock_embedder_factory.return_value = MagicMock() + mock_vector_store = MagicMock() + mock_vector_factory.return_value = mock_vector_store + mock_llm_factory.return_value = MagicMock() + mock_sqlite.return_value = MagicMock() + + from mem0.memory.main import AsyncMemory + config = MemoryConfig() + memory = AsyncMemory(config) + + mock_vector_store.get.side_effect = ConnectionError("vector store unreachable") + + with pytest.raises(ConnectionError, match="vector store unreachable"): + await memory._update_memory("mem-1", "new data", {"new data": [0.1, 0.2]}) + + mock_vector_store.update.assert_not_called() + + @patch('mem0.utils.factory.EmbedderFactory.create') @patch('mem0.utils.factory.VectorStoreFactory.create') @patch('mem0.utils.factory.LlmFactory.create') diff --git a/tests/test_server_params.py b/tests/test_server_params.py index 6ee58de69..f5700ad4c 100644 --- a/tests/test_server_params.py +++ b/tests/test_server_params.py @@ -12,6 +12,8 @@ from unittest.mock import MagicMock, patch import pytest +from mem0.exceptions import ValidationError as Mem0ValidationError + pytest.importorskip("fastapi", reason="fastapi not installed") from fastapi.testclient import TestClient @@ -701,3 +703,44 @@ class TestSearchValidationErrors: ) resp = client.post("/search", json={"query": "food"}) assert resp.status_code == 400 + + +# =========================================================================== +# add / update / delete: map core errors to 4xx instead of 502 +# =========================================================================== + +class TestWriteHandlerErrorMapping: + """ValueError("... not found") -> 404, other ValueError / Mem0ValidationError + -> 400. A real outage still surfaces as 502 via upstream_error().""" + + def test_update_not_found_returns_404(self, client, mock_memory): + mock_memory.update.side_effect = ValueError("Memory with id mem-1 not found") + resp = client.put("/memories/mem-1", json={"text": "new"}) + assert resp.status_code == 404 + assert "not found" in resp.json()["detail"] + + def test_delete_not_found_returns_404(self, client, mock_memory): + mock_memory.delete.side_effect = ValueError("Memory with id mem-1 not found") + resp = client.delete("/memories/mem-1") + assert resp.status_code == 404 + + def test_update_other_value_error_returns_400(self, client, mock_memory): + mock_memory.update.side_effect = ValueError("data must be a non-empty string") + resp = client.put("/memories/mem-1", json={"text": "new"}) + assert resp.status_code == 400 + + def test_add_validation_error_returns_400(self, client, mock_memory): + mock_memory.add.side_effect = Mem0ValidationError( + message="messages must be str, dict, or list[dict]", error_code="VALIDATION_003" + ) + resp = client.post("/memories", json={ + "messages": [{"role": "user", "content": "hi"}], "user_id": "u1", + }) + assert resp.status_code == 400 + + def test_add_real_outage_still_returns_502(self, client, mock_memory): + mock_memory.add.side_effect = RuntimeError("vector store unreachable") + resp = client.post("/memories", json={ + "messages": [{"role": "user", "content": "hi"}], "user_id": "u1", + }) + assert resp.status_code == 502