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

Bug in hiredis Pub/Sub handling on errors in the first subscribe. #116

Closed
antirez opened this issue Jul 22, 2012 · 3 comments
Closed

Bug in hiredis Pub/Sub handling on errors in the first subscribe. #116

antirez opened this issue Jul 22, 2012 · 3 comments

Comments

@antirez
Copy link
Contributor

antirez commented Jul 22, 2012

Hello!

if you try to subscribe to a Pub/Sub channel with the async API and the server reply with an error such as -ERR Loading... the hiredis client will crash.

My point of view is that while there is some kind of problem in hiredis that's worth fixing, in this specific case it also shows a problem with the Redis server that should instead allow to subscribe to Pub/Sub while the server is loading data. I'm fixing the server but I think it's worth fixing hiredis as well, given that the fix for the server will probably not be back ported to Redis 2.4, and there may be in the future some other reason to reply with an error to the first subscribe.

Cheers,
Salvatore

@antirez
Copy link
Contributor Author

antirez commented Jul 22, 2012

p.s. this likely also happens when authentication is on but no AUTH was sent. It may be a simpler way to reproduce the bug.

@pietern
Copy link
Contributor

pietern commented Aug 1, 2012

This should be fixed with 7ec4df9.

The client will now disconnect the context when it receives a spontaneous error reply. I believe this is the best strategy at this point, because it can't retry subscriptions, or do other fancy state recovery when the server starts accepting commands.

@pietern pietern closed this as completed Aug 1, 2012
@antirez
Copy link
Contributor Author

antirez commented Aug 12, 2012

Hello Pieter, thanks for the fix! About the strategy, what I can say is that in Sentinel if subscribe fails what I do is, actually, to manually disconnect the channel, and retry from scratch, so at application level I reasoned likewise.

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

No branches or pull requests

2 participants