Skip to content

Commit 3051655

Browse files
fix(login): don't abort browser login when a single poll request stalls (#26)
A single poll of /cli-session/<nonce>/key that outlived the 5s per-request http.Client timeout escaped the retry loop as fatal: since Go 1.16 the client's timeout error matches errors.Is(err, context.DeadlineExceeded), so it was indistinguishable from the overall login deadline and rendered as "login timed out" after ~5 seconds instead of the advertised 90. Classify the terminal condition via ctx.Err() (the outer deadline or a cancel) instead of the error sentinel; per-request stalls now fold into pollTransient and the loop keeps polling. The client timeout moves to a package-level var so the regression test can shrink it — the test stalls the first response past the client timeout, serves the key on the second, and asserts the poll survives (fails before the fix, passes after). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent eb6dfc9 commit 3051655

2 files changed

Lines changed: 50 additions & 3 deletions

File tree

cmd/login.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ import (
2828
// 5-minute server-side TTL.
2929
const pollInterval = 1500 * time.Millisecond
3030

31+
// pollRequestTimeout bounds a SINGLE poll attempt. A request that
32+
// outlives it is a transient failure (retry), never the overall login
33+
// deadline. Var rather than const so tests can shrink it.
34+
var pollRequestTimeout = 5 * time.Second
35+
3136
var (
3237
loginProfile string
3338
loginURL string
@@ -240,7 +245,7 @@ func browserSessionPollLogin(out io.Writer, asJSON bool, profileName, baseURL st
240245
func pollSessionKey(ctx context.Context, baseURL, nonce string, interval time.Duration) (string, error) {
241246
endpoint := fmt.Sprintf("%s/ai-api/v1/cli-session/%s/key",
242247
strings.TrimRight(baseURL, "/"), nonce)
243-
client := &http.Client{Timeout: 5 * time.Second}
248+
client := &http.Client{Timeout: pollRequestTimeout}
244249

245250
for {
246251
key, status, err := pollSessionOnce(ctx, client, endpoint)
@@ -278,9 +283,15 @@ func pollSessionOnce(ctx context.Context, client *http.Client, endpoint string)
278283
}
279284
resp, err := client.Do(req)
280285
if err != nil {
281-
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
282-
return "", pollPending, err
286+
if ctx.Err() != nil {
287+
// The OVERALL login deadline (or a cancel) fired — stop.
288+
return "", pollPending, ctx.Err()
283289
}
290+
// Everything else is transient and must keep the loop polling.
291+
// That includes the client's own per-request timeout, whose
292+
// error matches errors.Is(err, context.DeadlineExceeded) since
293+
// Go 1.16 — checking the error instead of ctx.Err() here used
294+
// to abort the whole login as "timed out" after one slow poll.
284295
return "", pollTransient, nil
285296
}
286297
defer resp.Body.Close()

cmd/login_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,42 @@ func TestPollSessionKey_FatalOn400(t *testing.T) {
171171
}
172172
}
173173

174+
func TestPollSessionKey_SlowRequestIsTransientNotTimeout(t *testing.T) {
175+
// Regression: a single poll request that outlived the per-request
176+
// http.Client timeout used to be reported as context.DeadlineExceeded
177+
// (the client wraps its timeout error to match that sentinel since
178+
// Go 1.16), which the login flow renders as "login timed out" — long
179+
// before the overall 90s deadline. A stalled request must instead be
180+
// treated as transient and the loop must keep polling.
181+
prev := pollRequestTimeout
182+
pollRequestTimeout = 50 * time.Millisecond
183+
defer func() { pollRequestTimeout = prev }()
184+
185+
var calls atomic.Int32
186+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
187+
if calls.Add(1) == 1 {
188+
time.Sleep(250 * time.Millisecond) // outlive the client timeout
189+
return
190+
}
191+
w.WriteHeader(http.StatusOK)
192+
_, _ = w.Write([]byte(`{"plaintext_key":"sk_live_AFTER_STALL"}`))
193+
}))
194+
defer srv.Close()
195+
196+
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
197+
defer cancel()
198+
got, err := pollSessionKey(ctx, srv.URL, "n", fastPoll)
199+
if err != nil {
200+
t.Fatalf("a single stalled request should be transient, got err = %v", err)
201+
}
202+
if got != "sk_live_AFTER_STALL" {
203+
t.Errorf("got %q, want sk_live_AFTER_STALL", got)
204+
}
205+
if calls.Load() < 2 {
206+
t.Errorf("expected a retry after the stalled request, got %d calls", calls.Load())
207+
}
208+
}
209+
174210
func TestPollSessionKey_TimeoutReturnsDeadlineExceeded(t *testing.T) {
175211
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
176212
w.WriteHeader(http.StatusNoContent) // always pending

0 commit comments

Comments
 (0)