fix: Include RetryAfter in AbuseRateLimitError.Error output#4181
fix: Include RetryAfter in AbuseRateLimitError.Error output#4181gmlewis merged 1 commit intogoogle:masterfrom
RetryAfter in AbuseRateLimitError.Error output#4181Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
48bab6a to
6bf9f02
Compare
|
hmm, trying to figure out CLA magic |
6bf9f02 to
4ceebec
Compare
When RetryAfter is populated, append '[retry after <duration>]' to the error string. This makes the retry information visible to callers that surface .Error() (e.g. logging, AI agent frameworks) without needing to use errors.As to inspect the struct directly. This is consistent with RateLimitError.Error() which already includes timing via formatRateReset. Fixes google#4180.
4ceebec to
ca9426b
Compare
|
OK CLA is good. Ready for review. |
|
BTW @gmlewis I wonder whther it might be good in the repo settings to enable copilot code review agent on PR's ? I've gotten used to it. Thats if you didn't try it already and found insufficient value. |
I no longer have authorization to modify |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4181 +/- ##
=======================================
Coverage 93.69% 93.69%
=======================================
Files 210 210
Lines 19760 19763 +3
=======================================
+ Hits 18514 18517 +3
Misses 1049 1049
Partials 197 197 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
RetryAfter in AbuseRateLimitError.Error output
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @danmoseley!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra - @munlicode
|
Thank you, @Not-Dhananjay-Mishra! |
When
AbuseRateLimitErrorincludes a populatedRetryAfterfield (from theRetry-AfterHTTP header), the.Error()string silently drops it:This is inconsistent with
RateLimitError.Error(), which already includes timing viaformatRateReset:Why this matters now
This gap has existed since 2016, but was minor because Go callers can use
errors.Asto inspect.RetryAfterdirectly.It has become more impactful with AI coding agents. The github-mcp-server exposes GitHub API operations as MCP tools, and AI agents receive tool results as plain strings — there is no way for the model to inspect the underlying Go struct. When an agent hits a secondary rate limit, it sees no retry duration and must choose between retrying immediately (worsening the situation) or waiting an arbitrary amount of time.
Including the duration in
.Error()gives agent frameworks and models the information they need to back off correctly.After this change
When
RetryAfteris nil or zero, the output is unchanged.Fixes #4180.
Note
This PR was authored with the assistance of GitHub Copilot.