Skip to content

Commit 1d804e2

Browse files
author
gluck
committed
fix(anthropicsdk): a spent quota fails the turn instead of sleeping through it
anthropic-sdk-go computes its own retry delay: if retryAfterDelay, ok := parseRetryAfterHeader(res); ok { return max(0, retryAfterDelay) // no ceiling } The 8-second cap in that function guards only the fallback exponential path. When the server sends a Retry-After the SDK sleeps for exactly that long, inside RequestConfig.Execute, with no callback, no log line and no stream event. We set no MaxRetries either, so the ceiling was the SDK default. That is right for a per-minute throttle. It is wrong for a usage WINDOW - a spent balance or a quota period - where Retry-After is "seconds until your window resets" and arrives in the thousands. Observed: one aria, two consecutive turns, 22 and 77 minutes of total silence, each ended by the user giving up rather than by the retry completing. `figaro status` said "idle" throughout. The account was simply out of credit for that model, and figaro responded by holding the conversation hostage until the balance renewed. Two things were wrong and both are fixed here. 1. It waited at all. Sleeping cannot make a spent quota unspent. What it CAN do is hold the agent loop, so the human who could fork, downshift the model, or top up the account cannot get a prompt in edgewise. Past maxRetryAfter (60s) the transport now sets x-should-retry:false - the SDK's own documented override, checked ahead of the status code in shouldRetry - so Execute returns the 429 immediately. The turn ends, and the aria is idle and promptable within the second. Retry-After is clamped as well, belt and braces, so no other consumer of that response can be talked into an hour-long nap either. A short throttle is still retried: the cap is a ceiling, not a refusal. 2. It said nothing. A rate limit produced no log record of any kind - the SDK swallowed it and slept - and the error that eventually surfaced said "rate_limit_error (429)" with no hint of the scale. Now every refusal writes a slog.Warn with the status, the wait asked for, the request id, the provider's own reset hint and what we did about it; and the error carries the wait, the reset time and what to do: anthropic: rate_limit_error (429): the provider asked us to wait 1h0m0s (limit resets 2026-08-14T20:00:00Z); that is longer than figaro will hold a turn open (1m0s), so the turn was ended rather than slept through - the aria is free, send again after the reset, or fork to a different model [req_011Ce3TTTY6NjCj66TBPgp3K] option.WithMaxRetries is also now set explicitly, so the worst-case wait is arithmetic this repo controls rather than a property of a dependency's release notes. There is a test asserting that product, one asserting a quota refusal returns at once, and one pinning the x-should-retry contract so we find out if a future SDK stops honoring it. retryCapTransport sits ABOVE wirelog in the chain so the ledger and the span still record what the server really sent; a diagnostic that recorded our own policy back to us would be worthless.
1 parent c9a59f1 commit 1d804e2

4 files changed

Lines changed: 517 additions & 6 deletions

File tree

‎internal/provider/anthropicsdk/anthropicsdk.go‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,17 @@ func New(knobs provider.Knobs, resolver auth.TokenResolver, cacheOpen func(aria
104104
rr = "tag"
105105
}
106106
return &Provider{
107-
resolver: resolver,
108-
model: knobs.Model,
109-
maxTokens: knobs.MaxTokens,
110-
reminder: rr,
111-
httpClient: &http.Client{Timeout: 10 * time.Minute, Transport: &wirelog.Transport{Inner: http.DefaultTransport}},
107+
resolver: resolver,
108+
model: knobs.Model,
109+
maxTokens: knobs.MaxTokens,
110+
reminder: rr,
111+
httpClient: &http.Client{
112+
Timeout: 10 * time.Minute,
113+
// retryCapTransport sits ABOVE wirelog so the ledger and the
114+
// span record the Retry-After the server really sent; only the
115+
// SDK's retry loop sees the clamped value.
116+
Transport: &retryCapTransport{Inner: &wirelog.Transport{Inner: http.DefaultTransport}},
117+
},
112118
CacheOpen: cacheOpen,
113119
CacheNamespace: providerName,
114120
}, nil
@@ -183,6 +189,10 @@ func (p *Provider) Send(ctx context.Context, in provider.SendInput, bus provider
183189
if dir := in.Snapshot.Lookup("system.environment.figaro_wire_dir"); dir != nil && *dir != "" {
184190
ctx = wirelog.WithLogging(ctx, in.AriaID, *dir)
185191
}
192+
// The SDK discards the response once it gives up retrying, so the
193+
// Retry-After that explains the failure would be lost. Carry a note the
194+
// transport can write into.
195+
ctx, note := withRateLimitNote(ctx)
186196

187197
cache, err := p.cacheFor(in.AriaID)
188198
if err != nil {
@@ -227,7 +237,7 @@ func (p *Provider) Send(ctx context.Context, in provider.SendInput, bus provider
227237
return nil
228238
})
229239
if err != nil {
230-
return cleanAPIError(err)
240+
return annotateRateLimit(cleanAPIError(err), note)
231241
}
232242
if len(msg.Content) == 0 {
233243
return nil

‎internal/provider/anthropicsdk/auth.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ func (p *Provider) authOptions(token, betas string) []option.RequestOption {
4040
opts := []option.RequestOption{
4141
option.WithoutEnvironmentDefaults(),
4242
option.WithHTTPClient(p.httpClient),
43+
// Set explicitly rather than inheriting the SDK's default, so the
44+
// worst-case wait a turn can absorb is a property of figaro and not
45+
// of a dependency's release notes. Paired with the Retry-After clamp
46+
// in retry.go this bounds a rate-limited turn at maxRetries *
47+
// maxRetryAfter instead of at the provider's window length.
48+
option.WithMaxRetries(maxRetries),
4349
}
4450
if isOAuthToken(token) {
4551
// Drop the SDK's default x-api-key (set to "" via WithoutEnvironmentDefaults
Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
package anthropicsdk
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"log/slog"
8+
"net/http"
9+
"strconv"
10+
"strings"
11+
"sync"
12+
"time"
13+
14+
"github.com/jack-work/figaro/internal/wirelog"
15+
)
16+
17+
// Why this file exists.
18+
//
19+
// anthropic-sdk-go computes its own retry delay, and its rule is:
20+
//
21+
// if retryAfterDelay, ok := parseRetryAfterHeader(res); ok {
22+
// return max(0, retryAfterDelay) // no ceiling
23+
// }
24+
//
25+
// The 8-second cap in that function applies only to the FALLBACK exponential
26+
// path. When the server sends a Retry-After the SDK sleeps for exactly that
27+
// long, inside RequestConfig.Execute, with no callback, no log line and no
28+
// stream event.
29+
//
30+
// That is fine for a per-minute throttle. It is not fine for a long-window
31+
// usage cap, where Retry-After is "seconds until your window resets" and can
32+
// be thousands. An aria sat in that sleep for 77 minutes, twice, and every
33+
// surface figaro has reported it as idle-but-unresponsive. There is no wait
34+
// long enough to be worth serving in silence: a caller who is told "rate
35+
// limited until 20:00" can fork, downshift the model, or go to lunch, and a
36+
// caller who is told nothing can only conclude the program is broken.
37+
//
38+
// So: clamp the header on the way in, cap the retry count explicitly rather
39+
// than inheriting a default, log every clamp, and put the number the server
40+
// actually asked for into the error we finally return.
41+
42+
var (
43+
// maxRetryAfter is the longest wait we are willing to sit through inside
44+
// a turn. Under it, retrying is the right thing: a brief throttle clears
45+
// and the turn continues.
46+
//
47+
// OVER it, retrying is not just useless but harmful. A wait measured in
48+
// tens of minutes is a usage WINDOW - a spent balance or a quota period -
49+
// and no amount of sleeping inside this turn will change the answer. All
50+
// it does is hold the aria busy, so the human who could fork, downshift
51+
// the model, or top up the account cannot even get a prompt in. We refuse
52+
// the retry, return the 429 immediately, and hand the aria back.
53+
maxRetryAfter = 60 * time.Second
54+
55+
// maxRetries is set explicitly instead of inheriting the SDK's default,
56+
// so the worst-case silent wait is a property of this file and not of a
57+
// dependency's release notes.
58+
maxRetries = 2
59+
)
60+
61+
// rateLimitNote is what the transport saw the last time the provider refused
62+
// us. It rides on the request context so Send can put it in the error after
63+
// the SDK has given up and thrown the response away.
64+
type rateLimitNote struct {
65+
mu sync.Mutex
66+
seen bool
67+
status int
68+
askedFor time.Duration // Retry-After exactly as sent
69+
requestID string
70+
resetHint string // provider's own "when" from its ratelimit headers
71+
clampCount int
72+
}
73+
74+
func (n *rateLimitNote) record(status int, askedFor time.Duration, requestID, resetHint string, clamped bool) {
75+
n.mu.Lock()
76+
defer n.mu.Unlock()
77+
n.seen = true
78+
n.status = status
79+
n.askedFor = askedFor
80+
n.requestID = requestID
81+
if resetHint != "" {
82+
n.resetHint = resetHint
83+
}
84+
if clamped {
85+
n.clampCount++
86+
}
87+
}
88+
89+
func (n *rateLimitNote) snapshot() (rateLimitNote, bool) {
90+
n.mu.Lock()
91+
defer n.mu.Unlock()
92+
return rateLimitNote{
93+
seen: n.seen, status: n.status, askedFor: n.askedFor,
94+
requestID: n.requestID, resetHint: n.resetHint, clampCount: n.clampCount,
95+
}, n.seen
96+
}
97+
98+
type noteKey struct{}
99+
100+
func withRateLimitNote(ctx context.Context) (context.Context, *rateLimitNote) {
101+
n := &rateLimitNote{}
102+
return context.WithValue(ctx, noteKey{}, n), n
103+
}
104+
105+
func noteFromContext(ctx context.Context) *rateLimitNote {
106+
n, _ := ctx.Value(noteKey{}).(*rateLimitNote)
107+
return n
108+
}
109+
110+
// retryCapTransport clamps an over-long Retry-After before the SDK's retry
111+
// loop can honor it.
112+
//
113+
// It sits ABOVE wirelog in the chain so the ledger and the span record the
114+
// header the server really sent; only the SDK sees the clamped value.
115+
type retryCapTransport struct {
116+
Inner http.RoundTripper
117+
Max time.Duration
118+
Aria string
119+
}
120+
121+
func (t *retryCapTransport) RoundTrip(req *http.Request) (*http.Response, error) {
122+
inner := t.Inner
123+
if inner == nil {
124+
inner = http.DefaultTransport
125+
}
126+
resp, err := inner.RoundTrip(req)
127+
if err != nil || resp == nil {
128+
return resp, err
129+
}
130+
if !transientStatus(resp.StatusCode) {
131+
return resp, nil
132+
}
133+
134+
asked := wirelog.RetryAfter(resp.Header)
135+
max := t.Max
136+
if max <= 0 {
137+
max = maxRetryAfter
138+
}
139+
clamped := asked > max
140+
141+
requestID := resp.Header.Get("request-id")
142+
if requestID == "" {
143+
requestID = resp.Header.Get("x-request-id")
144+
}
145+
resetHint := firstResetHint(resp.Header)
146+
147+
if n := noteFromContext(req.Context()); n != nil {
148+
n.record(resp.StatusCode, asked, requestID, resetHint, clamped)
149+
}
150+
151+
// Say it out loud. Before this, a rate limit produced no log record of
152+
// any kind: the SDK swallowed it and slept.
153+
action := "retrying"
154+
if clamped {
155+
action = "giving up now (wait exceeds cap; the turn will fail so the aria stays promptable)"
156+
}
157+
slog.Warn("anthropic refused a request",
158+
"aria", t.Aria,
159+
"status", resp.StatusCode,
160+
"retry_after", asked.String(),
161+
"cap", max.String(),
162+
"action", action,
163+
"request_id", requestID,
164+
"reset", resetHint)
165+
166+
if clamped {
167+
// x-should-retry is the SDK's own documented override, checked ahead
168+
// of the status code in requestconfig.shouldRetry. Setting it false
169+
// makes Execute return this response NOW instead of sleeping: the
170+
// turn fails, the error carries the wait, and the aria is idle and
171+
// promptable again within the second.
172+
//
173+
// Retry-After is clamped as well, so that any other consumer of this
174+
// response - a future SDK that stops honoring x-should-retry, a
175+
// middleware, a test - still cannot be talked into an hour-long nap.
176+
resp.Header.Set("x-should-retry", "false")
177+
resp.Header.Set("Retry-After", formatSeconds(max))
178+
}
179+
return resp, nil
180+
}
181+
182+
// transientStatus mirrors what the SDK will retry: rate limit, anthropic's
183+
// overloaded, and 5xx.
184+
func transientStatus(code int) bool {
185+
return code == http.StatusTooManyRequests || code == 529 || (code >= 500 && code <= 599)
186+
}
187+
188+
func formatSeconds(d time.Duration) string {
189+
s := int64(d / time.Second)
190+
if s < 1 {
191+
s = 1
192+
}
193+
return strconv.FormatInt(s, 10)
194+
}
195+
196+
// firstResetHint pulls the provider's own answer to "when will this clear"
197+
// out of its ratelimit headers. Which one is present varies by which limit
198+
// was hit, so take the earliest that names a reset.
199+
func firstResetHint(h http.Header) string {
200+
best := ""
201+
for k, vs := range h {
202+
if len(vs) == 0 {
203+
continue
204+
}
205+
lk := strings.ToLower(k)
206+
if !strings.Contains(lk, "ratelimit") || !strings.Contains(lk, "reset") {
207+
continue
208+
}
209+
if best == "" || vs[0] < best {
210+
best = vs[0]
211+
}
212+
}
213+
return best
214+
}
215+
216+
// annotateRateLimit turns a bare "429" into an answer.
217+
//
218+
// By the time the SDK gives up it has thrown away the response, so the error
219+
// that reaches the user says "rate_limit_error (429)" and nothing about how
220+
// long the wait actually is. That is the difference between a user who forks
221+
// or downshifts and a user who files a bug about a hang. The note carries
222+
// what the transport saw.
223+
func annotateRateLimit(err error, note *rateLimitNote) error {
224+
if err == nil || note == nil {
225+
return err
226+
}
227+
n, ok := note.snapshot()
228+
if !ok || n.askedFor <= 0 {
229+
return err
230+
}
231+
msg := fmt.Sprintf("%v: the provider asked us to wait %s", err, n.askedFor.Round(time.Second))
232+
if n.resetHint != "" {
233+
msg += fmt.Sprintf(" (limit resets %s)", n.resetHint)
234+
}
235+
if n.clampCount > 0 {
236+
// Say what to DO. A wait this long is a spent quota, and the actions
237+
// that help are all outside this turn.
238+
msg += fmt.Sprintf("; that is longer than figaro will hold a turn open (%s), so the turn was ended"+
239+
" rather than slept through - the aria is free, send again after the reset,"+
240+
" or fork to a different model", maxRetryAfter)
241+
}
242+
if n.requestID != "" {
243+
msg += fmt.Sprintf(" [%s]", n.requestID)
244+
}
245+
return errors.New(msg)
246+
}

0 commit comments

Comments
 (0)