Skip to content

fix(agent): rebuild connection pool for socket-layer transport errors (BrokenPipe, ConnectionReset, etc.) - #52226

Open
DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/52216-brokenpipe-transport-recovery
Open

fix(agent): rebuild connection pool for socket-layer transport errors (BrokenPipe, ConnectionReset, etc.)#52226
DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/52216-brokenpipe-transport-recovery

Conversation

@DavidMetcalfe

Copy link
Copy Markdown
Contributor

Summary

The transport-recovery gate at agent/agent_runtime_helpers.py:818 consulted a stale frozenset (_TRANSIENT_TRANSPORT_ERRORS, only 7 type names) that missed socket-layer errors the canonical classifier at agent/error_classifier.py:_TRANSPORT_ERROR_TYPES already treats as retryable. Result: providers whose cloud gateway closes sockets during long thinking phases (NVIDIA Nemotron 3 Ultra on hosted NIM's ~120s upstream idle kill — first-party reproduction at NVIDIA/NemoClaw#4846 — also Anthropic Opus 4.7 thinking and OpenAI o1/o3) burned all 3 outer retries against the same dead connection pool, then surfaced API call failed after 3 retries: [Errno 32] Broken pipe.

One source-file line change:

# agent/agent_runtime_helpers.py:1081
_TRANSIENT_TRANSPORT_ERRORS = _TRANSPORT_ERROR_TYPES

Single source of truth — any future addition to _TRANSPORT_ERROR_TYPES automatically flows through, no drift possible.

Test Plan

  • python3 -m pytest tests/run_agent/test_primary_runtime_restore.py tests/agent/test_error_classifier.py tests/run_agent/test_streaming.py -q238 passed, zero failures, zero regressions.
  • ruff check agent/agent_runtime_helpers.py tests/run_agent/test_primary_runtime_restore.py — clean.
  • Negative test: temporarily reverted the source change and confirmed the 8 new tests fail with the exact symptom from [Bug]: BrokenPipeError skips connection-pool rebuild in transport-recovery gate #52216 (_try_recover_primary_transport refused recovery for BrokenPipeError; the gate at agent_runtime_helpers.py:818 is out of sync with agent/error_classifier.py:_TRANSPORT_ERROR_TYPES). Then restored the fix and confirmed all 39 tests in test_primary_runtime_restore.py pass.
  • Cross-vendor dual review via agy -p:
    • Gemini 3.5 Flash (Medium) — passed: true, zero blockers/should-fix/nits.
    • GPT-OSS 120B (Medium) — passed: true, zero blockers/should-fix/nits.
    • Both reviewers verified: alias is a Python object reference (not a copy), no callers mutate either frozenset, no import cycle (verified agent/error_classifier.py has no reverse import of agent.agent_runtime_helpers), the parametrized test correctly mirrors the production type(api_error).__name__ check at line 818, and the identity assertion (is, not ==) catches future re-introduction of a duplicated frozenset literal.

Notes

  • Scope is the smallest possible. 1 source line change + 1 import extension + 8 regression tests. No new modules, no new knobs, no user-visible config additions, no timeout-default changes.
  • No regression risk on aggregator providers. The recovery gate's existing aggregator skip at line 822 (if agent._is_openrouter_url(): return False) and provider skip at lines 825-826 (Nous, Nous-Research) are preserved. Aggregators that manage their own retry infra still skip the rebuild.
  • No regression risk on persistent upstream errors. The recovery fires at most once per API call block (gated by _retry.primary_recovery_attempted at agent/conversation_loop.py:3421). For truly persistent upstream issues that the rebuild cannot fix, the next outer retry surfaces the same error and the classifier's broader signals route to fallback or abort — no recovery loop.
  • Behavioral regression risk audited. Every type in _TRANSPORT_ERROR_TYPES is genuinely transient:
    • ReadTimeout, ConnectTimeout, PoolTimeout — explicit timeouts, transient.
    • ConnectError, RemoteProtocolError, ConnectionError, ConnectionResetError, ConnectionAbortedError, BrokenPipeError, ServerDisconnectedError, ReadError — socket-level disconnects, transient (a stale-connection-pool rebuild is exactly the right recovery).
    • TimeoutError — generic timeout, transient.
    • SSLError, SSLZeroReturnError, SSLWantReadError, SSLWantWriteError, SSLEOFError, SSLSyscallError — TLS handshake/record failures, transient.
    • APIConnectionError, APITimeoutError — OpenAI SDK wrappers for the above, transient.
  • Related but NOT in this PR. Issue [Bug]: Reasoning models inherit chat-model stale-timeout defaults — broken-pipe on long-thinking models like Nemotron 3 Ultra, OpenAI o1/o3, Opus 4.x thinking #52217 (sister issue filed in the same session) covers the upstream-side mitigation: a per-reasoning-model stale-timeout floor that auto-scales timeouts for known reasoning models. That requires a new module (agent/reasoning_timeouts.py) and integration into two stale-detector scaling blocks, and is shipped separately.

Fixes #52216.

…nPipe, ConnectionReset, etc.)

The transport-recovery gate at `agent/agent_runtime_helpers.py:818`
consulted a stale frozenset (`_TRANSIENT_TRANSPORT_ERRORS`, only 7 type
names) that missed socket-layer errors the classifier at
`agent/error_classifier.py:_TRANSPORT_ERROR_TYPES` already treats as
retryable: `BrokenPipeError`, `ConnectionResetError`,
`ConnectionAbortedError`, `ServerDisconnectedError`, `ReadError`,
plus the SSL family.

Result: reasoning models on providers whose cloud gateway closes
sockets during long thinking phases (NVIDIA Nemotron 3 Ultra on hosted
NIM's ~120s upstream idle kill, also Anthropic Opus 4.7 thinking and
OpenAI o1/o3) burned all 3 outer retries against the same dead
connection pool, then surfaced `API call failed after 3 retries:
[Errno 32] Broken pipe`.

Fix: alias the recovery set to the canonical classifier set. Single
source of truth — any future addition to `_TRANSPORT_ERROR_TYPES`
automatically flows through, no drift possible.

```python
_TRANSIENT_TRANSPORT_ERRORS = _TRANSPORT_ERROR_TYPES
```

Verified:
- 238 tests passing across `test_error_classifier.py`,
  `test_primary_runtime_restore.py`, `test_streaming.py`.
- 8 new regression tests in `test_primary_runtime_restore.py`:
  - 7 parametrized over the socket-layer types (ConnectionError,
    ConnectionResetError, ConnectionAbortedError, BrokenPipeError,
    TimeoutError, ReadError, ServerDisconnectedError) — all fail
    without the source fix, all pass with it.
  - 1 identity check (`_TRANSIENT_TRANSPORT_ERRORS is _TRANSPORT_ERROR_TYPES`)
    guards against future re-introduction of a duplicated frozenset.
- Ruff lint clean on both modified files.
- No import cycle (verified: `agent/error_classifier.py` has no reverse
  import of `agent.agent_runtime_helpers`).
- Zero mutations on either set anywhere in the codebase.

Fixes NousResearch#52216.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused recovery fix. The premise still holds on current main: agent/agent_runtime_helpers.py:980 rejects BrokenPipeError because its local set at :1334 omits it, while agent/error_classifier.py:419-437 includes it in _TRANSPORT_ERROR_TYPES; the exhausted-retry path reaches this gate at agent/conversation_loop.py:3925-3942.

The alias is a small, direct way to remove that drift. agent/error_classifier.py:783-788 continues to classify certificate-verification failures as non-retryable before the recovery path is considered, so sharing the named transport set does not override that fail-fast behavior.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: BrokenPipeError skips connection-pool rebuild in transport-recovery gate

3 participants