fix: BA-18 — request/idle timeout on the http(s) providers (v0.34.0)#26
Merged
Conversation
…socket was a ~2h hang, not an error)
The four http(s) providers (Anthropic, OpenAI, Gemini, Ollama) built their
request with only `req.on('error')` wired — no socket timeout, no AbortSignal —
so a socket the server silently dropped, or a response that never starts, was
bounded only by the OS TCP timeout (~2h on Linux). It presented to the caller as
a HANG, not a failure (no event, no error, no progress), which made every
retry/casualty policy above it inert. Same BA-4/5/6/13/17 under-modeled-boundary
family: a hung socket has no slot in the neutral result shape, so it rounded
toward "still working."
Part 1 (real): new `timeoutMs` option on all four providers (default 600000ms /
10 min, per-call overridable, 0/Infinity disable). One shared src/provider-http.js
(resolveTimeoutMs/applyRequestTimeout) so the four cannot drift. Bounds on socket
INACTIVITY (req.setTimeout) — a slow-but-streaming response is not killed, only a
silent socket trips; the 10-min default clears any single non-streaming
completion (TTFB ≈ generation time). On trip: a retryable TimeoutError
(code:'ETIMEDOUT'). CLIPipe already bounded its child; unchanged.
Part 2 (name mismatch, no code): the ask cited "withRetry has zero call sites."
There is no withRetry — the primitive is Retry.call, and it IS wired around
provider.generate (Loop({retry}), plus run-plan's stepRetry), with
DEFAULT_RETRY_ON already classifying ETIMEDOUT transient. Covered by tests +
docs; the intended contract is caller-side wiring, and the provider-level
timeoutMs protects a caller who uses the provider directly.
Disable-edge semantics are fail-safe (code-review findings 2 & 3): a per-call
null/undefined INHERITS the instance value (a per-call null never re-enables the
default over an instance-level disable), and a NaN/negative/non-finite value
(e.g. Number(process.env.X) on an unset var) falls back to the default rather
than silently disabling the bound (which would round back toward the ~2h hang).
Per-call timeoutMs is now documented in each generate() JSDoc (finding 4).
Finding 1 (finite default regresses >10min non-streaming completions) is the
signed-off design choice — aligned with Anthropic's own ~10-min non-streaming
ceiling, with 0/Infinity as the documented escape hatch and the Ollama cold-load
caveat noted.
Validation: POC (poc/ba18-timeout.mjs, real loopback servers, 9/9); +21 tests
(all 4 providers fire + no false trip, resolveTimeoutMs incl. the two disable-edge
fixes, the Retry seam); 5 mutations proven RED incl. a guard-OFF negative control
that reproduces the hang and one per review fix; live verify on the real https
wire 4/4 (poc/ba18-verify-shipped.mjs).
Also folded in (separate, pre-existing): a flaky mcp-bridge reaping test whose
300ms connect-timeout raced node's ~400-500ms cold-start on a slow host — the
child was SIGKILLed before it wrote its pid witness (ENOENT), so the test failed
on this machine (passed on the faster release host). Raised that test's connect
timeout to 2000ms (still ≪ the 10000ms slow-init, so it still exercises
fail-to-connect + reaping). No product change.
Docs synced same commit: CHANGELOG (0.34.0), CLAUDE.md, bareagent.context.md
(+header), docs/02-features/errors.md, PRD §22. Version bumped 0.33.1 → 0.34.0
(new public option = MINOR).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EWGqnWr4Qn3gDnHrE3DaZi
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
1 participant
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.
Fixes bareloop's upstream ask BA-18: the four http(s) providers (Anthropic, OpenAI, Gemini, Ollama) built their request with only
req.on('error')wired — no socket timeout — so a silently-dropped or never-answering socket hunggenerate()until the OS TCP timeout (~2h). A hang, not an error, so every retry policy above it was inert (adopter saw 38-min and 2h24m hangs).What changed
timeoutMsoption on all four http(s) providers (default 600000ms / 10 min, per-call overridable,0/Infinitydisable). One sharedsrc/provider-http.js(resolveTimeoutMs/applyRequestTimeout) so the four can't drift. Bounds on socket inactivity — a slow-but-streaming response isn't killed; only a silent socket trips. On trip: a retryableTimeoutError(code:'ETIMEDOUT'). CLIPipe already bounded its child; unchanged.withRetry; the primitive isRetry.call, already wired aroundprovider.generateviaLoop({retry})+run-plan'sstepRetry, andDEFAULT_RETRY_ONalready classifiesETIMEDOUTtransient. Proven by tests + documented; no code needed.null/undefinedinherits the instance;NaN/negative fall back to the default rather than silently disabling.Validation
resolveTimeoutMsedges, the Retry seam); 5 mutations proven RED incl. a guard-OFF negative control reproducing the hang; live verify on the real https wire 4/4./shipReady ·/securityClean ·/code-review(high) 4 findings all resolved + mutation-proven.Version: 0.33.1 → 0.34.0 (new public option = minor).
🤖 Generated with Claude Code
https://claude.ai/code/session_01EWGqnWr4Qn3gDnHrE3DaZi