Made rate limiting fail open, not closed
// CONTEXT
Adding a Redis-backed sliding-window rate limiter middleware in front of every route: 5 req/min on auth endpoints, 100 req/min elsewhere.
// WHAT THE OBVIOUS VERSION WOULD DO
Fail closed: if Redis is unreachable, reject the request (503/429). It's the safer-sounding default — a rate limiter that silently stops enforcing feels broken, so the instinct is to block when you can't count. The problem is what that instinct actually buys you, which is the next section.
// THE CODE THAT DID IT
src/ledgercore/api/middleware/rate_limit.py:
redis = getattr(request.app.state, 'redis', None)
if redis is None:
return await call_next(request)
...
try:
count: int = await redis.incr(key)
if count == 1:
await redis.expire(key, _WINDOW_SECONDS)
except Exception:
return await call_next(request)// WHAT I FOUND
Two separate escape hatches, both landing on the same decision: if app.state.redis was never wired up, or if INCR/EXPIRE throws for any reason, the request is let through instead of rejected. The alternative — fail closed and reject every request when Redis is unreachable — turns a Redis outage into a full API outage. Rate limiting is a defense-in-depth layer, not the primary security boundary (auth + RBAC are), so I decided an unlimited-but-available API beats a correctly-limited-but-down one.
// LIMIT
The bare except Exception swallows everything, not just connection errors — and there's no logging/metrics library anywhere in the codebase (grep across src/ and pyproject.toml turns up nothing: no logger, no Sentry, no Prometheus). So a Redis outage doesn't just fail open — it fails silently: no log line, no metric, no alert. The only visible symptom would be someone noticing rate limiting isn't kicking in.
// TAKEAWAY
Fail-open without an observable signal is just "broken and I won't know" — the fail-open decision was right, but it needs at minimum a log line in the except block to be an operational choice rather than a blind spot.