Skip to content

Commit

Permalink
notifier: respawn workers if they're all dead
Browse files Browse the repository at this point in the history
Potentially fixes airbrake/airbrake#463
(falling back to sync delivery because there are no running async
workers)

I noticed I can reliably reproduce this message when I invoke the Rails
console and manually send a notice via `Airbrake.notify`. It happens
because by default Rails launches [Spring][1], which preloads the app
by using the `fork()` system call. And when that happens, as we know,
our worker threads die.

The fix is simply to respawn workers. It works reliably with the Rails
console and I assume it will fix the above mentioned issue.

[1]: https://github.com/rails/spring/
  • Loading branch information
kyrylo committed Jan 13, 2016
1 parent e6e73db commit c85cbbf
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Airbrake Ruby Changelog
([#25](https://github.com/airbrake/airbrake-ruby/pull/25))
* Made sure that generated notices always have a backtrace
([#21](https://github.com/airbrake/airbrake-ruby/pull/21))
* Made the asynchronous delivery mechanism more robust
([#26](https://github.com/airbrake/airbrake-ruby/pull/26))

### [v1.0.2][v1.0.2] (January 3, 2016)

Expand Down
17 changes: 14 additions & 3 deletions lib/airbrake-ruby/async_sender.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ def initialize(config)
@unsent = SizedQueue.new(config.queue_size)
@sender = SyncSender.new(config)
@closed = false
@workers = ThreadGroup.new
@workers = nil

config.workers.times { @workers.add(spawn_worker) }
@workers.enclose
spawn_workers
end

##
Expand Down Expand Up @@ -77,6 +76,18 @@ def has_workers?
!@closed && @workers.list.any?
end

##
# Spawns N workers (according to the config option) and creates a new
# enclosed ThreadGroup.
#
# @since v1.0.3
# @return [void]
def spawn_workers
@workers = ThreadGroup.new
@config.workers.times { @workers.add(spawn_worker) }
@workers.enclose
end

private

def spawn_worker
Expand Down
6 changes: 4 additions & 2 deletions lib/airbrake-ruby/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,17 @@ def convert_to_exception(ex)
e
end

def send_notice(exception, params, sender = default_sender)
def send_notice(exception, params, sender = select_sender)
notice = build_notice(exception, params)
@filter_chain.refine(notice)
return if notice.ignored?

sender.send(notice)
end

def default_sender
def select_sender
@async_sender.spawn_workers unless @async_sender.has_workers?

if @async_sender.has_workers?
@async_sender
else
Expand Down
31 changes: 26 additions & 5 deletions spec/notifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,17 +377,38 @@ def to_json(*)
out = StringIO.new

airbrake = described_class.new(airbrake_params.merge(logger: Logger.new(out)))
airbrake.
instance_variable_get(:@async_sender).
instance_variable_get(:@workers).
list.
each(&:kill)
async_sender = airbrake.instance_variable_get(:@async_sender)
# Make sure we don't revive workers.
allow(async_sender).to receive(:spawn_workers).and_return(nil)
async_sender.instance_variable_get(:@workers).list.each(&:kill)

sleep 1

expect(async_sender.has_workers?).to be_falsey
expect(airbrake.notify('bingo')).to be_nil
expect(async_sender.has_workers?).to be_falsey

expect(out.string).to match(/falling back to sync delivery/)
end

it "respawns workers if the workers list is empty" do
out = StringIO.new

airbrake = described_class.new(airbrake_params.merge(logger: Logger.new(out)))
async_sender = airbrake.instance_variable_get(:@async_sender)
async_sender.instance_variable_get(:@workers).list.each(&:kill)

sleep 1

expect(async_sender.has_workers?).to be_falsey
expect(airbrake.notify('bingo')).to be_nil

sleep 1

expect(async_sender.has_workers?).to be_truthy

expect(out.string).not_to match(/falling back to sync delivery/)
end
end

describe "#add_filter" do
Expand Down

0 comments on commit c85cbbf

Please sign in to comment.