Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added server ping worker. #74

Merged
merged 6 commits into from
Aug 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2018-05-06 23:29:13 -0400 using RuboCop version 0.55.0.
# on 2018-08-21 19:45:36 -0400 using RuboCop version 0.55.0.
# 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
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
### Changelog

#### 0.6.2 (Next)
#### 0.7.0 (Next)

* [#60](https://github.com/slack-ruby/slack-ruby-bot-server/pull/60): Log caught Standard::Error backtrace at debug-level - [@alexagranov](https://github.com/alexagranov).
* [#65](https://github.com/slack-ruby/slack-ruby-bot-server/pull/65): Updated Capybara and selenium-webdriver - [@dblock](https://github.com/dblock).
* [#67](https://github.com/slack-ruby/slack-ruby-bot-server/pull/67): Only load the OTR::ActiveRecord::ConnectionManagement middleware when the OTR module is included. This module isn't needed when using Rails - [@darbyfrey](https://github.com/darbyfrey).
* [#74](https://github.com/slack-ruby/slack-ruby-bot-server/pull/74): Added ping worker, will restart offline bots - [@dblock](https://github.com/dblock).
* Your contribution here.

#### 0.6.1 (3/29/2017)
Expand Down
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,22 @@ SlackRubyBotServer.configure do |config|
end
```

#### Ping Worker

Each `SlackRubyBotServer::Server` instance will start a ping worker that will periodically check the online status of the bot via the Slack web [auth.test](https://api.slack.com/methods/auth.test) and [users.getPresence](https://api.slack.com/methods/users.getPresence) APIs, and will forcefully close the connection if the bot goes offline, causing an automatic reconnect.

You can configure the ping interval, number of retries before restart, and disable the ping worker entirely.

```ruby
SlackRubyBotServer.configure do |config|
config.ping = {
enabled: true, # set to false to disable the ping worker
ping_interval: 30, # interval in seconds
retry_count: 3 # number of unsuccessful retries until a restart
}
end
```

### Access Tokens

By default the implementation of [Team](lib/slack-ruby-bot-server/models/team) stores a `bot_access_token` that grants a certain amount of privileges to the bot user as described in [Slack OAuth Docs](https://api.slack.com/docs/oauth). You may not want a bot user at all, or may require different auth scopes, such as `users.profile:read` to access user profile information via `Slack::Web::Client#users_profile_get`. To obtain the non-bot access token make the following changes.
Expand Down
18 changes: 18 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
Upgrading Slack-Ruby-Bot-Server
===============================

### Upgrading to >= 0.7.0

#### New Ping Worker

Version 0.7.0 will automatically start a ping worker that checks for the bot's online status and forcefully terminate and restart disconnected bots. Set the ping `enabled` option to `false` to disable this behavior.

```ruby
SlackRubyBotServer.configure do |config|
config.ping = {
enabled: false
}
end
```

If you are currently using a custom ping worker as suggested in [slack-ruby-client#208](https://github.com/slack-ruby/slack-ruby-client/issues/208), delete it.

See [#74](https://github.com/slack-ruby/slack-ruby-bot-server/pull/74) for more information.

### Upgrading to >= 0.6.0

#### Mongoid and ActiveRecord support
Expand Down
1 change: 1 addition & 0 deletions lib/slack-ruby-bot-server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'grape-swagger'
require 'slack-ruby-bot'
require 'slack-ruby-bot-server/server'
require 'slack-ruby-bot-server/ping'
require 'slack-ruby-bot-server/config'

require 'slack-ruby-bot-server/ext'
Expand Down
2 changes: 1 addition & 1 deletion lib/slack-ruby-bot-server/api/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Api
class Middleware
def self.logger
@logger ||= begin
$stdout.sync = true
STDOUT.sync = true
Logger.new(STDOUT)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/slack-ruby-bot-server/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def self.instance

def logger
@logger ||= begin
$stdout.sync = true
STDOUT.sync = true
Logger.new(STDOUT)
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/slack-ruby-bot-server/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ module Config
extend self

attr_accessor :server_class
attr_accessor :ping
attr_accessor :database_adapter

def reset!
self.ping = nil
self.server_class = SlackRubyBotServer::Server
self.database_adapter = if defined?(::Mongoid)
:mongoid
Expand Down
99 changes: 99 additions & 0 deletions lib/slack-ruby-bot-server/ping.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
module SlackRubyBotServer
class Ping
include Celluloid

attr_reader :client
attr_reader :options
attr_reader :error_count

def initialize(client, options = {})
@options = options
@client = client
@error_count = 0
end

def start!
every ping_interval do
check!
end
end

private

def check!
if online?
@error_count = 0
else
down!
end
rescue StandardError => e
case e.message
when 'account_inactive', 'invalid_auth' then
logger.warn "Error pinging team #{owner.id}: #{e.message}, terminating."
terminate
else
logger.warn "Error pinging team #{owner.id}: #{e.message}."
end
end

def offline?
!online?
end

def online?
presence.online
end

def presence
owner.ping![:presence]
end

def down!
logger.warn "DOWN: #{owner}, #{retries_left} #{retries_left == 1 ? 'retry' : 'retries'} left"
@error_count += 1
return if retries_left?
restart!
end

def restart!
logger.warn "RESTART: #{owner}"
driver.emit(:close, WebSocket::Driver::CloseEvent.new(1001, 'bot offline')) if driver
terminate
end

def ping_interval
options[:ping_interval] || 60
end

def retries_left?
retries_left >= 0
end

def retries_left
retry_count - error_count
end

def retry_count
options[:retry_count] || 2
end

def socket
client.instance_variable_get(:@socket) if client
end

def driver
socket.instance_variable_get(:@driver) if socket
end

def logger
@logger ||= begin
STDOUT.sync = true
Logger.new(STDOUT)
end
end

def owner
client.owner
end
end
end
20 changes: 19 additions & 1 deletion lib/slack-ruby-bot-server/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,36 @@ class Server < SlackRubyBot::Server
def initialize(attrs = {})
attrs = attrs.dup
@team = attrs.delete(:team)
@ping_options = attrs.delete(:ping) || {}
raise 'Missing team' unless @team
attrs[:token] = @team.token
super(attrs)
client.owner = @team
open!
end

def restart!(wait = 1)
# when an integration is disabled, a live socket is closed, which causes the default behavior of the client to restart
# it would keep retrying without checking for account_inactive or such, we want to restart via service which will disable an inactive team
logger.info "#{team.name}: socket closed, restarting ..."
SlackRubyBotServer::Service.instance.restart! team, self, wait
open!
end

private

attr_reader :ping_options

def create_ping
return unless !ping_options.key?(:enabled) || ping_options[:enabled]
SlackRubyBotServer::Ping.new(client, ping_options)
end

def open!
client.owner = team
client.on :open do |_event|
worker = create_ping
worker.start! if worker
end
end
end
end
4 changes: 3 additions & 1 deletion lib/slack-ruby-bot-server/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ def create!(team)
def start!(team)
run_callbacks :starting, team
logger.info "Starting team #{team}."
server = SlackRubyBotServer::Config.server_class.new(team: team)
options = { team: team }
options[:ping] = SlackRubyBotServer::Config.ping if SlackRubyBotServer::Config.ping
server = SlackRubyBotServer::Config.server_class.new(options)
start_server! team, server
run_callbacks :started, team
server
Expand Down
2 changes: 1 addition & 1 deletion lib/slack-ruby-bot-server/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module SlackRubyBotServer
VERSION = '0.6.2'.freeze
VERSION = '0.7.0'.freeze
end
127 changes: 127 additions & 0 deletions spec/slack-ruby-bot-server/ping_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
require 'spec_helper'

describe SlackRubyBotServer::Ping do
let(:team) { Team.new(token: 'token') }
let(:options) { {} }
let(:server) { SlackRubyBotServer::Server.new({ team: team }.merge(options)) }
let(:client) { server.send(:client) }

subject do
server.send(:create_ping)
end

context 'with defaults' do
before do
allow(subject.wrapped_object).to receive(:every).and_yield
end

it 'defaults retry count' do
expect(subject.send(:retry_count)).to eq 2
end

it 'calculates retries left' do
expect(subject.send(:retries_left)).to eq 2
end

it 'defaults ping interval' do
expect(subject.send(:ping_interval)).to eq 60
end

it 'checks for connection' do
expect(subject.wrapped_object).to receive(:check!)
subject.start!
end

context 'after a failed check' do
before do
allow(subject.wrapped_object).to receive(:online?).and_return(false)
subject.start!
end

it 'decrements retries left' do
expect(subject.send(:retries_left)).to eq 1
end

it 'sets error count' do
expect(subject.send(:error_count)).to eq 1
end

context 'after a successful check' do
before do
allow(subject.wrapped_object).to receive(:online?).and_return(true)
subject.start!
end

it 're-increments retries left' do
expect(subject.send(:retries_left)).to eq 2
end

it 'resets error count' do
expect(subject.send(:error_count)).to eq 0
end
end
end

it 'terminates the ping worker after account_inactive' do
allow(subject.wrapped_object).to receive(:online?).and_raise('account_inactive')
expect(subject.wrapped_object).to receive(:terminate)
subject.start!
end

it 'terminates after a number of retries' do
allow(subject.wrapped_object).to receive(:online?).and_return(false)
expect(subject.wrapped_object).to receive(:terminate)
3.times { subject.start! }
end
end

context 'with options' do
context 'ping interval' do
let(:options) { { ping: { ping_interval: 42 } } }

it 'is used' do
expect(subject.send(:ping_interval)).to eq 42
expect(subject.wrapped_object).to receive(:every).with(42)
subject.start!
end
end

context 'retry count' do
let(:options) { { ping: { retry_count: 42 } } }

it 'is set' do
expect(subject.send(:retry_count)).to eq 42
end

it 'adjusts retries left' do
expect(subject.send(:retries_left)).to eq 42
end
end

context 'enabled' do
context 'nil' do
let(:options) { { ping: { enabled: nil } } }

it 'does not create a worker' do
expect(subject).to be_nil
end
end

context 'false' do
let(:options) { { ping: { enabled: false } } }

it 'does not create a worker' do
expect(subject).to be_nil
end
end

context 'true' do
let(:options) { { ping: { enabled: true } } }

it 'creates a worker' do
expect(subject).to_not be_nil
end
end
end
end
end
Loading