Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pipeline with transactions causes unhandled warnings #884

Merged
merged 3 commits into from
Jun 8, 2019

Conversation

luin
Copy link
Collaborator

@luin luin commented Jun 8, 2019

Pipeline#exec() is a little tricky: it has an init process when called first time (indicated by whether Pipeline#nodeifiedPromise is true). Transaction#exec() is similar to that, it also has an init process that parse the result from Pipeline#exec() before returning to users. However, #nodeifiedPromise is not checked here, so it may fail which causes the unhandled warnings when retries.

Closes: #883.

To be frank, it also take me a lot of time in order to understand the code, especially the complex relation between Pipeline & Transaction. We indeed need to do a refactor on this.

Pipeline#exec() is a little tricky: it has a init process when
called first time (indicated by whether Pipeline#nodeifiedPromise
is true). Transaction#exec() is similar to that, it has a init
process that parse the result from Pipeline#exec before returning
to users. However, #nodeifiedPromise is not checked here.

Closes: #883
@luin luin force-pushed the fix/transaction-cluster-retries branch from f805e2d to 7b3489f Compare June 8, 2019 08:18
@jstewmon
Copy link
Contributor

jstewmon commented Jun 8, 2019

The patch resolves the issue with my reproduction setup, but the new test passes with and without the change.

@luin
Copy link
Collaborator Author

luin commented Jun 8, 2019

@jstewmon You're right. Just fixed.

@jstewmon
Copy link
Contributor

jstewmon commented Jun 8, 2019

LGTM! Thanks for addressing so quickly!

@luin luin merged commit bbfd2fc into master Jun 8, 2019
@luin luin deleted the fix/transaction-cluster-retries branch June 8, 2019 13:31
ioredis-robot pushed a commit that referenced this pull request Jun 8, 2019
## [4.10.2](v4.10.1...v4.10.2) (2019-06-08)

### Bug Fixes

* pipeline with transactions causes unhandled warnings ([#884](#884)) ([bbfd2fc](bbfd2fc)), closes [#883](#883)
@ioredis-robot
Copy link
Collaborator

🎉 This PR is included in version 4.10.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

janus-dev87 added a commit to janus-dev87/ioredis-work that referenced this pull request Mar 1, 2024
## [4.10.2](redis/ioredis@v4.10.1...v4.10.2) (2019-06-08)

### Bug Fixes

* pipeline with transactions causes unhandled warnings ([#884](redis/ioredis#884)) ([bbfd2fc](redis/ioredis@bbfd2fc)), closes [#883](redis/ioredis#883)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cluster UnhandledPromiseRejectionWarning
3 participants