Skip to content

fix(executor): keep accounts routable after a transport failure - #437

Merged
caigee-cmd merged 2 commits into
caigee-cmd:mainfrom
yoruuuchan:fix/transport-failure-no-cooldown
Oct 29, 2026
Merged

caigee-cmd merged 2 commits into
caigee-cmd:mainfrom
yoruuuchan:fix/transport-failure-no-cooldown

Conversation

@yoruuuchan

@yoruuuchan yoruuuchan commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • A dial, TLS handshake, or reset failure never reaches the upstream, but it fell through ClassifyError to the generic unavailable branch: 15s account cooldown + failover. The account gets parked although its credential, quota, and rate limit were never exercised.
  • On a single-account route that is a routing outage, not a hiccup: every request that follows is answered with 429 rate_limit "all accounts are cooling down" (Retry-After = the remainder of the 15s), so the real cause — the transport error — never reaches the client. With a 1-account provider route there is no other account to fail over to, so the account cools itself in a loop: handshake timeout → 502 → 15s cooldown → 429s → cooldown expires → handshake timeout again.
  • This PR classifies a *url.Error as upstream_unreachable: 502, no cooldown, no failover, message kept. Detection is shape-based: http.Client returns *url.Error only when no response arrived, so an upstream (or proxy) 502 whose body merely quotes net/http: TLS handshake timeout keeps the existing 15s cooldown.

Observed upstream shape:

Post "https://api1.qoder.sh/algo/api/v2/service/pro/sse/agent_chat_generation?...": net/http: TLS handshake timeout

GET / on the same host succeeded seconds later, and other overseas providers showed the same handshake timeouts in the same window — i.e. an intermittent egress problem, not a sick account.

Test plan

  • go test ./... — CI green (go, windows-host, container, frontend, worker, secrets). Untouched packages are unaffected.
  • go vet ./... — CI green
  • cd worker && npm test — untouched, CI green
  • cd frontend && npm run build && npm run lint — untouched, CI green
  • Added changelog/unreleased/transport-failure-no-cooldown.md
  • No tokens, auth blobs, raw captures, or host details in the diff
  • New tests: TestClassifyErrorTransportFailureKeepsAccountRoutable (asserts the account is still picked by PickRoute afterwards), TestClassifyErrorWrappedTransportFailure, and TestClassifyErrorUpstreamQuotedHandshakeTimeoutStillCools (pins that detection is shape-based, not message-based)

Note for local runners: on my Windows box a clean 2da8c45 checkout already fails TestRepeatedFailuresBackOff and TestBackoffResetsOnKindChange plus a few env-dependent packages. Verified by stashing this diff. CI does not reproduce it, so it is local-environment noise, not this PR.

Two judgment calls

  1. Failover: false. Every account in a deployment shares one egress path unless it sets its own proxy_url, so failing over just repeats the same 8–11s handshake per account and ends at the same 502. If you prefer to keep one failover attempt for per-account-proxy setups, flipping this field is the whole change — upstream_empty already does Failover: true, Cooldown: 0 for the same "the account is fine" reasoning.
  2. Scope is pre-headers only. A connection lost mid-body still cools the account through ObserveStreamFailure, which has its own documented reason (bytes are already on the wire) and is untouched here.

yoruuuchan and others added 2 commits October 29, 2026 18:02
A dial, TLS handshake, or reset failure never reaches the upstream, but it
fell through to the generic unavailable branch: 15s cooldown plus failover.
On a single-account route — the normal personal setup — that parks the only
usable account, and every following request is answered with
"all accounts are cooling down" instead of the real transport error.

Classify a *url.Error as upstream_unreachable: 502, no cooldown, no failover.
Detection is shape-based, so an upstream 502 body that merely quotes
"TLS handshake timeout" still cools the account as before.
@caigee-cmd
caigee-cmd merged commit b042a49 into caigee-cmd:main Oct 29, 2026
5 of 6 checks passed
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.

2 participants