fix(passthrough): tz guard, stale ref, async cleanup (greptile P2s on #30384)

Four P2 threads resolved:

1. **tz-aware vs tz-naive comparison can crash the stream**
   (`streaming_handler.py:51`). Wrapped the `true_start < start_time`
   comparison in try/except for `TypeError`. tzinfo mismatch now
   skips the override silently rather than propagating before the
   first chunk is yielded.
2. **Stale line-number reference** (`streaming_handler.py:68`). Dropped
   the `litellm_logging.py:1834-1837` pointer and rephrased the
   comment to describe the fallback semantically — no line number
   to rot.
3. **`_build_response_with_chunks` was `async` for no reason**
   (test_streaming_handler.py:37). Plain `def`, removed
   `asyncio.run(...)` wrapper at all four call sites.
4. **`captured_start_time["start_time"]` could raise `KeyError`
   instead of an informative assertion** if the logging task
   never executed. Added an explicit `assert "start_time" in ...`
   with a clear message before the value check, in both affected
   tests.

4/4 pass locally.
This commit is contained in:
songkuan-zheng 2026-06-16 04:20:28 +00:00
parent 034658d7fb
commit 53e7b940a8
2 changed files with 23 additions and 9 deletions

View file

@ -43,11 +43,19 @@ class PassThroughStreamingHandler:
# entered the proxy. Without this override, SpendLogs.startTime
# is artificially deflated by the full TTFT, making
# `endTime - startTime` shorter than reality.
# Guard the `<` comparison: a tz-aware vs tz-naive mix raises
# `TypeError: can't compare offset-naive and offset-aware datetimes`,
# which would propagate before the first chunk is yielded and
# break the whole stream.
true_start = getattr(litellm_logging_obj, "start_time", None)
if isinstance(true_start, datetime) and (
not isinstance(start_time, datetime) or true_start < start_time
):
start_time = true_start
if isinstance(true_start, datetime):
try:
if not isinstance(start_time, datetime) or true_start < start_time:
start_time = true_start
except TypeError:
# tzinfo mismatch — skip the override rather than
# crashing the stream. The caller-supplied start_time stays.
pass
raw_bytes: List[bytes] = []
logging_scheduled = False

View file

@ -34,7 +34,7 @@ def _build_logging_obj(true_start: datetime, completion_start_time=None):
return logging_obj
async def _build_response_with_chunks(chunks):
def _build_response_with_chunks(chunks):
"""Wrap a list of bytes chunks into an httpx.Response-shaped mock that
yields them via aiter_bytes()."""
@ -68,7 +68,7 @@ def test_chunk_processor_uses_logging_obj_start_time_when_earlier(monkeypatch):
logging_obj = _build_logging_obj(true_start=earlier)
response = asyncio.run(_build_response_with_chunks([b"chunk-1", b"chunk-2"]))
response = _build_response_with_chunks([b"chunk-1", b"chunk-2"])
captured_start_time = {}
async def fake_log_streaming_request(*args, **kwargs):
@ -91,6 +91,9 @@ def test_chunk_processor_uses_logging_obj_start_time_when_earlier(monkeypatch):
)
_drain(gen)
assert (
"start_time" in captured_start_time
), "logging task never reached the route_streaming_logging_to_handler stub"
assert captured_start_time["start_time"] == earlier, (
"Handler must override the later caller-supplied start_time with the "
"earlier logging-obj start_time."
@ -106,7 +109,7 @@ def test_chunk_processor_keeps_caller_start_time_when_earlier(monkeypatch):
logging_obj = _build_logging_obj(true_start=later)
response = asyncio.run(_build_response_with_chunks([b"chunk-1"]))
response = _build_response_with_chunks([b"chunk-1"])
captured_start_time = {}
async def fake_log_streaming_request(*args, **kwargs):
@ -129,6 +132,9 @@ def test_chunk_processor_keeps_caller_start_time_when_earlier(monkeypatch):
)
_drain(gen)
assert (
"start_time" in captured_start_time
), "logging task never reached the route_streaming_logging_to_handler stub"
assert (
captured_start_time["start_time"] == earlier
), "Caller-supplied start_time should win when it's already the earlier value."
@ -143,7 +149,7 @@ def test_chunk_processor_records_completion_start_on_first_chunk(monkeypatch):
completion_start_time=None,
)
response = asyncio.run(_build_response_with_chunks([b"chunk-1", b"chunk-2"]))
response = _build_response_with_chunks([b"chunk-1", b"chunk-2"])
monkeypatch.setattr(
PassThroughStreamingHandler,
@ -175,7 +181,7 @@ def test_chunk_processor_does_not_overwrite_existing_completion_start(monkeypatc
completion_start_time=pre_set,
)
response = asyncio.run(_build_response_with_chunks([b"chunk-1"]))
response = _build_response_with_chunks([b"chunk-1"])
monkeypatch.setattr(
PassThroughStreamingHandler,