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

"Cannot send commands in PubSub mode" error on ping #2345

Closed
TarSzator opened this issue Dec 12, 2022 · 20 comments · Fixed by #2373 · May be fixed by WalletConnect/relay#7
Closed

"Cannot send commands in PubSub mode" error on ping #2345

TarSzator opened this issue Dec 12, 2022 · 20 comments · Fixed by #2373 · May be fixed by WalletConnect/relay#7
Labels

Comments

@TarSzator
Copy link

As I understand the sentence

The commands that are allowed in the context of a subscribed client are SUBSCRIBE, SSUBSCRIBE, SUNSUBSCRIBE, PSUBSCRIBE, UNSUBSCRIBE, PUNSUBSCRIBE, PING, RESET, and QUIT.

on Redis Pub/Sub

I should not get the error

Cannot send commands in PubSub mode

on a ping call with the redis client

Environment:

  • Node.js Version: 18.12.1
  • Redis Server Version: 7.0.5
  • Node Redis Version: 4.5.1
  • Platform: Alpine in Docker (Host MacOS 13.0.1 on Apple M1 Max)
@TarSzator TarSzator added the Bug label Dec 12, 2022
leibale added a commit to leibale/node-redis that referenced this issue Dec 12, 2022
@leibale
Copy link
Collaborator

leibale commented Dec 12, 2022

this is a known limitation with the current PubSub implementation in the client (which validates PubSub mode on the client side).. it's already fixed in #2344 (which not only "add support for sharded PubSub", but also improves "normal PubSub")

edit:
Hopefully I'll release it this week

@TarSzator
Copy link
Author

Thank you @leibale 😄 So fingers corsed that you get around to it this week 🤞🏼

@TarSzator
Copy link
Author

@leibale Where you able to release?

@leibale
Copy link
Collaborator

leibale commented Dec 19, 2022

@TarSzator Unfortenly not, I had to work on something else at the end of last week :(
The features missing in the PR is: PubSub and Sharded PubSub for cluster. if I won't get to it in the next few days I'll release a version without it and implement that in another PR. In the meantime you can install it directly from my fork (make sure to add a postinstall/prepare script that will transpile TypeScript (npm run build-all should work).

@TarSzator
Copy link
Author

Thank you @leibale for the response and the workaround hints.

I like your plan for the release 😃

@leibale
Copy link
Collaborator

leibale commented Dec 22, 2022

@TarSzator apparently PubSub & Sharded PubSub + Cluster is a little bit more complicated than I anticipated, but it's almost ready... sorry for all the delays...

@TarSzator
Copy link
Author

@leibale Thanks for keeping this issue up2date.

Still planing to finish PubSub & Sharded PubSub + Cluster or push this to a later release?

@leibale
Copy link
Collaborator

leibale commented Dec 22, 2022

I'll release a version with or without it (depends if I'll finish it in time) tomorrow

@leibale
Copy link
Collaborator

leibale commented Dec 26, 2022

@TarSzator I can't release an RC version of redis with an RC version or @redis/client without releasing a version for all the sub-packages (because the current "peer dependency" in all the sub-packages does not include RC versions). If you are using @redis/client directly I can release an RC version of that.

Sorry for all the delays.

@TarSzator
Copy link
Author

I should be able to easily switch to @redis/client for now. So it would already solve my issue when you for now only release the fix on @redis/client

@leibale
Copy link
Collaborator

leibale commented Dec 26, 2022

@TarSzator @redis/client@1.5.0-next.0 is on npm 🎉

@leibale
Copy link
Collaborator

leibale commented Dec 28, 2022

@TarSzator have you tried the "next" version? does it work?

@TarSzator
Copy link
Author

@leibale Works fine for me. Thanks a lot. When will you release it as fully?

@TarSzator
Copy link
Author

@leibale Is there an ETA for v1.5.0?

@leibale
Copy link
Collaborator

leibale commented Jan 10, 2023

I already missed the "promised" day a couple of times, so I won't make that mistake again...
You can watch #2373 to get updates, the two missing parts are:

  1. cluster + shared pub-sub (60-70% done)
  2. tests (5-10% done)

Sorry for all the delays.

edit:
docs is missing as well

@TarSzator
Copy link
Author

Understandable @leibale

No rush, mainly just wanted to understand the next steps.
So I understand that you will release the client when you can also release the complete redis package.

@leibale
Copy link
Collaborator

leibale commented Jan 11, 2023

@TarSzator Once #2373 will be ready I'll merge it + (about) 10 more PRs and release them all

@leibale
Copy link
Collaborator

leibale commented Jan 18, 2023

@TarSzator just an update.. I'm pretty sure that it works, just need to clean the code a bit + add more tests.. Hopefully my next comment will be after releasing it.. :)

@TarSzator
Copy link
Author

@leibale Thanks a lot for keeping me in the loop

@leibale
Copy link
Collaborator

leibale commented Jan 25, 2023

@TarSzator redis@4.6.0/@redis/client@1.5.0 is on npm 🎉

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