fix(redis): describe the client that ran the failed command

A command can outlive the client that issued it: the PING health check drops
`state.client` after consecutive failures, which is the same unhealthy stretch
in which that command is timing out. Reading the global in the failure path
then described the replacement — reporting `no-client` or a fresh `connecting`
for a failure belonging to the connection before it, misclassifying the very
timeout the diagnostic exists to explain.

Take the client as an argument, and withhold the ages when `state` no longer
holds it rather than dating a connection its timestamps never measured.

Author: waleedlatif1

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

@@ -440,6 +440,28 @@ describe('redis config', () => {
       )
     })
 
+    it('describes the client that ran the command, not one that replaced it mid-flight', async () => {
+      // The PING check drops `state.client` after consecutive failures — the same
+      // unhealthy stretch in which the command is timing out. Reading the global
+      // then would report the replacement and misclassify the very failure this
+      // diagnostic exists to explain.
+      mockRedisInstance.status = 'connecting'
+      mockRedisInstance.set.mockImplementationOnce(async () => {
+        resetForTesting()
+        throw new Error('Command timed out')
+      })
+
+      await expect(acquireLock(lockKey, value, ttlSeconds)).rejects.toThrow('Command timed out')
+      expect(mockLogger.error).toHaveBeenCalledWith(
+        'Redis lock acquire failed',
+        expect.objectContaining({
+          // Timestamps belong to whatever `state` holds now, so they are withheld
+          // rather than dated against a connection they never measured.
+          redis: expect.objectContaining({ status: 'connecting', clientAgeMs: null }),
+        })
+      )
+    })
+
     it('stays quiet on the taken and contended paths, which poll routes run constantly', async () => {
       mockRedisInstance.set.mockResolvedValueOnce('OK')
       await acquireLock(lockKey, value, ttlSeconds)

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

@@ -153,22 +153,31 @@ function describeRedisUrl(
  *
  * Derives only non-sensitive facts from REDIS_URL — never the URL itself, which
  * carries the AUTH token.
+ *
+ * Pass the client whose command is being diagnosed when it may not be the one
+ * `state` still holds. A command can outlive its client — the PING health check
+ * drops `state.client` after consecutive failures, which is the same unhealthy
+ * stretch in which that command is timing out — and reading the global then
+ * describes the replacement, reporting `no-client` or a fresh `connecting` for a
+ * failure that belongs to the connection before it.
  */
-export function describeRedisConnection(): RedisConnectionDiagnostics {
+export function describeRedisConnection(
+  client: Redis | null = state.client
+): RedisConnectionDiagnostics {
   let url: string | null = null
   try {
     url = getConfiguredRedisUrl()
   } catch {
     url = null
   }
 
-  const client = state.client
-
   // Ages describe the client currently held. A discarded client leaves its
   // timestamps behind until the next `getRedisClient()` rebuilds them, and
-  // reporting those against `no-client` would date a connection that no longer
-  // exists. The counters below are deliberately cumulative for the process.
-  const ageOf = (at: number | null) => (client === null ? null : elapsedSince(at))
+  // reporting those against `no-client` — or against a client that has since
+  // been replaced — would date a connection these timestamps never measured.
+  // The counters below are deliberately cumulative for the process.
+  const timestampsDescribeClient = client !== null && client === state.client
+  const ageOf = (at: number | null) => (timestampsDescribeClient ? elapsedSince(at) : null)
 
   return {
     status: client?.status ?? 'no-client',
@@ -418,7 +427,7 @@ export async function acquireLock(
     logger.error('Redis lock acquire failed', {
       lockKey,
       error: toError(error).message,
-      redis: describeRedisConnection(),
+      redis: describeRedisConnection(redis),
     })
     // Best effort, and the same compare-and-delete `releaseLock` runs on the
     // success path: it deletes only while `value` still owns the key. If Redis