Fix slack-ruby#78: ping thread terminates on a failed restart.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 #### 0.8.1 (Next)
 
+* [#79](https://github.com/slack-ruby/slack-ruby-bot-server/pull/79): Fix: ping worker terminates on a failed restart - [@dblock](https://github.com/dblock).
 * Your contribution here.
 
 #### 0.8.0 (2018/9/8)

lib/slack-ruby-bot-server/ping.rb

@@ -68,32 +68,55 @@ def down!
       @error_count += 1
       return true if retries_left?
       restart!
-      false
     end
 
     def restart!
       logger.warn "RESTART: #{owner}"
-      begin
-        connection.close
-      rescue Async::Wrapper::Cancelled
-        # ignore, from connection.close
-      end
+      close_connection
+      close_driver
+      emit_close
+      false
+    rescue StandardError => e
+      logger.warn "Error restarting team #{owner.id}: #{e.message}."
+      true
+    end
+
+    def close_connection
+      return unless connection
+      connection.close
+    rescue Async::Wrapper::Cancelled
+      # ignore, from connection.close
+    rescue StandardError => e
+      logger.warn "Error closing connection for #{owner.id}: #{e.message}."
+      raise e
+    end
+
+    def close_driver
+      return unless driver
       driver.close
+    rescue StandardError => e
+      logger.warn "Error closing driver for #{owner.id}: #{e.message}."
+      raise e
+    end
+
+    def emit_close
+      return unless driver
       driver.emit(:close, WebSocket::Driver::CloseEvent.new(1001, 'bot offline'))
     rescue StandardError => e
-      logger.warn "Error restarting team #{owner.id}: #{e.message}."
+      logger.warn "Error sending :close event to driver for #{owner.id}: #{e.message}."
+      raise e
     end
 
     def ping_interval
       options[:ping_interval] || 60
     end
 
     def retries_left?
-      retries_left >= 0
+      retry_count - error_count >= 0
     end
 
     def retries_left
-      retry_count - error_count
+      [0, retry_count - error_count].max
     end
 
     def retry_count

spec/slack-ruby-bot-server/ping_spec.rb

@@ -60,6 +60,21 @@
           expect(subject.send(:error_count)).to eq 0
         end
       end
+
+      context 'after two more failed checks' do
+        before do
+          allow(subject).to receive(:online?).and_return(false).twice
+          2.times { subject.send(:check!) }
+        end
+
+        it 'does not decrement retries left below zero' do
+          expect(subject.send(:retries_left)).to eq 0
+        end
+
+        it 'sets error count' do
+          expect(subject.send(:error_count)).to eq 3
+        end
+      end
     end
 
     it 'terminates the ping worker after account_inactive' do