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

Ensure node exists before being redirected via an ASK #341

Merged
merged 1 commit into from
Jun 28, 2016

Conversation

jpallen
Copy link

@jpallen jpallen commented Jun 28, 2016

If a master node in the Redis cluster does not have any slots, it is not returned by the cluster slots command, and so does not have a client in the connectionPool. If slots are migrated to this new master node, the old master node returns an ASK error with the new node details. At the moment ioredis looks up the node in the connectionPool but it doesn't exist, so the following error is returned:

TypeError: Cannot call method 'asking' of undefined
  at tryConnection (/var/www/document-updater/releases/5543/node_modules/ioredis/lib/cluster/index.js:464:19)
  at Object._this.handleError.ask (/var/www/document-updater/releases/5543/node_modules/ioredis/lib/cluster/index.js:404:11)
  at Cluster.handleError (/var/www/document-updater/releases/5543/node_modules/ioredis/lib/cluster/index.js:503:52)
  at Command.command.reject (/var/www/document-updater/releases/5543/node_modules/ioredis/lib/cluster/index.js:389:13)
  at Redis.exports.returnError (/var/www/document-updater/releases/5543/node_modules/ioredis/lib/redis/parser.js:75:18)

It looks like a similar issues with a MOVED reply was fixed in 0dcb768. This patch applies the same fix to the ASK error.

@luin luin merged commit 5d9d0d3 into redis:master Jun 28, 2016
@luin
Copy link
Collaborator

luin commented Jun 28, 2016

Released in v2.2.0. Thank you for the patch! 🍻

@jpallen
Copy link
Author

jpallen commented Jun 28, 2016

Wow, that was super quick! I see you've already done another release too, thanks so much :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants