Skip to content

feat: add an HTTP connect timeout so unreachable RPC hosts fail fast - #172

Open
flyq wants to merge 1 commit into
mainfrom
liquan/feat/rpc-connect-timeout
Open

feat: add an HTTP connect timeout so unreachable RPC hosts fail fast#172
flyq wants to merge 1 commit into
mainfrom
liquan/feat/rpc-connect-timeout

Conversation

@flyq

@flyq flyq commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

RpcClient now builds one shared reqwest client with a connect-phase bound (RpcClientConfig::connect_timeout, default 3s) and injects it into every alloy provider (data, witness, report). An endpoint whose TCP handshake gets no reply — a dead host, or a firewall silently dropping SYNs — now fails and rotates within ~3s instead of consuming the full 20s per_attempt_timeout on every attempt.

Motivation

Observed on a mainnet debug-trace-server deploy: the internal witness generator endpoint sat behind a firewall that silently dropped SYNs, and every chain-sync block burned the whole 20s per-attempt budget on provider 0 before rotating to the working fallback (which then answered in ~60ms), capping sync throughput at ~0.1 blocks/s. Connection refusal (RST) already fails in milliseconds; only the no-reply case had no bound tighter than per_attempt_timeout — and that one cannot simply be lowered without also cutting the budget for legitimately slow multi-MB witness transfers. The two failure modes need separate knobs.

Change

  • New RpcClientConfig::connect_timeout field (default 3s, enough for a WAN handshake that loses its first SYN) wired through a single shared reqwest::Client (crates/stateless-common/src/rpc_client.rs:299); connection pools are keyed per host, so sharing one client across providers is behavior-neutral.
  • Providers are now constructed via alloy_rpc_client::ClientBuilder::http_with_client + RootProvider::new instead of ProviderBuilder::connect_http; the alloy builder path preserves the is_local guessing of the previous shape.
  • New workspace dependency alloy-rpc-client (same 1.0.23 family as the existing alloy deps, default-features = false; its reqwest feature is already enabled through alloy-provider via feature unification). Cargo.lock gains only the new dependency edge.

Testing

New test_connect_timeout_rotates_past_unreachable_provider: a backlog-1 listener with a pre-filled accept queue models the SYN-drop case; with connect_timeout=200ms and per_attempt_timeout=60s, get_latest_block_number rotates to the healthy second endpoint well inside a 5s outer bound (the test self-skips if the host kernel refuses to saturate the queue). Negative check performed locally: lifting the connect bound to 50s makes the test fail via the outer timeout, so the simulation genuinely hangs the connect phase rather than getting an RST. cargo test -p stateless-common: 41 passed; fmt/clippy/cargo-sort clean.

🤖 Generated with Claude Code

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mega-maxwell

mega-maxwell Bot commented Aug 1, 2026

Copy link
Copy Markdown

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

🛠️ Review did not finish

Attempted head 4664f997 · updated 2026-08-01T10:03:04+00:00

This round did not publish: MODEL_ACTION_FAILED in phase review_retry. Anything listed below is from the last round that did. Re-run the workflow or push a new commit to try again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant