fix(server): return 404/400 instead of 502 for not-found and invalid input (#5634)
This commit is contained in:
+4
-2
@@ -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'")
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user