Merge pull request #525 from rollbar/fix-notifiers-on-threads

Fix per-thread notifiers

lib/rollbar.rb

@@ -23,33 +23,61 @@
 module Rollbar
   PUBLIC_NOTIFIER_METHODS = %w(debug info warn warning error critical log logger
                                process_item process_from_async_handler scope
-                               send_failsafe log_info log_debug
-                               log_warning log_error silenced preconfigure
-                               reconfigure unconfigure scope_object configuration with_config)
+                               send_failsafe log_info log_debug log_warning
+                               log_error silenced scope_object with_config).freeze
 
   class << self
     extend Forwardable
 
     def_delegators :notifier, *PUBLIC_NOTIFIER_METHODS
 
     attr_writer :plugins
+    attr_writer :root_notifier
 
     def notifier
-      Thread.current[:_rollbar_notifier] ||= Notifier.new
+      # Use the global instance @root_notifier so we don't fall
+      # in a infinite loop
+      Thread.current[:_rollbar_notifier] ||= Notifier.new(@root_notifier)
     end
 
     def notifier=(notifier)
       Thread.current[:_rollbar_notifier] = notifier
     end
 
-    # Configures the current thread notifier and
-    # loads the plugins
+    # It's the first notifier instantiated in the
+    # process. We store it so all the next per-thread
+    # notifiers can inherit its configuration
+    # The methods Rollbar.configure, Rollbar.reconfigure,
+    # Rollbar.preconfigure and Rollbar.unconfigure work
+    # on this notifier.
+    # Before v2.13.0 these methods worked on the global
+    # configuration, so in the practice the behavior is the same,
+    # since they work on the root notifier's configuration
+    def root_notifier
+      @root_notifier ||= notifier
+    end
+
+    def preconfigure(&block)
+      root_notifier.preconfigure(&block)
+    end
+
+    # Configures the root notifier and loads the plugins
     def configure(&block)
-      notifier.configure(&block)
+      root_notifier.configure(&block)
 
       plugins.load!
     end
 
+    # Reconfigures the root notifier
+    def reconfigure(&block)
+      root_notifier.reconfigure(&block)
+    end
+
+    # Unconfigures the root notifier
+    def unconfigure
+      root_notifier.unconfigure
+    end
+
     # Returns the configuration for the current notifier.
     # The current notifier is Rollbar.notifier and exists
     # one per thread.
@@ -86,10 +114,11 @@ def reset_notifier!
       notifier.reset!
     end
 
-    # Clears the current thread notifier. In the practice
-    # this should be used only on the specs
+    # Clears the current thread notifier and the root notifier.
+    # In the practice this should be used only on the specs
     def clear_notifier!
       self.notifier = nil
+      self.root_notifier = nil
     end
 
     # Create a new Notifier instance using the received options and

spec/rollbar_spec.rb

@@ -45,6 +45,49 @@
     end
   end
 
+  shared_examples 'stores the root notifier' do
+
+  end
+
+  describe '.configure' do
+    before { Rollbar.clear_notifier! }
+
+    it 'stores the root notifier' do
+      Rollbar.configure { |c| }
+      expect(Rollbar.root_notifier).to be(Rollbar.notifier)
+    end
+  end
+
+  describe '.preconfigure' do
+    before { Rollbar.clear_notifier! }
+
+    it 'stores the root notifier' do
+      Rollbar.preconfigure { |c| }
+      expect(Rollbar.root_notifier).to be(Rollbar.notifier)
+    end
+  end
+
+  describe '.reconfigure' do
+    before { Rollbar.clear_notifier! }
+
+    it 'stores the root notifier' do
+      Rollbar.reconfigure { |c| }
+      expect(Rollbar.root_notifier).to be(Rollbar.notifier)
+    end
+  end
+
+  describe '.unconfigure' do
+    before { Rollbar.clear_notifier! }
+
+    it 'stores the root notifier' do
+      expect(Rollbar.root_notifier).to receive(:unconfigure)
+
+      Rollbar.unconfigure
+
+      expect(Rollbar.root_notifier).to be(Rollbar.notifier)
+    end
+  end
+
   context 'Notifier' do
     describe '#log' do
       let(:exception) do
@@ -1334,11 +1377,12 @@ class DummyClass
   end
 
   describe '.clear_notifier' do
-    it 'resets the notifier' do
-      notifier1_id = Rollbar.notifier.object_id
+    before { Rollbar.notifier }
 
+    it 'resets the notifier' do
       Rollbar.clear_notifier!
-      expect(Rollbar.notifier.object_id).not_to be_eql(notifier1_id)
+      expect(Rollbar.instance_variable_get('@notifier')).to be_nil
+      expect(Rollbar.instance_variable_get('@root_notifier')).to be_nil
     end
   end