diff --git a/autobot-backend/chat_history/messages.py b/autobot-backend/chat_history/messages.py index dd0289399..5c2494f70 100644 --- a/autobot-backend/chat_history/messages.py +++ b/autobot-backend/chat_history/messages.py @@ -196,7 +196,11 @@ async def get_session_messages( Returns: List[Dict[str, Any]]: List of messages in the session. """ - messages = await self.load_session(session_id) + try: + messages = await self.load_session(session_id) + except (PermissionError, ValueError) as exc: + logger.error("Failed to load session %s: %s", session_id, exc) + return [] # Use model-aware limit if not explicitly provided if limit is None: @@ -220,7 +224,11 @@ async def get_session_message_count(self, session_id: str) -> int: Returns: int: Number of messages in the session. """ - messages = await self.load_session(session_id) + try: + messages = await self.load_session(session_id) + except (PermissionError, ValueError) as exc: + logger.error("Failed to load session %s: %s", session_id, exc) + return 0 return len(messages) def _message_matches_filter( diff --git a/autobot-backend/chat_history/session.py b/autobot-backend/chat_history/session.py index 7ffcb716c..83590f4b6 100644 --- a/autobot-backend/chat_history/session.py +++ b/autobot-backend/chat_history/session.py @@ -283,40 +283,42 @@ async def load_session(self, session_id: str) -> List[Dict[str, Any]]: """ Load a specific chat session with Redis cache-first strategy. + Raises: + PermissionError: If access to the session file is denied. + ValueError: If the session data is corrupted or cannot be decoded. + + Issue #1906: Propagate specific exceptions instead of silently returning []. + Args: session_id: The session identifier Returns: - List of messages in the session + List of messages in the session, or [] if the session does not exist. """ - try: - self._sanitize_session_id(session_id) - - # Try Redis cache first (Issue #315 - uses helper) - cached_messages = self._try_get_from_cache(session_id) - if cached_messages is not None: - return cached_messages + self._sanitize_session_id(session_id) - logger.debug("Cache MISS for session %s", session_id) + # Try Redis cache first (Issue #315 - uses helper) + cached_messages = self._try_get_from_cache(session_id) + if cached_messages is not None: + return cached_messages - # Load from file (Issue #620 - uses helper) - chat_data = await self._load_session_from_file(session_id) - if chat_data is None: - return [] + logger.debug("Cache MISS for session %s", session_id) - # Process messages (Issue #620 - uses helper) - cleaned_messages = await self._process_loaded_messages( - session_id, chat_data - ) + # Load from file (Issue #620 - uses helper). + # FileNotFoundError and PermissionError propagate to caller. + # ValueError (corrupted data) propagates to caller. + chat_data = await self._load_session_from_file(session_id) + if chat_data is None: + # Session file not found — not an error, just an empty result. + return [] - # Warm up Redis cache with cleaned data - await self._warm_cache_safe(session_id, chat_data) + # Process messages (Issue #620 - uses helper) + cleaned_messages = await self._process_loaded_messages(session_id, chat_data) - return cleaned_messages + # Warm up Redis cache with cleaned data + await self._warm_cache_safe(session_id, chat_data) - except Exception as e: - logger.error("Error loading chat session %s: %s", session_id, e) - return [] + return cleaned_messages async def _warm_cache_safe( self, session_id: str, chat_data: Dict[str, Any] diff --git a/autobot-backend/chat_history_manager.py b/autobot-backend/chat_history_manager.py index 273bc46b8..58e67a350 100644 --- a/autobot-backend/chat_history_manager.py +++ b/autobot-backend/chat_history_manager.py @@ -319,27 +319,37 @@ def list_sessions(self) -> List[Dict[str, Any]]: return [] def load_session(self, session_id: str) -> List[Dict[str, Any]]: - """Loads a specific chat session.""" - try: - chats_directory = self._get_chats_directory() - chat_file = f"{chats_directory}/chat_{session_id}.json" + """ + Load a specific chat session. - if not os.path.exists(chat_file): - logging.warning(f"Chat session {session_id} not found") - return [] + Raises: + PermissionError: If access to the session file is denied. + ValueError: If the session data is corrupted or cannot be decoded. - with open(chat_file, "r", encoding="utf-8") as f: - file_content = f.read() + Issue #1906: Propagate specific exceptions instead of silently returning []. - # Decrypt data if encryption is enabled - chat_data = self._decrypt_data(file_content) + Args: + session_id: The session identifier. - return chat_data.get("messages", []) + Returns: + List of messages, or [] if the session file does not exist. + """ + chats_directory = self._get_chats_directory() + chat_file = f"{chats_directory}/chat_{session_id}.json" - except Exception as e: - logging.error(f"Error loading chat session {session_id}: {str(e)}") + if not os.path.exists(chat_file): + logging.warning("Chat session %s not found", session_id) return [] + # PermissionError propagates to caller — access denied is a real problem. + with open(chat_file, "r", encoding="utf-8") as f: + file_content = f.read() + + # ValueError from _decrypt_data propagates to caller — corrupted data + # should not be silently swallowed. + chat_data = self._decrypt_data(file_content) + return chat_data.get("messages", []) + def _load_existing_chat_data( self, chat_file: str, session_id: str ) -> Dict[str, Any]: