Repository navigation
fix(client): apply the command timeout and abortSignal to MULTI and pipelines on pool, cluster and sentinel - #3537
Arthur031221 wants to merge 2 commits into
Conversation
…ipelines on pool, cluster and sentinel
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72ad432670
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| isReadonly, | ||
| undefined, | ||
| (client, options) => client._executeMulti( | ||
| (client, options) => client.withCommandOptions(batchOptions)._executeMulti( |
There was a problem hiding this comment.
In the @redis/client Mocha suite, the existing fakes in packages/client/lib/cluster/batch-routing.spec.ts (for example lines 11–20 and 39–48, plus the MOVED/ASK fakes) expose _executeMulti or _executePipeline but no withCommandOptions; because they are cast to never, type checking does not catch this, and the routing cases now fail here with TypeError: client.withCommandOptions is not a function before their assertions. Update those doubles (including the analogous pipeline path) or inject the batch options without adding this method requirement so the package suite remains runnable.
AGENTS.md reference: AGENTS.md:L54-L56
Useful? React with 👍 / 👎.
The cluster now calls withCommandOptions() on the node client before _executeMulti and _executePipeline, so the fakes in batch-routing.spec.ts need the method too.
Description
Second of the two PRs for #3504, following #3515.
The
timeoutandabortSignalset throughwithCommandOptions(),withAbortSignal()or the options of a pool, cluster or sentinel weren't passed tomulti().exec()andexecAsPipeline(). The batch ran with the node client's own 5 s default, so a call withtimeout: 100waited 5 s while a single command rejected on time.The three wrappers now pass both to the node client for each batch, through a small
pickBatchOptionshelper, as single commands do. An unset key keeps the node client's default andtimeout: undefinedopts out. As agreed in the issue, Sentinel batches follow the single command retry rule.On a real Redis, a pool
multi().exec()withtimeout: 100rejected after 5001 ms before and about 102 ms now.Added tests in the pool, cluster and sentinel specs.
Checklist
npm testpass with this change (including linting)?Assisted by Claude/Codex.
Note
Medium Risk
Changes batch execution paths across pool, cluster, and sentinel; behavior aligns with single commands but affects timeout/abort semantics for all queued MULTI and pipeline calls.
Overview
Pool, cluster, and sentinel now forward
timeoutandabortSignalfromwithCommandOptions(),withAbortSignal(), or constructorcommandOptionsintomulti().exec()andexecAsPipeline(), matching single-command behavior.A new
pickBatchOptionshelper copies only those keys when they are present (unset keeps the leased node client’s default;timeout: undefinedstill opts out). Pool, cluster, and sentinel wrap the leased client withwithCommandOptions(batchOptions)before_executeMulti/_executePipeline.Tests cover per-call timeout, abort signals, and opt-out via
timeout: undefinedon the pool; cluster and sentinel get similar coverage.blockSetImmediateis shared fromtest-utilsfor those tests, and cluster routing mocks stubwithCommandOptions.Reviewed by Cursor Bugbot for commit 17f2539. Bugbot is set up for automated code reviews on this repo. Configure here.