Rate limiter refill bug causes queued requests to always time out

[juhanisaa]

Bug

The refill() method in src/services/rate-limiter.ts calculates token replenishment using the full refillIntervalMs (60s) as the divisor instead of the per-token interval (refillIntervalMs / refillRate = 7.5s).

This means after the initial burst of 8 tokens is consumed, the next token only becomes available after 60s — but the queue timeout is 30s. Queued requests always time out before a token can be refilled.

Root cause

// Current (broken): tokens refill all-at-once after full interval
const intervalsElapsed = Math.floor(elapsed / this.refillIntervalMs);
const tokensToAdd = intervalsElapsed * this.refillRate;

Should be:

// Fixed: tokens refill individually at evenly spaced intervals
const msPerToken = this.refillIntervalMs / this.refillRate;
const tokensToAdd = Math.floor(elapsed / msPerToken);

Additionally, queued requests only schedule a single setTimeout for refill checking. If multiple requests are queued, only the first one triggers a check — the rest are never processed.

Impact

With default config (8 tokens/min, 30s queue timeout): any request that arrives after the initial 8-request burst will hang for 30s and then fail with Rate limit queue timeout.

Suggested fix

1. Change refill() to calculate msPerToken = refillIntervalMs / refillRate and use that for both token calculation and lastRefill update
2. Replace the one-shot setTimeout with a setInterval-based scheduleRefill() that runs while requests are queued

[markusl]

Confirmed and fixed in #2. Thanks for the precise diagnosis — your root-cause analysis and suggested fix #1 were spot on.

The core bug is real. refill() floored elapsed time on the full refillIntervalMs (60s), so a drained bucket only replenished after the whole interval. Since queued requests time out at 30s, anything past the initial 8-request burst always failed with Rate limit queue timeout. The fix computes msPerToken = refillIntervalMs / refillRate and floors against that (plus carries the sub-token remainder forward), so tokens trickle back every 7.5s and queued requests resolve at ~7.5s.

One correction on the secondary claim: the report says "only the first queued request triggers a check — the rest are never processed." That part doesn't hold — each acquire() schedules its own setTimeout(() => this.refill(), msUntilNextToken). Those checks were firing fine; they were just no-ops because of the flooring bug. With refill() fixed they work correctly, so the setInterval rewrite (suggested fix #2) isn't necessary. If we later want a single shared timer for efficiency, that's a separate, optional cleanup.

Added regression tests using the production config (8 / 60s) that fail on the old code and pass with the fix: a 9th queued request now resolves at 7.5s instead of timing out, and the remainder carries across refills.