diff --git a/src/selfhost/redis-token-cache.ts b/src/selfhost/redis-token-cache.ts index 7246bfd6c4..035436d158 100644 --- a/src/selfhost/redis-token-cache.ts +++ b/src/selfhost/redis-token-cache.ts @@ -13,14 +13,24 @@ const REDIS_TOKEN_CACHE_METRIC = "loopover_redis_token_cache_total"; const keyFor = (installationId: number): string => `gh:insttoken:${installationId}`; -function recordTokenCacheMetric(result: "hit" | "miss"): void { +function recordTokenCacheMetric(result: "hit" | "miss" | "error"): void { incr(REDIS_TOKEN_CACHE_METRIC, { result }); } export function createRedisTokenCache(redis: Redis): InstallationTokenStore { return { async get(installationId: number) { - const raw = await redis.get(keyFor(installationId)); + // Fail open on a connection error, same contract as redis-cache.ts's webhook-dedup cache: the caller + // (github/app.ts's readCachedToken -> createInstallationToken) has no try/catch of its own, so an + // uncaught error here would hard-fail GitHub App token minting on every Redis hiccup instead of just + // costing one extra real mint. Unlike redis-cache.ts, still record a metric so the failure isn't invisible. + let raw: string | null; + try { + raw = await redis.get(keyFor(installationId)); + } catch { + recordTokenCacheMetric("error"); + return null; + } if (!raw) { recordTokenCacheMetric("miss"); return null; diff --git a/test/unit/selfhost-redis-token-cache.test.ts b/test/unit/selfhost-redis-token-cache.test.ts index 42013b6d10..43fb08cb2f 100644 --- a/test/unit/selfhost-redis-token-cache.test.ts +++ b/test/unit/selfhost-redis-token-cache.test.ts @@ -4,7 +4,7 @@ import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; import { createRedisTokenCache } from "../../src/selfhost/redis-token-cache"; /** Minimal ioredis stand-in that records the TTL passed to set(). */ -function fakeRedis(): { +function fakeRedis(options: { getThrows?: boolean } = {}): { redis: Redis; store: Map; ttl: () => number; @@ -13,6 +13,7 @@ function fakeRedis(): { let lastTtl = -1; const redis = { async get(k: string) { + if (options.getThrows) throw new Error("connection refused"); return store.get(k) ?? null; }, async set(k: string, v: string, _ex: "EX", ttl: number) { @@ -96,4 +97,17 @@ describe("createRedisTokenCache (#perf installation-token persistence)", () => { 'loopover_redis_token_cache_total{result="miss"} 1', ); }); + + it("regression: fails open (returns null) and records an error metric on a Redis connection failure (#6288)", async () => { + // Unlike a cache miss/malformed value, a connection failure must never throw uncaught here: the caller + // (github/app.ts's readCachedToken -> createInstallationToken) has no try/catch of its own, so an uncaught + // rejection would hard-fail GitHub App token minting on every Redis hiccup instead of costing one extra + // real mint. This must still be observable, unlike redis-cache.ts's silent fail-open. + const { redis } = fakeRedis({ getThrows: true }); + await expect(createRedisTokenCache(redis).get(9)).resolves.toBeNull(); + + expect(await renderMetrics()).toContain( + 'loopover_redis_token_cache_total{result="error"} 1', + ); + }); });