From d2c1a056ac63da9f49e40743321eeba2d22e8446 Mon Sep 17 00:00:00 2001 From: MAC Date: Fri, 26 Jun 2026 06:34:05 +0100 Subject: [PATCH] fix(api): surface shared-store (Redis/Upstash) liveness in /health (BUG-111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Upstash Redis becomes unreachable, UpstashStore silently falls back to per-replica in-memory enforcement for rate limits, WS connection caps, and auth bans — the only signal is a scattered logger.warn() per failed call. Operators had no way to see this degradation from /health. Add ping() to the SharedStore interface: InMemoryStore is trivially always healthy (it's the deliberately chosen backend), UpstashStore issues a real Redis PING without the fallback masking applied to its other methods. health.ts now reports this as a new `sharedStore` check. --- src/middleware/shared-store.ts | 29 ++++++++++++++ src/routes/health.ts | 14 ++++++- tests/middleware/shared-store.test.ts | 45 +++++++++++++++++++++ tests/routes/health.test.ts | 56 +++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 1 deletion(-) diff --git a/src/middleware/shared-store.ts b/src/middleware/shared-store.ts index d8763aa..c4500fd 100644 --- a/src/middleware/shared-store.ts +++ b/src/middleware/shared-store.ts @@ -105,6 +105,18 @@ export interface SharedStore { /** Evict stale auth-failure records (best-effort; no-op on Redis). */ evictExpiredAuthFailures(windowMs: number, banDurationMs: number): Promise; + + /** + * Liveness probe for the backing store, used by /health. + * + * InMemoryStore is trivially always healthy — it's the deliberately + * chosen single-replica backend, not a degraded state. UpstashStore + * issues a real Redis PING (no fallback masking) so operators can detect + * when multi-replica abuse-control guarantees have silently degraded to + * per-replica enforcement, instead of only finding out from scattered + * warning logs on individual failed calls. + */ + ping(): Promise; } // --------------------------------------------------------------------------- @@ -240,6 +252,10 @@ export class InMemoryStore implements SharedStore { if (stale) this.authFailures.delete(ip); } } + + async ping(): Promise { + return true; + } } // --------------------------------------------------------------------------- @@ -496,6 +512,19 @@ export class UpstashStore implements SharedStore { async evictExpiredAuthFailures(_windowMs: number, _banDurationMs: number): Promise { // Redis TTL handles eviction automatically. No-op here. } + + async ping(): Promise { + try { + const result = await this.cmd("PING"); + return typeof result === "string" && result.toUpperCase() === "PONG"; + } catch (err) { + logger.warn( + "UpstashStore.ping failed — Redis unreachable, abuse controls are running in per-replica fallback mode", + { error: err instanceof Error ? err.message : String(err) } + ); + return false; + } + } } // --------------------------------------------------------------------------- diff --git a/src/routes/health.ts b/src/routes/health.ts index 92050f3..8546c2a 100644 --- a/src/routes/health.ts +++ b/src/routes/health.ts @@ -5,6 +5,7 @@ import { withRpcFallback } from "../utils/rpc-fallback.js"; import { HEALTH_RPC_TIMEOUT_MS } from "../utils/rpc-timeout.js"; import { getWebSocketMetrics } from "./ws.js"; import { requireApiKey } from "../middleware/auth.js"; +import { getSharedStore } from "../middleware/shared-store.js"; const logger = createLogger("api:health"); const startTime = Date.now(); @@ -24,7 +25,7 @@ export function healthRoutes(): Hono { if (cachedHealth && Date.now() - cachedHealth.checkedAt < HEALTH_CACHE_TTL_MS) { return c.json(cachedHealth.body, cachedHealth.statusCode as 200 | 503); } - const checks: { db: boolean; rpc: boolean; ws: boolean } = { db: false, rpc: false, ws: false }; + const checks: { db: boolean; rpc: boolean; ws: boolean; sharedStore: boolean } = { db: false, rpc: false, ws: false, sharedStore: false }; let status: "ok" | "degraded" | "down" = "ok"; // Check RPC connectivity @@ -63,6 +64,17 @@ export function healthRoutes(): Hono { checks.ws = false; } + // Check the shared abuse-control store — when Upstash is configured but + // unreachable, rate limits/connection caps/auth bans silently degrade to + // per-replica enforcement. ping() surfaces that instead of leaving it to + // scattered warning logs (see shared-store.ts for the multi-replica + // rationale). + try { + checks.sharedStore = await getSharedStore().ping(); + } catch { + checks.sharedStore = false; + } + // Determine overall status const failedChecks = Object.values(checks).filter(v => !v).length; if (failedChecks === 0) { diff --git a/tests/middleware/shared-store.test.ts b/tests/middleware/shared-store.test.ts index 9deec45..00eb361 100644 --- a/tests/middleware/shared-store.test.ts +++ b/tests/middleware/shared-store.test.ts @@ -246,6 +246,51 @@ describe("UpstashStore — graceful fallback", () => { }); }); +// --------------------------------------------------------------------------- +// ping() liveness probe (BUG-111) +// --------------------------------------------------------------------------- + +describe("ping() liveness probe (BUG-111)", () => { + it("InMemoryStore.ping() is always true — it's the deliberately chosen backend", async () => { + const store = new InMemoryStore(); + expect(await store.ping()).toBe(true); + }); + + it("UpstashStore.ping() returns true on a successful PONG", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify([{ result: "PONG" }]), { status: 200 }) + ); + + const store = new UpstashStore("https://test.upstash.io", "token"); + expect(await store.ping()).toBe(true); + + globalThis.fetch = originalFetch; + }); + + it("UpstashStore.ping() returns false (not the silent in-memory fallback) when Redis is unreachable", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = vi.fn().mockRejectedValue(new Error("Network unreachable")); + + const store = new UpstashStore("https://test.upstash.io", "token"); + expect(await store.ping()).toBe(false); + + globalThis.fetch = originalFetch; + }); + + it("UpstashStore.ping() returns false on a non-PONG response", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify([{ result: null }]), { status: 200 }) + ); + + const store = new UpstashStore("https://test.upstash.io", "token"); + expect(await store.ping()).toBe(false); + + globalThis.fetch = originalFetch; + }); +}); + // --------------------------------------------------------------------------- // getSharedStore() singleton selection // --------------------------------------------------------------------------- diff --git a/tests/routes/health.test.ts b/tests/routes/health.test.ts index 6e08651..ee8eaa1 100644 --- a/tests/routes/health.test.ts +++ b/tests/routes/health.test.ts @@ -9,6 +9,13 @@ vi.mock("../../src/routes/ws.js", () => ({ })), })); +// Mock shared store +vi.mock("../../src/middleware/shared-store.js", () => ({ + getSharedStore: vi.fn(() => ({ + ping: vi.fn().mockResolvedValue(true), + })), +})); + // Mock @percolator/shared vi.mock("@percolator/shared", () => ({ getSupabase: vi.fn(), @@ -32,6 +39,7 @@ vi.mock("@percolator/shared", () => ({ const { getConnection, getSupabase } = await import("@percolator/shared"); const { getWebSocketMetrics } = await import("../../src/routes/ws.js"); +const { getSharedStore } = await import("../../src/middleware/shared-store.js"); describe("health routes", () => { let mockConnection: any; @@ -102,6 +110,7 @@ describe("health routes", () => { mockConnection.getSlot.mockRejectedValue(new Error("RPC error")); mockSupabase.select.mockRejectedValue(new Error("DB error")); vi.mocked(getWebSocketMetrics).mockImplementation(() => { throw new Error("WS unavailable"); }); + vi.mocked(getSharedStore).mockReturnValue({ ping: vi.fn().mockResolvedValue(false) } as any); const app = healthRoutes(); const res = await app.request("/health"); @@ -112,6 +121,53 @@ describe("health routes", () => { expect(data.checks.rpc).toBe(false); expect(data.checks.db).toBe(false); expect(data.checks.ws).toBe(false); + expect(data.checks.sharedStore).toBe(false); + }); + + describe("shared store health check (BUG-111)", () => { + it("flags sharedStore as unhealthy when ping() resolves false", async () => { + mockConnection.getSlot.mockResolvedValue(100); + mockSupabase.select.mockResolvedValue({ count: 5, error: null }); + vi.mocked(getSharedStore).mockReturnValue({ ping: vi.fn().mockResolvedValue(false) } as any); + + const app = healthRoutes(); + const res = await app.request("/health"); + const data = await res.json(); + expect(data.checks.sharedStore).toBe(false); + expect(data.status).not.toBe("ok"); + }); + + it("flags sharedStore as unhealthy when ping() throws", async () => { + mockConnection.getSlot.mockResolvedValue(100); + mockSupabase.select.mockResolvedValue({ count: 5, error: null }); + vi.mocked(getSharedStore).mockReturnValue({ + ping: vi.fn().mockRejectedValue(new Error("Redis unreachable")), + } as any); + + const app = healthRoutes(); + const res = await app.request("/health"); + const data = await res.json(); + expect(data.checks.sharedStore).toBe(false); + }); + + it("reports sharedStore healthy when ping() resolves true", async () => { + mockConnection.getSlot.mockResolvedValue(100); + mockSupabase.select.mockResolvedValue({ count: 5, error: null }); + // A prior test in this file overrides the ws mock's implementation; + // clearAllMocks() doesn't undo that, so restore an explicit healthy + // value here rather than depending on test execution order. + vi.mocked(getWebSocketMetrics).mockReturnValue({ + totalConnections: 0, + limits: { maxGlobalConnections: 1000 }, + } as any); + vi.mocked(getSharedStore).mockReturnValue({ ping: vi.fn().mockResolvedValue(true) } as any); + + const app = healthRoutes(); + const res = await app.request("/health"); + const data = await res.json(); + expect(data.checks.sharedStore).toBe(true); + expect(data.status).toBe("ok"); + }); }); it("should include uptime in response", async () => {