trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns14g rEventsFlows

A test pins the rate limiter's sliding window: no double burst across a minute boundary, refused requests not counted, admin allowance and the off switch #6725

Merged torquecrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

torque/forge:torque/6713-a-test-pins-the-rate-limiter-s-sliding-w → main 1 commit fork

  • Merged. torque/6713-a-test-pins-the-rate-limiter-s-sliding-w was merged into main as c974887.

Review

Sign in to review this change.
2 files changed +230 −3
src/core/ratelimit.ts +5 −3
@@ -62,9 +62,11 @@ export function limitsFromEnv(
6262 const perMinute = positive(raw, DEFAULT_LIMIT);
6363 return {
6464 perMinute,
65 adminPerMinute: positive(
66 env["FORGE_RATE_LIMIT_ADMIN"],
67 Math.max(perMinute, DEFAULT_ADMIN_LIMIT),
65 // An admin token is never held tighter than an ordinary one, whatever
66 // FORGE_RATE_LIMIT_ADMIN says.
67 adminPerMinute: Math.max(
68 perMinute,
69 positive(env["FORGE_RATE_LIMIT_ADMIN"], DEFAULT_ADMIN_LIMIT),
6870 ),
6971 enabled: true,
7072 };
test/ratelimit.test.ts added +225
@@ -0,0 +1,225 @@
1 /**
2 * The rate limiter driven directly, with a fake clock.
3 *
4 * The HTTP tests in agents.test.ts see 429s; these pin the arithmetic under
5 * them: the window slides (no double burst across a minute boundary), a
6 * refusal is not counted, Retry-After and X-RateLimit-Reset point at the
7 * moment the oldest hit leaves the window, and the environment parses the
8 * way the module comment promises.
9 */
10 import { describe, expect, test } from "bun:test";
11 import {
12 limitsFromEnv,
13 RateLimiter,
14 type RateLimits,
15 WINDOW_MS,
16 } from "../src/core/ratelimit.ts";
17
18 /** A limiter on a clock the test moves by hand; `at` is milliseconds. */
19 function clocked(limits: Partial<RateLimits> = {}) {
20 const clock = { at: 0 };
21 const limiter = new RateLimiter(
22 { perMinute: 3, adminPerMinute: 30, enabled: true, ...limits },
23 () => clock.at,
24 );
25 return { clock, limiter };
26 }
27
28 describe("RateLimiter", () => {
29 test("at the limit the next take is refused, pointing at when the oldest hit leaves", () => {
30 const { clock, limiter } = clocked({ perMinute: 3 });
31 for (const at of [1_000, 2_000, 3_000]) {
32 clock.at = at;
33 expect(limiter.take("k", "ro").allowed).toBe(true);
34 }
35 clock.at = 10_500;
36 const refused = limiter.take("k", "ro");
37 expect(refused.allowed).toBe(false);
38 expect(refused.remaining).toBe(0);
39 expect(refused.limit).toBe(3);
40 // Oldest hit at 1 000 ms leaves at 1 000 + 60 000 = 61 000 ms.
41 // Wait = 61 000 − 10 500 = 50 500 ms → ceil to 51 s.
42 expect(refused.retryAfter).toBe(51);
43 expect(refused.reset).toBe(61);
44
45 clock.at = 60_999;
46 expect(limiter.take("k", "ro").allowed).toBe(false);
47 clock.at = 61_000;
48 const back = limiter.take("k", "ro");
49 expect(back.allowed).toBe(true);
50 // Hits at 2 000, 3 000, 61 000 are live: 3 of 3 used.
51 expect(back.remaining).toBe(0);
52 });
53
54 test("allowed takes count down remaining and reset a window from now", () => {
55 const { clock, limiter } = clocked({ perMinute: 3 });
56 clock.at = 5_250;
57 const first = limiter.take("k", "rw");
58 expect(first).toEqual({
59 allowed: true,
60 limit: 3,
61 remaining: 2,
62 reset: Math.ceil((5_250 + WINDOW_MS) / 1000),
63 retryAfter: 0,
64 });
65 expect(limiter.take("k", "rw").remaining).toBe(1);
66 expect(limiter.take("k", "rw").remaining).toBe(0);
67 });
68
69 test("a burst at 59.9 s and another at 60.1 s: the second is refused", () => {
70 const limit = 10;
71 const { clock, limiter } = clocked({ perMinute: limit });
72 clock.at = 59_900;
73 let first = 0;
74 for (let i = 0; i < limit; i++) if (limiter.take("k", "ro").allowed) first++;
75 expect(first).toBe(limit);
76
77 // A fixed per-minute bucket starts afresh at 60 000 ms and would let all
78 // ten through: twenty requests in 200 ms. The sliding window still holds
79 // the ten from 59.9 s until 119.9 s.
80 clock.at = 60_100;
81 let second = 0;
82 for (let i = 0; i < limit; i++) if (limiter.take("k", "ro").allowed) second++;
83 expect(second).toBe(0);
84
85 clock.at = 119_899;
86 expect(limiter.take("k", "ro").allowed).toBe(false);
87 clock.at = 119_900;
88 expect(limiter.take("k", "ro").allowed).toBe(true);
89 });
90
91 test("a burst spread across the boundary is mostly refused on the far side", () => {
92 const limit = 10;
93 const { clock, limiter } = clocked({ perMinute: limit });
94 // Eight at 59.9 s leave two for the next minute, not ten.
95 clock.at = 59_900;
96 for (let i = 0; i < 8; i++) expect(limiter.take("k", "ro").allowed).toBe(true);
97 clock.at = 60_100;
98 let allowed = 0;
99 for (let i = 0; i < limit; i++) if (limiter.take("k", "ro").allowed) allowed++;
100 expect(allowed).toBe(2);
101 });
102
103 test("hammering while refused does not push the window forward", () => {
104 const { clock, limiter } = clocked({ perMinute: 2 });
105 clock.at = 0;
106 limiter.take("k", "ro");
107 clock.at = 1_000;
108 limiter.take("k", "ro");
109
110 clock.at = 30_000;
111 const promised = limiter.take("k", "ro");
112 expect(promised.allowed).toBe(false);
113 expect(promised.retryAfter).toBe(30);
114 expect(promised.reset).toBe(60);
115
116 // A poll loop with no sleep in it, every 10 ms until the promised time.
117 for (clock.at = 30_000; clock.at < 60_000; clock.at += 10) {
118 const verdict = limiter.take("k", "ro");
119 expect(verdict.allowed).toBe(false);
120 expect(verdict.reset).toBe(60);
121 }
122
123 clock.at = promised.reset * 1000;
124 expect(limiter.take("k", "ro").allowed).toBe(true);
125 });
126
127 test("admin scope uses adminPerMinute; ro and rw share one budget", () => {
128 const { limiter } = clocked({ perMinute: 2, adminPerMinute: 5 });
129
130 expect(limiter.take("token", "ro").allowed).toBe(true);
131 expect(limiter.take("token", "rw").allowed).toBe(true);
132 const rw = limiter.take("token", "rw");
133 expect(rw.allowed).toBe(false);
134 expect(rw.limit).toBe(2);
135 expect(limiter.take("token", "ro").allowed).toBe(false);
136
137 for (let i = 0; i < 5; i++) {
138 const verdict = limiter.take("admin", "admin");
139 expect(verdict.allowed).toBe(true);
140 expect(verdict.limit).toBe(5);
141 expect(verdict.remaining).toBe(4 - i);
142 }
143 expect(limiter.take("admin", "admin").allowed).toBe(false);
144 });
145
146 test("keys are counted apart", () => {
147 const { limiter } = clocked({ perMinute: 1 });
148 expect(limiter.take("a", "ro").allowed).toBe(true);
149 expect(limiter.take("a", "ro").allowed).toBe(false);
150 expect(limiter.take("b", "ro").allowed).toBe(true);
151 });
152
153 test("disabled, every take is allowed and nothing is counted", () => {
154 const { limiter } = clocked({ perMinute: 0, adminPerMinute: 0, enabled: false });
155 for (let i = 0; i < 1_000; i++) expect(limiter.take("k", "rw").allowed).toBe(true);
156 expect(limiter.size).toBe(0);
157 expect(limiter.enabled).toBe(false);
158 });
159
160 test("setLimits keeps the counted hits", () => {
161 const { clock, limiter } = clocked({ perMinute: 5 });
162 clock.at = 1_000;
163 for (let i = 0; i < 3; i++) limiter.take("k", "ro");
164
165 limiter.setLimits({ perMinute: 3, adminPerMinute: 30, enabled: true });
166 const tightened = limiter.take("k", "ro");
167 expect(tightened.allowed).toBe(false);
168 expect(tightened.limit).toBe(3);
169
170 limiter.setLimits({ perMinute: 10, adminPerMinute: 30, enabled: true });
171 const loosened = limiter.take("k", "ro");
172 expect(loosened.allowed).toBe(true);
173 // Three earlier hits plus this one: 10 − 4.
174 expect(loosened.remaining).toBe(6);
175 });
176 });
177
178 describe("limitsFromEnv", () => {
179 test("unset gives 600 ordinary and 6 000 admin", () => {
180 expect(limitsFromEnv({})).toEqual({
181 perMinute: 600,
182 adminPerMinute: 6_000,
183 enabled: true,
184 });
185 });
186
187 test("FORGE_RATE_LIMIT=0 turns it off", () => {
188 expect(limitsFromEnv({ FORGE_RATE_LIMIT: "0" }).enabled).toBe(false);
189 expect(
190 limitsFromEnv({ FORGE_RATE_LIMIT: "0", FORGE_RATE_LIMIT_ADMIN: "50" }).enabled,
191 ).toBe(false);
192 });
193
194 test("non-numeric and negative values fall back to 600", () => {
195 for (const raw of ["lots", "-5", "-0.5", "NaN", "Infinity"]) {
196 const limits = limitsFromEnv({ FORGE_RATE_LIMIT: raw });
197 expect(limits.enabled).toBe(true);
198 expect(limits.perMinute).toBe(600);
199 }
200 });
201
202 test("a fractional limit is floored", () => {
203 expect(limitsFromEnv({ FORGE_RATE_LIMIT: "12.9" }).perMinute).toBe(12);
204 });
205
206 test("the admin limit is never below the ordinary one", () => {
207 // Raising the ordinary limit past the admin default lifts admin with it.
208 expect(limitsFromEnv({ FORGE_RATE_LIMIT: "10000" }).adminPerMinute).toBe(10_000);
209 // An explicit admin limit below the ordinary one is lifted to it.
210 expect(
211 limitsFromEnv({ FORGE_RATE_LIMIT: "10000", FORGE_RATE_LIMIT_ADMIN: "100" })
212 .adminPerMinute,
213 ).toBe(10_000);
214 // A bad admin value falls back to the admin default.
215 expect(
216 limitsFromEnv({ FORGE_RATE_LIMIT: "5", FORGE_RATE_LIMIT_ADMIN: "-1" })
217 .adminPerMinute,
218 ).toBe(6_000);
219 // An explicit admin limit above the ordinary one is kept.
220 expect(
221 limitsFromEnv({ FORGE_RATE_LIMIT: "5", FORGE_RATE_LIMIT_ADMIN: "50" })
222 .adminPerMinute,
223 ).toBe(50);
224 });
225 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs