fix(pipeline): clear watch state after successful EXEC - #1498
fix(pipeline): clear watch state after successful EXEC#1498seidelmartin wants to merge 3 commits into
Conversation
Real Redis always clears watching keys and dirty flag after EXEC, regardless of whether the transaction succeeded or was aborted. The abort path already did this, but the success path did not — causing the pipeline's own commands to re-dirty the client via modifiedKeyEvents, making subsequent pipelines incorrectly return null. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes transaction (WATCH/MULTI/EXEC) state handling in ioredis-mock by ensuring the watch/dirty state is cleared on a successful EXEC, matching real Redis behavior and preventing subsequent transactions on the same connection from being incorrectly aborted.
Changes:
- Clear
redis.watchingand resetredis.dirtyon the successfulPipeline.exec()path. - Add an integration regression test to verify a second
MULTI/EXECsucceeds after a successful firstEXEC.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/pipeline.js |
Clears watch/dirty state on successful exec() to prevent re-dirtying during execution and subsequent false aborts. |
test/integration/exec.js |
Adds regression coverage to ensure subsequent MULTI/EXEC calls are not aborted after a successful EXEC. |
| // Real Redis always clears watch state after EXEC, whether it succeeded or aborted. | ||
| // Clear before running batch so commands within the pipeline don't re-dirty the state. | ||
| this.redis.watching.clear() | ||
| this.redis.dirty = false |
There was a problem hiding this comment.
Good catch. Fixed in 1ee6189 — added an _isMulti flag set by redis.multi(), and scoped both the _isDirty() abort check and the watch-state clearing in exec() to MULTI transactions only. Regular pipelines now run unconditionally without touching watch state. Added a regression test covering the WATCH + pipeline + MULTI/EXEC scenario.
Regular pipelines (redis.pipeline()) should not check or clear WATCH state — only MULTI transactions (redis.multi()) interact with watch semantics in real Redis. Introduce _isMulti flag so exec() can distinguish the two cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: e4651ec The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Pipeline.exec()had an asymmetric implementation: the abort path (dirty/expired watched key) correctly clearedredis.watchingandredis.dirty, but the success path did not — causing the pipeline's own commands to re-dirty the client via_signalModifiedKey, making all subsequent MULTI/EXEC pipelines incorrectly returnnull.EXECalways clears watch state regardless of outcome. However, onlyMULTI/EXECinteracts with watch semantics — regular pipelines (redis.pipeline()) should run unconditionally without checking or clearing watch state._isMultiflag (set byredis.multi()) to distinguish the two cases, and scoped both the dirty abort check and watch-state clearing inexec()to MULTI transactions only.Test plan
should clear watch state after a successful exec so subsequent pipelines are not aborted— verifies MULTI/EXEC clears watch state for subsequent transactionsshould not clear watch state after a regular pipeline exec— verifies WATCH + regular pipeline + MULTI/EXEC correctly aborts when a watched key is modified🤖 Generated with Claude Code