Repository layer has zero Redis-outage error handling — a Redis blip returns 500s instead of degrading gracefully

[prodbycorne]

Overview

src/services/priceOracle.js is carefully written to treat every Redis call as fallible — every cache.get/cache.set call is wrapped in try/catch with a fallback path, and this behavior is explicitly covered by test/resilience.test.js. src/middleware/rateLimit.js similarly fails open on Redis errors, with a comment documenting the intent.

None of the other Redis-backed modules follow this pattern. webhookRepository.js, deliveryRepository.js, apiKeys.js, airdrops.js, and alerts.js all call cache.getClient() and then invoke raw ioredis methods (sadd, smembers, zadd, zrevrange, lrange, rpush, llen, ...) or cache.get/cache.set with no error handling whatsoever:

// src/repositories/webhookRepository.js
async function list() {
  const redis = cache.getClient();
  const ids = await redis.smembers(IDS_KEY); // unhandled — throws straight up to the route handler
  const records = await Promise.all(ids.map((id) => cache.get(key(id))));
  return records.filter(Boolean).map(normalize);
}

// src/services/airdrops.js
async function create(data) {
  ...
  const redis = cache.getClient();
  await cache.set(airdropKey(id), airdrop);   // unhandled
  await redis.sadd(IDS_KEY, id);              // unhandled
  ...
}

Because getClient() uses lazyConnect: true and enableOfflineQueue: false (src/services/cache.js), any command issued while Redis is unreachable, mid-reconnect, or simply slow rejects immediately rather than queuing. Every one of these unhandled rejections propagates up through Express route handlers' try/catch blocks (which do exist at the route layer) and becomes a generic 500 INTERNAL_ERROR for every single webhook, airdrop, alert, and API-key operation during any Redis blip — a stark contrast to the price oracle, which was deliberately engineered to keep serving (possibly stale) data through the exact same failure mode. This means the one truly non-critical subsystem (price display) degrades gracefully, while airdrop management, webhook CRUD, and API key management — arguably more operationally important to keep at least partially functional — go fully down on any transient Redis hiccup.

Requirements

• Add explicit error handling at the repository layer (not just the route layer) for all Redis-backed repositories (webhookRepository, deliveryRepository, apiKeys, airdrops, alerts).
• Reads (list, findById, get) should degrade gracefully where a sensible degraded response exists (e.g. return an empty list with a response flag such as degraded: true rather than a hard 500), matching the redis_unavailable flag pattern already established in priceOracle.js's response shape.
• Writes (create, update, remove) that cannot succeed without Redis should fail with a clear, distinct error (e.g. a new AppError code like SERVICE_UNAVAILABLE / 503) rather than an opaque 500, so API consumers and monitoring can distinguish "your request was invalid" from "the backing store is currently down."
• Every route handler already has a catch (err) { return next(err) } — ensure the error itself now carries enough signal (a specific AppError subtype/code) to map to the correct HTTP status, rather than relying on the generic errorHandler fallback to 500 for everything.
• Add a dedicated AppError code (e.g. SERVICE_UNAVAILABLE, 503) to src/errors/AppError.js's ERROR_CODES map.

Acceptance Criteria

• Simulating a Redis outage (mocked getClient() throwing/rejecting) against GET /api/v1/webhooks, GET /api/v1/airdrops, GET /api/v1/alerts, and GET /api/v1/keys results in a clear, documented degraded response or a 503 SERVICE_UNAVAILABLE — never an unhandled-exception-driven generic 500.
• Write operations under simulated Redis outage return 503 SERVICE_UNAVAILABLE with a descriptive message rather than crashing the request with a stack-trace-bearing 500.
• AppError.codes includes a new SERVICE_UNAVAILABLE entry mapped to HTTP 503.
• New tests per affected repository simulate a Redis failure and assert the graceful/typed error behavior.
• /health endpoint's existing redis_connected/redis_unavailable fields remain accurate and are cross-referenced in the new error responses' details where useful.

Additional Notes

Additional edge cases / failure modes

• apiKeys.js's validateApiKey() (called from src/middleware/auth.js's requireApiKey() on every protected request) is on the hot path for every authenticated endpoint in the system — if Redis is down, cache.get(hashPath(hashed)) throws unhandled today, meaning every single protected endpoint (not just the api-keys CRUD routes) fails with a raw 500 during a Redis blip, including endpoints (like GET /prices/:asset_code/refresh) that otherwise have nothing to do with the repositories explicitly named in the Requirements. This is arguably the highest-impact single fix in this issue and deserves its own explicit mention/test, since it's not obviously implied by "fix the repository layer" — auth itself is a repository consumer here.
• The "fail open vs. fail closed" decision for validateApiKey() specifically needs to be made deliberately: unlike rateLimit.js's documented fail-open behavior (better to allow a request through than to block all traffic on a Redis blip), failing open on authentication (treating an unreachable Redis as "let the request through unauthenticated") would be a serious security regression — this repository almost certainly must fail closed (reject with 503, not silently authenticate) even though the general spirit of this issue is "degrade gracefully." Call this out explicitly as an intentional asymmetry, not an oversight, in the PR description.
• airdrops.js's getCurrentLedger() calls the Stellar Horizon API, not Redis — it's a separate external dependency with its own failure mode (routes/airdrops.js's POST /airdrops and PATCH /airdrops/:id both call it before validation even runs) that isn't Redis-related but suffers the identical "no error handling, unhandled rejection surfaces as generic 500" problem; worth deciding whether this issue's scope extends to Horizon-call resilience too, or whether that's tracked separately (it isn't listed among the 30 issues by title, so likely out of scope here — but flag the parallel unhandled-rejection pattern since fixing "Redis error handling" without touching this adjacent identical-shaped gap in the same file might look inconsistent in review).
• "Reads degrade gracefully with degraded: true" needs a precise contract for which reads can safely degrade to empty vs. which must hard-fail: listRecipients()/list() degrading to an empty page is misleading if a caller can't tell "there are zero airdrops" from "Redis is down and we lied about there being zero" — the degraded: true flag (mirroring redis_unavailable in priceOracle.js) is the right mechanism, but every degraded-read response needs it consistently applied, not just the top-level list envelope, since pagination.total: 0 alongside a missing/false degraded flag would look identical to a genuinely empty list to a naive client.
• webhookRepository.listActiveForEvent() (called synchronously from webhookDispatcher.dispatch() on every event) throwing during a Redis outage means all webhook dispatch silently fails during that window — combined with webhookDispatcher.dispatch() uses Promise.all — one failing target can obscure delivery outcome for every other subscriber #77's Promise.all/allSettled fix and webhookDispatcher.dispatch() has no idempotency protection against duplicate event_id delivery #75's idempotency fix, confirm the failure mode here (an exception before targets is even computed) is distinguishable from "zero targets are subscribed" so a caller doesn't misinterpret a Redis outage as "no one wanted this event."

Implementation sketch

// src/errors/AppError.js
const ERROR_CODES = Object.freeze({
  VALIDATION_ERROR: { statusCode: 400 },
  UNAUTHORIZED: { statusCode: 401 },
  NOT_FOUND: { statusCode: 404 },
  RATE_LIMITED: { statusCode: 429 },
  UPSTREAM_ERROR: { statusCode: 502 },
  SERVICE_UNAVAILABLE: { statusCode: 503 }, // new
  INTERNAL_ERROR: { statusCode: 500 },
});

// shared helper, e.g. src/repositories/redisGuard.js
async function withRedisRead(fn, fallback) {
  try {
    return await fn();
  } catch (err) {
    logger.warn('Redis read failed, degrading', { error: err.message });
    return fallback;
  }
}

async function withRedisWrite(fn) {
  try {
    return await fn();
  } catch (err) {
    throw new AppError('SERVICE_UNAVAILABLE', 'Storage backend is temporarily unavailable', 503, { cause: err.message });
  }
}

Wrap each repository's read functions with withRedisRead(..., { list: [], degraded: true })-style fallbacks and each write function's body with withRedisWrite(...). apiKeys.validateApiKey() should use withRedisWrite-style fail-closed behavior (propagate SERVICE_UNAVAILABLE) rather than withRedisRead's degrade-to-fallback behavior, per the fail-closed auth requirement above.

Test / reproduction plan

1. Mock cache.getClient() to return a client whose methods reject; call GET /api/v1/webhooks, /airdrops, /alerts, /keys; assert each responds with either a degraded: true empty-list body or a 503, per the chosen policy for that endpoint, and never an unhandled-exception 500.
2. Same outage simulation against POST /api/v1/webhooks, /airdrops, /alerts, /keys; assert 503 SERVICE_UNAVAILABLE with a descriptive AppError.
3. Same outage simulation against a protected endpoint via requireApiKey() (e.g. GET /prices/:asset_code/refresh); assert the request fails closed with 503, not with an auth bypass and not with a raw 500.
4. Assert AppError.codes.SERVICE_UNAVAILABLE.statusCode === 503.
5. Confirm /health's existing Redis-status fields remain accurate during the simulated outage and that a 503 response's details can reference the same underlying signal.

Related issues in this batch

• webhookDispatcher.dispatch() uses Promise.all — one failing target can obscure delivery outcome for every other subscriber #77 (Promise.all masking partial failures) — once repository calls throw typed SERVICE_UNAVAILABLE errors instead of raw ioredis exceptions, the per-target error messages surfaced in webhookDispatcher.dispatch() uses Promise.all — one failing target can obscure delivery outcome for every other subscriber #77's structured result become far more actionable ("storage unavailable" vs. an opaque driver error string).
• webhookDispatcher.dispatch() has no idempotency protection against duplicate event_id delivery #75 / deliveryRepository.popDueRetries() is not atomic — horizontally-scaled workers can double-process the same retry #76 (dispatch idempotency / atomic retry claim) — both fixes add new Redis operations (atomic claims, Lua scripts) that also need this issue's error-handling treatment; sequence-wise, landing this issue's shared withRedisRead/withRedisWrite helpers first gives those fixes a consistent pattern to build on rather than each inventing its own try/catch.
• Redis SMEMBERS-backed pagination is unstable across airdrops, alerts, and API-key listing endpoints #82 (SMEMBERS pagination instability) — touches the exact same read functions (list, listPaginated, listKeys) this issue wraps with degrade-gracefully handling; coordinate the two changes (or land one before the other) to avoid merge conflicts across airdrops.js, alerts.js, and apiKeys.js.
• Health check endpoint reports only Redis status — misses price-source, webhook-worker, and DB connectivity #92 (health check missing subsystem coverage, in the peer agent's Repository create() functions perform non-atomic two-step Redis writes, risking orphaned or dangling records on crash #84-Background jobs (price refresh, webhook retry worker) have no leader election — every horizontally-scaled replica runs them independently #98 range) — the degraded/redis_unavailable signal this issue standardizes across repositories is the same signal a more complete health check would want to aggregate; worth a cross-reference in review even though that issue is outside this batch's ownership.