Visitar URL original
fix(httpclient): honour Retry-After and rate limit reset headers by olblak · Pull Request #10949 · updatecli/updatecli · GitHub
Skip to content

fix(httpclient): honour Retry-After and rate limit reset headers - #10949

Merged
olblak merged 3 commits into
updatecli:mainfrom
olblak:fix/httpclient/retry-after
Oct 21, 2026
Merged

fix(httpclient): honour Retry-After and rate limit reset headers#10949
olblak merged 3 commits into
updatecli:mainfrom
olblak:fix/httpclient/retry-after

Conversation

@olblak

@olblak olblak commented Oct 21, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

The retry client waited a fixed 1s, 2s and 4s between retries, 7s in all, which can't outlast a GitLab throttle window of a minute, so throttled GitLab calls failed.

A retry now waits as long as the Retry-After header asks, as seconds or an HTTP date. On a 429 without it, the wait comes from Ratelimit-Reset (GitLab) or X-RateLimit-Reset (GitHub). Those headers are read on a 429 only, as both forges send them on every response.

The waits for one request are bounded to two minutes in all, a wait stops when the request context is done, and the previous response is drained before waiting so the connection isn't held meanwhile.

Test

To test this pull request, you can run the following commands:

cd pkg/core/httpclient
go test ./...

Additional Information

Checklist

  • I have updated the documentation via pull request in website repository.
  • I have tested this pull request manually with a custom Updatecli build and it works as expected.

Tradeoff

Potential improvement

Summary by CodeRabbit

  • Bug Fixes
    • HTTP requests now respect server-provided retry timing and rate-limit reset information.
    • Retry delays fall back to exponential backoff when server guidance is unavailable.
    • Retry waits can be interrupted when a request is canceled.
    • Requests stop retrying after a cumulative two-minute wait limit.
    • Retry logging now includes the request method, URL, and delay.

@olblak
fix(httpclient): honour Retry-After and rate limit reset headers
edba64c 
The retry client waited a fixed 1s, 2s and 4s between retries, 7s in all,
which can't outlast a GitLab throttle window of a minute, so throttled
GitLab calls failed.

A retry now waits as long as the Retry-After header asks, as seconds or an
HTTP date. On a 429 without it, the wait comes from Ratelimit-Reset
(GitLab) or X-RateLimit-Reset (GitHub). Those headers are read on a 429
only, as both forges send them on every response.

The waits for one request are bounded to two minutes in all, a wait stops
when the request context is done, and the previous response is drained
before waiting so the connection isn't held meanwhile.

Signed-off-by: Olblak <me@olblak.com>
@olblak olblak added bug Something isn't working core All things related to Updatecli core engine labels Oct 21, 2026
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d81516ad-cae1-4289-b06d-dedf2234e499
📥 Commits

Reviewing files that changed from the base of the PR and between 3827554 and 9b73fbc.

📒 Files selected for processing: 3
  • pkg/core/httpclient/retry.go
  • pkg/core/httpclient/retry_test.go
  • pkg/plugins/resources/gitlab/client/main.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The HTTP retry transport now uses server-provided retry headers when available, falls back to exponential backoff, and limits cumulative retry waiting to two minutes. Retry waits can be interrupted by request cancellation. Tests cover delay selection and retry behavior.

Changes: HTTP retry behavior

Layer / File(s) Summary
Retry delay selection and parsing
pkg/core/httpclient/retry.go, pkg/core/httpclient/retry_test.go		 The retry logic parses Retry-After seconds or dates and rate-limit reset headers, then falls back to backoff when needed. Tests cover parsing, precedence, and status-specific behavior.
Retry execution and validation
pkg/core/httpclient/retry.go, pkg/core/httpclient/retry_test.go, pkg/plugins/resources/gitlab/client/main.go		 The transport enforces a two-minute cumulative wait limit, drains responses, logs retry details, and returns the request context error when a wait is canceled. Tests cover wait limits, immediate retries, and cancellation. The GitLab client comment names the supported retry headers.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant RetryTransport
  participant HTTPServer
  participant RequestContext
  RetryTransport->>HTTPServer: Send HTTP request
  HTTPServer-->>RetryTransport: Return response with retry headers
  RetryTransport->>RetryTransport: Select delay and check cumulative limit
  alt Delay is within limit
    RetryTransport->>RequestContext: Wait for delay or cancellation
    RequestContext-->>RetryTransport: Delay elapsed
    RetryTransport->>HTTPServer: Retry request
  else Request context is canceled
    RequestContext-->>RetryTransport: Cancellation signal
    RetryTransport-->>RetryTransport: Return context error
  else Delay exceeds limit
    RetryTransport-->>RetryTransport: Return latest response
  end
Loading

Suggested reviewers: loispostula

Merge Risk: ⚪ Minimal · up to 9b73f

Retry waits respect the existing request deadlines; no actionable merge-blocking risk remains after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: honoring Retry-After and rate-limit reset headers in the HTTP client.
Description check Passed The description explains the problem, implementation, behavior, test command, and tradeoffs. The Tradeoff and Potential improvement sections are empty, and no issue reference replaces the Fix #XXX tem…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, implementation, behavior, test command, and tradeoffs. The Tradeoff and Potential improvement sections are empty, and no issue reference replaces the Fix #XXX template line, but the description is otherwise complete and relevant.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@olblak olblak mentioned this pull request Oct 21, 2026
Merged
2 tasks
@olblak
olblak merged commit 0c12681 into updatecli:main Oct 21, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Something isn't working core All things related to Updatecli core engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@olblak