filter_chain: prioritize library filter over user ones

CHANGELOG.md

@@ -9,6 +9,9 @@ Airbrake Ruby Changelog
 * Fixed the `host` option not recognizing hosts with subpaths such as
   `https://example.com/subpath/`
   ([#192](https://github.com/airbrake/airbrake-ruby/pull/192))
+* Fixed the order of invokation of library & user defined filters, so the user
+  filters are always invoked after all the library filters
+  ([#195](https://github.com/airbrake/airbrake-ruby/pull/195))
 
 ### [v2.0.0][v2.0.0] (March 21, 2017)
 

lib/airbrake-ruby.rb

@@ -20,6 +20,9 @@
 require 'airbrake-ruby/filters/keys_filter'
 require 'airbrake-ruby/filters/keys_whitelist'
 require 'airbrake-ruby/filters/keys_blacklist'
+require 'airbrake-ruby/filters/gem_root_filter'
+require 'airbrake-ruby/filters/system_exit_filter'
+require 'airbrake-ruby/filters/root_directory_filter'
 require 'airbrake-ruby/filter_chain'
 require 'airbrake-ruby/notifier'
 

lib/airbrake-ruby/filter_chain.rb

@@ -8,22 +8,9 @@ module Airbrake
   # @since v1.0.0
   class FilterChain
     ##
-    # Replaces paths to gems with a placeholder.
-    # @return [Proc]
-    GEM_ROOT_FILTER = proc do |notice|
-      return unless defined?(Gem)
-
-      notice[:errors].each do |error|
-        Gem.path.each do |gem_path|
-          error[:backtrace].each do |frame|
-            # If the frame is unparseable, then 'file' is nil, thus nothing to
-            # filter (all frame's data is in 'function' instead).
-            next unless (file = frame[:file])
-            file.sub!(/\A#{gem_path}/, '[GEM_ROOT]'.freeze)
-          end
-        end
-      end
-    end
+    # @return [String] the namespace for filters, which are executed first,
+    #   before any other filters
+    LIB_NAMESPACE = '#<Airbrake::'.freeze
 
     ##
     # Filters to be executed last. By this time all permutations on a notice
@@ -34,35 +21,34 @@ class FilterChain
       Airbrake::Filters::KeysWhitelist
     ].freeze
 
-    ##
-    # Skip over SystemExit exceptions, because they're just noise.
-    # @return [Proc]
-    SYSTEM_EXIT_FILTER = proc do |notice|
-      if notice[:errors].any? { |error| error[:type] == 'SystemExit' }
-        notice.ignore!
-      end
-    end
-
     ##
     # @param [Airbrake::Config] config
     def initialize(config)
       @filters = []
       @keys_filters = []
 
-      [SYSTEM_EXIT_FILTER, GEM_ROOT_FILTER].each do |filter|
-        add_filter(filter)
+      [Airbrake::Filters::SystemExitFilter,
+       Airbrake::Filters::GemRootFilter].each do |filter|
+        @filters << filter.new
       end
 
       root_directory = config.root_directory
-      add_filter(root_directory_filter(root_directory)) if root_directory
+      return unless root_directory
+
+      @filters << Airbrake::Filters::RootDirectoryFilter.new(root_directory)
     end
 
     ##
     # Adds a filter to the filter chain.
+    #
     # @param [#call] filter The filter object (proc, class, module, etc)
+    # @return [void]
     def add_filter(filter)
       return @keys_filters << filter if KEYS_FILTERS.include?(filter.class)
-      @filters << filter
+      return @filters << filter unless filter.to_s.start_with?(LIB_NAMESPACE)
+
+      i = @filters.rindex { |f| f.to_s.start_with?(LIB_NAMESPACE) }
+      @filters.insert(i + 1, filter) if i
     end
 
     ##
@@ -77,18 +63,5 @@ def refine(notice)
         filter.call(notice)
       end
     end
-
-    private
-
-    def root_directory_filter(root_directory)
-      proc do |notice|
-        notice[:errors].each do |error|
-          error[:backtrace].each do |frame|
-            next unless (file = frame[:file])
-            file.sub!(/\A#{root_directory}/, '[PROJECT_ROOT]'.freeze)
-          end
-        end
-      end
-    end
   end
 end

lib/airbrake-ruby/filters/gem_root_filter.rb

@@ -0,0 +1,26 @@
+module Airbrake
+  module Filters
+    ##
+    # Replaces paths to gems with a placeholder.
+    class GemRootFilter
+      ##
+      # @return [String]
+      GEM_ROOT_LABEL = '[GEM_ROOT]'.freeze
+
+      def call(notice)
+        return unless defined?(Gem)
+
+        notice[:errors].each do |error|
+          Gem.path.each do |gem_path|
+            error[:backtrace].each do |frame|
+              # If the frame is unparseable, then 'file' is nil, thus nothing to
+              # filter (all frame's data is in 'function' instead).
+              next unless (file = frame[:file])
+              file.sub!(/\A#{gem_path}/, GEM_ROOT_LABEL)
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/airbrake-ruby/filters/root_directory_filter.rb

@@ -0,0 +1,24 @@
+module Airbrake
+  module Filters
+    ##
+    # Replaces root directory with a label.
+    class RootDirectoryFilter
+      ##
+      # @return [String]
+      PROJECT_ROOT_LABEL = '[PROJECT_ROOT]'.freeze
+
+      def initialize(root_directory)
+        @root_directory = root_directory
+      end
+
+      def call(notice)
+        notice[:errors].each do |error|
+          error[:backtrace].each do |frame|
+            next unless (file = frame[:file])
+            file.sub!(/\A#{@root_directory}/, PROJECT_ROOT_LABEL)
+          end
+        end
+      end
+    end
+  end
+end

lib/airbrake-ruby/filters/system_exit_filter.rb

@@ -0,0 +1,16 @@
+module Airbrake
+  module Filters
+    ##
+    # Skip over SystemExit exceptions, because they're just noise.
+    class SystemExitFilter
+      ##
+      # @return [String]
+      SYSTEM_EXIT_TYPE = 'SystemExit'.freeze
+
+      def call(notice)
+        return if notice[:errors].none? { |error| error[:type] == SYSTEM_EXIT_TYPE }
+        notice.ignore!
+      end
+    end
+  end
+end

spec/filter_chain_spec.rb

@@ -54,6 +54,22 @@
         @chain.refine(notice)
         expect(notice[:params][:bingo]).to eq('[Filtered]')
       end
+
+      it "executes library filters before user ones" do
+        nums = []
+
+        @chain.add_filter(proc { nums << 2 })
+
+        priority_filter = proc { nums << 1 }
+        def priority_filter.to_s
+          '#<Airbrake::'
+        end
+        @chain.add_filter(priority_filter)
+
+        @chain.refine(notice)
+
+        expect(nums).to eq([1, 2])
+      end
     end
 
     describe "default backtrace filters" do