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

Potential risk that blocking calls never return #132

Closed
pfk84 opened this issue Jul 25, 2022 · 2 comments
Closed

Potential risk that blocking calls never return #132

pfk84 opened this issue Jul 25, 2022 · 2 comments
Labels

Comments

@pfk84
Copy link

pfk84 commented Jul 25, 2022

I've noticed that when executing a BLPOP and the Redis-Server "goes away" without closing the socket (very rare occasions, especially on production) the promise never returns making the whole connection unusable. If the server closes the connection, all is fine.

I know the library has by design no special handling for any Redis command but maybe in this case internally adding React\Promise\Timer\timeout would make sense, at least that's how I handle these cases now...

@SimonFrings
Copy link
Contributor

@pfk84 Thanks for bringing this up, definitely a tricky one 👍

I think a feature that fixes this issue (like adding a timeout or something similar) will be a thing somewhere in the future, but this doesn't mean that implementing it will be a breeze. Do we add a default timeout value and is this the way to go? Everyone has a different use case, some developers use BLPOP with a few seconds timeout, some others have this thing running for several hours. As you can see it depends on different use cases, so finding a norm between all this is not easy.

Like always: PRs are welcome! 😉

As you said, using an internal timeout works for your use case and should work in most/all cases, but it would definitely be more convenient if this library supported this itself. Maybe we can achieve an interim solution by adding some sort of documentation to the README.md that addresses this issue and suggests a way to handle this.

What do you think about this?

@clue
Copy link
Owner

clue commented Apr 28, 2023

@pfk84 Thank you for bringing this up! I've just filed #142 to keep track of the underlying feature request. I agree that detecting dead connections is something we should look into and provide better out-of-the-box support for.

I believe this has been answered, so I'm closing this for now. Please feel free to report back otherwise 👍

@clue clue closed this as completed Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants