Skip to content
New issue

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

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

Already on GitHub? # 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)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cluster UnhandledPromiseRejectionWarning
3 participants