Implemented ping support for Celluloid and EventMachine.

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2018-10-31 11:24:37 -0400 using RuboCop version 0.58.2.
+# on 2018-10-31 14:26:53 -0400 using RuboCop version 0.58.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new

CHANGELOG.md

@@ -1,9 +1,7 @@
 ### 0.13.2 (Next)
 
+* [#226](https://github.com/slack-ruby/slack-ruby-client/pull/226), [#232](https://github.com/slack-ruby/slack-ruby-client/pull/232), [#236](https://github.com/slack-ruby/slack-ruby-client/pull/236), [#234](https://github.com/slack-ruby/slack-ruby-client/pull/234): Added periodic ping that reconnects on failure - [@RodneyU215](https://github.com/RodneyU215), [@dblock](https://github.com/dblock), [@ioquatix](https://github.com/ioquatix).
 * Your contribution here.
-* [#234](https://github.com/slack-ruby/slack-ruby-client/pull/234): Removing the immediate retry to give the client time to reconnect - [@RodneyU215](https://github.com/RodneyU215).
-* [#232](https://github.com/slack-ruby/slack-ruby-client/pull/232): Addressing a few issues with #run_ping! (fixes #231) - [@RodneyU215](https://github.com/RodneyU215).
-* [#226](https://github.com/slack-ruby/slack-ruby-client/pull/226): Added periodic ping that reconnects on failure for `async-websocket` - [@RodneyU215](https://github.com/RodneyU215), [@dblock](https://github.com/dblock), [@ioquatix](https://github.com/ioquatix).
 
 ### 0.13.1 (2018/9/30)
 

lib/slack/real_time/concurrency/async.rb

@@ -20,7 +20,11 @@ class Socket < Slack::RealTime::Socket
           def start_async(client)
             @reactor = Reactor.new
             Thread.new do
-              @reactor.every(client.websocket_ping) { client.run_ping! } if client.run_ping?
+              if client.run_ping?
+                @reactor.every(client.websocket_ping) do
+                  client.run_ping!
+                end
+              end
               @reactor.run do |task|
                 task.async do
                   client.run_loop

lib/slack/real_time/concurrency/celluloid.rb

@@ -47,7 +47,7 @@ def run_loop
 
           def close
             @closing = true
-            driver.close
+            driver.close if driver
             super
           end
 
@@ -73,7 +73,16 @@ def start_async(client)
           end
 
           def run_client_loop
+            if @client.run_ping?
+              current_actor.every @client.websocket_ping do
+                @client.run_ping!
+              end
+            end
+
             @client.run_loop
+          rescue StandardError => e
+            logger.debug("#{self.class}##{__method__}") { e }
+            raise e
           end
 
           def connected?

lib/slack/real_time/concurrency/eventmachine.rb

@@ -29,20 +29,36 @@ class Socket < Slack::RealTime::Socket
           def start_async(client)
             @thread = ensure_reactor_running
 
+            if client.run_ping?
+              EventMachine.add_periodic_timer(client.websocket_ping) do
+                client.run_ping!
+              end
+            end
+
+            client.run_loop
+
+            @thread
+          end
+
+          def restart_async(client, new_url)
+            @url = new_url
+            @last_message_at = current_time
+            @thread = ensure_reactor_running
+
             client.run_loop
 
             @thread
           end
 
           def disconnect!
             super
-            close
+            EventMachine.stop if @thread
+            @thread = nil
           end
 
           def close
+            driver.close if driver
             super
-            EventMachine.stop if @thread
-            @thread = nil
           end
 
           def send_data(message)

lib/slack/real_time/socket.rb

@@ -58,7 +58,7 @@ def start_async(_client)
         raise NotImplementedError, "Expected #{self.class} to implement #{__method__}."
       end
 
-      def restart_async(_client)
+      def restart_async(_client, _url)
         raise NotImplementedError, "Expected #{self.class} to implement #{__method__}."
       end
 

spec/integration/integration_spec.rb

@@ -19,16 +19,18 @@
     Slack.configure do |slack|
       slack.logger = logger
     end
+
+    @queue = QueueWithTimeout.new
   end
 
   after do
     Slack.config.reset
   end
 
-  let(:queue) { QueueWithTimeout.new }
-
   let(:client) { Slack::RealTime::Client.new(token: ENV['SLACK_API_TOKEN']) }
 
+  let(:queue) { @queue }
+
   def start
     # starts the client and pushes an item on a queue when connected
     client.start_async do |driver|
@@ -47,7 +49,7 @@ def start
     client.on :close do
       logger.info 'Disconnecting ...'
       # pushes another item to the queue when disconnected
-      queue.push nil
+      queue.push nil if @queue
     end
   end
 
@@ -62,9 +64,11 @@ def start_server
   end
 
   def wait_for_server
+    return unless @queue
     logger.debug '#wait_for_server'
     queue.pop_with_timeout(5)
     logger.debug '#wait_for_server, joined'
+    @queue = nil
   end
 
   def stop_server
@@ -119,15 +123,37 @@ def stop_server
     start_server
   end
 
-  # We currently only send regular pings when using 'async-websocket'. See Issue #223.
-  if ENV['CONCURRENCY'] == 'async-websocket'
-    it 'sends pings' do
+  context 'with websocket_ping set' do
+    before do
       client.websocket_ping = 2
+    end
+    it 'sends pings' do
+      @reply_to = nil
       client.on :pong do |data|
-        expect(data.reply_to).to be 1
+        @reply_to = data.reply_to
+        client.stop!
+      end
+      start_server
+      wait_for_server
+      expect(@reply_to).to be 1
+    end
+  end
+
+  context 'with websocket_ping not set' do
+    before do
+      client.websocket_ping = 0
+    end
+    it 'does not send pings' do
+      @reply_to = nil
+      client.on :pong do |data|
+        @reply_to = data.reply_to
+      end
+      client.on :hello do
         client.stop!
       end
       start_server
+      wait_for_server
+      expect(@reply_to).to be nil
     end
   end