From 32ecd241168c57fcc1de28c75b3cb72524af8ea2 Mon Sep 17 00:00:00 2001 From: d Date: Tue, 17 Mar 2026 13:09:26 +0000 Subject: [PATCH] fix: address P2 review feedback - exception handling and warning accuracy - Add try/except around _initialize_shared_aiohttp_session call to catch and log exceptions (instead of letting them bubble to outer handler) - Fix warning message when re-checked session is None (was incorrectly logging closed session ID on a None session) - Add debug logging to outer except handler instead of bare pass - Add test for _initialize_shared_aiohttp_session raising exception --- litellm/proxy/route_llm_request.py | 35 +++++++++++++++---- .../proxy/test_aiohttp_session_recovery.py | 30 ++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/litellm/proxy/route_llm_request.py b/litellm/proxy/route_llm_request.py index 376bf07f2b5..96e6f705cca 100644 --- a/litellm/proxy/route_llm_request.py +++ b/litellm/proxy/route_llm_request.py @@ -169,10 +169,23 @@ async def add_shared_session_to_data(data: dict) -> None: data["shared_session"] = session return - verbose_proxy_logger.warning( - f"SESSION REUSE: Shared aiohttp session is closed (ID: {id(session)}), recreating..." - ) - new_session = await proxy_server._initialize_shared_aiohttp_session() + # session could be None here (if another coroutine set it to None) + # or closed — either way we need to recreate + if session is not None: + verbose_proxy_logger.warning( + f"SESSION REUSE: Shared aiohttp session is closed (ID: {id(session)}), recreating..." + ) + else: + verbose_proxy_logger.warning( + "SESSION REUSE: Shared aiohttp session is None after re-check, recreating..." + ) + try: + new_session = await proxy_server._initialize_shared_aiohttp_session() + except Exception: + verbose_proxy_logger.exception( + "SESSION REUSE: Exception during shared session recreation" + ) + new_session = None if new_session is not None: proxy_server.shared_aiohttp_session = new_session data["shared_session"] = new_session @@ -185,8 +198,18 @@ async def add_shared_session_to_data(data: dict) -> None: "SESSION REUSE: No shared session available for this request" ) except Exception: - # Silently continue without session reuse if import fails or session is unavailable - pass + # Continue without session reuse — this outer handler covers import failures + # and other unexpected errors to avoid breaking the request path. + # Inner recovery logic has its own specific exception handling. + try: + from litellm._logging import verbose_proxy_logger + + verbose_proxy_logger.debug( + "SESSION REUSE: Unexpected error in session setup, continuing without reuse", + exc_info=True, + ) + except Exception: + pass async def route_request( # noqa: PLR0915 - Complex routing function, refactoring tracked separately diff --git a/tests/test_litellm/proxy/test_aiohttp_session_recovery.py b/tests/test_litellm/proxy/test_aiohttp_session_recovery.py index 763744609e3..224a4aa1392 100644 --- a/tests/test_litellm/proxy/test_aiohttp_session_recovery.py +++ b/tests/test_litellm/proxy/test_aiohttp_session_recovery.py @@ -94,6 +94,36 @@ async def test_add_shared_session_handles_recreation_failure(): assert "shared_session" not in data +@pytest.mark.asyncio +async def test_add_shared_session_handles_recreation_exception(): + """When _initialize_shared_aiohttp_session raises, data should not contain shared_session.""" + import litellm.proxy.route_llm_request as route_module + from litellm.proxy import proxy_server as proxy_server_module + from litellm.proxy.route_llm_request import add_shared_session_to_data + + # Reset the module-level lock so each test uses the current event loop + route_module._shared_session_lock = None + + closed_session = MagicMock() + closed_session.closed = True + + with patch.object( + proxy_server_module, + "shared_aiohttp_session", + closed_session, + ): + with patch.object( + proxy_server_module, + "_initialize_shared_aiohttp_session", + new_callable=AsyncMock, + side_effect=RuntimeError("connection pool exhausted"), + ): + data = {} + await add_shared_session_to_data(data) + # Should gracefully handle exception — no shared_session attached + assert "shared_session" not in data + + @pytest.mark.asyncio async def test_add_shared_session_no_session_available(): """When no session was ever created, data should not contain shared_session."""