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