Fix: resolve doubly linked list push issue by ntvviktor · Pull Request #3085 · redis/node-redis
mayankkapoor pushed a commit to mayankkapoor/node-redis that referenced this pull request
…ent fails to reconnect, throwing `"None of the sentinels are available"`. This is a regression introduced in v5.9.0 (commit `73413e086c` — "fix: resolve doubly linked list push issue (redis#3085)"). **Root Cause:** `#handleSentinelFailure()` (added in v5.9.0) permanently removes the sentinel node from `#sentinelRootNodes` via `splice()` when its connection drops. When `#reset()` then calls `observe()`, the sentinel list is empty (or missing that node), so reconnection is impossible. **Secondary issue:** The sentinel list update in `transform()` (line 1350) compares only array lengths (`analyzed.sentinelList.length != this.#sentinelRootNodes.length`), so even if reconnection succeeds via another sentinel, a removed node may never be restored if the list sizes happen to match. - [ ] **Task 1:** Fix `#handleSentinelFailure` — stop permanently removing sentinel nodes - **File:** `packages/client/lib/sentinel/index.ts` (lines 934–942) - Remove the `splice()` call. A transient TCP disconnect (which happens during sentinel restart) should not permanently delete the node from the root list. Just call `#reset()` to trigger reconnection, matching v5.8.3 behavior. - Before (broken): ```typescript #handleSentinelFailure(node: RedisNode) { const found = this.#sentinelRootNodes.findIndex( (rootNode) => rootNode.host === node.host && rootNode.port === node.port ); if (found !== -1) { this.#sentinelRootNodes.splice(found, 1); } this.#reset(); } ``` - After (fixed): ```typescript #handleSentinelFailure(node: RedisNode) { this.#reset(); } ``` - [ ] **Task 2:** Fix sentinel list update condition in `transform()` - **File:** `packages/client/lib/sentinel/index.ts` (line 1350) - Replace the length-only comparison with a proper content comparison so that nodes removed or added are always detected. - Before: ```typescript if (analyzed.sentinelList.length != this.#sentinelRootNodes.length) { ``` - After — compare actual content: ```typescript if (!areSentinelListsEqual(analyzed.sentinelList, this.#sentinelRootNodes)) { ``` - Add a helper (either inline or as a small private method) that compares host+port of each node, not just array length. - [ ] **Task 3:** Run existing tests to confirm no regressions - Run `npm test` in the `packages/client` directory - Look for and run any sentinel-specific test files - [ ] **Task 4:** Verify the fix scenario - Confirm `#sentinelRootNodes` retains the original sentinel entry after disconnect - Confirm `observe()` retries that sentinel and succeeds once it's back online - Confirm no `"None of the sentinels are available"` error after sentinel restart