Skip to content

Commit 9d3bd82

Browse files
committed
Fix frequent reconnections when timers under/overshoot
The ping timer is set exactly on the interval. Sometimes the timer runs a bit before the ping time so the ping misses. If the next timer overshoots, the ping is not sent and the connection is restarted (because the last message time is > ping_time*2). Scheduling the timer more frequently than the actual ping time fixes these issues as the timer is guaranteed to ping at least once during the ping_time*2 interval.
1 parent 2924970 commit 9d3bd82

File tree

6 files changed

+10
-3
lines changed

6 files changed

+10
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### 0.14.4 (Next)
22

3+
* [#289](https://github.com/slack-ruby/slack-ruby-client/pull/289): Fix reconnects when ping timers under/overshoot - [@georgyangelov](https://github.com/georgyangelov).
34
* Your contribution here.
45

56
### 0.14.3 (2019/7/23)

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,8 @@ This setting determines how long the socket can be idle before sending a ping me
376376

377377
It's important to note that if a ping message was sent and no response was received within the amount of time specified in `websocket_ping`; the client will attempt to reestablish it's connection to the message server.
378378

379+
Note that the ping may take between `websocket_ping` and `websocket_ping * 3/2` seconds to actually trigger when there is no activity on the socket. This is because the timer that checks whether to ping is triggered at every `websocket_ping / 2` interval (see [#289](https://github.com/slack-ruby/slack-ruby-client/pull/289) for further explanation).
380+
379381
To disable this feature; set `websocket_ping` to 0.
380382

381383
### Connection Methods

lib/slack/real_time/concurrency/async.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def start_reactor(client)
3434

3535
# The timer task will naturally exit after the driver is set to nil.
3636
while @restart
37-
subtask.sleep client.websocket_ping
37+
subtask.sleep client.websocket_ping_timer
3838
client.run_ping! if @restart
3939
end
4040
end

lib/slack/real_time/concurrency/celluloid.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def run_client_loop
8282
def run_ping_loop
8383
return unless @client.run_ping?
8484

85-
@ping_timer = every @client.websocket_ping do
85+
@ping_timer = every @client.websocket_ping_timer do
8686
@client.run_ping!
8787
end
8888
end

lib/slack/real_time/concurrency/eventmachine.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def start_async(client)
3030
@thread = ensure_reactor_running
3131

3232
if client.run_ping?
33-
EventMachine.add_periodic_timer(client.websocket_ping) do
33+
EventMachine.add_periodic_timer client.websocket_ping_timer do
3434
client.run_ping!
3535
end
3636
end

lib/slack/real_time/config.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ def concurrency
3333
(val = @concurrency).respond_to?(:call) ? val.call : val
3434
end
3535

36+
def websocket_ping_timer
37+
websocket_ping / 2
38+
end
39+
3640
private
3741

3842
def detect_concurrency

0 commit comments

Comments
 (0)