mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-05 08:07:05 +00:00
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
This commit is contained in:
parent
9e09bbc1df
commit
32ecd24116
2 changed files with 59 additions and 6 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue