From e7cb01a79ce840928926265a2f3abc8e3f44f83d Mon Sep 17 00:00:00 2001 From: Alex Drankou Date: Fri, 15 May 2026 16:09:19 +0200 Subject: [PATCH 1/2] refactor: replace @vercel/kv with node-redis client --- .env.example | 6 +- .github/workflows/test.yml | 5 +- README.md | 9 +- bun.lock | 23 +++-- package.json | 2 +- scripts/check-app-env.ts | 7 -- src/core/shared/clients/kv.ts | 89 ++++++++++++++--- src/lib/env.ts | 9 +- tests/unit/kv.test.ts | 177 +++++++++++++++++++++++++++++----- 9 files changed, 261 insertions(+), 66 deletions(-) diff --git a/.env.example b/.env.example index cacadb567..c7b3adae8 100644 --- a/.env.example +++ b/.env.example @@ -33,11 +33,9 @@ NEXT_PUBLIC_SUPABASE_ANON_KEY=your_supabase_anon_key ### ZeroBounce API key for email validation # ZEROBOUNCE_API_KEY= -### Optional KV database configuration ### Required only for KV-backed capabilities such as alternate-email warning dedupe. -### Must be Vercel/Upstash Redis REST compatible; raw redis://localhost:6379 is not supported by @vercel/kv. -# KV_REST_API_TOKEN= -# KV_REST_API_URL= +### Use redis:// for local/self-hosted Redis or rediss:// for TLS-enabled Redis. +# REDIS_URL=redis://localhost:6379 ### Plain API key for customer support integration # (Required if NEXT_PUBLIC_INCLUDE_REPORT_ISSUE=1) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 55683c7f5..51448749b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,10 +45,7 @@ jobs: runs-on: ubuntu-latest needs: unit-tests env: - KV_URL: redis://localhost:6379 - KV_REST_API_READ_ONLY_TOKEN: test-read-only-token - KV_REST_API_TOKEN: test-api-token - KV_REST_API_URL: https://test-kv-api.example.com + REDIS_URL: redis://localhost:6379 SUPABASE_SERVICE_ROLE_KEY: test-service-role-key BILLING_API_URL: https://billing.e2b-test.dev NEXT_PUBLIC_E2B_DOMAIN: e2b-test.dev diff --git a/README.md b/README.md index 18189ae6a..93216cdfb 100644 --- a/README.md +++ b/README.md @@ -105,15 +105,12 @@ cp .env.example .env.local #### a. Key-Value Store Setup Redis/KV is optional for standard dashboard deployments, including local, enterprise, and on-prem environments. The dashboard can boot and run core auth and dashboard workflows without KV configured. -KV is currently used for optional capability checks and for deduplicating ZeroBounce alternate-email warnings. If you need those capabilities, configure a Vercel/Upstash Redis REST-compatible store: +KV is currently used for optional capability checks and for deduplicating ZeroBounce alternate-email warnings. If you need those capabilities, configure a Redis-compatible store: ``` - KV_REST_API_URL=your_redis_rest_api_url - KV_REST_API_TOKEN=your_redis_api_write_token + REDIS_URL=redis://localhost:6379 ``` -> **Note**: `@vercel/kv` expects a Redis REST API. A raw Redis server such as `redis://localhost:6379` is not compatible without an Upstash-compatible REST proxy. - -> **Health check**: When `KV_REST_API_URL` and `KV_REST_API_TOKEN` are set, `/api/health` will report `503 degraded` if KV is unreachable. Leave both unset to opt out of the KV health check entirely. +> **Health check**: When `REDIS_URL` is set, `/api/health` will report `503 degraded` if Redis is unreachable. Leave it unset to opt out of the Redis health check entirely. 6. Start the development server: ```bash diff --git a/bun.lock b/bun.lock index 2c9788506..30cf71ba3 100644 --- a/bun.lock +++ b/bun.lock @@ -1,5 +1,6 @@ { "lockfileVersion": 1, + "configVersion": 0, "workspaces": { "": { "name": "@e2b/dashboard", @@ -48,7 +49,6 @@ "@trpc/tanstack-react-query": "^11.7.1", "@types/micromatch": "^4.0.9", "@vercel/analytics": "^1.5.0", - "@vercel/kv": "^3.0.0", "@vercel/otel": "^1.13.0", "@vercel/speed-insights": "^1.2.0", "cheerio": "^1.0.0", @@ -83,6 +83,7 @@ "react-icons": "^5.4.0", "react-shiki": "^0.5.2", "recharts": "^2.15.1", + "redis": "^5.12.1", "semver": "^7.7.2", "serialize-error": "^12.0.0", "server-only": "^0.0.1", @@ -762,6 +763,16 @@ "@radix-ui/rect": ["@radix-ui/rect@1.1.1", "", {}, "sha512-HPwpGIzkl28mWyZqG52jiqDJ12waP11Pa1lGoiyUkIEuMLBP0oeK/C89esbXrxsky5we7dfd8U58nm0SgAWpVw=="], + "@redis/bloom": ["@redis/bloom@5.12.1", "", { "peerDependencies": { "@redis/client": "^5.12.1" } }, "sha512-PUUfv+ms7jgPSBVoo/DN4AkPHj4D5TZSd6SbJX7egzBplkYUcKmHRE8RKia7UtZ8bSQbLguLvxVO+asKtQfZWA=="], + + "@redis/client": ["@redis/client@5.12.1", "", { "dependencies": { "cluster-key-slot": "1.1.2" }, "peerDependencies": { "@node-rs/xxhash": "^1.1.0", "@opentelemetry/api": ">=1 <2" }, "optionalPeers": ["@node-rs/xxhash", "@opentelemetry/api"] }, "sha512-7aPGWeqA3uFm43o19umzdl16CEjK/JQGtSXVPevplTaOU3VJA/rseBC1QvYUz9lLDIMBimc4SW/zrW4S89BaCA=="], + + "@redis/json": ["@redis/json@5.12.1", "", { "peerDependencies": { "@redis/client": "^5.12.1" } }, "sha512-eOze75esLve4vfqDel7aMX08CNaiLLQS2fV8mpRN9NxPe1rVR4vQyYiW/OgtGUysF6QOr9ANhfxABKNOJfXdKg=="], + + "@redis/search": ["@redis/search@5.12.1", "", { "peerDependencies": { "@redis/client": "^5.12.1" } }, "sha512-ItlxbxC9cKI6IU1TLWoczwJCRb6TdmkEpWv05UrPawqaAnWGRu3rcIqsc5vN483T2fSociuyV1UkWIL5I4//2w=="], + + "@redis/time-series": ["@redis/time-series@5.12.1", "", { "peerDependencies": { "@redis/client": "^5.12.1" } }, "sha512-c6JL6E3EcZJuNqKFz+KM+l9l5mpcQiKvTwgA3blt5glWJ8hjDk0yeHN3beE/MpqYIQ8UEX44ItQzgkE/gCBELQ=="], + "@redocly/ajv": ["@redocly/ajv@8.11.4", "", { "dependencies": { "fast-deep-equal": "^3.1.1", "json-schema-traverse": "^1.0.0", "require-from-string": "^2.0.2", "uri-js-replace": "^1.0.1" } }, "sha512-77MhyFgZ1zGMwtCpqsk532SJEc3IJmSOXKTCeWoMTAvPnQOkuOgxEip1n5pG5YX1IzCTJ4kCvPKr8xYyzWFdhg=="], "@redocly/config": ["@redocly/config@0.22.2", "", {}, "sha512-roRDai8/zr2S9YfmzUfNhKjOF0NdcOIqF7bhf4MVC5UxpjIysDjyudvlAiVbpPHp3eDRWbdzUgtkK1a7YiDNyQ=="], @@ -1006,12 +1017,8 @@ "@ungap/structured-clone": ["@ungap/structured-clone@1.3.0", "", {}, "sha512-WmoN8qaIAo7WTYWbAZuG8PYEhn5fkz7dZrqTBZ7dtt//lL2Gwms1IcnQ5yHqjDfX8Ft5j4YzDM23f87zBfDe9g=="], - "@upstash/redis": ["@upstash/redis@1.35.6", "", { "dependencies": { "uncrypto": "^0.1.3" } }, "sha512-aSEIGJgJ7XUfTYvhQcQbq835re7e/BXjs8Janq6Pvr6LlmTZnyqwT97RziZLO/8AVUL037RLXqqiQC6kCt+5pA=="], - "@vercel/analytics": ["@vercel/analytics@1.5.0", "", { "peerDependencies": { "@remix-run/react": "^2", "@sveltejs/kit": "^1 || ^2", "next": ">= 13", "react": "^18 || ^19 || ^19.0.0-rc", "svelte": ">= 4", "vue": "^3", "vue-router": "^4" }, "optionalPeers": ["@remix-run/react", "@sveltejs/kit", "next", "react", "svelte", "vue", "vue-router"] }, "sha512-MYsBzfPki4gthY5HnYN7jgInhAZ7Ac1cYDoRWFomwGHWEX7odTEzbtg9kf/QSo7XEsEAqlQugA6gJ2WS2DEa3g=="], - "@vercel/kv": ["@vercel/kv@3.0.0", "", { "dependencies": { "@upstash/redis": "^1.34.0" } }, "sha512-pKT8fRnfyYk2MgvyB6fn6ipJPCdfZwiKDdw7vB+HL50rjboEBHDVBEcnwfkEpVSp2AjNtoaOUH7zG+bVC/rvSg=="], - "@vercel/otel": ["@vercel/otel@1.14.0", "", { "peerDependencies": { "@opentelemetry/api": ">=1.7.0 <2.0.0", "@opentelemetry/api-logs": ">=0.46.0 <0.200.0", "@opentelemetry/instrumentation": ">=0.46.0 <0.200.0", "@opentelemetry/resources": ">=1.19.0 <2.0.0", "@opentelemetry/sdk-logs": ">=0.46.0 <0.200.0", "@opentelemetry/sdk-metrics": ">=1.19.0 <2.0.0", "@opentelemetry/sdk-trace-base": ">=1.19.0 <2.0.0" } }, "sha512-4FXrYvjmHh3ia2v/TN/iiz0oVIw1xSnkW/MzRtsUGpu4jlcfu1qXJIfugQ1iAZnRolyTKn8FxhoWA6EhiAIZBg=="], "@vercel/speed-insights": ["@vercel/speed-insights@1.2.0", "", { "peerDependencies": { "@sveltejs/kit": "^1 || ^2", "next": ">= 13", "react": "^18 || ^19 || ^19.0.0-rc", "svelte": ">= 4", "vue": "^3", "vue-router": "^4" }, "optionalPeers": ["@sveltejs/kit", "next", "react", "svelte", "vue", "vue-router"] }, "sha512-y9GVzrUJ2xmgtQlzFP2KhVRoCglwfRQgjyfY607aU0hh0Un6d0OUyrJkjuAlsV18qR4zfoFPs/BiIj9YDS6Wzw=="], @@ -1122,6 +1129,8 @@ "clsx": ["clsx@2.1.1", "", {}, "sha512-eYm0QWBtUrBWZWG0d386OGAw16Z995PiOVo2B7bjWSbHedGl5e0ZWaq65kOGgUSNesEIDkB9ISbTg/JK9dhCZA=="], + "cluster-key-slot": ["cluster-key-slot@1.1.2", "", {}, "sha512-RMr0FhtfXemyinomL4hrWcYJxmX6deFdCxpJzhDttxgO1+bcCnkk+9drydLVDmAMG7NE6aN/fl4F7ucU/90gAA=="], + "cmdk": ["cmdk@1.1.1", "", { "dependencies": { "@radix-ui/react-compose-refs": "^1.1.1", "@radix-ui/react-dialog": "^1.1.6", "@radix-ui/react-id": "^1.1.0", "@radix-ui/react-primitive": "^2.0.2" }, "peerDependencies": { "react": "^18 || ^19 || ^19.0.0-rc", "react-dom": "^18 || ^19 || ^19.0.0-rc" } }, "sha512-Vsv7kFaXm+ptHDMZ7izaRsP70GgrW9NBNGswt9OZaVBLlE0SNpDq8eu/VGXyF9r7M0azK3Wy7OlYXsuyYLFzHg=="], "color-convert": ["color-convert@2.0.1", "", { "dependencies": { "color-name": "~1.1.4" } }, "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ=="], @@ -1642,6 +1651,8 @@ "recharts-scale": ["recharts-scale@0.4.5", "", { "dependencies": { "decimal.js-light": "^2.4.1" } }, "sha512-kivNFO+0OcUNu7jQquLXAxz1FIwZj8nrj+YkOKc5694NbjCvcT6aSZiIzNzd2Kul4o4rTto8QVR9lMNtxD4G1w=="], + "redis": ["redis@5.12.1", "", { "dependencies": { "@redis/bloom": "5.12.1", "@redis/client": "5.12.1", "@redis/json": "5.12.1", "@redis/search": "5.12.1", "@redis/time-series": "5.12.1" } }, "sha512-LDsoVvb/CpoV9EN3FXvgvSHNJWuCIzl9MiO3ppOevuGLpSGJhwfQjpEwfFJcQvNSddHADDdZaWx0HnmMxRXG7g=="], + "regex": ["regex@6.0.1", "", { "dependencies": { "regex-utilities": "^2.3.0" } }, "sha512-uorlqlzAKjKQZ5P+kTJr3eeJGSVroLKoHmquUj4zHWuR+hEyNqlXsSKlYYF5F4NI6nl7tWCs0apKJ0lmfsXAPA=="], "regex-recursion": ["regex-recursion@6.0.2", "", { "dependencies": { "regex-utilities": "^2.3.0" } }, "sha512-0YCaSCq2VRIebiaUviZNs0cBz1kg5kVS2UKUfNIx8YVs1cN3AV7NTctO5FOKBA+UT2BPJIWZauYHPqJODG50cg=="], @@ -1814,8 +1825,6 @@ "unbash": ["unbash@3.0.0", "", {}, "sha512-FeFPZ/WFT0mbRCuydiZzpPFlrYN8ZUpphQKoq4EeElVIYjYyGzPMxQR/simUwCOJIyVhpFk4RbtyO7RuMpMnHA=="], - "uncrypto": ["uncrypto@0.1.3", "", {}, "sha512-Ql87qFHB3s/De2ClA9e0gsnS6zXG27SkTiSJwjCc9MebbfapQfuPzumMIUMi38ezPZVNFcHI9sUIepeQfw8J8Q=="], - "undici": ["undici@7.16.0", "", {}, "sha512-QEg3HPMll0o3t2ourKwOeUAZ159Kn9mx5pnzHRQO8+Wixmh88YdZRiIwat0iNzNNXn0yoEtXJqFpyW7eM8BV7g=="], "undici-types": ["undici-types@6.20.0", "", {}, "sha512-Ny6QZ2Nju20vw1SRHe3d9jVu6gJ+4e3+MMpqu7pqE5HT6WsTSlce++GQmK5UXS8mzV8DSYHrQH+Xrf2jVcuKNg=="], diff --git a/package.json b/package.json index 5a8b7b36d..75c28c222 100644 --- a/package.json +++ b/package.json @@ -95,7 +95,6 @@ "@trpc/tanstack-react-query": "^11.7.1", "@types/micromatch": "^4.0.9", "@vercel/analytics": "^1.5.0", - "@vercel/kv": "^3.0.0", "@vercel/otel": "^1.13.0", "@vercel/speed-insights": "^1.2.0", "cheerio": "^1.0.0", @@ -130,6 +129,7 @@ "react-icons": "^5.4.0", "react-shiki": "^0.5.2", "recharts": "^2.15.1", + "redis": "^5.12.1", "semver": "^7.7.2", "serialize-error": "^12.0.0", "server-only": "^0.0.1", diff --git a/scripts/check-app-env.ts b/scripts/check-app-env.ts index 367bdd401..88871a756 100644 --- a/scripts/check-app-env.ts +++ b/scripts/check-app-env.ts @@ -6,13 +6,6 @@ loadEnvConfig(projectDir) const schema = serverSchema .merge(clientSchema) - .refine( - (data) => Boolean(data.KV_REST_API_URL) === Boolean(data.KV_REST_API_TOKEN), - { - message: 'KV_REST_API_URL and KV_REST_API_TOKEN must be set together', - path: ['KV_REST_API_URL'], - } - ) .refine( (data) => { if (data.NEXT_PUBLIC_INCLUDE_BILLING === '1') { diff --git a/src/core/shared/clients/kv.ts b/src/core/shared/clients/kv.ts index a15cacb10..f40068b1a 100644 --- a/src/core/shared/clients/kv.ts +++ b/src/core/shared/clients/kv.ts @@ -1,4 +1,6 @@ -import { kv } from '@vercel/kv' +import 'server-only' +import { createClient } from 'redis' +import { l, serializeErrorForLog } from './logger/logger' export type OptionalKvResult = | { ok: true; configured: true; value: T } @@ -15,19 +17,71 @@ export type KvCapabilityStatus = | { configured: true; available: true; status: 'ok' } | { configured: true; available: false; status: 'error'; error: unknown } -function getKvConfigStatus() { - const hasUrl = Boolean(process.env.KV_REST_API_URL) - const hasToken = Boolean(process.env.KV_REST_API_TOKEN) +type KvConfigStatus = 'not_configured' | 'misconfigured' | 'configured' +type RedisClient = ReturnType - if (!(hasUrl || hasToken)) { +let redisClient: RedisClient | null = null +let redisConnectPromise: Promise | null = null + +function isValidRedisUrl(url: string) { + try { + const parsedUrl = new URL(url) + return parsedUrl.protocol === 'redis:' || parsedUrl.protocol === 'rediss:' + } catch { + return false + } +} + +function getKvConfigStatus(): KvConfigStatus { + const redisUrl = process.env.REDIS_URL + + if (!redisUrl) { return 'not_configured' } - if (hasUrl && hasToken) { - return 'configured' + if (!isValidRedisUrl(redisUrl)) { + return 'misconfigured' } - return 'misconfigured' + return 'configured' +} + +function createRedisClient() { + const client = createClient({ + url: process.env.REDIS_URL, + }) + + client.on('error', (error) => { + l.error( + { + key: 'redis_client:error', + error: serializeErrorForLog(error), + }, + 'Redis client error' + ) + }) + + return client +} + +async function getRedisClient() { + if (redisClient?.isReady) { + return redisClient + } + + if (!redisClient) { + redisClient = createRedisClient() + } + + if (!redisConnectPromise) { + redisConnectPromise = redisClient.connect().catch((error) => { + redisConnectPromise = null + redisClient = null + throw error + }) + } + + return redisConnectPromise } export function isKvConfigured() { @@ -42,7 +96,8 @@ export async function pingKv(): Promise { } try { - await kv.ping() + const redis = await getRedisClient() + await redis.ping() return { configured: true, available: true, status: 'ok' } } catch (error) { return { configured: true, available: false, status: 'error', error } @@ -59,7 +114,14 @@ export async function getKvValue( } try { - return { ok: true, configured: true, value: await kv.get(key) } + const redis = await getRedisClient() + const value = await redis.get(key) + + return { + ok: true, + configured: true, + value: value === null ? null : (JSON.parse(value) as T), + } } catch (error) { return { ok: false, configured: true, reason: 'error', error } } @@ -76,7 +138,12 @@ export async function setKvValue( } try { - return { ok: true, configured: true, value: await kv.set(key, value) } + const redis = await getRedisClient() + return { + ok: true, + configured: true, + value: await redis.set(key, JSON.stringify(value)), + } } catch (error) { return { ok: false, configured: true, reason: 'error', error } } diff --git a/src/lib/env.ts b/src/lib/env.ts index ab016be32..281577c41 100644 --- a/src/lib/env.ts +++ b/src/lib/env.ts @@ -2,8 +2,13 @@ import { z } from 'zod' export const serverSchema = z.object({ SUPABASE_SERVICE_ROLE_KEY: z.string().min(1), - KV_REST_API_TOKEN: z.string().min(1).optional(), - KV_REST_API_URL: z.url().optional(), + REDIS_URL: z + .url() + .refine( + (url) => url.startsWith('redis://') || url.startsWith('rediss://'), + 'REDIS_URL must use redis:// or rediss://' + ) + .optional(), ENABLE_USER_BOOTSTRAP: z.string().optional(), DASHBOARD_API_ADMIN_TOKEN: z.string().min(1).optional(), diff --git a/tests/unit/kv.test.ts b/tests/unit/kv.test.ts index 47abfb881..854c1ae05 100644 --- a/tests/unit/kv.test.ts +++ b/tests/unit/kv.test.ts @@ -1,53 +1,182 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { ping } = vi.hoisted(() => ({ - ping: vi.fn(), +const { createClient, redisClient } = vi.hoisted(() => { + const client = { + isReady: false, + on: vi.fn(), + connect: vi.fn(), + ping: vi.fn(), + get: vi.fn(), + set: vi.fn(), + } + + client.connect.mockImplementation(async () => { + client.isReady = true + return client + }) + + return { + createClient: vi.fn(() => client), + redisClient: client, + } +}) + +vi.mock('redis', () => ({ + createClient, })) -vi.mock('@vercel/kv', () => ({ - kv: { - ping, +vi.mock('@/core/shared/clients/logger/logger', () => ({ + l: { + error: vi.fn(), }, + serializeErrorForLog: vi.fn((error) => error), })) -import { pingKv } from '@/core/shared/clients/kv' +const originalRedisUrl = process.env.REDIS_URL -const originalKvEnv = { - KV_REST_API_TOKEN: process.env.KV_REST_API_TOKEN, - KV_REST_API_URL: process.env.KV_REST_API_URL, +async function loadKvClient() { + vi.resetModules() + return import('@/core/shared/clients/kv') } -function resetKvEnv() { - if (originalKvEnv.KV_REST_API_TOKEN === undefined) { - delete process.env.KV_REST_API_TOKEN +function resetRedisEnv() { + if (originalRedisUrl === undefined) { + delete process.env.REDIS_URL } else { - process.env.KV_REST_API_TOKEN = originalKvEnv.KV_REST_API_TOKEN - } - - if (originalKvEnv.KV_REST_API_URL === undefined) { - delete process.env.KV_REST_API_URL - } else { - process.env.KV_REST_API_URL = originalKvEnv.KV_REST_API_URL + process.env.REDIS_URL = originalRedisUrl } } describe('optional KV client', () => { beforeEach(() => { vi.clearAllMocks() - delete process.env.KV_REST_API_TOKEN - delete process.env.KV_REST_API_URL + redisClient.isReady = false + redisClient.connect.mockImplementation(async () => { + redisClient.isReady = true + return redisClient + }) + delete process.env.REDIS_URL }) afterEach(() => { - resetKvEnv() + resetRedisEnv() }) - it('reports KV as not configured when both env values are omitted', async () => { + it('reports KV as not configured when REDIS_URL is omitted', async () => { + const { pingKv } = await loadKvClient() + await expect(pingKv()).resolves.toEqual({ configured: false, available: false, status: 'not_configured', }) - expect(ping).not.toHaveBeenCalled() + expect(createClient).not.toHaveBeenCalled() + }) + + it('reports KV as misconfigured when REDIS_URL is not a Redis URL', async () => { + process.env.REDIS_URL = 'https://example.com' + const { pingKv } = await loadKvClient() + + await expect(pingKv()).resolves.toEqual({ + configured: false, + available: false, + status: 'misconfigured', + }) + expect(createClient).not.toHaveBeenCalled() + }) + + it('reports KV as healthy when Redis ping succeeds', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.ping.mockResolvedValue('PONG') + const { pingKv } = await loadKvClient() + + await expect(pingKv()).resolves.toEqual({ + configured: true, + available: true, + status: 'ok', + }) + expect(createClient).toHaveBeenCalledWith({ + url: 'redis://localhost:6379', + }) + expect(redisClient.on).toHaveBeenCalledWith('error', expect.any(Function)) + expect(redisClient.connect).toHaveBeenCalledTimes(1) + expect(redisClient.ping).toHaveBeenCalledTimes(1) + }) + + it('reports KV errors when Redis connection fails', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + const error = new Error('redis unavailable') + redisClient.connect.mockRejectedValue(error) + const { pingKv } = await loadKvClient() + + await expect(pingKv()).resolves.toEqual({ + configured: true, + available: false, + status: 'error', + error, + }) + }) + + it('connects once across repeated operations', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.ping.mockResolvedValue('PONG') + redisClient.get.mockResolvedValue('true') + const { getKvValue, pingKv } = await loadKvClient() + + await pingKv() + await getKvValue('warned-alternate-email:user@example.com') + + expect(redisClient.connect).toHaveBeenCalledTimes(1) + }) + + it('serializes values as JSON when setting KV values', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.set.mockResolvedValue('OK') + const { setKvValue } = await loadKvClient() + + await expect(setKvValue('flag', true)).resolves.toEqual({ + ok: true, + configured: true, + value: 'OK', + }) + expect(redisClient.set).toHaveBeenCalledWith('flag', 'true') + }) + + it('parses JSON values when getting KV values', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.get.mockResolvedValue('{"enabled":true}') + const { getKvValue } = await loadKvClient() + + await expect(getKvValue<{ enabled: boolean }>('settings')).resolves.toEqual( + { + ok: true, + configured: true, + value: { enabled: true }, + } + ) + }) + + it('returns null when getting a missing KV value', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.get.mockResolvedValue(null) + const { getKvValue } = await loadKvClient() + + await expect(getKvValue('missing')).resolves.toEqual({ + ok: true, + configured: true, + value: null, + }) + }) + + it('reports KV errors when stored JSON cannot be parsed', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.get.mockResolvedValue('not-json') + const { getKvValue } = await loadKvClient() + + await expect(getKvValue('invalid')).resolves.toMatchObject({ + ok: false, + configured: true, + reason: 'error', + }) }) }) From 11d43576ba59796b657c93835fd110e3be71a3ca Mon Sep 17 00:00:00 2001 From: Alex Drankou Date: Fri, 15 May 2026 19:25:49 +0200 Subject: [PATCH 2/2] fix(kv): fail fast and rebuild singleton on Redis disconnect --- src/core/shared/clients/kv.ts | 15 +++++++++++ tests/unit/kv.test.ts | 50 ++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/core/shared/clients/kv.ts b/src/core/shared/clients/kv.ts index f40068b1a..1a920f532 100644 --- a/src/core/shared/clients/kv.ts +++ b/src/core/shared/clients/kv.ts @@ -49,6 +49,10 @@ function getKvConfigStatus(): KvConfigStatus { function createRedisClient() { const client = createClient({ url: process.env.REDIS_URL, + // Fail commands fast when the client isn't ready instead of queueing them + // behind the auto-reconnect. Preserves the {status: 'error'} envelope on + // pingKv() and keeps /api/health snappy during Redis outages. + disableOfflineQueue: true, }) client.on('error', (error) => { @@ -61,6 +65,17 @@ function createRedisClient() { ) }) + // When the socket fully ends (TCP close, retries exhausted, manual close), + // drop the cached singletons so the next getRedisClient() call rebuilds the + // client instead of returning the stale resolved promise. The identity guard + // protects against a late 'end' from a previous client clobbering a fresh one. + client.on('end', () => { + if (redisClient === client) { + redisClient = null + redisConnectPromise = null + } + }) + return client } diff --git a/tests/unit/kv.test.ts b/tests/unit/kv.test.ts index 854c1ae05..8f08af5c8 100644 --- a/tests/unit/kv.test.ts +++ b/tests/unit/kv.test.ts @@ -1,9 +1,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { createClient, redisClient } = vi.hoisted(() => { + const handlers: Record void> = {} const client = { isReady: false, - on: vi.fn(), + on: vi.fn((event: string, handler: (arg?: unknown) => void) => { + handlers[event] = handler + return client + }), + emit: (event: string, arg?: unknown) => handlers[event]?.(arg), connect: vi.fn(), ping: vi.fn(), get: vi.fn(), @@ -97,12 +102,55 @@ describe('optional KV client', () => { }) expect(createClient).toHaveBeenCalledWith({ url: 'redis://localhost:6379', + disableOfflineQueue: true, }) expect(redisClient.on).toHaveBeenCalledWith('error', expect.any(Function)) + expect(redisClient.on).toHaveBeenCalledWith('end', expect.any(Function)) expect(redisClient.connect).toHaveBeenCalledTimes(1) expect(redisClient.ping).toHaveBeenCalledTimes(1) }) + it('fails fast when the client disconnects mid-session', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.ping.mockResolvedValueOnce('PONG') + const { pingKv } = await loadKvClient() + + // First ping: healthy + await expect(pingKv()).resolves.toMatchObject({ status: 'ok' }) + + // Simulate disconnect: isReady flips and the next ping rejects fast + // because the singleton was created with disableOfflineQueue: true. + redisClient.isReady = false + const closedError = new Error('The client is closed') + redisClient.ping.mockRejectedValueOnce(closedError) + + await expect(pingKv()).resolves.toEqual({ + configured: true, + available: false, + status: 'error', + error: closedError, + }) + }) + + it('rebuilds the singleton after the client emits end', async () => { + process.env.REDIS_URL = 'redis://localhost:6379' + redisClient.ping.mockResolvedValue('PONG') + const { pingKv } = await loadKvClient() + + await pingKv() + expect(createClient).toHaveBeenCalledTimes(1) + expect(redisClient.connect).toHaveBeenCalledTimes(1) + + // Permanent disconnect — drives the 'end' handler, which should null out + // the cached singletons so the next call rebuilds from scratch. + redisClient.isReady = false + redisClient.emit('end') + + await pingKv() + expect(createClient).toHaveBeenCalledTimes(2) + expect(redisClient.connect).toHaveBeenCalledTimes(2) + }) + it('reports KV errors when Redis connection fails', async () => { process.env.REDIS_URL = 'redis://localhost:6379' const error = new Error('redis unavailable')