Failover handlers when async_handler fails

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,44 @@ 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
+      raise unless configuration.failover_handlers.any?
+
+      async_failover(payload)
+    end
+
+    def async_failover(payload)
+      index = 0
+      failover_handlers = configuration.failover_handlers
+
+      begin
+        handler = failover_handlers[index]
+        handler.call(payload)
+      rescue => e
+        index += 1
+
+        if index >= failover_handlers.size
+          report_internal_error(e)
+        else
+          retry
+        end
+      end
+    end
+
     def build_payload(data)
       payload = {
         'access_token' => configuration.access_token,
@@ -534,21 +559,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.

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'

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

lib/rollbar/delay/resque.rb

@@ -0,0 +1,27 @@
+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
+        def self.queue; 'default'; end
+
+        def self.perform(payload)
+          new.perform(payload)
+        end
+
+        def perform(payload)
+          Rollbar.process_payload(payload)
+        end
+      end
+    end
+  end
+end

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

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

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

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

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

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,59 @@ def backtrace
 
         Rollbar.report_exception(@exception)
       end
+
+      context 'with async failover handlers' do
+        let(:async_handler) do
+          proc { |_| fail 'this handler will crash' }
+        end
+
+        let(:exception) { StandardError.new('the error') }
+
+        before do
+          Rollbar.reconfigure do |config|
+            config.use_async = true
+            config.async_handler = async_handler
+            config.failover_handlers = handlers
+          end
+        end
+
+        context 'if the handler success' do
+          let(:handler) { proc { |_| 'success' } }
+          let(:handlers) { [handler] }
+
+          it 'calls the failover handler and doesnt report internal error' do
+            Rollbar.should_not_receive(:report_internal_error)
+            handler.should_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
+            Rollbar.should_not_receive(:report_internal_error)
+            handler2.should_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
+            Rollbar.should_receive(:report_internal_error)
+
+            Rollbar.report_exception(exception)
+          end
+        end
+      end
     end
 
     describe "#use_sucker_punch", :if => defined?(SuckerPunch) do
@@ -514,7 +568,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 +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