Skip to content

feat: return a typed RateLimitError carrying the retry hint - #65

Merged
jferrl merged 1 commit into
mainfrom
feat/rate-limit-error
Sep 18, 2026
Merged

jferrl merged 1 commit into
mainfrom
feat/rate-limit-error

Conversation

@jferrl

@jferrl jferrl commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

ErrRateLimited is a bare sentinel, so a caller running its own backoff can tell that GitHub throttled the request but not for how long. The client had already read Retry-After and X-RateLimit-Reset, capped the result and picked a fallback, then discarded all of it into an error string the caller would have to re-parse from a response it never sees. RateLimitError carries that work forward.

Changes

  • Adds RateLimitError{StatusCode, RetryAfter, Message}, extracted with errors.As
  • It unwraps to ErrRateLimited, so errors.Is(err, ErrRateLimited) keeps working; every pre-existing test asserting the sentinel passes untouched
  • The retry path recovers the delay with errors.As rather than threading it through a third return value, so doCreateInstallationToken returns the usual two

Rationale

The rendered message is byte-identical to the old fmt.Errorf output, and Test_RateLimitError_MessageFormat pins that string because callers log and match on it. RetryAfter is zero when GitHub says to retry immediately or when the reset instant has already passed; the field documents that as "retry now" rather than "no hint", since the two are indistinguishable from the value alone. Test_RateLimitError_NotReturnedForTerminalFailures guards the boundary from the other side: a 403 permission failure and a plain 404 must match neither errors.As nor errors.Is.

Migration Notes

Purely additive. errors.Is(err, ErrRateLimited) and the error string are unchanged, so existing callers need no edits.

ErrRateLimited was a bare sentinel, so a caller running its own backoff
could tell that GitHub had throttled the request but not for how long.
The client had already parsed Retry-After and X-RateLimit-Reset, capped
the result and chosen a fallback, then discarded all of it into an error
string the caller would have to re-parse.

RateLimitError carries StatusCode, RetryAfter and the response message.
It unwraps to ErrRateLimited, so errors.Is keeps working and the rendered
message is byte-identical to before; a test pins that string, since
callers log and match on it.

The retry path now recovers the delay with errors.As instead of threading
it through a third return value, so doCreateInstallationToken returns the
usual two.

RetryAfter is zero when GitHub says to retry immediately or when the
reset instant has already passed, which is documented as "retry now"
rather than "no hint", since the two are indistinguishable in the value
alone.
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.00%. Comparing base (2a37180) to head (6f8b6fe).

Files with missing lines Patch % Lines
github.go 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
+ Coverage   96.94%   97.00%   +0.05%     
==========================================
  Files           4        4              
  Lines         295      300       +5     
==========================================
+ Hits          286      291       +5     
  Misses          9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jferrl
jferrl merged commit bb157e1 into main Sep 18, 2026
9 of 10 checks passed
@jferrl
jferrl deleted the feat/rate-limit-error branch September 18, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant