Consider a client connected only after receiving a welcome message. (#24

)

* Fix a typo in action_cable_client (callaback vs callback).

* Consider a client connected only after receiving a welcome message.

Given an `action_cable_client` which:

1. connects to an action cable server
2. subscribes to the ChatChannel.
3. receives a message from that channel, then disconnect and starts
   the process from 1.

at a certain point the client connects, but doesn't receive the
subscription confirmation and therefore misses broadcast messages.

A full example repository is available here:

https://github.com/wpp/action-cable-debugging

Turns out ActionCable expects a client not to send any messages
(including subscription messages) before receiving a "welcome".

Quoting from an issue I mistakenly opened on the rails project:

> client must consider a connection to be ready/open only after
> receiving "welcome" message (and not WebSocket OPEN event). Thus you
> should not send any message before that. Sending messages after
> receiving "welcome" from server guarantees that the socket is OPEN
> (from the server point of view).

> Hence the client doesn't implement Action Cable protocol correctly.

> Take a look, for example, at Action Cable js client to see how it
> should be implemented.

If you have a look at the ActionCable JS client mentioned:

https://github.com/rails/rails/blob/master/actioncable/app/assets/javascripts/action_cable/connection.coffee#L90

you can confirm that's what they are doing.

lib/action_cable_client.rb

@@ -22,7 +22,7 @@ class Commands
   attr_reader :_message_factory
   # The queue should store entries in the format:
   # [ action, data ]
-  attr_accessor :message_queue, :_subscribed, :_subscribed_callaback, :_pinged_callback
+  attr_accessor :message_queue, :_subscribed, :_subscribed_callback, :_pinged_callback, :_connected_callback
 
   def_delegator :_websocket_client, :onerror, :errored
   def_delegator :_websocket_client, :send, :send_msg
@@ -91,7 +91,7 @@ def received
   #     # do things after the client is connected to the server
   #   end
   def connected
-    _websocket_client.onopen do
+    self._connected_callback = Proc.new do
       subscribe
       yield
     end
@@ -111,7 +111,7 @@ def connected
   #     # do things after successful subscription confirmation
   #   end
   def subscribed(&block)
-    self._subscribed_callaback = block
+    self._subscribed_callback = block
   end
 
   # @return [Boolean] is the client subscribed to the channel?
@@ -147,6 +147,8 @@ def handle_received_message(message)
 
     if is_ping?(json)
       _pinged_callback&.call(json)
+    elsif is_welcome?(json)
+      _connected_callback&.call(json)
     elsif !subscribed?
       check_for_subscribe_confirmation(json)
     else
@@ -162,7 +164,7 @@ def check_for_subscribe_confirmation(message)
     return unless  Message::TYPE_CONFIRM_SUBSCRIPTION == message_type
 
     self._subscribed = true
-    _subscribed_callaback&.call
+    _subscribed_callback&.call
   end
 
   # {"identifier" => "_ping","message" => 1460201942}
@@ -172,6 +174,12 @@ def is_ping?(message)
     Message::IDENTIFIER_PING == message_identifier
   end
 
+  # {"type" => "welcome"}
+  def is_welcome?(message)
+    message_identifier = message[Message::TYPE_KEY]
+    Message::IDENTIFIER_WELCOME == message_identifier
+  end
+
   def subscribe
     msg = _message_factory.create(Commands::SUBSCRIBE)
     send_msg(msg.to_json)

lib/action_cable_client/message.rb

@@ -4,6 +4,7 @@ class ActionCableClient
   class Message
     IDENTIFIER_KEY = 'identifier'
     IDENTIFIER_PING = 'ping'
+    IDENTIFIER_WELCOME = 'welcome'
     # Type is never sent, but is received
     # TODO: find a better place for this constant
     TYPE_KEY = 'type'

spec/unit/action_cable_client_spec.rb

@@ -99,9 +99,9 @@
 
     context '#subscribed' do
       it 'sets the callback' do
-        expect(@client._subscribed_callaback).to eq nil
+        expect(@client._subscribed_callback).to eq nil
         @client.subscribed {}
-        expect(@client._subscribed_callaback).to_not eq nil
+        expect(@client._subscribed_callback).to_not eq nil
       end
 
       it 'once the callback is set, receiving a subscription confirmation invokes the callback' do
@@ -110,14 +110,22 @@
           callback_called = true
         end
 
-        expect(@client).to receive(:_subscribed_callaback).and_call_original
+        expect(@client).to receive(:_subscribed_callback).and_call_original
         message = { 'identifier' => 'ping', 'type' => 'confirm_subscription' }
         @client.send(:check_for_subscribe_confirmation, message)
         expect(callback_called).to eq true
       end
     end
 
     context '#connected' do
+      it 'sets the callback' do
+        expect(@client._connected_callback).to eq(nil)
+
+        @client.connected {}
+
+        expect(@client._connected_callback).to_not eq(nil)
+      end
+
       it 'subscribes' do
         # TODO: how do I stub a method chain that takes a block?
         # allow{ |b| @client._websocket_client.callback }.to yield_with_no_args