fix(gmail): retry 403 rateLimitExceeded and 5xx with exponential backoff - #80
Open
jqueguiner wants to merge 2 commits into
Open
fix(gmail): retry 403 rateLimitExceeded and 5xx with exponential backoff#80jqueguiner wants to merge 2 commits into
jqueguiner wants to merge 2 commits into
Conversation
Gmail enforces a per-user quota shared by every process on the account, so a burst from one client can rate-limit another. The API client had no retry: all 14 call sites ended in .error_for_status(), turning a transient 429 into a fatal error. Adds a bounded retry with full jitter, honouring Retry-After when present. Non-retryable statuses (401, 403, 404) still fail fast, so a scope error or a missing thread does not burn quota waiting for an answer that cannot change. Two existing tests asserted that a 429/500 failed on the first response. They now assert the status survives once retries are exhausted, which is the property that actually matters. Refs MaximeGaudin#79
Retrying only 429 missed the failure this was meant to fix. Gmail does not answer 429 for the per-user quota: it answers 403 with `reason: rateLimitExceeded`. Measured over 2026-09-06 to 2026-09-11 on a shared account: 16 `403 rateLimitExceeded`, 3 `404 notFound`, 0 Google 429. 403 is now decided by Google's reason, not the status. Retried on `rateLimitExceeded`, `userRateLimitExceeded`, `quotaExceeded`, `backendError`. Every other 403 still fails fast, including `ACCESS_TOKEN_SCOPE_INSUFFICIENT` and permission errors: retrying those burns quota and hides an auth problem. The reason parser lives in `error.rs`, next to `is_insufficient_scope_body`, so one module knows Google's error shapes. It reads both `error.errors[].reason` and `error.details[].reason`, and matches reason tokens only, never loose prose like "quota". Deciding on a 403 consumes the response, so the body is buffered once and the response rebuilt from its own parts. A non-retryable 403 reaches the caller with its status, headers, URL and body intact, which is what lets `json_or_scope_error` still map a scope error to `InsufficientScope`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Retry transient Gmail API failures instead of surfacing them as fatal errors.
api/retry.rs: bounded exponential backoff with full jitter, honouringRetry-After(both the seconds form and the HTTP-date form).500 ms base, 16 s cap.
reasonsays the quota was hit:rateLimitExceeded,userRateLimitExceeded,quotaExceeded,backendError.ACCESS_TOKEN_SCOPE_INSUFFICIENTandpermission errors included. So do 401 and 404.
.send()becomes.send_retrying(&self.retry).First of the items in #79, smallest first.
Why
Gmail's quota (
Total Query Cost) is counted per user per minute and is shared byevery process authenticated as that account. One client's burst rate-limits the
others.
The status that carries it is 403, not 429. Counted on a shared account running
several agent sessions, over 2026-09-06 to 2026-09-11:
403 rateLimitExceeded404 notFound429(any)So a retry predicate keyed on 429 would have caught none of them. 429 and 5xx are
still retried, since Gmail can send either, but 403 is the case that actually
happens.
403 cannot be decided by its status: it is also what Gmail returns for a missing
scope, a denied permission or a domain policy, none of which clear by waiting. The
discriminator is Google's
reasonfield in the response body. Retrying a scopeerror burns quota and hides a real auth problem, so the predicate is a positive
list of quota reasons, not "403 unless proven otherwise".
Before this change all 14 call sites in
api/client.rsended in.error_for_status(), soRetry-Afterwas discarded and the read was lost.Jitter is full jitter on purpose. Without it, several clients that hit the same
limit wake up together and collide again.
One parser, in
error.rserror.rsalready knew how to read a Google reason (is_insufficient_scope_body,matching
ACCESS_TOKEN_SCOPE_INSUFFICIENT). The quota check went next to it ratherthan into a second parser in
retry.rs, so one module knows Google's error shapes.It reads both shapes Google uses for the same field,
error.errors[].reasonanderror.details[].reason, and matches reason tokens only. Loose prose is not areason:
"You have exceeded your daily quota of patience."is asserted notretryable. A scope error also wins over a quota match, in case a body carries both.
Reading the body costs the response
Deciding on a 403 means consuming it, and a consumed response reaching the caller
empty would be a regression:
json_or_scope_errormapsInsufficientScopebyparsing the body, so an emptied 403 would degrade to a generic
Apierror.So the body is buffered once and the response rebuilt from its own parts, status,
headers and URL included.
buffer_body_preserves_status_headers_and_bodyassertseach of those, and
resolve_signature_403_scope_error_keeps_its_bodyasserts theend-to-end consequence: the scope error still arrives as
InsufficientScope.Two existing tests changed, deliberately
create_draft_429_preserves_statusandget_thread_500_preserves_statusassertedthat a 429/500 failed on the first response. That is the behaviour this PR
changes, so leaving them untouched was not an option.
They are renamed to
..._after_retriesand now assert the property that stillmatters: when every attempt fails, the status reaches the caller unchanged. The
mocks answer the error status every time, so the assertion is about the final
outcome, not the attempt count.
retryable_excludes_real_answersno longer asserts!is_retryable(FORBIDDEN)as afinal word. 403 moved out of that predicate and into
forbidden_is_decided_by_the_body_not_the_status, which asserts both halves: thestatus alone does not retry it, and the body is what decides.
The "does not retry" side is covered explicitly with wiremock
.expect(1), on bothget_message_does_not_retry_404andget_message_does_not_retry_403_scope_error: astray retry makes the mock verification fail.
Verified
cargo test -p void-gmail, run on this branch, based onmainatadd6674:cargo clippy -p void-gmail --all-targetsis clean.I checked the new tests bite, in both directions. Forcing
is_retryable_quota_bodyto returnfalsefails 2:Forcing it to return
truefails 3, which is the half that matters more, since apredicate that retries every 403 is worse than one that retries none:
Forcing
is_retryableto returnfalsefails 4 (the 429 and 5xx tests).Notes
http = "1". It is already in the lockfile as areqwest 0.13 dependency, and reqwest re-exports its
StatusCodeandheadertypes, so this pins nothing new. It is needed for the
http::Responsetoreqwest::Responseround-trip that rebuilds a buffered body.Not in this PR
your call on the approach first.
client. If you want sends to be less eager to retry than reads, say so and I will
split it.