Skip to content

fix(streaming): close SDK streams closed before their first read - #1405

Merged
HareeshBahuleyan merged 3 commits into
mainfrom
feature/stream-close-before-first-read
Sep 16, 2026
Merged

HareeshBahuleyan merged 3 commits into
mainfrom
feature/stream-close-before-first-read

Conversation

@HareeshBahuleyan

@HareeshBahuleyan HareeshBahuleyan commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

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-provider chunk_iterator() over openai's AsyncStream never closed that stream at all.

The streaming wrapper in handle_exceptions becomes _ExceptionHandlingAsyncIterator, which holds the provider stream directly so aclose() reaches it whether or not anything was read. BaseOpenAIProvider gets OpenAIChunkStream for the same reason one level down; the XML-reasoning provider uses it too. The Azure provider's local chunks() 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 on OpenaiProvider (exhaustion, early exit, failure, cancellation, zero consumption) releases the HTTP body for both chat and responses.

PR Type

  • 🐛 Bug Fix

Relevant issues

Follow-up to #1319 and #1400.

Checklist

  • I understand the code I am submitting.
  • I have added unit tests that prove my fix/feature works
  • I have run this code locally and verified it fixes the issue.
  • New and existing tests pass locally
  • Documentation was updated where necessary
  • I have read and followed the contribution guidelines
  • AI Usage:
    • No AI was used.
    • AI was used for drafting/refactoring.
    • This is fully AI-generated.

AI Usage Information

  • AI Model used: Claude Fable 5.1
  • AI Developer Tool used: Claude Code
  • Any other info you'd like to share: The fix was first written inside fix(azureopenai): migrate to the v1 API #1400 and split out here so it lands for every OpenAI-based provider. Unit suite: 2,754 passed, 69 skipped locally.

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 :)

  • I am an AI Agent filling out this form (check box if true)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved streaming reliability by releasing connections when streams finish, fail, are cancelled, or are closed early.
    • Standardised response handling across OpenAI-compatible providers.
    • Improved error handling for streaming responses, including cleanup when closed before reading begins.
    • Ensured closed streams remain closed and do not resume processing.
    • Improved cleanup for streams that process XML reasoning content.

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>
@HareeshBahuleyan
HareeshBahuleyan deployed to integration-tests September 16, 2026 08:49 — with GitHub Actions Active
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d78f9c97-e9da-4eda-a670-4ca28145ad50

📥 Commits

Reviewing files that changed from the base of the PR and between 9c5764e and 231bf4c.

📒 Files selected for processing (2)
  • src/any_llm/utils/exception_handler.py
  • tests/unit/test_exception_handler.py

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


Walkthrough

The 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.

Changes

Async stream lifecycle

Layer / File(s) Summary
OpenAI stream wrapper
src/any_llm/providers/openai/base.py, src/any_llm/providers/azureopenai/azureopenai.py, tests/unit/providers/test_openai_base_provider.py
OpenAIChunkStream converts OpenAI chunks and closes the SDK stream on completion or failure. Azure OpenAI now inherits the base conversion. Tests cover exhaustion, early close, failure, cancellation, zero consumption, and close failures.
XML reasoning stream integration
src/any_llm/providers/openai/xml_reasoning.py, tests/unit/test_xml_reasoning.py
XMLReasoningOpenAIProvider applies XML reasoning to chunks produced by OpenAIChunkStream. The wrapper makes closing terminal and closes the source once.
Exception-handling stream wrapper
src/any_llm/utils/exception_handler.py, tests/unit/test_exception_handler.py
streaming_wrapper uses _ExceptionHandlingAsyncIterator, which closes the wrapped iterator on completion, errors, cancellation, and explicit close. Tests cover unread and already-read streams.

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 231bf

No actionable merge-blocking risk remains from the reviewed change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.95% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: closing SDK streams when streaming starts are closed before the first read.
Description check ✅ Passed The description explains the bug, implementation, affected providers, tests, issue references, PR type, checklist status, and AI usage. The unchecked documentation item is non-critical because no docu…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/stream-close-before-first-read

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 path_filters to narrow the review scope.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/any_llm/providers/azureopenai/azureopenai.py 100.00% <100.00%> (ø)
src/any_llm/providers/openai/base.py 92.42% <100.00%> (+0.62%) ⬆️
src/any_llm/providers/openai/xml_reasoning.py 97.01% <100.00%> (+1.27%) ⬆️
src/any_llm/utils/exception_handler.py 99.35% <100.00%> (+0.11%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment thread src/any_llm/providers/openai/base.py Outdated
Comment thread src/any_llm/providers/openai/xml_reasoning.py
Comment thread src/any_llm/utils/exception_handler.py
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.
@HareeshBahuleyan
HareeshBahuleyan deployed to integration-tests September 16, 2026 09:00 — with GitHub Actions Active
@HareeshBahuleyan HareeshBahuleyan added the run-integration-tests Put this label on a PR to trigger the integration test suite: works with forks label Sep 16, 2026
@github-actions github-actions Bot removed the run-integration-tests Put this label on a PR to trigger the integration test suite: works with forks label Sep 16, 2026
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.
@HareeshBahuleyan
HareeshBahuleyan deployed to integration-tests September 16, 2026 09:21 — with GitHub Actions Active
@HareeshBahuleyan HareeshBahuleyan added the run-integration-tests Put this label on a PR to trigger the integration test suite: works with forks label Sep 16, 2026
@github-actions github-actions Bot removed the run-integration-tests Put this label on a PR to trigger the integration test suite: works with forks label Sep 16, 2026
@HareeshBahuleyan
HareeshBahuleyan merged commit 9ae1ab2 into main Sep 16, 2026
21 of 22 checks passed
@HareeshBahuleyan
HareeshBahuleyan deleted the feature/stream-close-before-first-read branch September 16, 2026 10:43
@github-actions github-actions Bot added the 1.28.0 Included in release 1.28.0 label Sep 18, 2026

This branch was successfully deployed

1 active deployment
integration-tests — 231bf4c3 Deployed Sep 16, 2026 by HareeshBahuleyan via run-docs-tests #2998
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.28.0 Included in release 1.28.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants