fix: recover from network drops without restarting#78
Merged
Conversation
After a host network drop/reconnect, Node's shared fetch connection pool holds dead keep-alive sockets. With no time limit on a request, a retry lands on a dead socket and hangs forever — every account and retry hits the same poison, so the proxy stays wedged until a manual restart. Bound each stage so a stuck request fails fast, letting Node evict the dead socket so the client's retry reconnects cleanly: - upstream-fetch.js: a headers-only deadline (time-to-first-byte) on both the direct-fetch and sx-tunnel paths, disarmed the instant headers arrive so long streaming bodies are never cut (TEAMCLAUDE_UPSTREAM_HEADERS_TIMEOUT_MS, default 120s). - server.js: a body-idle watchdog in streamResponse that resets per chunk and fires only when the stream goes silent mid-body (a drop after headers) — covers both fetch paths since both yield a web ReadableStream (TEAMCLAUDE_UPSTREAM_BODY_TIMEOUT_MS, default 120s). On timeout the truncated stream is torn down rather than cleanly ended, so the client retries instead of accepting a partial response. - server.js: broaden the transient-error classification to cover the new timeout codes plus undici's headers/body timeouts. - oauth.js: bound token refresh (TEAMCLAUDE_REFRESH_TIMEOUT_MS, default 30s); a hung refresh is coalesced across callers and would otherwise wedge every request for that account. Tests cover fast-fail on a dead socket, same-origin eviction+reconnect, a slow body not being cut once headers arrive, and the mid-stream body watchdog firing fast while leaving healthy slow streams alone. Based on #73 by @teocns; headers-timeout + refresh-timeout reimplemented from that PR, extended here with the zero-dependency mid-stream body watchdog (the PR's dispatcher-level alternative would add the undici runtime dep and miss the sx-tunnel path). Closes #73 Co-authored-by: teocns <teocns@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After the host network drops and reconnects, teamclaude keeps failing every request until you restart it. Accounts show as throttled or errored and Claude reports API errors, even though nothing is actually rate limited.
The cause is a poisoned connection pool. Node's shared
fetchpool holds dead keep-alive sockets across all requests and accounts. When the network drops the connections die silently, and because a request has no time limit it waits on a dead connection forever instead of failing. A hung request never errors, so the dead socket is never dropped from the pool, and every retry and every account keeps landing on the same poison — only a restart builds a fresh pool.Fix
Bound each stage so a stuck request fails fast instead of hanging. The fast failure lets Node evict the dead socket, the client retries, and the next request connects fresh — no restart needed.
src/upstream-fetch.js): a headers-only deadline (time-to-first-byte) on both the direct-fetchpath and the sx-tunnel path. It is cleared the instant headers arrive, so a long streaming body is never cut — it only converts an indefinite pre-headers hang into a fast, retryable failure.TEAMCLAUDE_UPSTREAM_HEADERS_TIMEOUT_MS, default 120s.src/server.jsstreamResponse): the headers timeout is disarmed once headers arrive, so a drop during the body would still hang. This watchdog guards eachreader.read(), resets on every chunk (a slow-but-healthy stream is fine), and fires only when the stream goes silent mid-body. On timeout the truncated stream is torn down rather than cleanly ended, so the client retries instead of accepting a partial response. Covers both fetch paths, since both hand back a webReadableStream.TEAMCLAUDE_UPSTREAM_BODY_TIMEOUT_MS, default 120s.src/server.js): broadened to recognize the new timeout codes plus undici'sUND_ERR_HEADERS_TIMEOUT/UND_ERR_BODY_TIMEOUT.src/oauth.js): token refresh now fails after 30s instead of hanging. A hung refresh is especially harmful —ensureTokenFreshcoalesces callers into a single promise, so one stuck refresh wedges every request for that account.TEAMCLAUDE_REFRESH_TIMEOUT_MS, default 30s.No new dependencies.
Notes
Recovery is per-socket: each fast failure evicts one dead connection, so after a flap with several in-flight requests it can take a few failed-then-retried requests to fully drain. It always converges without a restart.
Non-streaming caveat: the headers timeout is time-to-first-byte, which is cheap for streamed completions (Claude Code's default). A non-streaming (
stream: false) request that legitimately generates for longer than the window could trip it — raise the env var for such callers. Documented in the README.Relationship to #73
Based on #73 by @teocns. The headers-timeout and refresh-timeout are reimplemented from that PR (reviewed, not blindly cherry-picked). Extended here with a zero-dependency mid-stream body watchdog — #73's suggested dispatcher-level alternative (
setGlobalDispatcher(new Agent({ headersTimeout, bodyTimeout }))) would add theundiciruntime dependency the project deliberately avoids and would not cover the sx-tunnel path. Merging this closes #73.Tests
test/upstream-timeout.test.js: fast-fail on a dead socket, same-origin socket eviction + reconnect, a slow body not cut once headers arrive, the mid-stream body watchdog firing fast, and the watchdog not firing on a healthy slow stream. Full suite passes (143) and lint is clean.Closes #73
Co-authored-by: teocns teocns@gmail.com
🤖 Generated with Claude Code