diff --git a/README.md b/README.md index 2153d79d..b9c90713 100644 --- a/README.md +++ b/README.md @@ -239,7 +239,7 @@ config.delayed_job_enabled = false ## Asynchronous reporting -By default, all messages are reported synchronously. You can enable asynchronous reporting with [girl_friday](https://github.com/mperham/girl_friday) or [sucker_punch](https://github.com/brandonhilkert/sucker_punch) or [Sidekiq](https://github.com/mperham/sidekiq). +By default, all messages are reported synchronously. You can enable asynchronous reporting with [girl_friday](https://github.com/mperham/girl_friday), [sucker_punch](https://github.com/brandonhilkert/sucker_punch), [Sidekiq](https://github.com/mperham/sidekiq), [Resque](https://github.com/resque/resque) or using threading. ### Using girl_friday @@ -285,11 +285,40 @@ Start Sidekiq from the root directory of your Rails app and declare the name of $ bundle exec sidekiq -q rollbar ``` +### Using Resque + +Add the following in ```config/initializers/rollbar.rb```: + +```ruby +config.use_resque +``` + +You can also supply a custom Resque queue: + +```ruby +config.use_resque :queue => 'my_queue' +``` + +Now you can just start a new Resque worker processing jobs in that queue: + +```bash +$ QUEUE=my_queue bundle exec resque:work +``` + +### Using threading + +Add the following in ```config/initializers/rollbar.rb```: + +```ruby +config.use_thread +``` + ### Using another handler -You can supply your own handler using ```config.async_handler```. The handler should schedule the payload for later processing (i.e. with a delayed_job, in a resque queue, etc.) and should itself return immediately. For example: +You can supply your own handler using ```config.async_handler```. The object to set for `async_handler` should respond to `#call` and receive the payload. The handler should schedule the payload for later processing (i.e. with a delayed_job, in a resque queue, etc.) and should itself return immediately. For example: ```ruby +config.use_async config.async_handler = Proc.new { |payload| Thread.new { Rollbar.process_payload(payload) } } @@ -297,6 +326,21 @@ config.async_handler = Proc.new { |payload| Make sure you pass ```payload``` to ```Rollbar.process_payload``` in your own implementation. +## Failover handlers + +If you are using `async_handler` to process asynchronous the error it's possible that the handler fails before it calls `Rollbar.process_payload`. For example, for the Resque handler, the Redis connection could fail so the job is finally not processed. + +To ensure that the error is sent you can define a chain of failover handlers that Rollbar will use to send the payload in case that the primary handler fails. The failover handlers, as for `async_handler`, are just objects responding to `#call`. + +To configure the failover handlers you can add the following: + +```ruby +config.use_resque +config.failover_handlers = [Rollbar::Delay::GirlFriday, Rollbar::Delay::Thread] +``` + +With the configuration above Resque will be your primary asynchronous handler but if it fails queueing the job Rollbar will use GirlFriday at first, and just a thread in case that GirlFriday fails too. + ## Using with rollbar-agent For even more asynchrony, you can configure the gem to write to a file instead of sending the payload to Rollbar servers directly. [rollbar-agent](https://github.com/rollbar/rollbar-agent) can then be hooked up to this file to actually send the payload across. To enable, add the following in ```config/initializers/rollbar.rb```: diff --git a/lib/rollbar.rb b/lib/rollbar.rb index b2d68c23..a980d8df 100644 --- a/lib/rollbar.rb +++ b/lib/rollbar.rb @@ -16,6 +16,8 @@ require 'rollbar/active_record_extension' if defined?(ActiveRecord) require 'rollbar/util' require 'rollbar/railtie' if defined?(Rails) +require 'rollbar/delay/girl_friday' +require 'rollbar/delay/thread' unless ''.respond_to? :encode require 'iconv' @@ -28,6 +30,8 @@ class << self attr_writer :configuration attr_accessor :last_report + @file_semaphore = Mutex.new + # Similar to configure below, but used only internally within the gem # to configure it without initializing any of the third party hooks def preconfigure @@ -46,9 +50,8 @@ def preconfigure # end def configure # if configuration.enabled has not been set yet (is still 'nil'), set to true. - if configuration.enabled.nil? - configuration.enabled = true - end + configuration.enabled = true if configuration.enabled.nil? + yield(configuration) require_hooks @@ -225,6 +228,7 @@ def log_warning(message) puts "[Rollbar] #{message}" end end + def log_debug(message) begin logger.debug message @@ -234,6 +238,12 @@ def log_debug(message) end end + def default_async_handler + return Rollbar::Delay::GirlFriday if defined?(GirlFriday) + + Rollbar::Delay::Thread + end + private def attach_request_data(payload, request_data) @@ -425,29 +435,45 @@ def send_payload(payload) end def schedule_payload(payload) - if payload.nil? - return - end + return if payload.nil? log_info '[Rollbar] Scheduling payload' if configuration.use_async - unless configuration.async_handler - configuration.async_handler = method(:default_async_handler) - end - - if configuration.write_to_file - unless @file_semaphore - @file_semaphore = Mutex.new - end - end - - configuration.async_handler.call(payload) + process_async_payload(payload) else process_payload(payload) end end + def process_async_payload(payload) + configuration.async_handler ||= default_async_handler + configuration.async_handler.call(payload) + rescue + if configuration.failover_handlers.empty? + log_error '[Rollbar] Async handler failed, and there are no failover handlers configured. See the docs for "failover_handlers"' + return + end + + async_failover(payload) + end + + def async_failover(payload) + log_warning '[Rollbar] Primary async handler failed. Trying failovers...' + + failover_handlers = configuration.failover_handlers + + failover_handlers.each do |handler| + begin + handler.call(payload) + rescue + next unless handler == failover_handlers.last + + log_error "[Rollbar] All failover handlers failed while processing payload: #{MultiJson.dump(payload)}" + end + end + end + def build_payload(data) payload = { 'access_token' => configuration.access_token, @@ -534,21 +560,6 @@ def server_data data end - def default_async_handler(payload) - if defined?(GirlFriday) - unless @queue - @queue = GirlFriday::WorkQueue.new(nil, :size => 5) do |payload| - process_payload(payload) - end - end - - @queue.push(payload) - else - log_warning '[Rollbar] girl_friday not found to handle async call, falling back to Thread' - Thread.new { process_payload(payload) } - end - end - # Reports an internal error in the Rollbar library. This will be reported within the configured # Rollbar project. We'll first attempt to provide a report including the exception traceback. # If that fails, we'll fall back to a more static failsafe response. diff --git a/lib/rollbar/configuration.rb b/lib/rollbar/configuration.rb index 86e3c32f..f91d4d34 100644 --- a/lib/rollbar/configuration.rb +++ b/lib/rollbar/configuration.rb @@ -15,6 +15,7 @@ class Configuration attr_accessor :endpoint attr_accessor :environment attr_accessor :exception_level_filters + attr_accessor :failover_handlers attr_accessor :filepath attr_accessor :framework attr_accessor :ignored_person_ids @@ -53,6 +54,7 @@ def initialize 'AbstractController::ActionNotFound' => 'warning', 'ActionController::RoutingError' => 'warning' } + @failover_handlers = [] @framework = 'Plain' @ignored_person_ids = [] @person_method = 'current_user' @@ -77,6 +79,15 @@ def use_sidekiq(options = {}) @async_handler = Rollbar::Delay::Sidekiq.new(options) end + def use_resque(options = {}) + require 'rollbar/delay/resque' if defined?(Resque) + + Rollbar::Delay::Resque::Job.queue = options[:queue] if options[:queue] + + @use_async = true + @async_handler = Rollbar::Delay::Resque + end + def use_sidekiq=(value) deprecation_message = "#use_sidekiq=(value) has been deprecated in favor of #use_sidekiq(options = {}). Please update your rollbar configuration." defined?(ActiveSupport) ? ActiveSupport::Deprecation.warn(deprecation_message) : puts(deprecation_message) @@ -84,6 +95,12 @@ def use_sidekiq=(value) value.is_a?(Hash) ? use_sidekiq(value) : use_sidekiq end + def use_thread + require 'rollbar/delay/thread' + @use_async = true + @async_handler = Rollbar::Delay::Thread + end + def use_sucker_punch require 'rollbar/delay/sucker_punch' if defined?(SuckerPunch) @use_async = true diff --git a/lib/rollbar/delay/girl_friday.rb b/lib/rollbar/delay/girl_friday.rb new file mode 100644 index 00000000..48479b8f --- /dev/null +++ b/lib/rollbar/delay/girl_friday.rb @@ -0,0 +1,26 @@ +module Rollbar + module Delay + class GirlFriday + + class << self + attr_accessor :queue + + def call(payload) + new.call(payload) + end + end + + def queue_class + ::GirlFriday::WorkQueue + end + + def call(payload) + self.class.queue = queue_class.new(nil, :size => 5) do |payload| + Rollbar.process_payload(payload) + end + + self.class.queue.push(payload) + end + end + end +end diff --git a/lib/rollbar/delay/resque.rb b/lib/rollbar/delay/resque.rb new file mode 100644 index 00000000..ec814fa4 --- /dev/null +++ b/lib/rollbar/delay/resque.rb @@ -0,0 +1,31 @@ +require 'resque' + +module Rollbar + module Delay + class Resque + def self.call(payload) + new.call(payload) + end + + def call(payload) + ::Resque.enqueue(Job, payload) + end + + class Job + class << self + attr_accessor :queue + end + + self.queue = :default + + def self.perform(payload) + new.perform(payload) + end + + def perform(payload) + Rollbar.process_payload(payload) + end + end + end + end +end diff --git a/lib/rollbar/delay/thread.rb b/lib/rollbar/delay/thread.rb new file mode 100644 index 00000000..808b12fc --- /dev/null +++ b/lib/rollbar/delay/thread.rb @@ -0,0 +1,13 @@ +module Rollbar + module Delay + class Thread + def self.call(payload) + new.call(payload) + end + + def call(payload) + ::Thread.new { Rollbar.process_payload(payload) } + end + end + end +end diff --git a/rollbar.gemspec b/rollbar.gemspec index 064aed36..7df12aaa 100644 --- a/rollbar.gemspec +++ b/rollbar.gemspec @@ -25,4 +25,5 @@ Gem::Specification.new do |gem| gem.add_development_dependency 'sidekiq', '>= 2.13.0' if RUBY_VERSION != '1.8.7' gem.add_development_dependency 'genspec', '>= 0.2.7' gem.add_development_dependency 'sinatra' + gem.add_development_dependency 'resque' end diff --git a/spec/rollbar/configuration_spec.rb b/spec/rollbar/configuration_spec.rb new file mode 100644 index 00000000..1b7a23c7 --- /dev/null +++ b/spec/rollbar/configuration_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' +require 'rollbar/configuration' + +describe Rollbar::Configuration do + + describe '#use_thread' do + it 'enables async and sets a Thread as handler' do + subject.use_thread + + expect(subject.use_async).to be_eql(true) + expect(subject.async_handler).to be_eql(Rollbar::Delay::Thread) + end + end + + describe '#use_resque' do + it 'enables async and sets Resque as the handler' do + require 'resque' + subject.use_resque(:queue => 'errors') + + expect(subject.use_async).to be_eql(true) + expect(subject.async_handler).to be_eql(Rollbar::Delay::Resque) + end + end +end diff --git a/spec/rollbar/delay/girl_friday_spec.rb b/spec/rollbar/delay/girl_friday_spec.rb new file mode 100644 index 00000000..e6e04de8 --- /dev/null +++ b/spec/rollbar/delay/girl_friday_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +# require girl_friday in the test instead in the implementation +# just to let the user decide to load it or not +require 'girl_friday' +require 'rollbar/delay/girl_friday' + +describe Rollbar::Delay::GirlFriday do + describe '.call' do + let(:payload) do + { :key => 'value' } + end + + it 'push the payload into the queue' do + expect_any_instance_of(::GirlFriday::WorkQueue).to receive(:push).with(payload) + described_class.call(payload) + end + end +end diff --git a/spec/rollbar/delay/resque_spec.rb b/spec/rollbar/delay/resque_spec.rb new file mode 100644 index 00000000..caa9fcb2 --- /dev/null +++ b/spec/rollbar/delay/resque_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' +require 'rollbar/delay/resque' + +describe Rollbar::Delay::Resque do + describe '.call' do + let(:payload) do + { :key => 'value' } + end + + before do + allow(Resque).to receive(:inline?).and_return(true) + end + + it 'process the payload' do + loaded_hash = MultiJson.load(MultiJson.dump(payload)) + + expect(Rollbar).to receive(:process_payload).with(loaded_hash) + described_class.call(payload) + end + end +end diff --git a/spec/rollbar/delay/thread_spec.rb b/spec/rollbar/delay/thread_spec.rb new file mode 100644 index 00000000..62fbc4ae --- /dev/null +++ b/spec/rollbar/delay/thread_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe Rollbar::Delay::Thread do + describe '.call' do + let(:payload) { { :key => 'value' } } + + it 'process the payload in a new thread' do + expect(Rollbar).to receive(:process_payload).with(payload) + + th = described_class.call(payload) + th.join + end + end +end diff --git a/spec/rollbar_spec.rb b/spec/rollbar_spec.rb index 0b120dfb..d9889af9 100644 --- a/spec/rollbar_spec.rb +++ b/spec/rollbar_spec.rb @@ -12,6 +12,7 @@ end describe Rollbar do + let(:configuration) { Rollbar.configuration } describe '.report_exception' do before(:each) do @@ -443,14 +444,14 @@ def backtrace Rollbar.configure do |config| config.use_async = true - GirlFriday::WorkQueue::immediate! + GirlFriday::WorkQueue.immediate! end Rollbar.report_exception(@exception) Rollbar.configure do |config| config.use_async = false - GirlFriday::WorkQueue::queue! + GirlFriday::WorkQueue.queue! end end @@ -471,7 +472,7 @@ def backtrace Rollbar.configure do |config| config.use_async = false - config.async_handler = Rollbar.method(:default_async_handler) + config.async_handler = Rollbar.default_async_handler end end @@ -498,6 +499,84 @@ def backtrace Rollbar.report_exception(@exception) end + + context 'with async failover handlers' do + before do + Rollbar.reconfigure do |config| + config.use_async = true + config.async_handler = async_handler + config.failover_handlers = handlers + config.logger = logger_mock + end + end + + let(:exception) { StandardError.new('the error') } + + context 'if the async handler doesnt fail' do + let(:async_handler) { proc { |_| 'success' } } + let(:handler) { proc { |_| 'success' } } + let(:handlers) { [handler] } + + it 'doesnt call any failover handler' do + expect(handler).not_to receive(:call) + + Rollbar.report_exception(exception) + end + end + + context 'if the async handler fails' do + let(:async_handler) { proc { |_| fail 'this handler will crash' } } + + context 'if any failover handlers is configured' do + let(:handlers) { [] } + let(:log_message) do + '[Rollbar] Async handler failed, and there are no failover handlers configured. See the docs for "failover_handlers"' + end + + it 'logs the error but doesnt try to report an internal error' do + expect(logger_mock).to receive(:error).with(log_message) + + Rollbar.report_exception(exception) + end + end + + context 'if the first failover handler success' do + let(:handler) { proc { |_| 'success' } } + let(:handlers) { [handler] } + + it 'calls the failover handler and doesnt report internal error' do + expect(Rollbar).not_to receive(:report_internal_error) + expect(handler).to receive(:call) + + Rollbar.report_exception(exception) + end + end + + context 'with two handlers, the first failing' do + let(:handler1) { proc { |_| fail 'this handler fails' } } + let(:handler2) { proc { |_| 'success' } } + let(:handlers) { [handler1, handler2] } + + it 'calls the second handler and doesnt report internal error' do + expect(handler2).to receive(:call) + + Rollbar.report_exception(exception) + end + end + + context 'with two handlers, both failing' do + let(:handler1) { proc { |_| fail 'this handler fails' } } + let(:handler2) { proc { |_| fail 'this will also fail' } } + let(:handlers) { [handler1, handler2] } + + it 'reports internal error' do + expect(logger_mock).to receive(:error) + + Rollbar.report_exception(exception) + end + end + end + end end describe "#use_sucker_punch", :if => defined?(SuckerPunch) do @@ -514,7 +593,7 @@ def backtrace Rollbar.configure do |config| config.use_async = false - config.async_handler = Rollbar.method(:default_async_handler) + config.async_handler = Rollbar.default_async_handler end end end @@ -539,7 +618,7 @@ def backtrace Rollbar.configure do |config| config.use_async = false - config.async_handler = Rollbar.method(:default_async_handler) + config.async_handler = Rollbar.default_async_handler end end end