fix(redis): size cold-connection waits to survive a dead handshake

apps/sim/instrumentation-node.ts

@@ -407,6 +407,6 @@ export async function register() {
   // Not awaited: the connection is warmed in the background so the first request
   // that needs Redis does not pay the handshake inside its own command deadline,
   // but boot never waits on Redis to serve requests that do not touch it.
-  const { warmRedisConnection } = await import('./lib/core/config/redis')
+  const { warmRedisConnection } = await import('@/lib/core/config/redis')
   void warmRedisConnection()
 }

apps/sim/lib/core/config/redis-budget.test.ts

@@ -0,0 +1,44 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { coldConnectionBudgetMs } from '@/lib/core/config/redis-budget'
+
+describe('coldConnectionBudgetMs', () => {
+  it('charges an unanswered handshake at two command deadlines plus the half-close wait', () => {
+    // Unauthenticated: SETNAME/SETINFO time out, then INFO times out, then the
+    // half-closed socket waits for a FIN a wedged peer never sends.
+    expect(
+      coldConnectionBudgetMs({
+        connectTimeoutMs: 1_000,
+        commandTimeoutMs: 5_000,
+        disconnectTimeoutMs: 2_000,
+        reconnectDelayMs: 500,
+      })
+    ).toBe(2 * 5_000 + 2_000 + 500 + 1_000)
+  })
+
+  it('charges the connect deadline when a connection that never completes is the longer case', () => {
+    // Lowering the command deadline must not shrink the budget below what a
+    // connect that never completes costs before retryStrategy can fire.
+    expect(
+      coldConnectionBudgetMs({
+        connectTimeoutMs: 10_000,
+        commandTimeoutMs: 3_000,
+        disconnectTimeoutMs: 2_000,
+        reconnectDelayMs: 500,
+      })
+    ).toBe(10_000 + 500 + 1_000)
+  })
+
+  it('always leaves room for the healthy attempt that follows the reconnect', () => {
+    expect(
+      coldConnectionBudgetMs({
+        connectTimeoutMs: 0,
+        commandTimeoutMs: 0,
+        disconnectTimeoutMs: 0,
+        reconnectDelayMs: 0,
+      })
+    ).toBe(1_000)
+  })
+})

apps/sim/lib/core/config/redis-budget.ts

@@ -0,0 +1,45 @@
+/** Generous room for the healthy handshake that follows a recovered stall; a real one takes tens of milliseconds. */
+const HEALTHY_HANDSHAKE_ALLOWANCE_MS = 1_000
+
+export interface ColdConnectionBudgetOptions {
+  /** The client's TCP connect deadline: bounds a connection that never completes at all. */
+  connectTimeoutMs: number
+  /** The client's per-command deadline: bounds a handshake command the server never answers. */
+  commandTimeoutMs: number
+  /** How long ioredis waits after half-closing a dead socket for the peer's FIN before destroying it. */
+  disconnectTimeoutMs: number
+  /** What the client's `retryStrategy` returns for its first reconnect. */
+  reconnectDelayMs: number
+}
+
+/**
+ * How long a wait for a cold ioredis connection must allow before giving up,
+ * if it is to survive one dead attempt and still see a healthy one land.
+ *
+ * A dead attempt is diagnosed by whichever deadline governs the phase it
+ * stalls in. A connect that never completes costs `connectTimeoutMs`, and the
+ * socket is destroyed outright. A connection that opens but whose handshake
+ * is never answered costs command deadlines — one with a password, because a
+ * timed-out `AUTH` is fatal; two without, because `CLIENT SETNAME`/`SETINFO`
+ * must settle, by timing out, before the `INFO` ready check starts its own —
+ * and then `disconnectTimeoutMs` more, because ioredis half-closes the socket
+ * and a peer that is wedged never answers with a FIN. The budget takes the
+ * larger phase so it holds for either URL shape without parsing it. Only then
+ * does `retryStrategy` run and a fresh attempt begin.
+ *
+ * A wait sized to a single deadline expires while the first attempt is still
+ * being diagnosed, so a configured retry can never be the thing that saves it.
+ *
+ * The guarantee is one dead attempt from a fresh or previously-ready client.
+ * ioredis feeds `retryStrategy` its running attempt count, so a wait that
+ * begins while the client is already deep in a reconnect loop faces larger
+ * delays this does not model. The handshake sequence is ioredis 5's; keep this
+ * beside the pinned client, not in a generic package.
+ */
+export function coldConnectionBudgetMs(options: ColdConnectionBudgetOptions): number {
+  const deadAttemptMs = Math.max(
+    options.connectTimeoutMs,
+    2 * options.commandTimeoutMs + options.disconnectTimeoutMs
+  )
+  return deadAttemptMs + options.reconnectDelayMs + HEALTHY_HANDSHAKE_ALLOWANCE_MS
+}

