fix(streaming): rebuild Anthropic client on stream cleanup instead of OpenAI client - #28240
Conversation
… OpenAI client interruptible_streaming_api_call() has three connection-pool cleanup sites that called _replace_primary_openai_client() unconditionally. For api_mode=anthropic_messages this has two consequences: 1. _replace_primary_openai_client() fails (OPENAI_API_KEY unset on Anthropic-only configs), so dead connections are never purged. 2. The stale-stream detector's outer-poll site (L1977) is the only mechanism that can interrupt the worker thread while it blocks in for event in stream:. Because the Anthropic client is never closed, the thread stays blocked until the 900 s httpx read-timeout fires, producing a visible 15-minute hang for Telegram/gateway users on claude-opus-4-7. Fix: mirror the existing interrupt-path pattern (L1989-1997) at all three cleanup sites — if api_mode == "anthropic_messages", call _anthropic_client.close() + _rebuild_anthropic_client() instead of _replace_primary_openai_client(). _rebuild_anthropic_client() handles both direct Anthropic and Bedrock-hosted Claude correctly, unlike the inline build_anthropic_client() calls in open PR NousResearch#14430. PR NousResearch#14430 (open) covers only the outer stale-detector site (L1977). PR NousResearch#23678 (open) covers only the inner retry sites (L1774, L1833). This PR covers all three sites and uses _rebuild_anthropic_client() for Bedrock parity. Fixes NousResearch#28161
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused fix. I verified the underlying bug is still real on current main: agent/chat_completion_helpers.py:2362, agent/chat_completion_helpers.py:2415, and agent/chat_completion_helpers.py:2580 still call _replace_primary_openai_client() from Anthropic streaming cleanup paths, while the nearby interrupt path already uses _anthropic_client.close() + _rebuild_anthropic_client() at agent/chat_completion_helpers.py:2601.
Problems
- tests/run_agent/test_streaming.py:1038 still asserts mock_replace.call_count == 1 for an Anthropic parser retry. This PR changes that path to avoid _replace_primary_openai_client(), so the existing test needs to be updated to assert Anthropic close/rebuild instead.
- The new test file covers stream_retry_pool_cleanup and stale_stream_pool_cleanup, but not the mid-tool cleanup site that is still buggy on main at agent/chat_completion_helpers.py:2362.
Suggested changes
- Update the existing Anthropic parser retry test at tests/run_agent/test_streaming.py:1038.
- Add one regression case for stream_mid_tool_retry_pool_cleanup so all three fixed sites are covered.
This is an automated hermes-sweeper review.
| this silently fails (no OPENAI_API_KEY) and leaves the in-flight httpx stream unclosed, | ||
| blocking the worker thread until the 900s httpx read-timeout fires. | ||
|
|
||
| Tests cover: |
There was a problem hiding this comment.
This test list omits stream_mid_tool_retry_pool_cleanup, but the PR changes that cleanup site too; please add a regression case for the mid-tool retry branch so all three Anthropic cleanup paths are covered.
|
Superseded by #51851 — that PR is based on the current codebase and includes the |
|
Closed in favor of #53926, now merged to main (commit a0b9663). This was a popular bug — 12 independent PRs fixed the same issue (#28161): the three stream-cleanup paths in the streaming code rebuilt the OpenAI primary client unconditionally, which on Anthropic-native sessions both failed (no OPENAI_API_KEY) and left the wedged stream open, causing the ~15-minute hang. We salvaged @EloquentBrush0x's #28240 (the earliest dedicated fix for this issue) onto current main, resolved the conflict against the newer request-client cleanup helper, repointed the bug-encoding regression test, and added coverage for the two reachable Anthropic cleanup sites. Verified live: stale stream torn down in <1s instead of ~900s, Anthropic client closed+rebuilt, OpenAI rebuild never called on the Anthropic path. Thank you for the fix — closing as a duplicate of the merged work. Credit to everyone who reported and fixed this. |
What does this PR do?
Root cause:
interruptible_streaming_api_call()has three connection-pool cleanup sites that called_replace_primary_openai_client()unconditionally — at the mid-tool retry path (~L1774), the transient-error retry path (~L1833), and the stale-stream outer-poll loop (~L1977).For
api_mode=anthropic_messagesthis has two consequences:_replace_primary_openai_client()silently fails (noOPENAI_API_KEYon Anthropic-only configs), so the dead connection pool is never purged before the next retry.for event in stream:. Because neither the Anthropic client is closed nor the stream's underlying transport dropped, the thread stays blocked until the 900 s httpx read-timeout fires — producing the 15-minute hang reported in gateways runningclaude-opus-4-7.Fix: Mirror the existing interrupt-path pattern at L1989–1997 (already correct) at all three cleanup sites. For
api_mode == "anthropic_messages", call_anthropic_client.close()+_rebuild_anthropic_client()instead of_replace_primary_openai_client()._rebuild_anthropic_client()handles both direct Anthropic and Bedrock-hosted Claude correctly.Open PR #14430 covers only the outer stale-detector site and does not use
_rebuild_anthropic_client()(Bedrock not handled). Open PR #23678 covers only the two inner retry sites, leaving the stale-stream hang unaddressed. This PR covers all three sites.Related Issue
Fixes #28161
Type of Change
Changes Made
agent/chat_completion_helpers.py: guard all three_replace_primary_openai_client()call sites withapi_mode != "anthropic_messages"; add_anthropic_client.close() + _rebuild_anthropic_client()branch for Anthropic mode (+18 lines)tests/run_agent/test_28161_anthropic_stream_pool_cleanup.py: two new tests — stream retry cleanup and stale-stream detector cleanup for Anthropic modeHow to Test
Checklist