diff --git a/README.md b/README.md index 507ccaa..0f7609f 100644 --- a/README.md +++ b/README.md @@ -886,6 +886,7 @@ Notes: - Continues on errors: failed pages are logged and the copy proceeds. - Exclude patterns use simple globbing: `*` matches any sequence, `?` matches any single character, and special regex characters are treated literally. - Large trees may take time; the CLI applies a small delay between sibling page creations to avoid rate limits (configurable via `--delay-ms`). +- If the server answers `429` (for example a Data Center instance limited to a few requests per second), the CLI paces its requests: a positive `Retry-After` makes every request wait, and requests are spaced further apart until the server accepts them, then speeded up again. Traversals such as `copy-tree`, `export` and `children --recursive` therefore finish under a low shared limit instead of failing after a few retries, and a server that keeps refusing still fails after about as long as before. Without throttling nothing is slowed down. A `503` on a read is retried per request, as before. - Root title suffix defaults to ` (Copy)`; override with `--copy-suffix`. Child pages keep their original titles. - Use `--fail-on-error` to exit non-zero if any page fails to copy. diff --git a/lib/confluence-client.js b/lib/confluence-client.js index 0eaa8c6..2be0920 100644 --- a/lib/confluence-client.js +++ b/lib/confluence-client.js @@ -9,12 +9,21 @@ const { decodeHTML } = require('entities'); const MacroConverter = require('./macro-converter'); const { resolvePlantumlFormat } = require('./plantuml-format'); const { htmlToMarkdown, NAMED_ENTITIES } = require('./html-to-markdown'); +const { RequestGate } = require('./request-gate'); const { isJsonMode } = require('./output'); const WRITE_FORMATS = ['auto', 'storage', 'html', 'markdown']; const PAGE_LINK_LOOKUP_CONCURRENCY = 10; const RETRY_SAFE_METHODS = new Set(['get', 'head', 'options']); const MAX_RETRY_DELAY_MS = 60000; +// Throttled responses that are not evidence against a request (it was part of +// a burst that was already in flight, or the server kept serving others) do +// not use up its retries; this caps how many of those it may ride out, as +// many as its own retries, so a path that is always refused fails after the +// same number of attempts as before. +// New 429s without a Retry-After, beyond what one request would retry, before +// the gate gives up (see RequestGate). +const EXTRA_FRESH_THROTTLES = 4; async function returnNullForNotFound(request) { try { @@ -155,8 +164,19 @@ class ConfluenceClient { this.maxRetries = 3; this.retryBaseDelayMs = 1000; + // Shared across every request this client makes, so a throttled response + // slows them all down instead of each retrying in lockstep. + this.gate = new RequestGate(); + this.client.interceptors.request.use(async config => { + await this.gate.acquire(config); + return config; + }); + this.client.interceptors.response.use( - response => response, + response => { + this.gate.reportSuccess(); + return response; + }, async error => { const requestConfig = error.config; const status = error.response?.status; @@ -172,10 +192,41 @@ class ConfluenceClient { typeof requestConfig.data?.pipe !== 'function' ) { const attempt = requestConfig.__retryCount || 0; - if (attempt < this.maxRetries) { - requestConfig.__retryCount = attempt + 1; - await this.sleep(this.retryDelayMs(error.response, attempt)); - return this.client.request(requestConfig); + const free = requestConfig.__freeRetries || 0; + if (attempt < this.maxRetries && free < this.maxRetries) { + if (status !== 429) { + requestConfig.__retryCount = attempt + 1; + await this.sleep(this.retryDelayMs(error.response, attempt)); + return this.client.request(requestConfig); + } + // A 429 is shared with every other request of this client: the + // gate paces their starts and honors a positive Retry-After for + // all of them. The backoff grows with new 429s seen by the whole + // client but never past what one request would reach on its + // own. A rejection from a burst this request was already part of, + // or while the server was still serving others, waits like the + // rest but keeps its retries, and once the gate opens its circuit + // (a run of new 429s with no success) nothing more is retried. + const level = this.gate.level; + const pauseMs = this.retryAfterMs(error.response); + const { retry, fresh, progressed, until } = this.gate.reportThrottle(requestConfig, { + pauseMs, + // A server that names the wait is believed after as many + // failures as one request would have given up on. Without one + // the spacing needs a few doublings to find the server's pace. + maxFresh: this.maxRetries + (pauseMs > 0 ? 1 : EXTRA_FRESH_THROTTLES) + }); + if (retry) { + if (fresh && !progressed) { + requestConfig.__retryCount = attempt + 1; + } else { + requestConfig.__freeRetries = free + 1; + } + const exponent = Math.min(level, Math.max(0, this.maxRetries - 1)); + await this.sleep(this.retryDelayMs(error.response, exponent)); + this.gate.expire(until); + return this.client.request(requestConfig); + } } } if (error.response?.status === 401) { @@ -2522,20 +2573,26 @@ class ConfluenceClient { * up all attempts within milliseconds. */ retryDelayMs(response, attempt) { - const retryAfter = response?.headers?.['retry-after']; - if (retryAfter !== undefined && retryAfter !== null && retryAfter !== '') { - const seconds = Number(retryAfter); - const waitMs = Number.isFinite(seconds) - ? seconds * 1000 - : Date.parse(retryAfter) - Date.now(); - // NaN (unparsable) and non-positive waits fall through to the backoff. - if (waitMs > 0) { - return Math.min(waitMs, MAX_RETRY_DELAY_MS); - } - } + const retryAfterMs = this.retryAfterMs(response); + if (retryAfterMs > 0) return retryAfterMs; return Math.min(this.retryBaseDelayMs * 2 ** attempt, MAX_RETRY_DELAY_MS); } + /** + * The wait a response asks for with Retry-After (seconds or HTTP-date), + * capped at the maximum delay; 0 when it names no usable wait. + */ + retryAfterMs(response) { + const retryAfter = response?.headers?.['retry-after']; + if (retryAfter === undefined || retryAfter === null || retryAfter === '') return 0; + const seconds = Number(retryAfter); + const waitMs = Number.isFinite(seconds) + ? seconds * 1000 + : Date.parse(retryAfter) - Date.now(); + // NaN (unparsable) and non-positive waits name no wait. + return waitMs > 0 ? Math.min(waitMs, MAX_RETRY_DELAY_MS) : 0; + } + /** * Accumulate results across start/limit pages until the server stops * returning a next link or `maxResults` is reached (null = unlimited). diff --git a/lib/request-gate.js b/lib/request-gate.js new file mode 100644 index 0000000..28e9f0c --- /dev/null +++ b/lib/request-gate.js @@ -0,0 +1,146 @@ +// Coordinates the requests of one client that share a rate-limited server. +// +// Each request used to back off on its own, so a burst of N concurrent +// requests that hit a low shared limit (Confluence Data Center can allow as +// little as 3 requests per second) was rejected N times over, retried in +// lockstep, rejected again, and ran out of retries even though a patient +// client would have finished. Capping concurrency does not fix that: a fast +// server answers at once, so even one request at a time arrives faster than +// the limit. The gate paces request *starts* instead, and shares what it +// learns across every request the client makes. It only ever sees 429 +// responses; other retryable statuses keep their independent backoff. +// +// - A positive Retry-After pauses every request, not just the one that saw +// it. Without one (Data Center answers 429 with `Retry-After: 0`) there is +// nothing to wait for, so only the spacing below changes. +// - A new 429 doubles the minimum spacing between request starts, and each +// success shrinks it again. A rejection from a burst that was already in +// flight when the spacing last changed is stale: it neither widens the +// spacing again nor uses up the request's own retries. +// - After `maxFresh` new 429s with no success in between, the gate opens a +// circuit: nothing more is retried or delayed until a request succeeds, so +// a server that keeps refusing fails as fast as a single request would. +// - Without throttling there is no pause and no spacing: starts are released +// back to back, so concurrency and throughput are untouched and the gate +// only adds a microtask to each request. + +// Spacing applied after the first throttle, its ceiling, and the factor each +// success applies to it. Below MIN_INTERVAL_MS the spacing is dropped. The +// decay is deliberately memoryless: a 429 that has nothing to do with the +// request rate (a flaky proxy) is forgotten within a handful of successes +// instead of leaving the client throttled, and against a real limit the +// spacing settles around the server's pace, crossing it now and then. +const INITIAL_INTERVAL_MS = 100; +const MAX_INTERVAL_MS = 2000; +const SUCCESS_DECAY = 0.9; +const MIN_INTERVAL_MS = 25; + +// The gate waits with its own timer, not ConfluenceClient.sleep, which a +// caller may stub to skip the per-request backoff. +const defaultSleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +class RequestGate { + constructor({ now = () => Date.now(), sleep = defaultSleep } = {}) { + this.now = now; + this.sleep = sleep; + this.intervalMs = 0; + this.lastStart = -Infinity; + // Requests start one at a time, in arrival order. + this.tail = Promise.resolve(); + this.pauseUntil = 0; + // New 429s since the last success; scales a request's own backoff and + // trips the circuit. + this.level = 0; + this.open = false; + // Bumped whenever the spacing widens. A request stamped with an older + // epoch was sent before the change that its rejection would cause. + this.epoch = 0; + // Successful responses so far; a request that sees this grow while it is + // throttled knows the server is still making progress for others. + this.successes = 0; + } + + // Wait for this request's turn to start, then for any shared pause and the + // spacing since the previous start. `config` is stamped so a rejection can + // later be told apart as new or stale. + async acquire(config) { + const previous = this.tail; + let done; + this.tail = new Promise((resolve) => { done = resolve; }); + try { + await previous; + // Track the time waited until rather than re-reading the clock, so the + // loop ends even if `sleep` returns early; it only repeats when the + // pause was extended while this request slept. + let startAt = this.now(); + while (!this.open) { + const target = Math.max(this.pauseUntil, this.lastStart + this.intervalMs); + if (target <= startAt) break; + await this.sleep(target - startAt); + startAt = target; + } + this.lastStart = Math.max(startAt, this.now()); + config.__gateEpoch = this.epoch; + if (config.__gateSuccesses === undefined) config.__gateSuccesses = this.successes; + } finally { + done(); + } + } + + // Record a 429 that may be retried. `pauseMs` is a positive Retry-After + // (0 if there was none) and `maxFresh` how many new 429s in a row are + // allowed before the circuit opens. Returns `retry` (false once the circuit + // is open), `fresh` (a new 429 rather than one from a burst already in + // flight), `progressed` (some request has succeeded since this one first + // started, so the rejection is not evidence against it) and `until` (the + // shared pause it set, for `expire`). Only a fresh rejection with no + // progress should count against the request's own retries. + reportThrottle(config, { pauseMs = 0, maxFresh = Infinity } = {}) { + const fresh = config.__gateEpoch === this.epoch; + const progressed = this.successes > (config.__gateSuccesses ?? this.successes); + let until = 0; + if (fresh) { + this.epoch++; + this.level++; + this.intervalMs = this.intervalMs === 0 + ? INITIAL_INTERVAL_MS + : Math.min(this.intervalMs * 2, MAX_INTERVAL_MS); + if (this.level >= maxFresh) { + this.open = true; + } + } + // Nothing waits on a pause once the circuit is open, and one set now + // would come back to life when a later success closes it. + if (pauseMs > 0 && !this.open) { + until = Math.max(this.pauseUntil, this.now() + pauseMs); + this.pauseUntil = until; + } + return { retry: !this.open, fresh, progressed, until }; + } + + // The caller that set the pause has waited it out itself; clear it unless + // someone has extended it since. + expire(until) { + if (until && this.pauseUntil === until) this.pauseUntil = 0; + } + + reportSuccess() { + this.successes++; + this.level = 0; + if (this.open) { + this.open = false; + this.pauseUntil = 0; + } + if (this.intervalMs === 0) return; + const next = this.intervalMs * SUCCESS_DECAY; + this.intervalMs = next < MIN_INTERVAL_MS ? 0 : next; + } +} + +module.exports = { + RequestGate, + INITIAL_INTERVAL_MS, + MAX_INTERVAL_MS, + MIN_INTERVAL_MS, + SUCCESS_DECAY +}; diff --git a/tests/request-gate.test.js b/tests/request-gate.test.js new file mode 100644 index 0000000..4d6a804 --- /dev/null +++ b/tests/request-gate.test.js @@ -0,0 +1,254 @@ +const { + RequestGate, + INITIAL_INTERVAL_MS, + MAX_INTERVAL_MS, + MIN_INTERVAL_MS, + SUCCESS_DECAY +} = require('../lib/request-gate'); + +// A gate on a virtual clock: `sleep` advances time instead of waiting. +const makeGate = () => { + const clock = { now: 0, sleeps: [] }; + const gate = new RequestGate({ + now: () => clock.now, + sleep: async (ms) => { + clock.sleeps.push(ms); + clock.now += ms; + } + }); + return { gate, clock }; +}; + +const start = async (gate) => { + const config = {}; + await gate.acquire(config); + return config; +}; + +describe('RequestGate (#261)', () => { + describe('without throttling', () => { + test('starts are released back to back with no wait', async () => { + const { gate, clock } = makeGate(); + const configs = await Promise.all(Array.from({ length: 50 }, () => start(gate))); + + expect(clock.sleeps).toEqual([]); + expect(clock.now).toBe(0); + expect(configs.every((config) => config.__gateEpoch === 0)).toBe(true); + }); + + test('successes change nothing', () => { + const { gate } = makeGate(); + for (let i = 0; i < 100; i++) gate.reportSuccess(); + expect(gate.intervalMs).toBe(0); + expect(gate.level).toBe(0); + expect(gate.open).toBe(false); + }); + }); + + describe('a 429', () => { + test('without a Retry-After spaces later starts but does not pause them', async () => { + const { gate, clock } = makeGate(); + gate.reportThrottle(await start(gate), { pauseMs: 0 }); + + await start(gate); + + // Only the spacing since the previous start, nowhere near a pause. + expect(clock.now).toBeLessThanOrEqual(INITIAL_INTERVAL_MS); + }); + + test('with a Retry-After pauses every later start, not only the retrying request', async () => { + const { gate, clock } = makeGate(); + gate.reportThrottle(await start(gate), { pauseMs: 2000 }); + + const others = await Promise.all([start(gate), start(gate), start(gate)]); + + expect(clock.now).toBeGreaterThanOrEqual(2000); + expect(others).toHaveLength(3); + }); + + test('the first new throttle spaces starts and counts against the request', async () => { + const { gate } = makeGate(); + const result = gate.reportThrottle(await start(gate)); + + expect(result).toMatchObject({ retry: true, fresh: true }); + expect(gate.intervalMs).toBe(INITIAL_INTERVAL_MS); + expect(gate.level).toBe(1); + expect(gate.epoch).toBe(1); + }); + + test('rejections from the same burst are stale: they neither widen the spacing nor count', async () => { + const { gate } = makeGate(); + const burst = await Promise.all(Array.from({ length: 5 }, () => start(gate))); + + const results = burst.map((config) => gate.reportThrottle(config)); + + expect(results.map((r) => r.fresh)).toEqual([true, false, false, false, false]); + expect(gate.intervalMs).toBe(INITIAL_INTERVAL_MS); + expect(gate.level).toBe(1); + }); + + test('a rejection after the spacing changed is new again and doubles it', async () => { + const { gate } = makeGate(); + gate.reportThrottle(await start(gate)); + gate.reportThrottle(await start(gate)); + gate.reportThrottle(await start(gate)); + + expect(gate.intervalMs).toBe(INITIAL_INTERVAL_MS * 4); + expect(gate.level).toBe(3); + }); + + test('the spacing is capped', async () => { + const { gate } = makeGate(); + for (let i = 0; i < 20; i++) gate.reportThrottle(await start(gate)); + expect(gate.intervalMs).toBe(MAX_INTERVAL_MS); + }); + + test('starts are spaced by the interval', async () => { + const { gate, clock } = makeGate(); + gate.reportThrottle(await start(gate)); + const times = []; + for (let i = 0; i < 4; i++) { + await start(gate); + times.push(clock.now); + } + + for (let i = 1; i < times.length; i++) { + expect(times[i] - times[i - 1]).toBeGreaterThanOrEqual(INITIAL_INTERVAL_MS); + } + }); + + test('expire clears the pause the caller set, but not one that was extended', async () => { + const { gate } = makeGate(); + const config = await start(gate); + const { until } = gate.reportThrottle(config, { pauseMs: 1000 }); + gate.expire(until); + expect(gate.pauseUntil).toBe(0); + + const { until: first } = gate.reportThrottle(config, { pauseMs: 1000 }); + gate.reportThrottle(config, { pauseMs: 5000 }); + gate.expire(first); + expect(gate.pauseUntil).toBeGreaterThan(first); + expect(gate.expire(0)).toBeUndefined(); + }); + + test('a sleep that returns early cannot trap acquire in a loop', async () => { + const calls = []; + const gate = new RequestGate({ now: () => 0, sleep: async (ms) => { calls.push(ms); } }); + gate.reportThrottle(await start(gate), { pauseMs: 1000 }); + + await start(gate); + + expect(calls.length).toBeLessThan(5); + }); + }); + + describe('recovery', () => { + test('a success resets the backoff level', async () => { + const { gate } = makeGate(); + gate.reportThrottle(await start(gate)); + gate.reportSuccess(); + expect(gate.level).toBe(0); + }); + + test('each success shrinks the spacing until it is dropped', async () => { + const { gate } = makeGate(); + gate.reportThrottle(await start(gate)); + expect(gate.intervalMs).toBe(INITIAL_INTERVAL_MS); + + gate.reportSuccess(); + expect(gate.intervalMs).toBeCloseTo(INITIAL_INTERVAL_MS * SUCCESS_DECAY); + + let successes = 1; + while (gate.intervalMs !== 0 && successes < 1000) { + gate.reportSuccess(); + successes++; + } + expect(gate.intervalMs).toBe(0); + // About a dozen successes bring the first throttle's spacing back to nothing. + expect(successes).toBeLessThan(20); + expect(INITIAL_INTERVAL_MS * SUCCESS_DECAY ** successes).toBeLessThan(MIN_INTERVAL_MS); + }); + + test('a throttle with a Retry-After pauses and also reports progress only when others succeeded', async () => { + const { gate } = makeGate(); + const config = await start(gate); + expect(gate.reportThrottle(config).progressed).toBe(false); + + gate.reportSuccess(); + + expect(gate.reportThrottle(config).progressed).toBe(true); + }); + + test('isolated throttles do not ratchet the spacing up', async () => { + const { gate } = makeGate(); + // A throttle every 20 requests, as a flaky proxy might produce. + for (let round = 0; round < 50; round++) { + gate.reportThrottle(await start(gate)); + for (let i = 0; i < 20; i++) gate.reportSuccess(); + } + expect(gate.intervalMs).toBe(0); + }); + }); + + describe('the circuit', () => { + test('opens after maxFresh new throttles with no success in between', async () => { + const { gate } = makeGate(); + const results = []; + for (let i = 0; i < 4; i++) { + results.push(gate.reportThrottle(await start(gate), { maxFresh: 4 })); + } + + expect(results.map((r) => r.retry)).toEqual([true, true, true, false]); + expect(gate.open).toBe(true); + }); + + test('stale rejections do not count towards it', async () => { + const { gate } = makeGate(); + const burst = await Promise.all(Array.from({ length: 10 }, () => start(gate))); + + burst.forEach((config) => gate.reportThrottle(config, { maxFresh: 3 })); + + expect(gate.open).toBe(false); + expect(gate.level).toBe(1); + }); + + test('a success in between keeps it closed', async () => { + const { gate } = makeGate(); + for (let i = 0; i < 20; i++) { + gate.reportThrottle(await start(gate), { maxFresh: 3 }); + gate.reportThrottle(await start(gate), { maxFresh: 3 }); + gate.reportSuccess(); + } + expect(gate.open).toBe(false); + }); + + test('a pause requested by the throttle that opens it does not come back when a success closes it', async () => { + const { gate, clock } = makeGate(); + gate.reportThrottle(await start(gate), { pauseMs: 60000, maxFresh: 2 }); + const opened = gate.reportThrottle(await start(gate), { pauseMs: 60000, maxFresh: 2 }); + expect(opened.retry).toBe(false); + expect(gate.open).toBe(true); + + gate.reportSuccess(); + const before = clock.now; + await start(gate); + + expect(gate.pauseUntil).toBe(0); + // Only the short spacing between starts, not the 60 s pause. + expect(clock.now - before).toBeLessThan(1000); + }); + + test('while open nothing is paced or paused, and a success closes it', async () => { + const { gate, clock } = makeGate(); + for (let i = 0; i < 3; i++) gate.reportThrottle(await start(gate), { pauseMs: 5000, maxFresh: 3 }); + expect(gate.open).toBe(true); + const before = clock.now; + + await Promise.all([start(gate), start(gate), start(gate)]); + expect(clock.now).toBe(before); + + gate.reportSuccess(); + expect(gate.open).toBe(false); + }); + }); +}); diff --git a/tests/traversal-rate-limit.test.js b/tests/traversal-rate-limit.test.js new file mode 100644 index 0000000..c5924b8 --- /dev/null +++ b/tests/traversal-rate-limit.test.js @@ -0,0 +1,248 @@ +const MockAdapter = require('axios-mock-adapter'); +const ConfluenceClient = require('../lib/confluence-client'); + +// A deterministic stand-in for a Data Center instance with a low shared rate +// limit. Time is virtual (jest fake timers), so a token bucket refilling at +// `ratePerSecond` is exact and the whole run takes milliseconds of real time. +describe('page tree traversal under a low shared rate limit (#261)', () => { + const buildTree = (fanouts) => { + const children = new Map(); + let nextId = 2; + const grow = (id, depth) => { + if (depth >= fanouts.length) return; + const ids = Array.from({ length: fanouts[depth] }, () => String(nextId++)); + children.set(id, ids); + ids.forEach((child) => grow(child, depth + 1)); + }; + grow('1', 0); + return { children, total: nextId - 2 }; + }; + + const startServer = (client, tree, { capacity, ratePerSecond, retryAfter = '0' }) => { + const stats = { ok: 0, rejected: 0, rejectedTimes: [] }; + let tokens = capacity; + let last = Date.now(); + const mock = new MockAdapter(client.client); + mock.onGet(/\/content\/\d+\/child\/page$/).reply((config) => { + const now = Date.now(); + tokens = Math.min(capacity, tokens + ((now - last) / 1000) * ratePerSecond); + last = now; + if (tokens < 1) { + stats.rejected++; + stats.rejectedTimes.push(now); + return [429, {}, { 'retry-after': retryAfter }]; + } + tokens -= 1; + stats.ok++; + const id = config.url.match(/content\/(\d+)\/child/)[1]; + const results = (tree.children.get(id) || []).map((childId) => ({ + id: childId, + title: `Page ${childId}`, + type: 'page', + space: { key: 'ENG' }, + version: { number: 1 } + })); + return [200, { results }]; + }); + return { stats, mock }; + }; + + // Advance virtual time until the promise settles, within a virtual budget. + const settle = async (promise, budgetMs = 10 * 60 * 1000) => { + let outcome = null; + promise.then((value) => { outcome = { value }; }, (error) => { outcome = { error }; }); + const start = Date.now(); + while (outcome === null && Date.now() - start < budgetMs) { + await jest.advanceTimersByTimeAsync(50); + } + if (outcome === null) throw new Error(`still running after ${budgetMs} virtual ms`); + return { ...outcome, elapsedMs: Date.now() - start }; + }; + + let client; + beforeEach(() => { + jest.useFakeTimers(); + client = new ConfluenceClient({ domain: 'wiki.example.org', token: 'test-token' }); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + test('a 3 requests/second server does not exhaust retries on a 40-page tree', async () => { + const tree = buildTree([4, 3, 2]); + const { stats, mock } = startServer(client, tree, { capacity: 3, ratePerSecond: 3 }); + + const result = await settle(client.getAllDescendantPages('1', 10)); + mock.restore(); + + expect(result.error).toBeUndefined(); + expect(result.value).toHaveLength(tree.total); + expect(new Set(result.value.map((page) => page.id)).size).toBe(tree.total); + // Every page was listed once, and the rejections were a handful, not a storm. + expect(stats.ok).toBe(tree.total + 1); + expect(stats.rejected).toBeLessThan(tree.total / 2); + // Within a small multiple of the time the limit itself requires. + expect(result.elapsedMs).toBeLessThan((tree.total / 3) * 4 * 1000); + }); + + test.each([ + ['1 request/second with no burst', { capacity: 1, ratePerSecond: 1 }], + ['2 requests/second', { capacity: 2, ratePerSecond: 2 }], + ['a large burst but a low steady rate', { capacity: 10, ratePerSecond: 2 }], + ['a positive Retry-After', { capacity: 3, ratePerSecond: 3, retryAfter: '1' }], + ['no Retry-After header', { capacity: 3, ratePerSecond: 3, retryAfter: undefined }] + ])('completes a wide and deep tree under %s', async (_label, limits) => { + const tree = buildTree([6, 4, 3]); + const { stats, mock } = startServer(client, tree, limits); + + const result = await settle(client.getAllDescendantPages('1', 10)); + mock.restore(); + + expect(result.error).toBeUndefined(); + expect(result.value).toHaveLength(tree.total); + expect(stats.ok).toBe(tree.total + 1); + // No retry storm: far fewer rejected requests than pages. + expect(stats.rejected).toBeLessThan(tree.total); + }); + + test('without a limit nothing is paced or delayed', async () => { + const tree = buildTree([4, 3, 2]); + const { stats, mock } = startServer(client, tree, { capacity: 1000, ratePerSecond: 1000 }); + + const result = await settle(client.getAllDescendantPages('1', 10)); + mock.restore(); + + expect(result.value).toHaveLength(tree.total); + expect(stats.rejected).toBe(0); + // Done within the first 50 ms step of virtual time: no pause, no spacing. + expect(result.elapsedMs).toBeLessThanOrEqual(50); + }); + + test('a server that never lets a request through fails after bounded waiting', async () => { + const tree = buildTree([4, 3, 2]); + const { stats, mock } = startServer(client, tree, { capacity: 0, ratePerSecond: 0 }); + + const result = await settle(client.getAllDescendantPages('1', 10)); + mock.restore(); + + expect(result.error).toBeDefined(); + expect(result.error.response.status).toBe(429); + // The error surfaces as before, after a bounded number of requests and time. + expect(stats.ok).toBe(0); + expect(stats.rejected).toBeLessThan(100); + expect(result.elapsedMs).toBeLessThan(5 * 60 * 1000); + }); + + test.each([ + ['without Retry-After', undefined, 20 * 1000], + ['with Retry-After: 0', '0', 20 * 1000], + // Honoring a 60 s Retry-After three times is what one request did before. + ['with Retry-After: 60', '60', 4 * 60 * 1000] + ])('concurrent requests against a server that always refuses fail about as fast as before %s', async (_label, retryAfter, boundMs) => { + const { stats, mock } = startServer(client, buildTree([]), { capacity: 0, ratePerSecond: 0, retryAfter }); + const failures = []; + + const done = Promise.all( + Array.from({ length: 50 }, (_, i) => + client.client.get(`/content/${i}/child/page`).then(() => 'ok', (error) => { + failures.push(Date.now()); + return error.response.status; + }) + ) + ); + const outcome = await settle(done); + mock.restore(); + + expect(outcome.value.every((status) => status === 429)).toBe(true); + // Before this change the last of them failed after 7 s (3 retries of + // 1, 2 and 4 s) or three Retry-After waits, however many ran at once. + expect(outcome.elapsedMs).toBeLessThan(boundMs); + expect(stats.rejected).toBeLessThan(200); + }); + + test.each([ + ['Retry-After: 0', '0', 15 * 1000], + ['Retry-After: 5', '5', 60 * 1000] + ])('one page that is always refused fails after the same attempts as before while the others succeed (%s)', async (_label, retryAfter, boundMs) => { + const tree = buildTree([6, 4, 3]); + const stuck = tree.children.get('1')[0]; + const attempts = new Map(); + const mock = new MockAdapter(client.client); + mock.onGet(/\/content\/\d+\/child\/page$/).reply((config) => { + const id = config.url.match(/content\/(\d+)\/child/)[1]; + attempts.set(id, (attempts.get(id) || 0) + 1); + if (id === stuck) return [429, {}, { 'retry-after': retryAfter }]; + return [200, { results: (tree.children.get(id) || []).map((childId) => ({ + id: childId, title: `Page ${childId}`, type: 'page', space: { key: 'ENG' }, version: { number: 1 } + })) }]; + }); + + const result = await settle(client.getAllDescendantPages('1', 10)); + mock.restore(); + + // The traversal fails on that page as it did before, with the same four attempts, not hanging. + expect(result.error).toBeDefined(); + expect(result.error.response.status).toBe(429); + expect(attempts.get(stuck)).toBe(4); + expect(result.elapsedMs).toBeLessThan(boundMs); + }); + + test('503 responses keep their independent retries and never pace other requests', async () => { + // The first call of every fourth page fails with a 503; its retry succeeds. + const seen = new Set(); + const mock = new MockAdapter(client.client); + mock.onGet(/\/content\/\d+\/child\/page$/).reply((config) => { + const id = Number(config.url.match(/content\/(\d+)\/child/)[1]); + const firstCall = !seen.has(id); + seen.add(id); + return id % 4 === 0 && firstCall ? [503, {}] : [200, { results: [] }]; + }); + + const outcome = await settle(Promise.all(Array.from({ length: 40 }, (_, i) => client.getChildPages(String(i))))); + mock.restore(); + + expect(outcome.error).toBeUndefined(); + expect(client.gate.intervalMs).toBe(0); + expect(client.gate.epoch).toBe(0); + // Each 503 waited its own 1 s and nothing else did. + expect(outcome.elapsedMs).toBeLessThan(2000); + }); + + test('isolated 429s with a Retry-After do not leave the client throttled', async () => { + const tree = buildTree([6, 4, 3]); + let count = 0; + const mock = new MockAdapter(client.client); + mock.onGet(/\/content\/\d+\/child\/page$/).reply((config) => { + count++; + if (count % 25 === 0) return [429, {}, { 'retry-after': '1' }]; + const id = config.url.match(/content\/(\d+)\/child/)[1]; + return [200, { results: (tree.children.get(id) || []).map((childId) => ({ + id: childId, title: `Page ${childId}`, type: 'page', space: { key: 'ENG' }, version: { number: 1 } + })) }]; + }); + + const result = await settle(client.getAllDescendantPages('1', 10)); + mock.restore(); + + expect(result.error).toBeUndefined(); + expect(result.value).toHaveLength(tree.total); + expect(client.gate.open).toBe(false); + // Each throttle costs about its one-second Retry-After, nothing compounding. + const throttles = Math.floor(count / 25); + expect(result.elapsedMs).toBeLessThan((throttles + 2) * 1500); + }); + + test('a throttled request makes concurrent requests wait for the same pause', async () => { + const { stats, mock } = startServer(client, buildTree([]), { capacity: 1, ratePerSecond: 1, retryAfter: '3' }); + + const results = await settle(Promise.all(Array.from({ length: 5 }, () => client.getChildPages('1')))); + mock.restore(); + + expect(results.error).toBeUndefined(); + // Nobody retried inside the 3 second window the server asked for. + expect(stats.rejectedTimes.length).toBeGreaterThan(0); + const first = stats.rejectedTimes[0]; + expect(stats.rejectedTimes.filter((time) => time > first && time < first + 3000)).toEqual([]); + }); +});