Repository navigation
fix(streaming): restore HTTP/1.1 connection reuse after [DONE] - #4041
marcuswood-oai wants to merge 2 commits into
Conversation
Castiron custom codeEvaluated main: ✅ No new custom-code files detected. 46 mixed files remain; 0 existing customizations changed. Compared 46 existing customizations unchanged
6 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 37822801555 --repo openai/openai-python \
--name castiron-custom-code-37822801555-1 --dir /tmp/castiron-custom-code-37822801555-1
git apply --stat /tmp/castiron-custom-code-37822801555-1/custom-code.patch
cat /tmp/castiron-custom-code-37822801555-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 9301e319ea33ef28fba380f39a289dedc14652c1 83833ea92b0da18e51e32f1d39650aeff691430f
python3 scripts/castiron/custom_code_report.py report \
--base 9301e319ea33ef28fba380f39a289dedc14652c1 \
--head 83833ea92b0da18e51e32f1d39650aeff691430f --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-83833ea92b0d
cat /tmp/castiron-custom-code-83833ea92b0d/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| await asyncio.wait_for(draining.wait(), timeout=5) | ||
| task.cancel() | ||
| with pytest.raises(asyncio.CancelledError): | ||
| await task |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed 83833ea92b0da18e51e32f1d39650aeff691430f. No actionable findings.
The HTTP/1.1 cleanup resumes the active byte iterator after [DONE] without parsing trailing SSE data. Transport failures are suppressed only during cleanup; cancellation still propagates and the response closes in finally. HTTP/2 and early-close paths remain unchanged.
The documented timeout tradeoff remains: this uses the request's per-read timeout, with no total cleanup deadline. A delayed ending adds completion latency; continuous trailing bytes or disabled read timeouts can keep iteration open indefinitely.
Source-only review; I did not run tests or the benchmark locally. Hosted Python 3.10, Python 3.14, HTTPX2, build, lint, and CodeQL checks passed for this head.
This branch has not been deployed
2 participants
Changes being requested
Fully consumed HTTP/1.1 Chat Completions streams currently close at
[DONE]before the transport reads the HTTP body ending, so each request opens a new connection. Resume the existing byte iterator after[DONE]to let the transport return completed connections to the pool.The drain runs only for HTTP/1.1 after the completion marker and discards bytes without parsing trailing SSE data. Transport errors during that cleanup are suppressed; cancellation and unexpected errors still propagate, and the existing
finallycloses the response. Early exits and errors before[DONE]do not drain. HTTP/2 keeps its existing behavior.This deliberately uses the request's existing read timeout. There is no independent cleanup deadline: a delayed ending adds completion latency, and disabling read timeouts can permit an indefinite wait. Tokens are still yielded as they arrive.
Validation
Three focused tests cover sync/async HTTP/1.1 connection reuse and failed-cleanup recovery, HTTP/2 and discarded trailing bytes, and cancellation during the drain. They use the existing HTTPX2/legacy HTTPX test matrix.
Synthetic TCP/TLS benchmark
40 requests per condition, same SDK version and transport dependencies before/after. Medians below use an immediate HTTP ending; these are synthetic measurements, not production latency estimates.
Deliberately delaying the HTTP ending by 5 ms or 30 ms adds approximately that wait after the final token. HTTP/2 continues to reuse one connection and complete promptly without draining.
Additional context & links
Fixes #4040. Related to #3440.