Skip to content

Conversation

@davidsbatista
Copy link
Contributor

@davidsbatista davidsbatista commented Dec 2, 2025

Related Issues

Proposed Changes:

  • The _handle_async_stream_response() method in OpenAIChatGenerator did not handle asyncio.CancelledError exceptions. When a streaming task was cancelled mid-stream, the async for loop would exit without properly closing the OpenAI stream object - leading to tokens still being sent and charged + potential resource leaks, open connections.

  • Added exception handling in _handle_async_stream_response() to catch asyncio.CancelledError and gracefully close the stream with asyncio.shield() to ensure the cleanup operation completes even during cancellation.

How did you test it?

  • test_async_stream_closes_on_cancellation(): mocks an async stream, starts a streaming task, cancels it mid-stream, and verifies that stream.close() is called exactly once.

  • test_run_async_cancellation_integration(): Tests against the real OpenAI API by starting a long-running streaming task (essay generation), cancelling it after 2 seconds, and verifying graceful cancellation with partial chunks received.

  • Minor fix in test_openai_responses.py: added missing OPENAI_API_KEY environment variable setup via monkeypatch.

Notes for the reviewer

This PR also adds the same functionality for the following ChatGenerators (most are part of haystack-core-integrations only the first is not):

  • AzureOpenAIChatGenerator
  • NvidiaChatGenerator
  • AIMLAPIChatGenerator
  • MistralChatGenerator
  • STACKITChatGenerator
  • OpenRouterChatGenerator
  • TogetherAIChatGenerator

since they all inherit from the OpenAIChatGenerator.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have added unit tests and updated the docstrings.
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

@vercel
Copy link

vercel bot commented Dec 2, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
haystack-docs Ignored Ignored Preview Dec 10, 2025 11:53am

@davidsbatista davidsbatista marked this pull request as ready for review December 2, 2025 11:30
@davidsbatista davidsbatista requested a review from a team as a code owner December 2, 2025 11:30
@davidsbatista davidsbatista requested review from Amnah199 and mpangrazzi and removed request for a team December 2, 2025 11:30
Copy link
Contributor

@Amnah199 Amnah199 left a comment

Choose a reason for hiding this comment

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

LG to me! But I would wait for @mpangrazzi 's review

Copy link
Contributor

@mpangrazzi mpangrazzi left a comment

Choose a reason for hiding this comment

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

Looks good to me!

…celledError-closing-response-stream-37e1e85255e1dc41.yaml

Co-authored-by: Michele Pangrazzi <[email protected]>
@github-actions github-actions bot added the type:documentation Improvements on the docs label Dec 10, 2025
@davidsbatista davidsbatista merged commit ca9c8a5 into main Dec 10, 2025
21 checks passed
@davidsbatista davidsbatista deleted the fix/close-openao-asyncio-stream-on-cancellation branch December 10, 2025 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Close OpenAI stream on asyncio cancellation to save tokens

4 participants