Skip to content

fix(client): back off when Retry-After is zero or not in the future - #258

Merged
pchuri merged 1 commit into
pchuri:mainfrom
bestend:fix/retry-after-zero
Oct 1, 2026
Merged

pchuri merged 1 commit into
pchuri:mainfrom
bestend:fix/retry-after-zero

Conversation

@bestend

@bestend bestend commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Problem

The retry interceptor honors Retry-After literally, so Retry-After: 0 produces a 0 ms delay.

Confluence Data Center's rate limiter (a token bucket) sends Retry-After: 0 on 429 responses, and on 200 responses, together with x-ratelimit-limit: 3, x-ratelimit-fillrate: 3 and x-ratelimit-interval-seconds: 1. Every retry therefore fired immediately, hit the empty bucket again, and all attempts were used up within milliseconds. On Data Center 9.2.9 (3 requests per second), confluence children <pageId> --recursive sent 36 requests in 186 ms and failed with Rate limit exceeded.

A negative value, a whitespace-only value and an HTTP-date that is not in the future also resolved to 0 ms.

Fix

retryDelayMs honors Retry-After only when it names a positive wait: a positive number of seconds or a future HTTP-date, still capped at 60 s. A missing, unparsable, zero, negative or already-past value is treated like a missing header and falls back to the existing exponential backoff (1 s, 2 s, 4 s with the default base delay, capped at 60 s). Positive values behave exactly as before.

  • The x-ratelimit-* headers are deliberately not used as a hint. While Retry-After is positive they add nothing, and the 1 s base backoff already spans the 1 s refill interval reported next to Retry-After: 0.
  • No extra minimum delay was added: with the default base of 1000 ms the backoff is never below 1 s; it is only small when tests inject a tiny retryBaseDelayMs.

Verification

  • Added 12 unit tests: Retry-After of "0", numeric 0, "00", whitespace, negative seconds, unparsable text and an empty string; past and current HTTP-dates; positive seconds and future HTTP-dates (still honored and capped); the backoff cap; and the interceptor wiring, asserting the sleep durations for a 429 with Retry-After: 0 (1000, 2000) and with Retry-After: 3 (3000) without waiting in real time. 8 of them fail on main (the interceptor test sees [0, 0]); the other 4 guard existing behavior.
  • npm test: 1465 passed (1453 before). npm run lint: clean.
  • Simulated a local 3 requests/second token-bucket server that answers Retry-After: 0 on every response and ran getAllDescendantPages over a tree needing 36 requests: main fails after about 15 ms with 3 of 36 requests served; this branch spaces retries 1 s / 2 s / 4 s and serves 24 before failing.
  • On the Data Center 9.2.9 server, children <pageId> --recursive --max-depth 3 gives up after 0.3 s on v2.25.8 and after 7.3 s on this branch: the retries now wait, but the traversal still fails for the reason below.

Known limitation

This fixes the retry timing only. children --recursive runs up to 10 requests concurrently, which does not fit under 3 requests per second with 3 retries (7 s of waiting). In the same simulation, limiting the traversal to 3 concurrent requests completes the 36-request tree in about 11 s, and raising maxRetries to 10 completes it in about 15 s; jitter alone does not help. Both are behavior changes, so they are left for a follow-up.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Testing

  • Tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

🤖 Generated with Claude Code

The retry interceptor honored Retry-After literally, so `Retry-After: 0`
produced a 0 ms delay. Confluence Data Center's rate limiter sends
`Retry-After: 0` on 429 responses (and on 200s), so every retry fired
immediately, hit the empty token bucket again, and all attempts were
used up within milliseconds. Observed on Data Center 9.2.9 (3 requests
per second): `children --recursive` sent 36 requests in 186 ms and
failed with "Rate limit exceeded".

retryDelayMs now honors Retry-After only when it names a positive wait:
a positive number of seconds or an HTTP-date in the future, still capped
at 60 s. A missing, unparsable, zero, negative or already-past value is
treated like a missing header and falls back to the existing
exponential backoff (1 s, 2 s, 4 s with the default base delay), which
is itself capped at 60 s.

The x-ratelimit-* headers are deliberately not used as a hint: they add
nothing while Retry-After is positive, and the 1 s base backoff already
spans the 1 s refill interval reported alongside `Retry-After: 0`.

Tests cover zero, numeric zero, whitespace, negative and unparsable
values, past and current HTTP-dates, positive seconds and future
HTTP-dates (still honored and capped), the backoff cap, and the
interceptor wiring (sleep durations for a 429 with `Retry-After: 0` and
with `Retry-After: 3`) without waiting in real time.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@pchuri pchuri left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clear diagnosis and focused fix. Falling back to the existing exponential backoff when Retry-After provides no usable positive delay makes sense, while preserving positive delays and the existing cap.

I reviewed the current head and ran the client test suite (268/268 passing) and lint successfully. I did not find a blocking issue in the reviewed changes. I also appreciate the explicit note that recursive traversal can still exceed a low rate limit: concurrency and retry-budget changes can remain a separate follow-up rather than expanding this PR.

I have not independently repeated the live-server/simulation checks or run the full repository test suite yet. This looks ready to move toward merge after final validation. Thanks again!

@pchuri
pchuri merged commit 5d9b3b9 into pchuri:main Oct 1, 2026
github-actions Bot pushed a commit that referenced this pull request Oct 1, 2026
## [2.25.9](v2.25.8...v2.25.9) (2026-10-01)

### Bug Fixes

* **attachments:** make --replace work on Data Center/Server ([#257](#257)) ([e9163e4](e9163e4))
* **client:** back off when Retry-After is zero or not in the future ([#258](#258)) ([5d9b3b9](5d9b3b9))
* **deps:** update axios to 1.20.0 to unblock security audit ([#263](#263)) ([1f62546](1f62546))
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

🎉 This PR is included in version 2.25.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

pchuri added a commit that referenced this pull request Oct 4, 2026
…#272)

Each request backed off on its own, so a burst of concurrent requests (page tree traversal runs ten at a time) that hit a low shared limit, such as 3 requests per second on Data Center, was rejected together, retried in lockstep, and exhausted its retries even though a patient client would have finished. #258 fixed immediate retries on Retry-After: 0 but not this.

Add a RequestGate shared by every request of a client. It handles 429 only; a 503 keeps its independent per-request retries. A new 429 doubles the minimum spacing between request starts (100 ms, capped at 2 s) and each success shrinks it by 10%. Only a positive Retry-After pauses all requests. A rejection counts against a request own retries only when it is new and no request has succeeded since it first started, with free retries capped at maxRetries. After a run of new 429s with no success the gate stops retrying and delaying until a request succeeds, so a persistent refusal fails about as fast as before.

Without throttling nothing is paced or delayed. Retry counts, the delay for a single request, and the error surfaced when retries run out are unchanged.

Known limits: 429s unrelated to the request rate leave the client spacing requests for a while, limits stricter than about 0.5 requests per second are not fully tamed, and a positive Retry-After now pauses the whole client. Verified against simulated servers only, not a live Data Center.

Refs #261
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants