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

Allow automatic pings to be sent on pubsub channel #2371

Closed
wants to merge 1 commit into from

Conversation

mdouglass
Copy link

Description

If you turn on automatic pings (via pingInterval) on a pubsub channel, you get the error: “Cannot send commands in PubSub mode”. PING is one of the legal commands on a pubsub channel, so we override the ping call to set ignorePubSubMode true and allow the ping to be sent on that channel.

We are unable to use the new pingInterval setting on clients being used for pubsub because of an incorrect error that PING is not allowed on pubsub channels.


Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

If you turn on automatic pings (via pingInterval) on a pubsub channel, you get the error: “Cannot send commands in PubSub mode”. PING is one of the legal commands on a pubsub channel, so we override the ping call to set ignorePubSubMode true and allow the ping to be sent on that channel.
@mdouglass
Copy link
Author

fix for #2372

@leibale
Copy link
Contributor

leibale commented Jan 10, 2023

@mdouglass thanks for contributing! this issue was already fixed in #2344 (I removed the client-side validation for PubSub mode, so there is no need to use ignorePubSubMode anymore). This PR is not yet ready for a full release, but in the meantime you can use @redis/[email protected] (a "pre-release") which already includes a fix for this bug.

@leibale leibale closed this Jan 10, 2023
@mdouglass
Copy link
Author

Thanks, I had looked around for something pre-existing and somehow missed that. :)
We use redis instead of @redis/client -- do you know the timeline for a new release?

@leibale
Copy link
Contributor

leibale commented Jan 10, 2023

redis is using @redis/client under the hood, you can override the @redis/client version to this one and it should work.
See this for updates.

@mdouglass
Copy link
Author

mdouglass commented Jan 10, 2023

Great, thanks!

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