From c0b3b4dcfc060dfc6b3572c2c902a1b86dc26289 Mon Sep 17 00:00:00 2001 From: Nikolay Vashchenko Date: Tue, 7 Feb 2017 06:48:41 +0300 Subject: [PATCH 1/4] bugfixes for improper socket and EM closing --- .gitignore | 1 + lib/lita/adapters/slack/rtm_connection.rb | 11 +++++++++-- spec/lita/adapters/slack/rtm_connection_spec.rb | 12 +++++++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 8cc7c28..ed67e51 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,4 @@ test/tmp test/version_tmp tmp lita_config.rb +.idea/ diff --git a/lib/lita/adapters/slack/rtm_connection.rb b/lib/lita/adapters/slack/rtm_connection.rb index 0d416e0..af82ebf 100644 --- a/lib/lita/adapters/slack/rtm_connection.rb +++ b/lib/lita/adapters/slack/rtm_connection.rb @@ -49,7 +49,7 @@ def run(queue = nil, options = {}) websocket.on(:message) { |event| receive_message(event) } websocket.on(:close) do log.info("Disconnected from Slack.") - shut_down + EventLoop.safe_stop end websocket.on(:error) { |event| log.debug("WebSocket error: #{event.message}") } @@ -64,7 +64,7 @@ def send_messages(channel, strings) end def shut_down - if websocket + if websocket_open? log.debug("Closing connection to the Slack Real Time Messaging API.") websocket.close end @@ -115,6 +115,13 @@ def websocket_options options[:proxy] = { :origin => config.proxy } if config.proxy options end + + # States are defined in https://github.com/faye/faye-websocket-ruby/blob/master/lib/faye/websocket/api.rb + # Currently, it's the best available option to inspect websocket state + def websocket_open? + websocket && websocket.ready_state <= 1 + end + end end end diff --git a/spec/lita/adapters/slack/rtm_connection_spec.rb b/spec/lita/adapters/slack/rtm_connection_spec.rb index d733a80..32188f3 100644 --- a/spec/lita/adapters/slack/rtm_connection_spec.rb +++ b/spec/lita/adapters/slack/rtm_connection_spec.rb @@ -29,7 +29,7 @@ def with_websocket(subject, queue) end let(:token) { 'abcd-1234567890-hWYd21AmMH2UHAkx29vb5c1Y' } let(:queue) { Queue.new } - let(:proxy_url) { "http://foo:3128" } + let(:proxy_url) { "http://example.com:3128" } let(:config) { Lita::Adapters::Slack.configuration_builder.build } before do @@ -114,6 +114,16 @@ def with_websocket(subject, queue) # the WebSocket. subject.send(:receive_message, event) end + + context "when the WebSocket is closed from outside" do + it "shuts down the reactor" do + with_websocket(subject, queue) do |websocket| + websocket.close + expect(EM.stopping?).to be_truthy + end + end + end + end describe "#send_messages" do From 424eaaa264f7493c7882f15cf65a22a8f5ac8f6d Mon Sep 17 00:00:00 2001 From: Nikolay Vashchenko Date: Tue, 7 Feb 2017 06:50:30 +0300 Subject: [PATCH 2/4] fix test --- spec/lita/adapters/slack/rtm_connection_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lita/adapters/slack/rtm_connection_spec.rb b/spec/lita/adapters/slack/rtm_connection_spec.rb index 32188f3..5b7c5f8 100644 --- a/spec/lita/adapters/slack/rtm_connection_spec.rb +++ b/spec/lita/adapters/slack/rtm_connection_spec.rb @@ -136,6 +136,7 @@ def with_websocket(subject, queue) allow(Faye::WebSocket::Client).to receive(:new).and_return(websocket) allow(websocket).to receive(:on) allow(websocket).to receive(:close) + allow(websocket).to receive(:ready_state).and_return(1) allow(Lita::Adapters::Slack::EventLoop).to receive(:defer).and_yield end From 3975f0c0bcd67bfa1f6788776a6d7cb9c99863e3 Mon Sep 17 00:00:00 2001 From: Nikolay Vashchenko Date: Tue, 7 Feb 2017 07:29:40 +0300 Subject: [PATCH 3/4] using EM status to detect websocket status --- lib/lita/adapters/slack/event_loop.rb | 6 +++++- lib/lita/adapters/slack/rtm_connection.rb | 8 +------- spec/lita/adapters/slack/rtm_connection_spec.rb | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/lita/adapters/slack/event_loop.rb b/lib/lita/adapters/slack/event_loop.rb index b78376a..8404234 100644 --- a/lib/lita/adapters/slack/event_loop.rb +++ b/lib/lita/adapters/slack/event_loop.rb @@ -15,7 +15,11 @@ def run end def safe_stop - EM.stop if EM.reactor_running? + EM.stop if running? + end + + def running? + EM.reactor_running? && !EM.stopping? end end end diff --git a/lib/lita/adapters/slack/rtm_connection.rb b/lib/lita/adapters/slack/rtm_connection.rb index af82ebf..90bae4d 100644 --- a/lib/lita/adapters/slack/rtm_connection.rb +++ b/lib/lita/adapters/slack/rtm_connection.rb @@ -64,7 +64,7 @@ def send_messages(channel, strings) end def shut_down - if websocket_open? + if websocket && EventLoop.running? log.debug("Closing connection to the Slack Real Time Messaging API.") websocket.close end @@ -116,12 +116,6 @@ def websocket_options options end - # States are defined in https://github.com/faye/faye-websocket-ruby/blob/master/lib/faye/websocket/api.rb - # Currently, it's the best available option to inspect websocket state - def websocket_open? - websocket && websocket.ready_state <= 1 - end - end end end diff --git a/spec/lita/adapters/slack/rtm_connection_spec.rb b/spec/lita/adapters/slack/rtm_connection_spec.rb index 5b7c5f8..32188f3 100644 --- a/spec/lita/adapters/slack/rtm_connection_spec.rb +++ b/spec/lita/adapters/slack/rtm_connection_spec.rb @@ -136,7 +136,6 @@ def with_websocket(subject, queue) allow(Faye::WebSocket::Client).to receive(:new).and_return(websocket) allow(websocket).to receive(:on) allow(websocket).to receive(:close) - allow(websocket).to receive(:ready_state).and_return(1) allow(Lita::Adapters::Slack::EventLoop).to receive(:defer).and_yield end From 16ee5d295e6bcb4446ade1972f868693eafa1eb5 Mon Sep 17 00:00:00 2001 From: Nikolay Vashchenko Date: Thu, 9 Feb 2017 22:28:31 +0300 Subject: [PATCH 4/4] removing IDE leftover --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index ed67e51..8cc7c28 100644 --- a/.gitignore +++ b/.gitignore @@ -16,4 +16,3 @@ test/tmp test/version_tmp tmp lita_config.rb -.idea/