Skip to content

Commit 4f7e1ea

Browse files
appleboyclaude
andcommitted
fix(review): address PR feedback from Copilot review
- Fix double-close of response body by using explicit close in 401 path and defer in else branch instead of unconditional defer - Increase slow_down test timeout from 15s to 25s to prevent flakiness - Update pollForTokenWithProgress doc comment to say "additive backoff" instead of "exponential backoff" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7994db9 commit 4f7e1ea

2 files changed

Lines changed: 6 additions & 4 deletions

File tree

main.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ func performDeviceFlow(ctx context.Context, d tui.Displayer) (credstore.Token, e
527527
}
528528

529529
// pollForTokenWithProgress polls for token while reporting progress via Displayer.
530-
// Implements exponential backoff for slow_down errors per RFC 8628.
530+
// Implements additive backoff (+5s) for slow_down errors per RFC 8628 §3.5.
531531
func pollForTokenWithProgress(
532532
ctx context.Context,
533533
config *oauth2.Config,
@@ -824,7 +824,6 @@ func makeAPICallWithAutoRefresh(
824824
if err != nil {
825825
return fmt.Errorf("API request failed: %w", err)
826826
}
827-
defer resp.Body.Close()
828827

829828
// If 401, drain and close the first response body to allow connection reuse,
830829
// then refresh the token and retry.
@@ -866,6 +865,8 @@ func makeAPICallWithAutoRefresh(
866865
return fmt.Errorf("retry failed: %w", err)
867866
}
868867
defer resp.Body.Close()
868+
} else {
869+
defer resp.Body.Close()
869870
}
870871

871872
body, err := io.ReadAll(resp.Body)

polling_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,9 @@ func TestPollForToken_SlowDown(t *testing.T) {
125125
Interval: 1, // 1 second for testing
126126
}
127127

128-
// After 1 slow_down the interval becomes 1+5=6s, so we need ~8s total.
129-
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
128+
// After 1 slow_down the interval becomes 1+5=6s; with an additional authorization_pending
129+
// before success, the third attempt occurs after ~1s + 6s + 6s ≈ 13s, so use a generous timeout.
130+
ctx, cancel := context.WithTimeout(context.Background(), 25*time.Second)
130131
defer cancel()
131132

132133
token, err := pollForTokenWithProgress(ctx, config, deviceAuth, tui.NoopDisplayer{})

0 commit comments

Comments
 (0)