fix(streaming): close SDK streams closed before their first read - #1405
Conversation
Follow-up to #1319, which left two gaps: a generator closed before its first __anext__() never runs its body, so the SDK stream beneath it stays open, and the per-provider chunk_iterator() over openai's AsyncStream never closed that stream at all. Replace the streaming wrapper generator in handle_exceptions with _ExceptionHandlingAsyncIterator, which holds the provider stream directly so aclose() reaches it whether or not anything was read. Add OpenAIChunkStream to BaseOpenAIProvider for the same reason one level down, and use it in the XML-reasoning provider too. The Azure provider's local stream override is removed since the base now covers it. Tests: wrapper aclose() before the first read closes the SDK stream; every exit mode on OpenaiProvider, including zero consumption, releases the HTTP body for both chat and responses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. WalkthroughThe change replaces inline async generators with explicit stream wrappers. OpenAI and XML reasoning providers use shared lifecycle handling. Exception handling closes wrapped streams on completion, errors, cancellation, and explicit close. Tests cover these paths. ChangesAsync stream lifecycle
Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk remains from the reviewed change. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
XML-reasoning and Minimax streams can still leak, while cleanup failures may mask the original stream outcome.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Improves streaming cleanup for OpenAI-compatible providers, including streams closed before consumption.
Changes:
- Replaces generator-based exception wrapping with a close-aware iterator.
- Adds a close-aware OpenAI chunk converter.
- Expands transport-release tests across stream exit modes.
File summaries
| File | Description |
|---|---|
src/any_llm/utils/exception_handler.py |
Adds the close-aware exception iterator. |
src/any_llm/providers/openai/base.py |
Adds shared OpenAI stream cleanup. |
src/any_llm/providers/openai/xml_reasoning.py |
Integrates cleanup with XML reasoning streams. |
src/any_llm/providers/azureopenai/azureopenai.py |
Removes redundant Azure cleanup logic. |
tests/unit/test_exception_handler.py |
Tests closing before initial consumption. |
tests/unit/providers/test_openai_base_provider.py |
Tests transport release across exit modes. |
Review details
Suppressed comments (1)
src/any_llm/providers/openai/base.py:229
- This base-path replacement does not cover
MinimaxProvider._convert_completion_response_async(src/any_llm/providers/minimax/minimax.py:38-56), which overrides the method and still nests the raw SDK response beneath filtering and reasoning async generators. Consequently Minimax continues leaking the HTTP stream on zero consumption and early closure, so the fix does not reach every OpenAI-based provider as described. Update that override to retain a close-aware SDK iterator while preserving its filtering.
return OpenAIChunkStream(response, self._convert_completion_chunk_response)
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Forward XML stream cleanup before the first read, preserve original stream outcomes when SDK cleanup fails, and stop closed iterators from yielding buffered chunks. Add regression coverage for cleanup, cancellation, and terminal close behavior.
Initialize provider iterators lazily inside exception handling so startup failures are converted and the source is closed. Test unified and legacy exceptions and closing before iterator initialization.
Description
Follow-up to #1319, which left two gaps it named: a generator closed before its first
__anext__()never runs its body, so the SDK stream under it stays open, and the per-providerchunk_iterator()over openai'sAsyncStreamnever closed that stream at all.The streaming wrapper in
handle_exceptionsbecomes_ExceptionHandlingAsyncIterator, which holds the provider stream directly soaclose()reaches it whether or not anything was read.BaseOpenAIProvidergetsOpenAIChunkStreamfor the same reason one level down; the XML-reasoning provider uses it too. The Azure provider's localchunks()override from #1400 is removed since the base now covers it.Tests: wrapper
aclose()before the first read closes the SDK stream; every exit mode onOpenaiProvider(exhaustion, early exit, failure, cancellation, zero consumption) releases the HTTP body for both chat and responses.PR Type
Relevant issues
Follow-up to #1319 and #1400.
Checklist
AI Usage Information
When answering questions by the reviewer, please respond yourself, do not copy/paste the reviewer comments into an AI system and paste back its answer. We want to discuss with you, not your AI :)
🤖 Generated with Claude Code
Summary by CodeRabbit