pusher / pusher-http-ruby

Ruby library for Pusher Channels HTTP API
https://pusher.com/channels
MIT License
664 stars 124 forks source link

Pusher.cluster delegates to Pusher::Client#cluster which is not defined #158

Closed nertzy closed 3 years ago

nertzy commented 4 years ago

See https://github.com/pusher/pusher-http-ruby/blob/799a7e4a0b54e02125ad7d52ee8bed0c35f0acfa/lib/pusher.rb#L35 where Pusher.cluster is delegating to Pusher::Client#cluster.

Notice that Pusher::Client#cluster is not defined.

irb(main):009:0> Pusher.client
Traceback (most recent call last):
        2: from (irb):5
        1: from (irb):6:in `rescue in irb_binding'
NoMethodError (undefined method `client' for Pusher:Module)
nertzy commented 4 years ago

Here's my workaround for now:

require "pusher/client"

module Pusher
  class Client
    # Pusher::Client#cluster is not defined, even through Pusher.cluster
    # delegates to it.
    #
    # See https://github.com/pusher/pusher-http-ruby/issues/158
    def cluster
      /api-([^.]*).pusher.com/.match(host)[1]
    end
  end
end
stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you'd like this issue to stay open please leave a comment indicating how this issue is affecting you. Thank you.

elverkilde commented 3 years ago

Sorry about that, I just enabled stalebot, but I am looking into this.

elverkilde commented 3 years ago

hi @nertzy , sorry for the late reply. After looking a bit into this, I no longer consider this a bug. Client.cluster is indeed defined, but it is a setter, meaning you can only use it as an alternative way to configure the SDK, as described in the README.

You cannot use this to get the cluster, which is intended. I gave a more detailed explanation about this in the original issue (#135) which also describes the use case.

You (and anyone else) are obviously welcome to use the workaround posted here, but it can't work in the general case because we allow the user to specify any arbitrary host.

nertzy commented 2 years ago

@elverkilde Thanks for the update, and sorry for my late response as well.

If there is not supposed to be a method for fetching the client, then I think that a delegator should not be defined.

This line still defines the delegating method: https://github.com/pusher/pusher-http-ruby/blob/08af7fba7c52f95f9a6d7ac7c6da603fbda80502/lib/pusher.rb#L35

:cluster should be removed from that line so that there is no longer a Pusher.cluster method that raises when called.

Note that it's safe to remove the getter delegator because the setter is defined on the next line: https://github.com/pusher/pusher-http-ruby/blob/08af7fba7c52f95f9a6d7ac7c6da603fbda80502/lib/pusher.rb#L36