Why does this rate limiter keep blocking users after the window should reset?

programming web dev
1

Hey everyone, I’m working on a small API gateway for a side project, and I’m trying to keep the rate limiter simple before I add Redis later. Right now it mostly works, but some users stay blocked longer than the window, which feels risky if I scale this pattern up.

class RateLimiter {
  constructor(limit, windowMs) {
    this.limit = limit;
    this.windowMs = windowMs;
    this.hits = new Map();
  }

  allow(userId, now = Date.now()) {
    const entry = this.hits.get(userId) || { count: 0, start: now };

    if (now - entry.start > this.windowMs) {
      entry.count = 0;
    }

    entry.count++;
    this.hits.set(userId, entry);

    return entry.count <= this.limit;
  }
}

What am I missing in this window reset logic that would make a user keep getting denied after enough time has passed?

Hari

2

@HariSeldon Your entry is currently keeping the old window start time around, not starting a fresh window after reset.

You reset count, but start still points to the original timestamp. That means the limiter never establishes a clean new window, so later requests are evaluated against stale time and the behavior drifts. The boundary check should also usually be >= so the reset happens exactly when the window is reached.

The smallest fix is this:

if (now - entry.start >= this.windowMs) {
  entry.count = 0;
  entry.start = now;
}

Here’s the same change in your method:

allow(userId, now = Date.now()) {
  const entry = this.hits.get(userId) || { count: 0, start: now };

  if (now - entry.start >= this.windowMs) {
    entry.count = 0;
    entry.start = now;
  }

  entry.count++;
  this.hits.set(userId, entry);
  return entry.count <= this.limit;
}

With entry.start = now and >= this.windowMs in place:

  • entry.start = now begins a real new fixed window for that user.
  • >= ensures the reset happens right at the window boundary.

That makes the next request after expiry count as request 1 in a fresh window.

You can see it behave cleanly here:

class RateLimiter {
  constructor(limit, windowMs) {
    this.limit = limit;
    this.windowMs = windowMs;
    this.hits = new Map();
  }

  allow(userId, now = Date.now()) {
    const entry = this.hits.get(userId) || { count: 0, start: now };

    if (now - entry.start >= this.windowMs) {
      entry.count = 0;
      entry.start = now;
    }

    entry.count++;
    this.hits.set(userId, entry);
    return entry.count <= this.limit;
  }
}

const rl = new RateLimiter(2, 1000);

console.log(rl.allow("u1", 0));    // true
console.log(rl.allow("u1", 100));  // true
console.log(rl.allow("u1", 200));  // false
console.log(rl.allow("u1", 1000)); // true
console.log(rl.allow("u1", 1100)); // true
console.log(rl.allow("u1", 1200)); // false

Tiny practical snippet: if you want to avoid memory growth later, delete idle users from this.hits after long inactivity.

Yoshiii

3

@Yoshiii the entry.start = now bit is the real fix, and one small caveat is that this is still a fixed-window limiter so users can burst at the boundary and squeeze nearly 2x the limit if timing lines up.

Arthur