fix(redis): drop cached client and restart PING loop after forced reconnect#4501
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Adds a 200ms deadline to Reviewed by Cursor Bugbot for commit 331d735. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR fixes a stale-client bug in the Redis reconnection path and adds a release timeout to
Confidence Score: 5/5Safe to merge — the fix correctly breaks the stale-client cycle and the release timeout is a faithful copy of the existing acquire pattern. Both changed code paths are tightly scoped: the redis module clears its own state in the right order (global null → interval cleared → listeners → disconnect → pingInFlight reset in finally), and the isolated-vm change is a mechanical copy of an already-working pattern. New tests directly exercise the cache-drop and PING-restart scenarios that were missing before. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Interval as PING Interval
participant HC as Health Check
participant Global as Module State
participant Listener as Reconnect Listeners
participant OldClient as Old Redis Client
participant NewClient as New Redis Client
Interval->>HC: "tick (failure 2)"
HC->>Global: "globalRedisClient = null"
HC->>Global: "clearInterval / pingInterval = null"
HC->>Listener: "invoke callbacks"
Listener->>Global: "getRedisClient() called"
Global->>NewClient: "new Redis() + startPingHealthCheck"
HC->>OldClient: "disconnect(true)"
HC->>Global: "pingInFlight = false (finally)"
note over NewClient: "Fresh PING loop active against new client"
Reviews (1): Last reviewed commit: "fix(redis): drop cached client and resta..." | Re-trigger Greptile |
Summary
startPingHealthCheckwas disconnecting the dead client but leavingglobalRedisClientcached, so the nextgetRedisClient()returned the dead reference and every command sat in its offline queue until the 5scommandTimeout. Now the global is cleared and the PING interval is reset before disconnect, so the next call builds a fresh client with a fresh health check.releaseDistributedLeaseinisolated-vm.ts(matchingtryAcquireDistributedLease) so a hung Redis client doesn't make releases wait the full 5s.Type of Change
Testing
bun run test lib/core/config/redis.test.ts(13/13)bun run test lib/execution/isolated-vm.test.ts(9/9)bunx tsc --noEmit -p tsconfig.jsoncleanbun run lintcleanbun run check:api-validation:strictpassedChecklist