mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-12 10:13:37 +00:00
nguyenha935andnguyenha935
496b7ffce6
A reasoning model emits nothing at all while thinking, so the wait for the response's FIRST byte — not any streaming timeout — decides whether its answer is reachable. At 180s a large-prompt reasoning request routinely lost that race: the gateway reported "http2: timeout awaiting response headers" while the router's own usage log showed the same request completing normally moments later (promptTokens=77194 -> completionTokens=2789). The request was healthy; only the deadline was short. Raise it to 300s. The value is deliberately modest rather than generous because it is multiplied twice downstream: RetryDo wraps every provider call with Attempts: 3, and a workflow step requeues up to MaxTaskDispatches times on top of that, so 300s is already ~45min of worst-case wall clock per step. Removing the timeout entirely was considered and rejected. Agent run contexts come from scheduler queue.go via context.WithCancel, so they carry no deadline — this transport timeout is the only thing bounding a request that never answers. Without it a silently dead connection would hold its scheduler lane until the OS TCP keepalive expired, and workflow requeue could not rescue it because requeue only fires when a run ends. The doc comment claiming the design could "rely on ctx deadlines" was wrong and is corrected here. Tests lock the sizing from both sides (must exceed the latency that failed, must not exceed the ceiling the retry math was sized against) and assert the retry multiplier itself has not drifted. Co-authored-by: nguyenha935 <nguyenha935@users.noreply.github.com>
60 lines
2.7 KiB
Go
60 lines
2.7 KiB
Go
package providers
|
|
|
|
import (
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// A reasoning model emits nothing while thinking, so ResponseHeaderTimeout —
|
|
// not any streaming timeout — is what decides whether its answer is reachable.
|
|
// At 180s, claude-opus-5-thinking with a ~110k-token prompt routinely lost the
|
|
// race: GoClaw reported "http2: timeout awaiting response headers" while
|
|
// 9router's usage log showed the same request answering fine
|
|
// (promptTokens=77194 → completionTokens=2789). Live 2026-07-28.
|
|
func TestResponseHeaderTimeoutAccommodatesReasoningModels(t *testing.T) {
|
|
t.Parallel()
|
|
tr := NewDefaultTransport()
|
|
|
|
// The observed first-byte latency that failed at 180s. The timeout must sit
|
|
// above it with room to spare.
|
|
const observedSlowFirstByte = 180 * time.Second
|
|
if tr.ResponseHeaderTimeout <= observedSlowFirstByte {
|
|
t.Errorf("ResponseHeaderTimeout = %v, must exceed the %v first-byte latency that failed live",
|
|
tr.ResponseHeaderTimeout, observedSlowFirstByte)
|
|
}
|
|
|
|
// It must stay bounded. RetryDo multiplies it by Attempts, and a workflow
|
|
// step requeues up to MaxTaskDispatches times on top of that, so an
|
|
// over-generous value turns one slow provider into an hours-long workflow.
|
|
// 3 (retry) x 3 (requeue) x 300s = 45min worst case, which is the ceiling
|
|
// this value was chosen against.
|
|
const maxAcceptable = 300 * time.Second
|
|
if tr.ResponseHeaderTimeout > maxAcceptable {
|
|
t.Errorf("ResponseHeaderTimeout = %v, exceeds %v; retry x requeue multiplies this ~9x",
|
|
tr.ResponseHeaderTimeout, maxAcceptable)
|
|
}
|
|
}
|
|
|
|
// Removing ResponseHeaderTimeout entirely was considered and rejected: agent run
|
|
// contexts come from scheduler queue.go via context.WithCancel, so they carry no
|
|
// deadline. With no header timeout, a silently dead connection would hold its
|
|
// scheduler lane until the OS TCP keepalive expired (~2h), and workflow requeue
|
|
// could not rescue it because requeue only fires when a run ENDS. This timeout is
|
|
// the only bound on a request that never answers — it must stay non-zero.
|
|
func TestResponseHeaderTimeoutIsSet(t *testing.T) {
|
|
t.Parallel()
|
|
if got := NewDefaultTransport().ResponseHeaderTimeout; got == 0 {
|
|
t.Fatal("ResponseHeaderTimeout is 0 (unbounded); nothing else bounds an LLM call, " +
|
|
"so a dead connection would wedge its scheduler lane indefinitely")
|
|
}
|
|
}
|
|
|
|
// The retry multiplier this timeout is sized against must not drift silently.
|
|
func TestDefaultRetryAttemptsMatchTimeoutSizing(t *testing.T) {
|
|
t.Parallel()
|
|
if got := DefaultRetryConfig().Attempts; got != 3 {
|
|
t.Errorf("DefaultRetryConfig().Attempts = %d, want 3 — ResponseHeaderTimeout was sized "+
|
|
"assuming a 3x retry multiplier; re-check that sizing if this changed", got)
|
|
}
|
|
}
|