apps/sim/lib/core/config/redis.test.ts

@@ -47,14 +47,19 @@ vi.mock('ioredis', () => ({
 
 import {
   acquireLock,
+  CONNECT_TIMEOUT_MS,
   closeRedisConnection,
+  DISCONNECT_TIMEOUT_MS,
   describeRedisConnection,
   extendLock,
   getRedisClient,
   onRedisReconnect,
   resetForTesting,
+  SHARED_COMMAND_TIMEOUT_MS,
+  sharedReconnectDelayMs,
   warmRedisConnection,
 } from '@/lib/core/config/redis'
+import { coldConnectionBudgetMs } from '@/lib/core/config/redis-budget'
 
 describe('redis config', () => {
   beforeEach(() => {
@@ -471,7 +476,50 @@ describe('redis config', () => {
     })
   })
 
+  describe('sharedReconnectDelayMs', () => {
+    it('grows exponentially from the base and caps, with upward-only jitter', () => {
+      expect(sharedReconnectDelayMs(1, 0)).toBe(1_000)
+      expect(sharedReconnectDelayMs(2, 0)).toBe(2_000)
+      expect(sharedReconnectDelayMs(5, 0)).toBe(10_000)
+      expect(sharedReconnectDelayMs(6, 0)).toBe(10_000)
+      expect(sharedReconnectDelayMs(1, 1)).toBe(1_300)
+    })
+  })
+
   describe('warmRedisConnection', () => {
+    it('outlasts one dead handshake so the reconnect can be what warms it', async () => {
+      mockRedisInstance.status = 'connecting'
+      const warm = warmRedisConnection()
+
+      // The dead attempt's own diagnosis and half-close, then the longest first
+      // reconnect delay: the moment a healthy second attempt can begin.
+      await vi.advanceTimersByTimeAsync(
+        Math.max(CONNECT_TIMEOUT_MS, 2 * SHARED_COMMAND_TIMEOUT_MS + DISCONNECT_TIMEOUT_MS) +
+          sharedReconnectDelayMs(1, 1)
+      )
+      const client = getRedisClient()
+      Object.assign(client ?? {}, { status: 'ready' })
+      client?.emit('ready')
+
+      await expect(warm).resolves.toBe(true)
+    })
+
+    it('still gives up once the budget is spent', async () => {
+      mockRedisInstance.status = 'connecting'
+      const warm = warmRedisConnection()
+
+      await vi.advanceTimersByTimeAsync(
+        coldConnectionBudgetMs({
+          connectTimeoutMs: CONNECT_TIMEOUT_MS,
+          commandTimeoutMs: SHARED_COMMAND_TIMEOUT_MS,
+          disconnectTimeoutMs: DISCONNECT_TIMEOUT_MS,
+          reconnectDelayMs: sharedReconnectDelayMs(1, 1),
+        })
+      )
+
+      await expect(warm).resolves.toBe(false)
+    })
+
     it('resolves immediately when the connection is already usable', async () => {
       mockRedisInstance.status = 'ready'
 

apps/sim/lib/core/config/redis.ts

@@ -2,9 +2,10 @@ import { isIP } from 'node:net'
 import { createLogger } from '@sim/logger'
 import { toError } from '@sim/utils/errors'
 import { randomFloat } from '@sim/utils/random'
-import Redis, { type RedisOptions } from 'ioredis'
+import Redis from 'ioredis'
 import { env } from '@/lib/core/config/env'
 import { getConfiguredCacheProvider } from '@/lib/core/config/env-capabilities.server'
+import { coldConnectionBudgetMs } from '@/lib/core/config/redis-budget'
 
 const logger = createLogger('Redis')
 
@@ -42,13 +43,24 @@ function resolveRedisTlsOptions(url: string | undefined): { servername: string }
  * and TLS SNI when REDIS_URL targets an IP. Every Redis client we open should
  * spread this; callers add their own retry / timeout policy on top.
  */
-export function getRedisConnectionDefaults(
-  url: string | undefined
-): Pick<RedisOptions, 'keepAlive' | 'connectTimeout' | 'enableOfflineQueue' | 'tls'> {
+export interface RedisConnectionDefaults {
+  keepAlive: number
+  connectTimeout: number
+  disconnectTimeout: number
+  enableOfflineQueue: boolean
+  tls?: { servername: string }
+}
+
+export const CONNECT_TIMEOUT_MS = 10_000
+/** ioredis's own default, stated so readiness budgets can be derived from it. */
+export const DISCONNECT_TIMEOUT_MS = 2_000
+
+export function getRedisConnectionDefaults(url: string | undefined): RedisConnectionDefaults {
   const tls = resolveRedisTlsOptions(url)
   return {
     keepAlive: 1000,
-    connectTimeout: 10000,
+    connectTimeout: CONNECT_TIMEOUT_MS,
+    disconnectTimeout: DISCONNECT_TIMEOUT_MS,
     enableOfflineQueue: true,
     ...(tls ? { tls } : {}),
   }
@@ -203,12 +215,34 @@ export function describeRedisConnection(
 
 const PING_INTERVAL_MS = 15_000
 const MAX_PING_FAILURES = 2
+export const SHARED_COMMAND_TIMEOUT_MS = 5_000
+const RECONNECT_BASE_MS = 1_000
+const RECONNECT_MAX_BASE_MS = 10_000
+const RECONNECT_JITTER_RATIO = 0.3
+
+/**
+ * The shared client's reconnect delay for attempt `times`, with `jitter` in
+ * `[0, 1]` scaling the upward-only jitter band. Pure so the same formula can
+ * be evaluated for a budget — `sharedReconnectDelayMs(1, 1)` is the longest
+ * possible first reconnect — as well as from `retryStrategy`, which adds the
+ * bookkeeping around it.
+ */
+export function sharedReconnectDelayMs(times: number, jitter: number): number {
+  const base = Math.min(RECONNECT_BASE_MS * 2 ** (times - 1), RECONNECT_MAX_BASE_MS)
+  return Math.round(base + jitter * base * RECONNECT_JITTER_RATIO)
+}
+
 /**
- * Warm-up budget. Sized to outlast a slow handshake rather than a fast one,
- * because giving up early just returns the handshake to the first command's
- * deadline, which is the thing this exists to avoid.
+ * Warm-up budget: one dead attempt, its longest possible first reconnect
+ * delay, then a healthy attempt. Giving up sooner returns the handshake to the
+ * first command's deadline, which is the thing warming exists to avoid.
  */
-const REDIS_WARMUP_TIMEOUT_MS = 10_000
+const REDIS_WARMUP_TIMEOUT_MS = coldConnectionBudgetMs({
+  connectTimeoutMs: CONNECT_TIMEOUT_MS,
+  commandTimeoutMs: SHARED_COMMAND_TIMEOUT_MS,
+  disconnectTimeoutMs: DISCONNECT_TIMEOUT_MS,
+  reconnectDelayMs: sharedReconnectDelayMs(1, 1),
+})
 
 export function getConfiguredRedisUrl(): string | null {
   if (getConfiguredCacheProvider() === 'database') return null
@@ -296,17 +330,15 @@ export function getRedisClient(): Redis | null {
 
     state.client = new Redis(redisUrl, {
       ...defaults,
-      commandTimeout: 5000,
+      commandTimeout: SHARED_COMMAND_TIMEOUT_MS,
       maxRetriesPerRequest: 5,
 
       retryStrategy: (times) => {
         if (times > 10) {
           logger.error(`Redis reconnection attempt ${times}`, { nextRetryMs: 30000 })
           return 30000
         }
-        const base = Math.min(1000 * 2 ** (times - 1), 10000)
-        const jitter = randomFloat() * base * 0.3
-        const delay = Math.round(base + jitter)
+        const delay = sharedReconnectDelayMs(times, randomFloat())
         state.reconnects++
         logger.warn('Redis reconnecting', { attempt: times, nextRetryMs: delay })
         return delay