Promise.all on our rate limiter would make a rejected request spend the budget the rejection was protecting

Munchable's API routes all start the same way: work out who is asking, then run a short list of rate-limit checks. The helper that runs the list is nine lines:

/** Enforce a chain of limiter checks; returns the FIRST failing decision. */
export async function enforce(
  checks: Array<{ limiter: Ratelimit; key: string }>,
): Promise<LimitDecision> {
  for (const { limiter, key } of checks) {
    const r = await limiter.limit(key);
    if (!r.success) {
      return { success: false, limit: r.limit, remaining: r.remaining, reset: r.reset };
    }
  }
  return { success: true };
}

The barcode lookup route passes it two checks, one keyed on the account and one keyed on the trusted client IP:

{ limiter: lim.productDev, key: user.id },
{ limiter: lim.productIp, key: ip },

Two independent checks, and through that loop, two sequential network round trips to Redis, both of them on the critical path of a scan that somebody is standing in a supermarket aisle waiting for. Promise.all is right there. It is an obvious, safe-looking, five-character change.

It is not a safe change, and working out why took longer than making it.

A limiter check is a write, not a question

The thing that makes this interesting is that limiter.limit(key) is not a predicate. It is a state mutation. Asking a sliding-window limiter "may I" increments that window. There is no way to ask without spending.

Now look at what several of our routes actually chain:

ocrDevMin:    mk(Ratelimit.slidingWindow(8, '1 m'),   'rl:ocr:dev:m'),
ocrDevDay:    mk(Ratelimit.slidingWindow(40, '1 d'),  'rl:ocr:dev:d'),
contribDevMin: mk(Ratelimit.slidingWindow(10, '1 m'),  'rl:contrib:dev:m'),
contribDevDay: mk(Ratelimit.slidingWindow(100, '1 d'), 'rl:contrib:dev:d'),
resolveDevMin: mk(Ratelimit.slidingWindow(30, '1 m'),  'rl:resolve:dev:m'),
resolveDevDay: mk(Ratelimit.slidingWindow(300, '1 d'), 'rl:resolve:dev:d'),

These are pairs: a per-minute burst limit in front of a per-day budget. They are deliberately ordered. The minute check is cheap to hit and cheap to recover from, and it exists so that somebody holding a button down, or a loop that has gone wrong, bounces off the minute limit without eating their allowance for the whole day.

Switch enforce to Promise.all and that stops being true everywhere at once. A request that the minute limiter was going to reject now also increments the day counter on its way to being rejected. Hold the button for a minute and you have spent a meaningful slice of a daily budget on requests that were all refused. Nothing errors. No test fails. The limits simply stop meaning what the numbers say they mean, in a way you notice weeks later when somebody says they ran out of something.

The short circuit was not an accident of writing a for loop. It was the behaviour.

So the fast version is a different function

export async function enforceParallel(
  checks: Array<{ limiter: Ratelimit; key: string }>,
): Promise<LimitDecision> {
  const results = await Promise.all(checks.map(({ limiter, key }) => limiter.limit(key)));
  for (const r of results) {
    if (!r.success) {
      return { success: false, limit: r.limit, remaining: r.remaining, reset: r.reset };
    }
  }
  return { success: true };
}

Same signature, same return contract, one extra property the caller has to accept: every counter is always incremented. The comment above it in lib/rate-limit.ts is longer than the function, and the last line of it is the one that matters:

Do not repoint an existing enforce caller at this function without checking whether it relies on the skip-on-earlier-failure property.

Two routes use it today, the barcode lookup and the product search. Both have exactly two checks, the account and the IP, and neither is in front of the other in any meaningful sense: they are the same question asked about two different subjects, and the route is a cheap read either way. Nine other call sites keep the loop.

The point I would make to anyone writing a shared helper: when you find a performance change that is safe for your caller and unsafe for the others, the cheapest honest move is a second function with a comment explaining the trade. Not a boolean option, which puts the dangerous behaviour one keystroke away from every existing call site, and not a refactor of the original, which makes ten routes depend on you having thought about all ten.

The 429 body has to be deterministic

Concurrency brings a second problem that is nothing to do with budgets. When two checks both fail, which failure does the caller see?

The decision object is not just a boolean. Callers read numbers off it:

export function tooManyRequests(scope: string, d: LimitDecision) {
  const retryAfter = d.reset
    ? Math.max(1, Math.ceil((d.reset - Date.now()) / 1000))
    : 1;
  return {
    body: { error: 'rate_limited', scope, retryAfter },
    headers: { 'Retry-After': String(retryAfter), ...rateLimitHeaders(d) },
  };
}

So the failing limiter decides the Retry-After and the RateLimit-* headers the client gets. If the implementation returned whichever rejection settled first, a client hitting both limits would get a Retry-After of 3 seconds on one request and 40 seconds on the next, from the same state, because Redis round trips do not finish in a fixed order. A client that respects Retry-After would then behave erratically, and a human reading logs would be chasing a ghost.

That is why enforceParallel iterates the awaited array afterwards rather than using anything like a race. The first failure by array order wins, which makes it identical to enforce in what it reports and different only in what it spends. And since that is the kind of property a later refactor drops without noticing, it has a test whose fake limiters resolve out of order on purpose:

test('enforceParallel: the first-by-array-order failure wins, not the first to settle', async () => {
  const slowAndFailing = fakeLimiter({ success: false, limit: 60, remaining: 0, reset: 111 }, 20);
  const fastAndFailing = fakeLimiter({ success: false, limit: 300, remaining: 0, reset: 222 }, 0);
  const decision = await enforceParallel([
    { limiter: slowAndFailing.limiter, key: 'user-1' },
    { limiter: fastAndFailing.limiter, key: 'ip-1' },
  ]);
  assert.equal(decision.limit, 60);
  assert.equal(decision.reset, 111);
});

The sibling test asserts the opposite of what you would normally assert: that the second limiter was called after the first one failed. The behaviour difference is the feature, so the feature gets a test.

Two smaller things in the same file

Never key on the leftmost forwarded-for token. It is client-supplied, so keying a limiter on it means anyone can mint a fresh bucket per request:

export function getClientIdentifier(request: NextRequest): string {
  const realIp = request.headers.get('x-real-ip');
  if (realIp?.trim()) return realIp.trim();
  const fwd = request.headers.get('x-forwarded-for');
  if (fwd?.trim()) {
    const parts = fwd.split(',').map((s) => s.trim()).filter(Boolean);
    if (parts.length) return parts[parts.length - 1];
  }
  return 'unknown';
}

Platform-set header first, closest hop second.

analytics: false on every limiter. Upstash will happily record per-key events for you. For a product whose promise is that it does not accumulate information about people, a rate limiter quietly building a per-user event stream on a third party is exactly the kind of thing that makes a privacy page a lie, so it is off at construction.

The general shape

Rate limiting reads like a validation step and behaves like a transaction. Once you see a limiter check as a write, three things follow:

1. The order of chained checks is part of your policy, not an implementation detail.
2. Making checks concurrent changes which budgets a rejected request consumes, which is a product decision, not an optimisation.
3. When two checks fail, the reported one must be chosen by a rule, not by a race, because clients read numbers out of it.

The product side of this, for context: the app answers a question about a packaged food on the device, and the routes these limiters guard are deliberately narrow reads. The public version of the same answers lives on the website, for instance is onion low FODMAP or the index of all of them at munchable.app/answers, and those pages are static, which is why none of this latency work touches them. The round trip this change removed belongs to somebody standing in an aisle with one bar of signal, which is the only reason it was worth the hour of reading the comments around it.