From 5d546dfdf8b4ddb81d8ed459744fb5e7af4efab6 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Tue, 14 Jul 2020 10:27:39 +0100
Subject: [PATCH 01/30] Extract a helper to fetch the code from an event

This also uses 'to_a' to ensure the order of code is correct. Hashes are
compared without regard for order because usually the order doesn't
matter. I don't think the API cares about the order because we explicitly
number the lines, but it's useful to validate our code is doing what is
expected
---
 spec/fixtures/crashes/short_file.rb |  2 ++
 spec/spec_helper.rb                 |  8 ++++++
 spec/stacktrace_spec.rb             | 43 +++++++++++++----------------
 3 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/spec/fixtures/crashes/short_file.rb b/spec/fixtures/crashes/short_file.rb
index c3e6d19a4..bc95df820 100644
--- a/spec/fixtures/crashes/short_file.rb
+++ b/spec/fixtures/crashes/short_file.rb
@@ -1 +1,3 @@
+#
 raise 'hell'
+#
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index b172d183b..1b869e377 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -30,6 +30,14 @@ def get_exception_from_payload(payload)
   event["exceptions"].last
 end
 
+def get_code_from_payload(payload, index = 0)
+  exception = get_exception_from_payload(payload)
+
+  expect(exception["stacktrace"].size).to be > index
+
+  exception["stacktrace"][index]["code"]
+end
+
 def notify_test_exception(*args)
   Bugsnag.notify(RuntimeError.new("test message"), *args)
 end
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 909d4e025..7c30daa83 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -15,10 +15,10 @@
         Bugsnag.notify(e)
       end
 
-      expect(Bugsnag).to have_sent_notification{ |payload, headers|
-        exception = get_exception_from_payload(payload)
-        starting_line = __LINE__ - 13
-        expect(exception["stacktrace"][0]["code"]).to eq({
+      expect(Bugsnag).to have_sent_notification { |payload, headers|
+        starting_line = __LINE__ - 12
+
+        expect(get_code_from_payload(payload).to_a).to eq({
           (starting_line + 0).to_s => '        _a = 1',
           (starting_line + 1).to_s => '        _b = 2',
           (starting_line + 2).to_s => '        _c = 3',
@@ -26,7 +26,7 @@
           (starting_line + 4).to_s => '        _d = 4',
           (starting_line + 5).to_s => '        _e = 5',
           (starting_line + 6).to_s => '        _f = 6'
-          })
+        }.to_a)
       }
     end
 
@@ -35,19 +35,16 @@
 
       notify_test_exception
 
-      expect(Bugsnag).to have_sent_notification{ |payload, headers|
-        exception = get_exception_from_payload(payload)
-        expect(exception["stacktrace"][1]["code"]).to eq(nil)
+      expect(Bugsnag).to have_sent_notification { |payload, headers|
+        expect(get_code_from_payload(payload)).to eq(nil)
       }
     end
 
     it 'should send the first 7 lines of the file for exceptions near the top' do
       load 'spec/fixtures/crashes/start_of_file.rb' rescue Bugsnag.notify $!
 
-      expect(Bugsnag).to have_sent_notification{ |payload, headers|
-        exception = get_exception_from_payload(payload)
-
-        expect(exception["stacktrace"][0]["code"]).to eq({
+      expect(Bugsnag).to have_sent_notification { |payload, headers|
+        expect(get_code_from_payload(payload).to_a).to eq({
           "1" => "#",
           "2" => "raise 'hell'",
           "3" => "#",
@@ -55,17 +52,15 @@
           "5" => "#",
           "6" => "#",
           "7" => "#"
-        })
+        }.to_a)
       }
     end
 
     it 'should send the last 7 lines of the file for exceptions near the bottom' do
       load 'spec/fixtures/crashes/end_of_file.rb' rescue Bugsnag.notify $!
 
-      expect(Bugsnag).to have_sent_notification{ |payload, headers|
-        exception = get_exception_from_payload(payload)
-
-        expect(exception["stacktrace"][0]["code"]).to eq({
+      expect(Bugsnag).to have_sent_notification { |payload, headers|
+        expect(get_code_from_payload(payload)).to eq({
           "3" => "#",
           "4" => "#",
           "5" => "#",
@@ -77,15 +72,15 @@
       }
     end
 
-    it 'should send the last 7 lines of the file for exceptions near the bottom' do
+    it 'should send every line of a very short file' do
       load 'spec/fixtures/crashes/short_file.rb' rescue Bugsnag.notify $!
 
-      expect(Bugsnag).to have_sent_notification{ |payload, headers|
-        exception = get_exception_from_payload(payload)
-
-        expect(exception["stacktrace"][0]["code"]).to eq({
-          "1" => "raise 'hell'"
-        })
+      expect(Bugsnag).to have_sent_notification { |payload, headers|
+        expect(get_code_from_payload(payload).to_a).to eq({
+          "1" => "#",
+          "2" => "raise 'hell'",
+          "3" => "#"
+        }.to_a)
       }
     end
   end

From c2d906b828c344956b965511dd18f7847170eafd Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Thu, 2 Jul 2020 10:58:07 +0100
Subject: [PATCH 02/30] Add a more complete test for code in stack frames

---
 spec/fixtures/crashes/functions.rb | 29 +++++++++++++
 spec/stacktrace_spec.rb            | 65 ++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 spec/fixtures/crashes/functions.rb

diff --git a/spec/fixtures/crashes/functions.rb b/spec/fixtures/crashes/functions.rb
new file mode 100644
index 000000000..44c11fb0a
--- /dev/null
+++ b/spec/fixtures/crashes/functions.rb
@@ -0,0 +1,29 @@
+def foo
+  bar
+end
+
+def bar
+  baz
+end
+
+def baz
+  xyz
+end
+
+def xyz
+  raise 'uh oh'
+end
+
+def abc
+  puts 'abc'
+end
+
+def abcdef
+  puts 'abcdef'
+end
+
+def abcdefghi
+  puts 'abcdefghi'
+end
+
+foo
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 7c30daa83..8b50897b7 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -83,6 +83,71 @@
         }.to_a)
       }
     end
+
+    it 'should send code for each line in the stacktrace' do
+      load 'spec/fixtures/crashes/functions.rb' rescue Bugsnag.notify $!
+
+      expected_code = [
+        # The topmost frame is centered on where the exception was raised
+        {
+          "11" => "end",
+          "12" => "",
+          "13" => "def xyz",
+          "14" => "  raise 'uh oh'",
+          "15" => "end",
+          "16" => "",
+          "17" => "def abc"
+        },
+        # then we get 'baz' which is where 'xyz' was called
+        {
+          "7" => "end",
+          "8" => "",
+          "9" => "def baz",
+          "10" => "  xyz",
+          "11" => "end",
+          "12" => "",
+          "13" => "def xyz"
+        },
+        # then we get 'bar' which is where 'baz' was called
+        {
+          "3" => "end",
+          "4" => "",
+          "5" => "def bar",
+          "6" => "  baz",
+          "7" => "end",
+          "8" => "",
+          "9" => "def baz"
+        },
+        # then we get 'foo' which is where 'bar' was called - this is the first
+        # 7 lines because the call to 'bar' is on line 2
+        {
+          "1" => "def foo",
+          "2" => "  bar",
+          "3" => "end",
+          "4" => "",
+          "5" => "def bar",
+          "6" => "  baz",
+          "7" => "end"
+        },
+        # finally we get the call to 'foo' - this is the last 7 lines because
+        # the call is on the last line of the file
+        {
+          "23" => "end",
+          "24" => "",
+          "25" => "def abcdefghi",
+          "26" => "  puts 'abcdefghi'",
+          "27" => "end",
+          "28" => "",
+          "29" => "foo"
+        }
+      ]
+
+      expect(Bugsnag).to have_sent_notification { |payload, headers|
+        (0...expected_code.size).each do |index|
+          expect(get_code_from_payload(payload, index).to_a).to eq(expected_code[index].to_a)
+        end
+      }
+    end
   end
 
   context "file paths" do

From a4b0f12b393c91a88c5796a9a72173e317e9548a Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Thu, 2 Jul 2020 11:48:27 +0100
Subject: [PATCH 03/30] Add test for when the stacktrace spans two files

---
 spec/fixtures/crashes/file1.rb | 29 +++++++++++++
 spec/fixtures/crashes/file2.rb | 25 +++++++++++
 spec/stacktrace_spec.rb        | 76 ++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+)
 create mode 100644 spec/fixtures/crashes/file1.rb
 create mode 100644 spec/fixtures/crashes/file2.rb

diff --git a/spec/fixtures/crashes/file1.rb b/spec/fixtures/crashes/file1.rb
new file mode 100644
index 000000000..f1043647d
--- /dev/null
+++ b/spec/fixtures/crashes/file1.rb
@@ -0,0 +1,29 @@
+require_relative 'file2'
+
+module File1
+  def self.foo1
+    File2.foo2
+  end
+
+  def self.bar1
+    File2.bar2
+  end
+
+  def self.baz1
+    File2.baz2
+  end
+
+  def self.abc1
+    puts 'abc'
+  end
+
+  def self.abcdef1
+    puts 'abcdef1'
+  end
+
+  def self.abcdefghi1
+    puts 'abcdefghi1'
+  end
+end
+
+File1.foo1
diff --git a/spec/fixtures/crashes/file2.rb b/spec/fixtures/crashes/file2.rb
new file mode 100644
index 000000000..66c195d43
--- /dev/null
+++ b/spec/fixtures/crashes/file2.rb
@@ -0,0 +1,25 @@
+module File2
+  def self.foo2
+    File1.bar1
+  end
+
+  def self.bar2
+    File1.baz1
+  end
+
+  def self.baz2
+    raise 'uh oh'
+  end
+
+  def self.abc2
+    puts 'abc'
+  end
+
+  def self.abcdef2
+    puts 'abcdef2'
+  end
+
+  def self.abcdefghi2
+    puts 'abcdefghi2'
+  end
+end
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 8b50897b7..6ca7c4a4c 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -148,6 +148,82 @@
         end
       }
     end
+
+    it 'should send code for each line in the stacktrace when split over multiple files' do
+      load 'spec/fixtures/crashes/file1.rb' rescue Bugsnag.notify $!
+
+      expected_code = [
+        {
+          "8" => "  end",
+          "9" => "",
+          "10" => "  def self.baz2",
+          "11" => "    raise 'uh oh'",
+          "12" => "  end",
+          "13" => "",
+          "14" => "  def self.abc2"
+        },
+        {
+          "10" => "  end",
+          "11" => "",
+          "12" => "  def self.baz1",
+          "13" => "    File2.baz2",
+          "14" => "  end",
+          "15" => "",
+          "16" => "  def self.abc1"
+        },
+        {
+          "4" => "  end",
+          "5" => "",
+          "6" => "  def self.bar2",
+          "7" => "    File1.baz1",
+          "8" => "  end",
+          "9" => "",
+          "10" => "  def self.baz2"
+        },
+        {
+          "6" => "  end",
+          "7" => "",
+          "8" => "  def self.bar1",
+          "9" => "    File2.bar2",
+          "10" => "  end",
+          "11" => "",
+          "12" => "  def self.baz1",
+        },
+        {
+          "1" => "module File2",
+          "2" => "  def self.foo2",
+          "3" => "    File1.bar1",
+          "4" => "  end",
+          "5" => "",
+          "6" => "  def self.bar2",
+          "7" => "    File1.baz1"
+        },
+        {
+          "2" => "",
+          "3" => "module File1",
+          "4" => "  def self.foo1",
+          "5" => "    File2.foo2",
+          "6" => "  end",
+          "7" => "",
+          "8" => "  def self.bar1"
+        },
+        {
+          "23" => "",
+          "24" => "  def self.abcdefghi1",
+          "25" => "    puts 'abcdefghi1'",
+          "26" => "  end",
+          "27" => "end",
+          "28" => "",
+          "29" => "File1.foo1"
+        }
+      ]
+
+      expect(Bugsnag).to have_sent_notification { |payload, headers|
+        (0...expected_code.size).each do |index|
+          expect(get_code_from_payload(payload, index).to_a).to eq(expected_code[index].to_a)
+        end
+      }
+    end
   end
 
   context "file paths" do

From 629840123bd5c1d1a91127b79ff95665213e53b6 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Fri, 3 Jul 2020 11:00:51 +0100
Subject: [PATCH 04/30] Improve code extraction performance

We now open each file in a stacktrace once to save opening the same file
multiple times if it is in the stacktrace multiple times. It's quite
common to have the same file in multiple stacktrace lines, e.g. when
a private method raises. The performance gain is especially noticeable
when a a really long file is repeated in the stacktrace as we have to
iterate through each line to find the ones we're interested in

e.g. a trace may look like this:

'/some/file line 5 in "foo"'
'/some/file line 10 in "bar"'
'/some/file line 15 in "baz"'

Previously we would:
1. Open the file for frame 1
2. Read each line up to line 11
3. Trim lines down to lines 2-8
4. Open the file for frame 2
5. Read each line up to line 14
6. Trim lines down to lines 7-13
7. Open the file for frame 3
8. Read each line up to line 19
9. Trim lines down to lines 12-18

Now we:
1. Open the file once
2. Read from line 1 until line 19
3. Add the lines each frame cares about:
    a. Lines 1-11 for frame 1
    b. Lines 3-14 for frame 2
    c. Lines 8-19 for frame 3
3. Trim lines down:
    a. To lines 2-8 for frame 1
    b. To lines 7-13 for frame 2
    c. To lines 12-18 for frame 3

Trimming is necessary because we need to record more lines than we
actually want to keep, so that we can always record 7 lines of context,
even when the frame starts at line 1 or the last line in the file

The new way is still quite inefficient and requires iterating over the
stack frames multiple times, however this passes all tests and can be
improved after profiling it against the old method
---
 lib/bugsnag/code_extractor.rb | 116 ++++++++++++++++++++++++++++++++++
 lib/bugsnag/stacktrace.rb     |  63 +++++-------------
 2 files changed, 132 insertions(+), 47 deletions(-)
 create mode 100644 lib/bugsnag/code_extractor.rb

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
new file mode 100644
index 000000000..7ef2efdff
--- /dev/null
+++ b/lib/bugsnag/code_extractor.rb
@@ -0,0 +1,116 @@
+module Bugsnag
+  class CodeExtractor
+    NUMBER_OF_LINES_TO_FETCH = 11
+    HALF_NUMBER_OF_LINES_TO_FETCH = (NUMBER_OF_LINES_TO_FETCH / 2).ceil + 1
+    MAXIMUM_LINES_TO_KEEP = 7
+
+    def initialize
+      @files = {}
+    end
+
+    ##
+    # @param [String] path
+    # @param [Hash] trace
+    # @return [void]
+    def add_file(path, trace)
+      # If the file doesn't exist we don't care about it. For BC with the old
+      # method of extraction, set :code to nil
+      # TODO is this necessary? I don't think the API cares if code is 'null' or
+      #      missing entirely in the JSON as code sending is optional
+      unless File.exist?(path)
+        trace[:code] = nil
+
+        return
+      end
+
+      @files[path] ||= []
+      @files[path].push(trace)
+
+      # Record the line numbers we want to fetch for this trace
+      first_line_number = trace[:lineNumber] - HALF_NUMBER_OF_LINES_TO_FETCH
+
+      trace[:first_line_number] = first_line_number < 1 ? 1 : first_line_number
+      trace[:last_line_number] = trace[:lineNumber] + HALF_NUMBER_OF_LINES_TO_FETCH
+    end
+
+    ##
+    # Add the code to the hashes that were given in #add_file
+    #
+    # TODO the old method has a rescue around the entire extraction process
+    #      is this needed (presumably is)? Can we add tests that raise?
+    #      We will need to handle exceptions differently in each stage; e.g.
+    #      if we fail while reading the file then every trace that needs that
+    #      file will not have code attached. However if we fail while attaching
+    #      the code to a trace, we can skip to the next trace and try that one
+    #      (though I don't know why we would fail anywhere other than File IO)
+    #
+    # @return [void]
+    def extract!
+      @files.each do |path, traces|
+        line_numbers = Set.new
+
+        traces.each do |trace|
+          trace[:first_line_number].upto(trace[:last_line_number]) do |line_number|
+            line_numbers << line_number
+          end
+        end
+
+        extract_from(path, traces, line_numbers)
+      end
+    end
+
+    private
+
+    def extract_from(path, traces, line_numbers)
+      code = {}
+
+      File.open(path) do |file|
+        current_line_number = 0
+
+        file.each_line do |line|
+          current_line_number += 1
+
+          next unless line_numbers.include?(current_line_number)
+
+          # TODO test for 200 character limit
+          code[current_line_number] = line[0...200].rstrip
+        end
+      end
+
+      associate_code_with_trace(code, traces)
+    end
+
+    def associate_code_with_trace(code, traces)
+      traces.each do |trace|
+        trace[:code] = {}
+
+        code.each do |line_number, line|
+          # If we've gone past the last line we care about we can stop iteration
+          break if line_number > trace[:last_line_number]
+
+          # Skip lines that aren't in the range we want
+          next unless line_number >= trace[:first_line_number]
+
+          trace[:code][line_number] = line
+        end
+
+        trim_excess_lines(trace[:code], trace[:lineNumber])
+        trace.delete(:first_line_number)
+        trace.delete(:last_line_number)
+      end
+    end
+
+    def trim_excess_lines(code, line_number)
+      while code.length > MAXIMUM_LINES_TO_KEEP
+        last_line = code.keys.max
+        first_line = code.keys.min
+
+        if (last_line - line_number) > (line_number - first_line)
+          code.delete(last_line)
+        else
+          code.delete(first_line)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index ac31bb535..a8571dcc1 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -14,6 +14,11 @@ class Stacktrace
     def initialize(backtrace, configuration)
       @configuration = configuration
 
+      if configuration.send_code
+        require_relative 'code_extractor'
+        code_extractor = CodeExtractor.new
+      end
+
       backtrace = caller if !backtrace || backtrace.empty?
 
       @processed_backtrace = backtrace.map do |trace|
@@ -30,12 +35,12 @@ def initialize(backtrace, configuration)
         file = File.realpath(file) rescue file
 
         # Generate the stacktrace line hash
-        trace_hash = {}
-        trace_hash[:lineNumber] = line_str.to_i
+        trace_hash = { lineNumber: line_str.to_i }
 
-        if configuration.send_code
-          trace_hash[:code] = code(file, trace_hash[:lineNumber])
-        end
+        # Save a copy of the file path as we're about to modify it but need the
+        # raw version when extracting code (otherwise we can't open the file)
+        # TODO we need a test to cover this (or it may be unnecessary!)
+        raw_file_path = file
 
         # Clean up the file path in the stacktrace
         if defined?(@configuration.project_root) && @configuration.project_root.to_s != ''
@@ -44,7 +49,6 @@ def initialize(backtrace, configuration)
           trace_hash.delete(:inProject) if file.match(@configuration.vendor_path)
         end
 
-
         # Strip common gem path prefixes
         if defined?(Gem)
           file = Gem.path.inject(file) {|line, path| line.sub(/#{path}\//, "") }
@@ -56,11 +60,17 @@ def initialize(backtrace, configuration)
         trace_hash[:method] = method if method && (method =~ /^__bind/).nil?
 
         if trace_hash[:file] && !trace_hash[:file].empty?
+          # If we're going to send code then record the raw file path and the
+          # trace_hash, so we can extract from it later
+          code_extractor.add_file(raw_file_path, trace_hash) if configuration.send_code
+
           trace_hash
         else
           nil
         end
       end.compact
+
+      code_extractor.extract! if configuration.send_code
     end
     # rubocop:enable Metrics/CyclomaticComplexity
 
@@ -69,46 +79,5 @@ def initialize(backtrace, configuration)
     def to_a
       @processed_backtrace
     end
-
-    private
-
-    def code(file, line_number, num_lines = 7)
-      code_hash = {}
-
-      from_line = [line_number - num_lines, 1].max
-
-      # don't try and open '(irb)' or '-e'
-      return unless File.exist?(file)
-
-      # Populate code hash with line numbers and code lines
-      File.open(file) do |f|
-        current_line_number = 0
-        f.each_line do |line|
-          current_line_number += 1
-
-          next if current_line_number < from_line
-
-          code_hash[current_line_number] = line[0...200].rstrip
-
-          break if code_hash.length >= ( num_lines * 1.5 ).ceil
-        end
-      end
-
-      while code_hash.length > num_lines
-        last_line = code_hash.keys.max
-        first_line = code_hash.keys.min
-
-        if (last_line - line_number) > (line_number - first_line)
-          code_hash.delete(last_line)
-        else
-          code_hash.delete(first_line)
-        end
-      end
-
-      code_hash
-    rescue
-      @configuration.warn("Error fetching code: #{$!.inspect}")
-      nil
-    end
   end
 end

From 74657161f0c6a2ab2bea258553da1a6583325d08 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Fri, 3 Jul 2020 16:07:10 +0100
Subject: [PATCH 05/30] Make Stacktrace a module as it's really 1 method

---
 lib/bugsnag/report.rb     |  2 +-
 lib/bugsnag/stacktrace.rb | 52 ++++++++++++++++++---------------------
 spec/stacktrace_spec.rb   | 14 +++++------
 3 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb
index 66b3ed7fd..b42fed8d2 100644
--- a/lib/bugsnag/report.rb
+++ b/lib/bugsnag/report.rb
@@ -182,7 +182,7 @@ def generate_exception_list
         {
           errorClass: error_class(exception),
           message: exception.message,
-          stacktrace: Stacktrace.new(exception.backtrace, configuration).to_a
+          stacktrace: Stacktrace.process(exception.backtrace, configuration)
         }
       end
     end
diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index a8571dcc1..6a826a203 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -1,6 +1,7 @@
-module Bugsnag
-  class Stacktrace
+require_relative 'code_extractor'
 
+module Bugsnag
+  module Stacktrace
     # e.g. "org/jruby/RubyKernel.java:1264:in `catch'"
     BACKTRACE_LINE_REGEX = /^((?:[a-zA-Z]:)?[^:]+):(\d+)(?::in `([^']+)')?$/
 
@@ -10,18 +11,17 @@ class Stacktrace
     ##
     # Process a backtrace and the configuration into a parsed stacktrace.
     #
+    # @param [Array, nil] backtrace
+    # @param [Configuration] configuration
+    # @return [Array]
+    #
     # rubocop:todo Metrics/CyclomaticComplexity
-    def initialize(backtrace, configuration)
-      @configuration = configuration
-
-      if configuration.send_code
-        require_relative 'code_extractor'
-        code_extractor = CodeExtractor.new
-      end
+    def self.process(backtrace, configuration)
+      code_extractor = CodeExtractor.new
 
       backtrace = caller if !backtrace || backtrace.empty?
 
-      @processed_backtrace = backtrace.map do |trace|
+      processed_backtrace = backtrace.map do |trace|
         # Parse the stacktrace line
         if trace.match(BACKTRACE_LINE_REGEX)
           file, line_str, method = [$1, $2, $3]
@@ -43,13 +43,14 @@ def initialize(backtrace, configuration)
         raw_file_path = file
 
         # Clean up the file path in the stacktrace
-        if defined?(@configuration.project_root) && @configuration.project_root.to_s != ''
-          trace_hash[:inProject] = true if file.start_with?(@configuration.project_root.to_s)
-          file.sub!(/#{@configuration.project_root}\//, "")
-          trace_hash.delete(:inProject) if file.match(@configuration.vendor_path)
+        if defined?(configuration.project_root) && configuration.project_root.to_s != ''
+          trace_hash[:inProject] = true if file.start_with?(configuration.project_root.to_s)
+          file.sub!(/#{configuration.project_root}\//, "")
+          trace_hash.delete(:inProject) if file.match(configuration.vendor_path)
         end
 
         # Strip common gem path prefixes
+        # TODO test this
         if defined?(Gem)
           file = Gem.path.inject(file) {|line, path| line.sub(/#{path}\//, "") }
         end
@@ -59,25 +60,20 @@ def initialize(backtrace, configuration)
         # Add a method if we have it
         trace_hash[:method] = method if method && (method =~ /^__bind/).nil?
 
-        if trace_hash[:file] && !trace_hash[:file].empty?
-          # If we're going to send code then record the raw file path and the
-          # trace_hash, so we can extract from it later
-          code_extractor.add_file(raw_file_path, trace_hash) if configuration.send_code
+        # TODO test this
+        return nil unless trace_hash[:file] && !trace_hash[:file].empty?
 
-          trace_hash
-        else
-          nil
-        end
+        # If we're going to send code then record the raw file path and the
+        # trace_hash, so we can extract from it later
+        code_extractor.add_file(raw_file_path, trace_hash) if configuration.send_code
+
+        trace_hash
       end.compact
 
       code_extractor.extract! if configuration.send_code
-    end
-    # rubocop:enable Metrics/CyclomaticComplexity
 
-    ##
-    # Returns the processed backtrace
-    def to_a
-      @processed_backtrace
+      processed_backtrace
     end
+    # rubocop:enable Metrics/CyclomaticComplexity
   end
 end
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 6ca7c4a4c..39fa18b3b 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -238,7 +238,7 @@
         "/foo/bar/.bundle/lib/ignore_me.rb:4:in `to_s'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.new(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
 
       expect(stacktrace).to eq([
         { file: "/foo/bar/app/models/user.rb", lineNumber: 1, method: "something" },
@@ -258,7 +258,7 @@
         "abc.rb:1:in `defg'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.new(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
 
       expect(stacktrace).to eq([
         { code: nil, file: "./foo/bar/baz.rb", lineNumber: 1, method: "something" },
@@ -281,7 +281,7 @@
         "#{dir}/../spec/stacktrace_spec.rb:5:in `something_else'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.new(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
 
       expect(stacktrace).to eq([
         { file: "#{dir}/spec_helper.rb", lineNumber: 1, method: "something" },
@@ -309,13 +309,13 @@
     end
 
     def out_project_trace(stacktrace)
-      stacktrace.to_a.map do |trace_line|
-        trace_line[:file] if !trace_line[:inProject]
+      stacktrace.map do |trace_line|
+        trace_line[:file] unless trace_line[:inProject]
       end.compact
     end
 
     it "marks vendor/ and .bundle/ as out-project by default" do
-      stacktrace = Bugsnag::Stacktrace.new(backtrace, configuration)
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(out_project_trace(stacktrace)).to eq([
         "vendor/lib/ignore_me.rb",
@@ -325,7 +325,7 @@ def out_project_trace(stacktrace)
 
     it "allows vendor_path to be configured and filters out backtrace file paths" do
       configuration.vendor_path = /other_vendor\//
-      stacktrace = Bugsnag::Stacktrace.new(backtrace, configuration)
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(out_project_trace(stacktrace)).to eq(["other_vendor/lib/dont.rb"])
     end

From 582ead4a831d470e1e6adbf9a95673b06848a15e Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Fri, 3 Jul 2020 16:42:12 +0100
Subject: [PATCH 06/30] Fix report_spec as we've removed a frame

There's no Stacktrace.new anymore, so we get one less frame
---
 spec/report_spec.rb | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/spec/report_spec.rb b/spec/report_spec.rb
index 91126274d..04fa63330 100644
--- a/spec/report_spec.rb
+++ b/spec/report_spec.rb
@@ -1373,27 +1373,30 @@ def to_s
     notify_test_exception
     expect(Bugsnag).to have_sent_notification{ |payload, headers|
       exception = get_exception_from_payload(payload)
+
       bugsnag_count = 0
+
       exception["stacktrace"].each do |frame|
         if /.*lib\/bugsnag.*\.rb/.match(frame["file"])
           bugsnag_count += 1
           expect(frame["inProject"]).to be_nil
         end
       end
-      # 7 is used here as the called bugsnag frames for a `notify` call should be:
+
+      # 6 is used here as the called bugsnag frames for a `notify` call should be:
       # - Bugsnag.notify
       # - Report.new
       # - Report.initialize
       # - Report.generate_exceptions_list
       # - Report.generate_exceptions_list | raw_exceptions.map
       # - Report.generate_exceptions_list | raw_exceptions.map | block
-      # - Report.generate_exceptions_list | raw_exceptions.map | block | Stacktrace.new
-      # However, JRUBY does not include the two `new` frames, resulting in 5 bugsnag frames
+      # However, JRUBY does not include the `Report.new` frame, resulting in 5 bugsnag frames
       if defined?(JRUBY_VERSION)
         frame_count = 5
       else
-        frame_count = 7
+        frame_count = 6
       end
+
       expect(bugsnag_count).to equal frame_count
     }
   end

From 487b5262e906662659347efb3ffbc9752cab001d Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Fri, 3 Jul 2020 16:52:01 +0100
Subject: [PATCH 07/30] Add colons to TODOs so Rubocop is happy

---
 lib/bugsnag/code_extractor.rb | 20 ++++++++++----------
 lib/bugsnag/stacktrace.rb     |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
index 7ef2efdff..04d43257f 100644
--- a/lib/bugsnag/code_extractor.rb
+++ b/lib/bugsnag/code_extractor.rb
@@ -15,8 +15,8 @@ def initialize
     def add_file(path, trace)
       # If the file doesn't exist we don't care about it. For BC with the old
       # method of extraction, set :code to nil
-      # TODO is this necessary? I don't think the API cares if code is 'null' or
-      #      missing entirely in the JSON as code sending is optional
+      # TODO: is this necessary? I don't think the API cares if code is 'null' or
+      #       missing entirely in the JSON as code sending is optional
       unless File.exist?(path)
         trace[:code] = nil
 
@@ -36,13 +36,13 @@ def add_file(path, trace)
     ##
     # Add the code to the hashes that were given in #add_file
     #
-    # TODO the old method has a rescue around the entire extraction process
-    #      is this needed (presumably is)? Can we add tests that raise?
-    #      We will need to handle exceptions differently in each stage; e.g.
-    #      if we fail while reading the file then every trace that needs that
-    #      file will not have code attached. However if we fail while attaching
-    #      the code to a trace, we can skip to the next trace and try that one
-    #      (though I don't know why we would fail anywhere other than File IO)
+    # TODO: the old method has a rescue around the entire extraction process
+    #       is this needed (presumably is)? Can we add tests that raise?
+    #       We will need to handle exceptions differently in each stage; e.g.
+    #       if we fail while reading the file then every trace that needs that
+    #       file will not have code attached. However if we fail while attaching
+    #       the code to a trace, we can skip to the next trace and try that one
+    #       (though I don't know why we would fail anywhere other than File IO)
     #
     # @return [void]
     def extract!
@@ -72,7 +72,7 @@ def extract_from(path, traces, line_numbers)
 
           next unless line_numbers.include?(current_line_number)
 
-          # TODO test for 200 character limit
+          # TODO: test for 200 character limit
           code[current_line_number] = line[0...200].rstrip
         end
       end
diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index 6a826a203..1a3a2c4c4 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -60,7 +60,7 @@ def self.process(backtrace, configuration)
         # Add a method if we have it
         trace_hash[:method] = method if method && (method =~ /^__bind/).nil?
 
-        # TODO test this
+        # TODO: test this
         return nil unless trace_hash[:file] && !trace_hash[:file].empty?
 
         # If we're going to send code then record the raw file path and the

From 5340c3da8eae319656a3939df7f7ea75d0a39fde Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 10:08:12 +0100
Subject: [PATCH 08/30] Add test for reading paths in the project_root

These will end up being converted into unreadable paths, e.g.
"spec/fixtures/crashes/file1.rb" is readable from the root of
bugsnag-ruby but will be stripped to "file1.rb" by the project_root,
which is unreadable as it's relative to a different directory

This test ensures we read files based on the expanded file path,
rather than the final path we send to the Bugsnag API
---
 lib/bugsnag/stacktrace.rb |  3 +-
 spec/stacktrace_spec.rb   | 68 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index 1a3a2c4c4..66494631e 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -39,8 +39,7 @@ def self.process(backtrace, configuration)
 
         # Save a copy of the file path as we're about to modify it but need the
         # raw version when extracting code (otherwise we can't open the file)
-        # TODO we need a test to cover this (or it may be unnecessary!)
-        raw_file_path = file
+        raw_file_path = file.dup
 
         # Clean up the file path in the stacktrace
         if defined?(configuration.project_root) && configuration.project_root.to_s != ''
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 39fa18b3b..8458388b9 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -224,6 +224,74 @@
         end
       }
     end
+
+    it "can extract code from paths that will be mangled by the project root" do
+      # Set the project root to a nested directory, which will then be stripped
+      # from the file paths in the API call. This ensures that we read the files
+      # based off of the original path, rather than the final file path, e.g.
+      # "spec/fixtures/crashes/file1.rb" will be "file1.rb" in the API call, which
+      # isn't a path that's possible to read
+      project_root = "#{File.dirname(File.dirname(__FILE__))}/spec/fixtures/crashes"
+
+      configuration = Bugsnag::Configuration.new
+      configuration.project_root = project_root
+
+      backtrace = [
+        "spec/fixtures/crashes/file1.rb:13:in `baz1'",
+        "./spec/fixtures/crashes/functions.rb:17:in `abc'",
+        "#{project_root}/file2.rb:19:in `abcdef2'",
+      ]
+
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+
+      expect(stacktrace).to eq([
+        {
+          file: "file1.rb",
+          lineNumber: 13,
+          method: "baz1",
+          inProject: true,
+          code: {
+            10 => "  end",
+            11 => "",
+            12 => "  def self.baz1",
+            13 => "    File2.baz2",
+            14 => "  end",
+            15 => "",
+            16 => "  def self.abc1"
+          }
+        },
+        {
+          file: "functions.rb",
+          lineNumber: 17,
+          method: "abc",
+          inProject: true,
+          code: {
+            14 => "  raise 'uh oh'",
+            15 => "end",
+            16 => "",
+            17 => "def abc",
+            18 => "  puts 'abc'",
+            19 => "end",
+            20 => ""
+          },
+        },
+        {
+          file: "file2.rb",
+          lineNumber: 19,
+          method: "abcdef2",
+          inProject: true,
+          code: {
+            16 => "  end",
+            17 => "",
+            18 => "  def self.abcdef2",
+            19 => "    puts 'abcdef2'",
+            20 => "  end",
+            21 => "",
+            22 => "  def self.abcdefghi2"
+          },
+        },
+      ])
+    end
   end
 
   context "file paths" do

From 32bd10c10702db753d3487a2f5ad513f8fd62db5 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 10:33:26 +0100
Subject: [PATCH 09/30] Add a test for when regex parsing fails

---
 lib/bugsnag/stacktrace.rb |  2 +-
 spec/stacktrace_spec.rb   | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index 66494631e..e137ab77f 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -29,7 +29,7 @@ def self.process(backtrace, configuration)
           method, file, line_str = [$1, $2, $3]
         end
 
-        next(nil) if file.nil?
+        next if file.nil?
 
         # Expand relative paths
         file = File.realpath(file) rescue file
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 8458388b9..9865383f1 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -358,6 +358,24 @@
         { file: "#{dir}/stacktrace_spec.rb", lineNumber: 5, method: "something_else" },
       ])
     end
+
+    it "ignores lines in backtrace that it can't parse" do
+      configuration = Bugsnag::Configuration.new
+      configuration.send_code = false
+
+      backtrace = [
+        "/foo/bar/baz.rb:2:in `to_s'",
+        "this is not formatted correctly :O",
+        "/abc/xyz.rb:4:in `to_s'",
+      ]
+
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+
+      expect(stacktrace).to eq([
+        { file: "/foo/bar/baz.rb", lineNumber: 2, method: "to_s" },
+        { file: "/abc/xyz.rb", lineNumber: 4, method: "to_s" },
+      ])
+    end
   end
 
   context "with configurable vendor_path" do

From 065794db2d1f38cc5ca8cc17324cdef6f33606d2 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 10:45:13 +0100
Subject: [PATCH 10/30] Add a test for stripping Gem paths from files

---
 lib/bugsnag/stacktrace.rb |  1 -
 spec/stacktrace_spec.rb   | 24 ++++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index e137ab77f..166ca450b 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -49,7 +49,6 @@ def self.process(backtrace, configuration)
         end
 
         # Strip common gem path prefixes
-        # TODO test this
         if defined?(Gem)
           file = Gem.path.inject(file) {|line, path| line.sub(/#{path}\//, "") }
         end
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 9865383f1..add83dd8f 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -376,6 +376,30 @@
         { file: "/abc/xyz.rb", lineNumber: 4, method: "to_s" },
       ])
     end
+
+    it "trims Gem prefix from paths" do
+      gem_path = Gem.path.first
+
+      # Sanity check that we have a gem path to strip
+      expect(gem_path).not_to be_empty
+
+      configuration = Bugsnag::Configuration.new
+      configuration.send_code = false
+
+      backtrace = [
+        "/foo/bar/baz.rb:2:in `to_s'",
+        "#{gem_path}/abc/xyz.rb:4:in `to_s'",
+        "/not/gem/path/but/has/gem.rb:6:in `to_s'"
+      ]
+
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+
+      expect(stacktrace).to eq([
+        { file: "/foo/bar/baz.rb", lineNumber: 2, method: "to_s" },
+        { file: "abc/xyz.rb", lineNumber: 4, method: "to_s" },
+        { file: "/not/gem/path/but/has/gem.rb", lineNumber: 6, method: "to_s" },
+      ])
+    end
   end
 
   context "with configurable vendor_path" do

From db5ba056b91ec3c041582821f77e8e860de140bc Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 11:42:22 +0100
Subject: [PATCH 11/30] Add a test for skipping empty file paths

This exposed a bug in the library, which may have never been reported
because it was never hit or because it's quite subtle (the stacktrace
would be truncated, but that may not be obvious)

Previously we were using 'return' which returns from the method,
not the 'map' so we'd remove the entire stacktrace if one of the
files ended up empty

It's possible this condition is never hit and therefore no one has
encountered the bug to report it, but it's easy enough to fix and
now there's a test to ensure it does the right thing
---
 lib/bugsnag/stacktrace.rb |  5 ++---
 spec/stacktrace_spec.rb   | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index 166ca450b..8b1f1e23e 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -53,14 +53,13 @@ def self.process(backtrace, configuration)
           file = Gem.path.inject(file) {|line, path| line.sub(/#{path}\//, "") }
         end
 
+        next if !file || file.empty?
+
         trace_hash[:file] = file
 
         # Add a method if we have it
         trace_hash[:method] = method if method && (method =~ /^__bind/).nil?
 
-        # TODO: test this
-        return nil unless trace_hash[:file] && !trace_hash[:file].empty?
-
         # If we're going to send code then record the raw file path and the
         # trace_hash, so we can extract from it later
         code_extractor.add_file(raw_file_path, trace_hash) if configuration.send_code
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index add83dd8f..bba8c698b 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -400,6 +400,28 @@
         { file: "/not/gem/path/but/has/gem.rb", lineNumber: 6, method: "to_s" },
       ])
     end
+
+    it "ignores files that end up with empty paths" do
+      # I'm not sure how to trigger this naturally, but we can force an empty
+      # path by setting the entire file path as the project_root. This works
+      # because we'll remove 'project_root/', which leaves us with an empty path
+      configuration = Bugsnag::Configuration.new
+      configuration.project_root = '/abc/xyz'
+      configuration.send_code = false
+
+      backtrace = [
+        "/foo/bar/baz.rb:2:in `to_s'",
+        "/abc/xyz/:4:in `to_s'",
+        "/baz/bar/foo.rb:6:in `to_s'",
+      ]
+
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+
+      expect(stacktrace).to eq([
+        { file: "/foo/bar/baz.rb", lineNumber: 2, method: "to_s" },
+        { file: "/baz/bar/foo.rb", lineNumber: 6, method: "to_s" },
+      ])
+    end
   end
 
   context "with configurable vendor_path" do

From 37546691c569fe19b244c856c4a8490143d315a0 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 12:08:53 +0100
Subject: [PATCH 12/30] Remove TODO

Most other notifiers set 'code' to null/nil in this case and there's
not really a reason not to. If we omit it entirely we _could_ break
someone's code if they assume the key 'code' always exists
---
 lib/bugsnag/code_extractor.rb | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
index 04d43257f..ddfe6646a 100644
--- a/lib/bugsnag/code_extractor.rb
+++ b/lib/bugsnag/code_extractor.rb
@@ -13,10 +13,8 @@ def initialize
     # @param [Hash] trace
     # @return [void]
     def add_file(path, trace)
-      # If the file doesn't exist we don't care about it. For BC with the old
-      # method of extraction, set :code to nil
-      # TODO: is this necessary? I don't think the API cares if code is 'null' or
-      #       missing entirely in the JSON as code sending is optional
+      # If the file doesn't exist we can't extract code from it, so we can skip
+      # this file entirely
       unless File.exist?(path)
         trace[:code] = nil
 

From add06344043139b7fdb8e0e6c89009257b06a30b Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 12:13:33 +0100
Subject: [PATCH 13/30] Simplify how many lines we get from a file

:s

I think this used to make sense in one permutation, but:

11 / 2 == 5.5
ceil(5.5) == 6
6 + 1 == 7

So we were previously using MAXIMUM_LINES_TO_KEEP in a very
obsfucated way...
---
 lib/bugsnag/code_extractor.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
index ddfe6646a..cc58abcba 100644
--- a/lib/bugsnag/code_extractor.rb
+++ b/lib/bugsnag/code_extractor.rb
@@ -1,7 +1,5 @@
 module Bugsnag
   class CodeExtractor
-    NUMBER_OF_LINES_TO_FETCH = 11
-    HALF_NUMBER_OF_LINES_TO_FETCH = (NUMBER_OF_LINES_TO_FETCH / 2).ceil + 1
     MAXIMUM_LINES_TO_KEEP = 7
 
     def initialize
@@ -25,10 +23,12 @@ def add_file(path, trace)
       @files[path].push(trace)
 
       # Record the line numbers we want to fetch for this trace
-      first_line_number = trace[:lineNumber] - HALF_NUMBER_OF_LINES_TO_FETCH
+      # We grab extra lines so that we can compensate if the error is on the
+      # first or last line of a file
+      first_line_number = trace[:lineNumber] - MAXIMUM_LINES_TO_KEEP
 
       trace[:first_line_number] = first_line_number < 1 ? 1 : first_line_number
-      trace[:last_line_number] = trace[:lineNumber] + HALF_NUMBER_OF_LINES_TO_FETCH
+      trace[:last_line_number] = trace[:lineNumber] + MAXIMUM_LINES_TO_KEEP
     end
 
     ##

From 6628cd1a8d321a5a126694ad34a813111300f46e Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 14:56:14 +0100
Subject: [PATCH 14/30] Give CodeExtractor access to configuration

---
 lib/bugsnag/code_extractor.rb | 5 ++++-
 lib/bugsnag/stacktrace.rb     | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
index cc58abcba..3e7573a4f 100644
--- a/lib/bugsnag/code_extractor.rb
+++ b/lib/bugsnag/code_extractor.rb
@@ -2,8 +2,11 @@ module Bugsnag
   class CodeExtractor
     MAXIMUM_LINES_TO_KEEP = 7
 
-    def initialize
+    ##
+    # @param [Configuration] configuration
+    def initialize(configuration)
       @files = {}
+      @configuration = configuration
     end
 
     ##
diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index 8b1f1e23e..5b72597ac 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -17,7 +17,7 @@ module Stacktrace
     #
     # rubocop:todo Metrics/CyclomaticComplexity
     def self.process(backtrace, configuration)
-      code_extractor = CodeExtractor.new
+      code_extractor = CodeExtractor.new(configuration)
 
       backtrace = caller if !backtrace || backtrace.empty?
 

From 741608439f11c1d081fb9f1357176db901fb7577 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 14:56:31 +0100
Subject: [PATCH 15/30] Add some basic tests for CodeExtractor

This is largely covered by 'stacktrace_spec', but these tests can be a
bit more specific
---
 lib/bugsnag/code_extractor.rb                 |   1 -
 spec/code_extractor_spec.rb                   | 101 ++++++++++++++++++
 spec/fixtures/crashes/file_with_long_lines.rb |   7 ++
 3 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 spec/code_extractor_spec.rb
 create mode 100644 spec/fixtures/crashes/file_with_long_lines.rb

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
index 3e7573a4f..69d4d94a0 100644
--- a/lib/bugsnag/code_extractor.rb
+++ b/lib/bugsnag/code_extractor.rb
@@ -73,7 +73,6 @@ def extract_from(path, traces, line_numbers)
 
           next unless line_numbers.include?(current_line_number)
 
-          # TODO: test for 200 character limit
           code[current_line_number] = line[0...200].rstrip
         end
       end
diff --git a/spec/code_extractor_spec.rb b/spec/code_extractor_spec.rb
new file mode 100644
index 000000000..7eac67420
--- /dev/null
+++ b/spec/code_extractor_spec.rb
@@ -0,0 +1,101 @@
+require 'spec_helper'
+
+describe Bugsnag::CodeExtractor do
+  it "extracts code from a file and adds it to the given hash" do
+    file1_hash = { lineNumber: 5 }
+    file2_hash = { lineNumber: 7 }
+
+    code_extractor = Bugsnag::CodeExtractor.new(Bugsnag::Configuration.new)
+    code_extractor.add_file("spec/fixtures/crashes/file1.rb", file1_hash)
+    code_extractor.add_file("spec/fixtures/crashes/file2.rb", file2_hash)
+
+    code_extractor.extract!
+
+    expect(file1_hash).to eq({
+      lineNumber: 5,
+      code: {
+        2 => "",
+        3 => "module File1",
+        4 => "  def self.foo1",
+        5 => "    File2.foo2",
+        6 => "  end",
+        7 => "",
+        8 => "  def self.bar1"
+      }
+    })
+
+    expect(file2_hash).to eq({
+      lineNumber: 7,
+      code: {
+        4 => "  end",
+        5 => "",
+        6 => "  def self.bar2",
+        7 => "    File1.baz1",
+        8 => "  end",
+        9 => "",
+        10 => "  def self.baz2"
+      }
+    })
+  end
+
+  it "handles extracting code from the first & last line in a file" do
+    file1_hash = { lineNumber: 1 }
+    file2_hash = { lineNumber: 25 }
+
+    code_extractor = Bugsnag::CodeExtractor.new(Bugsnag::Configuration.new)
+    code_extractor.add_file("spec/fixtures/crashes/file1.rb", file1_hash)
+    code_extractor.add_file("spec/fixtures/crashes/file2.rb", file2_hash)
+
+    code_extractor.extract!
+
+    expect(file1_hash).to eq({
+      lineNumber: 1,
+      code: {
+        1 => "require_relative 'file2'",
+        2 => "",
+        3 => "module File1",
+        4 => "  def self.foo1",
+        5 => "    File2.foo2",
+        6 => "  end",
+        7 => ""
+      }
+    })
+
+    expect(file2_hash).to eq({
+      lineNumber: 25,
+      code: {
+        19 => "    puts 'abcdef2'",
+        20 => "  end",
+        21 => "",
+        22 => "  def self.abcdefghi2",
+        23 => "    puts 'abcdefghi2'",
+        24 => "  end",
+        25 => "end"
+      }
+    })
+  end
+
+  it "truncates lines to a maximum of 200 characters" do
+    hash = { lineNumber: 4 }
+
+    code_extractor = Bugsnag::CodeExtractor.new(Bugsnag::Configuration.new)
+    code_extractor.add_file("spec/fixtures/crashes/file_with_long_lines.rb", hash)
+
+    code_extractor.extract!
+
+    # rubocop:disable Layout/LineLength
+    expect(hash).to eq({
+      lineNumber: 4,
+      code: {
+        1 => "# rubocop:disable Layout/LineLength",
+        2 => "def a_super_long_function_name_that_would_be_really_impractical_to_use_but_luckily_this_is_just_for_a_test_to_prove_we_can_handle_really_long_lines_of_code_that_go_over_200_characters_and_some_more_pa",
+        3 => "  puts 'This is a shorter string'",
+        4 => "  puts 'A more realistic example of when a line would be really long is long strings such as this one, which extends over the 200 character limit by containing a lot of excess words for padding its le",
+        5 => "  puts 'and another shorter string for comparison'",
+        6 => "end",
+        7 => "# rubocop:enable Layout/LineLength",
+      }
+    })
+    # rubocop:enable Layout/LineLength
+  end
+end
diff --git a/spec/fixtures/crashes/file_with_long_lines.rb b/spec/fixtures/crashes/file_with_long_lines.rb
new file mode 100644
index 000000000..0150e4a4b
--- /dev/null
+++ b/spec/fixtures/crashes/file_with_long_lines.rb
@@ -0,0 +1,7 @@
+# rubocop:disable Layout/LineLength
+def a_super_long_function_name_that_would_be_really_impractical_to_use_but_luckily_this_is_just_for_a_test_to_prove_we_can_handle_really_long_lines_of_code_that_go_over_200_characters_and_some_more_padding
+  puts 'This is a shorter string'
+  puts 'A more realistic example of when a line would be really long is long strings such as this one, which extends over the 200 character limit by containing a lot of excess words for padding its length so that it is super long'
+  puts 'and another shorter string for comparison'
+end
+# rubocop:enable Layout/LineLength

From e355f1d9c744a8ec1b44627fb5da96e68bdfacd1 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 6 Jul 2020 15:45:29 +0100
Subject: [PATCH 16/30] Handle exceptions in extract!

---
 lib/bugsnag/code_extractor.rb | 37 ++++++++++++++++++++---------------
 spec/code_extractor_spec.rb   | 28 ++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
index 69d4d94a0..906d6480b 100644
--- a/lib/bugsnag/code_extractor.rb
+++ b/lib/bugsnag/code_extractor.rb
@@ -35,28 +35,33 @@ def add_file(path, trace)
     end
 
     ##
-    # Add the code to the hashes that were given in #add_file
-    #
-    # TODO: the old method has a rescue around the entire extraction process
-    #       is this needed (presumably is)? Can we add tests that raise?
-    #       We will need to handle exceptions differently in each stage; e.g.
-    #       if we fail while reading the file then every trace that needs that
-    #       file will not have code attached. However if we fail while attaching
-    #       the code to a trace, we can skip to the next trace and try that one
-    #       (though I don't know why we would fail anywhere other than File IO)
+    # Add the code to the hashes that were given in #add_file by modifying them
+    # in-place. They will have a new ':code' key containing a hash of line
+    # number => string of code for that line
     #
     # @return [void]
     def extract!
       @files.each do |path, traces|
-        line_numbers = Set.new
+        begin
+          line_numbers = Set.new
 
-        traces.each do |trace|
-          trace[:first_line_number].upto(trace[:last_line_number]) do |line_number|
-            line_numbers << line_number
+          traces.each do |trace|
+            trace[:first_line_number].upto(trace[:last_line_number]) do |line_number|
+              line_numbers << line_number
+            end
           end
-        end
 
-        extract_from(path, traces, line_numbers)
+          extract_from(path, traces, line_numbers)
+        rescue StandardError => e
+          # Clean up after ourselves
+          traces.each do |trace|
+            trace[:code] ||= nil
+            trace.delete(:first_line_number)
+            trace.delete(:last_line_number)
+          end
+
+          @configuration.warn("Error extracting code: #{e.inspect}")
+        end
       end
     end
 
@@ -85,7 +90,7 @@ def associate_code_with_trace(code, traces)
         trace[:code] = {}
 
         code.each do |line_number, line|
-          # If we've gone past the last line we care about we can stop iteration
+          # If we've gone past the last line we care about, we can stop iterating
           break if line_number > trace[:last_line_number]
 
           # Skip lines that aren't in the range we want
diff --git a/spec/code_extractor_spec.rb b/spec/code_extractor_spec.rb
index 7eac67420..17c26bf6a 100644
--- a/spec/code_extractor_spec.rb
+++ b/spec/code_extractor_spec.rb
@@ -98,4 +98,32 @@
     })
     # rubocop:enable Layout/LineLength
   end
+
+  it "rescues exceptions raised in extract!" do
+    file1_hash = { lineNumber: 1 }
+    file2_hash = { lineNumber: 25 }
+
+    code_extractor = Bugsnag::CodeExtractor.new(Bugsnag::Configuration.new)
+    code_extractor.add_file("spec/fixtures/crashes/file1.rb", file1_hash)
+    code_extractor.add_file("spec/fixtures/crashes/file2.rb", file2_hash)
+
+    file1_hash[:first_line_number] = nil
+
+    code_extractor.extract!
+
+    expect(file1_hash).to eq({ lineNumber: 1, code: nil })
+
+    expect(file2_hash).to eq({
+      lineNumber: 25,
+      code: {
+        19 => "    puts 'abcdef2'",
+        20 => "  end",
+        21 => "",
+        22 => "  def self.abcdefghi2",
+        23 => "    puts 'abcdefghi2'",
+        24 => "  end",
+        25 => "end"
+      }
+    })
+  end
 end

From 85a38c4dfaec74254306191b7ebda30794b6c834 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Tue, 7 Jul 2020 12:39:45 +0100
Subject: [PATCH 17/30] Improve YARD docs

---
 lib/bugsnag/code_extractor.rb | 26 +++++++++++++++++++++-----
 lib/bugsnag/stacktrace.rb     |  9 +++------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/lib/bugsnag/code_extractor.rb b/lib/bugsnag/code_extractor.rb
index 906d6480b..3a6802b2f 100644
--- a/lib/bugsnag/code_extractor.rb
+++ b/lib/bugsnag/code_extractor.rb
@@ -1,17 +1,20 @@
 module Bugsnag
+  # @api private
   class CodeExtractor
     MAXIMUM_LINES_TO_KEEP = 7
 
     ##
-    # @param [Configuration] configuration
+    # @param configuration [Configuration]
     def initialize(configuration)
       @files = {}
       @configuration = configuration
     end
 
     ##
-    # @param [String] path
-    # @param [Hash] trace
+    # Add a file and its corresponding trace hash to be processed.
+    #
+    # @param path [String] The full path to the file
+    # @param trace [Hash]
     # @return [void]
     def add_file(path, trace)
       # If the file doesn't exist we can't extract code from it, so we can skip
@@ -35,8 +38,8 @@ def add_file(path, trace)
     end
 
     ##
-    # Add the code to the hashes that were given in #add_file by modifying them
-    # in-place. They will have a new ':code' key containing a hash of line
+    # Add the code to the hashes that were given in {#add_file} by modifying
+    # them in-place. They will have a new ':code' key containing a hash of line
     # number => string of code for that line
     #
     # @return [void]
@@ -67,6 +70,11 @@ def extract!
 
     private
 
+    ##
+    # @param path [String]
+    # @param traces [Array<Hash>]
+    # @param line_numbers [Set<Integer>]
+    # @return [void]
     def extract_from(path, traces, line_numbers)
       code = {}
 
@@ -85,6 +93,10 @@ def extract_from(path, traces, line_numbers)
       associate_code_with_trace(code, traces)
     end
 
+    ##
+    # @param code [Hash{Integer => String}]
+    # @param traces [Array<Hash>]
+    # @return [void]
     def associate_code_with_trace(code, traces)
       traces.each do |trace|
         trace[:code] = {}
@@ -105,6 +117,10 @@ def associate_code_with_trace(code, traces)
       end
     end
 
+    ##
+    # @param code [Hash{Integer => String}]
+    # @param line_number [Integer]
+    # @return [void]
     def trim_excess_lines(code, line_number)
       while code.length > MAXIMUM_LINES_TO_KEEP
         last_line = code.keys.max
diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index 5b72597ac..86ed15c9e 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -11,12 +11,10 @@ module Stacktrace
     ##
     # Process a backtrace and the configuration into a parsed stacktrace.
     #
-    # @param [Array, nil] backtrace
-    # @param [Configuration] configuration
+    # @param backtrace [Array, nil] If nil, 'caller' will be used instead
+    # @param configuration [Configuration]
     # @return [Array]
-    #
-    # rubocop:todo Metrics/CyclomaticComplexity
-    def self.process(backtrace, configuration)
+    def self.process(backtrace, configuration) # rubocop:todo Metrics/CyclomaticComplexity
       code_extractor = CodeExtractor.new(configuration)
 
       backtrace = caller if !backtrace || backtrace.empty?
@@ -71,6 +69,5 @@ def self.process(backtrace, configuration)
 
       processed_backtrace
     end
-    # rubocop:enable Metrics/CyclomaticComplexity
   end
 end

From 0fe026245b5e94939a3a407b856d8e1790dff56d Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Wed, 15 Jul 2020 15:02:23 +0100
Subject: [PATCH 18/30] Remove unnecessary 'to_a' calls

These were required when Stacktrace was a class and needed to be
'new'-ed, but now it returns an array already
---
 spec/stacktrace_spec.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index bba8c698b..9cc0aaa6e 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -242,7 +242,7 @@
         "#{project_root}/file2.rb:19:in `abcdef2'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(stacktrace).to eq([
         {
@@ -306,7 +306,7 @@
         "/foo/bar/.bundle/lib/ignore_me.rb:4:in `to_s'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(stacktrace).to eq([
         { file: "/foo/bar/app/models/user.rb", lineNumber: 1, method: "something" },
@@ -326,7 +326,7 @@
         "abc.rb:1:in `defg'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(stacktrace).to eq([
         { code: nil, file: "./foo/bar/baz.rb", lineNumber: 1, method: "something" },
@@ -349,7 +349,7 @@
         "#{dir}/../spec/stacktrace_spec.rb:5:in `something_else'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(stacktrace).to eq([
         { file: "#{dir}/spec_helper.rb", lineNumber: 1, method: "something" },
@@ -369,7 +369,7 @@
         "/abc/xyz.rb:4:in `to_s'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(stacktrace).to eq([
         { file: "/foo/bar/baz.rb", lineNumber: 2, method: "to_s" },
@@ -392,7 +392,7 @@
         "/not/gem/path/but/has/gem.rb:6:in `to_s'"
       ]
 
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(stacktrace).to eq([
         { file: "/foo/bar/baz.rb", lineNumber: 2, method: "to_s" },
@@ -415,7 +415,7 @@
         "/baz/bar/foo.rb:6:in `to_s'",
       ]
 
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration).to_a
+      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
 
       expect(stacktrace).to eq([
         { file: "/foo/bar/baz.rb", lineNumber: 2, method: "to_s" },

From e2a3094ccb366b1d533660b3287736a3c8220d1a Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Fri, 17 Jul 2020 11:58:00 +0100
Subject: [PATCH 19/30] Remove empty filename check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is no longer needed because, other than misconfiguration, there's
no way for the filename to be empty here

it was implemented in a479b6ed, which also let users add their own filters
for the stacktrace file name. This would have been necessary then, but seems
like it's not anymore — the only way to trigger this in a real backtrace is
by setting the project root to a filename, which isn't a realistic use-case

See https://github.com/bugsnag/bugsnag-ruby/pull/604#pullrequestreview-450496663
---
 lib/bugsnag/stacktrace.rb |  2 --
 spec/stacktrace_spec.rb   | 22 ----------------------
 2 files changed, 24 deletions(-)

diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index 86ed15c9e..d9e2d99d9 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -51,8 +51,6 @@ def self.process(backtrace, configuration) # rubocop:todo Metrics/CyclomaticComp
           file = Gem.path.inject(file) {|line, path| line.sub(/#{path}\//, "") }
         end
 
-        next if !file || file.empty?
-
         trace_hash[:file] = file
 
         # Add a method if we have it
diff --git a/spec/stacktrace_spec.rb b/spec/stacktrace_spec.rb
index 9cc0aaa6e..cf4344d0a 100644
--- a/spec/stacktrace_spec.rb
+++ b/spec/stacktrace_spec.rb
@@ -400,28 +400,6 @@
         { file: "/not/gem/path/but/has/gem.rb", lineNumber: 6, method: "to_s" },
       ])
     end
-
-    it "ignores files that end up with empty paths" do
-      # I'm not sure how to trigger this naturally, but we can force an empty
-      # path by setting the entire file path as the project_root. This works
-      # because we'll remove 'project_root/', which leaves us with an empty path
-      configuration = Bugsnag::Configuration.new
-      configuration.project_root = '/abc/xyz'
-      configuration.send_code = false
-
-      backtrace = [
-        "/foo/bar/baz.rb:2:in `to_s'",
-        "/abc/xyz/:4:in `to_s'",
-        "/baz/bar/foo.rb:6:in `to_s'",
-      ]
-
-      stacktrace = Bugsnag::Stacktrace.process(backtrace, configuration)
-
-      expect(stacktrace).to eq([
-        { file: "/foo/bar/baz.rb", lineNumber: 2, method: "to_s" },
-        { file: "/baz/bar/foo.rb", lineNumber: 6, method: "to_s" },
-      ])
-    end
   end
 
   context "with configurable vendor_path" do

From d2c45127da5559d9c968759b97b96d9c0fe0f811 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Fri, 17 Jul 2020 12:03:02 +0100
Subject: [PATCH 20/30] Remove unnecessary Rubucop toggle

---
 lib/bugsnag/stacktrace.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bugsnag/stacktrace.rb b/lib/bugsnag/stacktrace.rb
index d9e2d99d9..2ce131a96 100644
--- a/lib/bugsnag/stacktrace.rb
+++ b/lib/bugsnag/stacktrace.rb
@@ -14,7 +14,7 @@ module Stacktrace
     # @param backtrace [Array, nil] If nil, 'caller' will be used instead
     # @param configuration [Configuration]
     # @return [Array]
-    def self.process(backtrace, configuration) # rubocop:todo Metrics/CyclomaticComplexity
+    def self.process(backtrace, configuration)
       code_extractor = CodeExtractor.new(configuration)
 
       backtrace = caller if !backtrace || backtrace.empty?

From acf4f264f74a3d5eb18af86bb2fdb73a58bb0f23 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Fri, 17 Jul 2020 17:39:43 +0100
Subject: [PATCH 21/30] Add 'on_error' callbacks, backed by middleware

These new 'on_error' callbacks will ultimately be a replacement for
the existing 'before_notify_callbacks'

The main problem these solve is that they are global, rather than
being scoped to the current thread. This means you can add a callback
in a Rails initializer and it will run as expected. With the current
'before_notify_callbacks', they only run in the same thread as they
are added and so won't run if created in an initializer. This is a
problem in most (all ?) frameworks, not just Rails

This adds some complexity to the MiddlewareStack, because it now has
to handle procs being passed as middleware. It's important that we
don't wrap the procs earlier for two reasons:

  1. We need to be able to remove them, which is much simpler if
     we can do this by object identity (because the current method
       works that way)
  2. We need to be able to abort the 'queue' of callbacks, if one
     returns false. This is much easier if we have a list of procs
     to run
---
 lib/bugsnag.rb                    |  29 +++
 lib/bugsnag/configuration.rb      |  27 +++
 lib/bugsnag/middleware_stack.rb   |  41 +++-
 lib/bugsnag/on_error_callbacks.rb |  33 +++
 spec/on_error_spec.rb             | 332 ++++++++++++++++++++++++++++++
 5 files changed, 459 insertions(+), 3 deletions(-)
 create mode 100644 lib/bugsnag/on_error_callbacks.rb
 create mode 100644 spec/on_error_spec.rb

diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb
index 1be5d084a..178ce0efc 100644
--- a/lib/bugsnag.rb
+++ b/lib/bugsnag.rb
@@ -169,6 +169,8 @@ def start_session
     # Allow access to "before notify" callbacks as an array.
     #
     # These callbacks will be called whenever an error notification is being made.
+    #
+    # @deprecated Use {Bugsnag#add_on_error} instead
     def before_notify_callbacks
       Bugsnag.configuration.request_data[:before_callbacks] ||= []
     end
@@ -227,6 +229,33 @@ def leave_breadcrumb(name, meta_data={}, type=Bugsnag::Breadcrumbs::MANUAL_BREAD
       configuration.breadcrumbs << breadcrumb unless breadcrumb.ignore?
     end
 
+    ##
+    # Add the given callback to the list of on_error callbacks
+    #
+    # The on_error callbacks will be called when an error is captured or reported
+    # and are passed a {Bugsnag::Report} object
+    #
+    # Returning false from an on_error callback will cause the error to be ignored
+    # and will prevent any remaining callbacks from being called
+    #
+    # @param callback [Proc]
+    # @return [void]
+    def add_on_error(callback)
+      configuration.add_on_error(callback)
+    end
+
+    ##
+    # Remove the given callback from the list of on_error callbacks
+    #
+    # Note that this must be the same Proc instance that was passed to
+    # {Bugsnag#add_on_error}, otherwise it will not be removed
+    #
+    # @param callback [Proc]
+    # @return [void]
+    def remove_on_error(callback)
+      configuration.remove_on_error(callback)
+    end
+
     ##
     # Returns the client's Cleaner object, or creates one if not yet created.
     #
diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb
index d017c111e..b8e8954f8 100644
--- a/lib/bugsnag/configuration.rb
+++ b/lib/bugsnag/configuration.rb
@@ -318,6 +318,33 @@ def disable_sessions
       @enable_sessions = false
     end
 
+    ##
+    # Add the given callback to the list of on_error callbacks
+    #
+    # The on_error callbacks will be called when an error is captured or reported
+    # and are passed a {Bugsnag::Report} object
+    #
+    # Returning false from an on_error callback will cause the error to be ignored
+    # and will prevent any remaining callbacks from being called
+    #
+    # @param callback [Proc]
+    # @return [void]
+    def add_on_error(callback)
+      middleware.use(callback)
+    end
+
+    ##
+    # Remove the given callback from the list of on_error callbacks
+    #
+    # Note that this must be the same Proc instance that was passed to
+    # {#add_on_error}, otherwise it will not be removed
+    #
+    # @param callback [Proc]
+    # @return [void]
+    def remove_on_error(callback)
+      middleware.remove(callback)
+    end
+
     private
 
     attr_writer :scopes_to_filter
diff --git a/lib/bugsnag/middleware_stack.rb b/lib/bugsnag/middleware_stack.rb
index a28ee0053..d9dcd95bc 100644
--- a/lib/bugsnag/middleware_stack.rb
+++ b/lib/bugsnag/middleware_stack.rb
@@ -1,3 +1,5 @@
+require "bugsnag/on_error_callbacks"
+
 module Bugsnag
   class MiddlewareStack
     ##
@@ -65,11 +67,26 @@ def insert_before(before, new_middleware)
       end
     end
 
+    ##
+    # Disable the given middleware. This removes them from the list of
+    # middleware and ensures they cannot be added again
+    #
+    # See also {#remove}
     def disable(*middlewares)
       @mutex.synchronize do
         @disabled_middleware += middlewares
 
-        @middlewares.delete_if {|m| @disabled_middleware.include?(m)}
+        @middlewares.delete_if {|m| @disabled_middleware.include?(m) }
+      end
+    end
+
+    ##
+    # Remove the given middleware from the list of middleware
+    #
+    # This is like {#disable} but allows the middleware to be added again
+    def remove(*middlewares)
+      @mutex.synchronize do
+        @middlewares.delete_if {|m| middlewares.include?(m) }
       end
     end
 
@@ -91,7 +108,7 @@ def run(report)
 
       begin
         # We reverse them, so we can call "call" on the first middleware
-        middleware_procs.reverse.inject(notify_lambda) { |n,e| e.call(n) }.call(report)
+        middleware_procs.reverse.inject(notify_lambda) {|n, e| e.call(n) }.call(report)
       rescue StandardError => e
         # KLUDGE: Since we don't re-raise middleware exceptions, this breaks rspec
         raise if e.class.to_s == "RSpec::Expectations::ExpectationNotMetError"
@@ -107,10 +124,28 @@ def run(report)
     end
 
     private
+
+    ##
     # Generates a list of middleware procs that are ready to be run
     # Pass each one a reference to the next in the queue
+    #
+    # @return [Array<Proc>]
     def middleware_procs
-      @middlewares.map{|middleware| proc { |next_middleware| middleware.new(next_middleware) } }
+      # Split the middleware into separate lists of Procs and Classes
+      procs, classes = @middlewares.partition {|middleware| middleware.is_a?(Proc) }
+
+      # Wrap the classes in a proc that, when called, news up the middleware and
+      # passes the next middleware in the queue
+      middleware_instances = classes.map do |middleware|
+        proc {|next_middleware| middleware.new(next_middleware) }
+      end
+
+      # Wrap the list of procs in a proc that, when called, wraps them in an
+      # 'OnErrorCallbacks' instance that also has a reference to the next middleware
+      wrapped_procs = proc {|next_middleware| OnErrorCallbacks.new(next_middleware, procs) }
+
+      # Return the combined middleware and wrapped procs
+      middleware_instances.push(wrapped_procs)
     end
   end
 end
diff --git a/lib/bugsnag/on_error_callbacks.rb b/lib/bugsnag/on_error_callbacks.rb
new file mode 100644
index 000000000..70fa7f5f0
--- /dev/null
+++ b/lib/bugsnag/on_error_callbacks.rb
@@ -0,0 +1,33 @@
+module Bugsnag
+  # @api private
+  class OnErrorCallbacks
+    def initialize(next_middleware, callbacks)
+      @next_middleware = next_middleware
+      @callbacks = callbacks
+    end
+
+    ##
+    # @param report [Report]
+    def call(report)
+      @callbacks.each do |callback|
+        begin
+          should_continue = callback.call(report)
+        rescue StandardError => e
+          Bugsnag.configuration.warn("Error occurred in on_error callback: '#{e}'")
+          Bugsnag.configuration.warn("on_error callback stacktrace: #{e.backtrace.inspect}")
+        end
+
+        # If a callback returns false, we ignore the report and stop running callbacks
+        # Note that we explicitly check for 'false' so that callbacks don't need
+        # to return anything (i.e. can return 'nil') and we still continue
+        next unless should_continue == false
+
+        report.ignore!
+
+        break
+      end
+
+      @next_middleware.call(report)
+    end
+  end
+end
diff --git a/spec/on_error_spec.rb b/spec/on_error_spec.rb
new file mode 100644
index 000000000..ab982944d
--- /dev/null
+++ b/spec/on_error_spec.rb
@@ -0,0 +1,332 @@
+require "spec_helper"
+
+describe "on_error callbacks" do
+  it "runs callbacks on notify" do
+    callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+    callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to eq({ "hello" => "world" })
+      expect(event["metaData"]["significant"]).to eq({ "hey" => "earth" })
+    end)
+  end
+
+  it "can add callbacks in a configure block" do
+    callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+    callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+
+    Bugsnag.configure do |config|
+      config.add_on_error(callback1)
+      config.add_on_error(callback2)
+    end
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to eq({ "hello" => "world" })
+      expect(event["metaData"]["significant"]).to eq({ "hey" => "earth" })
+    end)
+  end
+
+  it "can remove an already registered callback" do
+    callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+    callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+
+    Bugsnag.remove_on_error(callback1)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to be_nil
+      expect(event["metaData"]["significant"]).to eq({ "hey" => "earth" })
+    end)
+  end
+
+  it "can remove all registered callbacks" do
+    callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+    callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+
+    Bugsnag.remove_on_error(callback2)
+    Bugsnag.remove_on_error(callback1)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to be_nil
+      expect(event["metaData"]["significant"]).to be_nil
+    end)
+  end
+
+  it "does not remove an identical callback if it is not the same Proc" do
+    callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+    callback1_duplicate = proc {|report| report.add_tab(:important, { hello: "world" }) }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.remove_on_error(callback1_duplicate)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to eq({ "hello" => "world" })
+    end)
+  end
+
+  it "can re-add callbacks that have previously been removed" do
+    callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+    callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+
+    Bugsnag.remove_on_error(callback1)
+
+    Bugsnag.add_on_error(callback1)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to eq({ "hello" => "world" })
+      expect(event["metaData"]["significant"]).to eq({ "hey" => "earth" })
+    end)
+  end
+
+  it "will only add a callback once" do
+    called_count = 0
+
+    callback = proc do |report|
+      called_count += 1
+
+      report.add_tab(:important, { called: called_count })
+    end
+
+    1.upto(10) do |i|
+      Bugsnag.add_on_error(callback)
+    end
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(called_count).to be(1)
+      expect(event["metaData"]["important"]).to eq({ "called" => 1 })
+    end)
+  end
+
+  it "will ignore the report and stop calling callbacks if one returns false" do
+    logger = spy('logger')
+    Bugsnag.configuration.logger = logger
+
+    called_count = 0
+
+    callback1 = proc { called_count += 1 }
+    callback2 = proc { called_count += 1 }
+    callback3 = proc { false }
+    callback4 = proc { called_count += 1 }
+    callback5 = proc { called_count += 1 }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+    Bugsnag.add_on_error(callback3)
+    Bugsnag.add_on_error(callback4)
+    Bugsnag.add_on_error(callback5)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).not_to have_sent_notification
+    expect(called_count).to be(2)
+
+    expect(logger).to have_received(:debug).with("[Bugsnag]") do |&block|
+      expect(block.call).to eq("Not notifying RuntimeError due to ignore being signified in user provided middleware")
+    end
+  end
+
+  it "callbacks are called in the same order they are added (FIFO)" do
+    callback1 = proc do |report|
+      expect(report.meta_data[:important]).to be_nil
+
+      report.add_tab(:important, { magic_number: 9 })
+    end
+
+    callback2 = proc do |report|
+      expect(report.meta_data[:important]).to eq({ magic_number: 9 })
+
+      report.add_tab(:important, { magic_number: 99 })
+    end
+
+    callback3 = proc do |report|
+      expect(report.meta_data[:important]).to eq({ magic_number: 99 })
+
+      report.add_tab(:important, { magic_number: 999 })
+    end
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+    Bugsnag.add_on_error(callback3)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to eq({ "magic_number" => 999 })
+    end)
+  end
+
+  it "callbacks continue being called after a callback raises" do
+    logger = spy('logger')
+    Bugsnag.configuration.logger = logger
+
+    callback1 = proc {|report| report.add_tab(:important, { a: "b" }) }
+    callback2 = proc {|_report| raise "bad things" }
+    callback3 = proc {|report| report.add_tab(:important, { c: "d" }) }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+    Bugsnag.add_on_error(callback3)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to eq({ "a" => "b", "c" => "d" })
+    end)
+
+    message_index = 0
+    expected_messages = [
+      /^Error occurred in on_error callback: 'bad things'$/,
+      /^on_error callback stacktrace:/
+    ]
+
+    expect(logger).to have_received(:warn).with("[Bugsnag]").twice do |&block|
+      expect(block.call).to match(expected_messages[message_index])
+      message_index += 1
+    end
+  end
+
+  it "runs callbacks even if no other middleware is registered" do
+    # Reset the middleware stack so any default middleware are removed
+    Bugsnag.configuration.middleware = Bugsnag::MiddlewareStack.new
+
+    callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+    callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+
+    Bugsnag.add_on_error(callback1)
+    Bugsnag.add_on_error(callback2)
+
+    Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+    expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+      event = get_event_from_payload(payload)
+
+      expect(event["metaData"]["important"]).to eq({ "hello" => "world" })
+      expect(event["metaData"]["significant"]).to eq({ "hey" => "earth" })
+    end)
+  end
+
+  describe "using callbacks across threads" do
+    it "runs callbacks that are added in different threads" do
+      callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+      callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+      callback3 = proc {|report| report.add_tab(:crucial, { magic_number: 999 }) }
+
+      Bugsnag.add_on_error(callback1)
+
+      threads = [
+        Thread.new { Bugsnag.add_on_error(callback2) },
+        Thread.new { Bugsnag.add_on_error(callback3) }
+      ]
+
+      threads.each(&:join)
+
+      Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+      expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+        event = get_event_from_payload(payload)
+
+        expect(event["metaData"]["important"]).to eq({ "hello" => "world" })
+        expect(event["metaData"]["significant"]).to eq({ "hey" => "earth" })
+        expect(event["metaData"]["crucial"]).to eq({ "magic_number" => 999 })
+      end)
+    end
+
+    it "can remove callbacks that are added in different threads" do
+      callback1 = proc {|report| report.add_tab(:important, { hello: "world" }) }
+      callback2 = proc {|report| report.add_tab(:significant, { hey: "earth" }) }
+
+      # We need to create & join these one at a time so that callback1 has
+      # definitely been added before it is removed, otherwise this test will fail
+      # at random
+      Thread.new { Bugsnag.add_on_error(callback1) }.join
+      Thread.new { Bugsnag.remove_on_error(callback1) }.join
+      Thread.new { Bugsnag.add_on_error(callback2) }.join
+
+      Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+      expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+        event = get_event_from_payload(payload)
+
+        expect(event["metaData"]["important"]).to be_nil
+        expect(event["metaData"]["significant"]).to eq({ "hey" => "earth" })
+      end)
+    end
+
+    it "callbacks are called in FIFO order when added in separate threads" do
+      callback1 = proc do |report|
+        expect(report.meta_data[:important]).to be_nil
+
+        report.add_tab(:important, { magic_number: 9 })
+      end
+
+      callback2 = proc do |report|
+        expect(report.meta_data[:important]).to eq({ magic_number: 9 })
+
+        report.add_tab(:important, { magic_number: 99 })
+      end
+
+      callback3 = proc do |report|
+        expect(report.meta_data[:important]).to eq({ magic_number: 99 })
+
+        report.add_tab(:important, { magic_number: 999 })
+      end
+
+      # As above, we need to create & join these one at a time so that each
+      # callback is added in sequence
+      Thread.new { Bugsnag.add_on_error(callback1) }.join
+      Thread.new { Bugsnag.add_on_error(callback2) }.join
+      Thread.new { Bugsnag.add_on_error(callback3) }.join
+
+      Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+      expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+        event = get_event_from_payload(payload)
+
+        expect(event["metaData"]["important"]).to eq({ "magic_number" => 999 })
+      end)
+    end
+  end
+end

From 25cd43c42a105a1d35029e20c3271a7aca362eaa Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 20 Jul 2020 16:25:04 +0100
Subject: [PATCH 22/30] Add changelog entry

---
 CHANGELOG.md | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index be53a6bef..21e758dd4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,17 @@
 Changelog
 =========
 
+## TBD
+
+### Enhancements
+
+* Add `on_error` callbacks to replace `before_notify_callbacks`
+  | [#608](https://github.com/bugsnag/bugsnag-ruby/pull/608)
+
+### Deprecated
+
+* `before_notify_callbacks` have been deprecated in favour of `on_error` and will be removed in the next major release
+
 ## 6.14.0 (20 July 2020)
 
 ### Enhancements

From 7475f8824acdaa6350091a659f238aed7ce9779f Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Tue, 21 Jul 2020 11:47:01 +0100
Subject: [PATCH 23/30] Use add_on_error in example apps

---
 example/padrino/app/app.rb                    | 28 +++------------
 example/padrino/app/templates/index.md        | 14 +++-----
 example/padrino/config/boot.rb                |  8 +++++
 example/rack/server.rb                        | 35 +++++++-----------
 example/rack/templates/index.md               | 14 +++-----
 .../app/controllers/application_controller.rb | 26 +++-----------
 example/rails-42/app/views/index.md           | 14 +++-----
 .../rails-42/config/initializers/bugsnag.rb   |  8 +++++
 example/rails-42/config/routes.rb             |  4 +--
 example/rails-51/README.md                    |  4 +--
 .../app/controllers/application_controller.rb | 26 +++-----------
 .../app/controllers/que_controller.rb         |  5 ---
 .../app/controllers/resque_controller.rb      |  5 ---
 .../app/controllers/sidekiq_controller.rb     |  5 ---
 example/rails-51/app/jobs/que_callback.rb     | 14 --------
 example/rails-51/app/views/index.md           | 14 +++-----
 example/rails-51/app/views/que.md             |  4 ---
 .../rails-51/app/views/que/callbacks.html.erb |  1 -
 example/rails-51/app/views/resque.md          | 10 ++----
 .../app/views/resque/callbacks.html.erb       |  1 -
 example/rails-51/app/views/sidekiq.md         |  8 ++---
 .../app/views/sidekiq/callbacks.html.erb      |  1 -
 .../rails-51/app/workers/resque_workers.rb    | 18 +---------
 .../rails-51/app/workers/sidekiq_workers.rb   | 16 ---------
 .../rails-51/config/initializers/bugsnag.rb   |  8 +++++
 example/rails-51/config/routes.rb             |  4 ---
 example/rails-60/README.md                    |  2 +-
 .../app/controllers/application_controller.rb | 19 ----------
 .../app/controllers/que_controller.rb         |  5 ---
 .../app/controllers/resque_controller.rb      |  4 ---
 .../app/controllers/sidekiq_controller.rb     |  4 ---
 example/rails-60/app/jobs/que_callback.rb     | 14 --------
 .../app/views/application/data.html.erb       |  2 +-
 .../app/views/application/index.html.erb      | 15 ++------
 .../rails-60/app/views/que/callbacks.html.erb |  8 -----
 example/rails-60/app/views/que/index.html.erb | 14 --------
 .../app/views/resque/callbacks.html.erb       | 17 ---------
 .../rails-60/app/views/resque/index.html.erb  | 19 ++--------
 .../app/views/sidekiq/callbacks.html.erb      | 12 -------
 .../rails-60/app/views/sidekiq/index.html.erb | 13 +------
 .../rails-60/app/workers/resque_workers.rb    | 16 ---------
 .../rails-60/app/workers/sidekiq_workers.rb   | 17 ---------
 .../rails-60/config/initializers/bugsnag.rb   |  8 +++++
 example/rails-60/config/routes.rb             |  4 ---
 example/rails-60/tmp/.keep                    |  0
 example/rails-60/tmp/pids/.keep               |  0
 example/sinatra/config.ru                     | 36 +++++++------------
 example/sinatra/templates/index.md            | 14 +++-----
 48 files changed, 109 insertions(+), 429 deletions(-)
 delete mode 100644 example/rails-51/app/jobs/que_callback.rb
 delete mode 100644 example/rails-51/app/views/que/callbacks.html.erb
 delete mode 100644 example/rails-51/app/views/resque/callbacks.html.erb
 delete mode 100644 example/rails-51/app/views/sidekiq/callbacks.html.erb
 delete mode 100644 example/rails-60/app/jobs/que_callback.rb
 delete mode 100644 example/rails-60/app/views/que/callbacks.html.erb
 delete mode 100644 example/rails-60/app/views/resque/callbacks.html.erb
 delete mode 100644 example/rails-60/app/views/sidekiq/callbacks.html.erb
 delete mode 100644 example/rails-60/tmp/.keep
 delete mode 100644 example/rails-60/tmp/pids/.keep

diff --git a/example/padrino/app/app.rb b/example/padrino/app/app.rb
index 3630647ef..17ba9878b 100644
--- a/example/padrino/app/app.rb
+++ b/example/padrino/app/app.rb
@@ -75,21 +75,6 @@ class App < Padrino::Application
         'bugsnag.com for a new notification!')
     end
 
-    get '/crash_with_callback' do
-      Bugsnag.before_notify_callbacks << proc { |report|
-        new_tab = {
-          message: 'Padrino demo says: Everything is great',
-          code: 200
-        }
-        report.add_tab(:diagnostics, new_tab)
-      }
-
-      msg = 'Bugsnag Padrino demo says: It crashed! But, due to the attached callback' +
-        ' the exception has meta information. Go check' +
-        ' bugsnag.com for a new notification (see the Diagnostics tab)!'
-      raise RuntimeError.new(msg)
-    end
-
     get '/notify' do
       Bugsnag.notify(RuntimeError.new("Bugsnag Padrino demo says: False alarm, your application didn't crash"))
 
@@ -100,21 +85,16 @@ class App < Padrino::Application
 
     get '/notify_data' do
       error = RuntimeError.new("Bugsnag Padrino demo says: False alarm, your application didn't crash")
-      Bugsnag.notify error do |report|
-        report.add_tab(:user, {
-          :username => "bob-hoskins",
-          :email => 'bugsnag@bugsnag.com',
-          :registered_user => true
-        })
+      Bugsnag.notify(error) do |report|
         report.add_tab(:diagnostics, {
-          :message => 'Padrino demo says: Everything is great',
-          :code => 200
+          message: 'Padrino demo says: Everything is great',
+          code: 200
         })
       end
 
       "Bugsnag Padrino demo says: It didn't crash! " +
         'But still go check <a href="https://bugsnag.com">https://bugsnag.com</a>' +
-        ' for a new notification. Check out the User tab for the meta data'
+        ' for a new notification. Check out the Diagnostics tab for the meta data'
     end
 
     get '/notify_severity' do
diff --git a/example/padrino/app/templates/index.md b/example/padrino/app/templates/index.md
index 4f2d4be06..c2151a982 100644
--- a/example/padrino/app/templates/index.md
+++ b/example/padrino/app/templates/index.md
@@ -8,18 +8,14 @@ While testing the examples open [your dashboard](https://app.bugsnag.com) in ord
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
 
-2. [Crash and use callbacks](/crash_with_callback)
-<br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
-
-3. [Notify](/notify)
+2. [Notify](/notify)
 <br/>
     Sends Bugsnag a report on demand using `bugsnag.notify`.  Allows details of handled errors or information to be sent to the Bugsnag dashboard without crashing your code.
 
-4. [Notify with data](/notify_data)
+3. [Notify with data](/notify_data)
 <br/>
-    Same as `notify` but allows you to attach additional data within a `block`, similar to the `before_notify_callbacks` example above.  In this case we're adding information about the user to go into the `user` tab, and additional diagnostics as a `diagnostics` tab.
+    Same as `notify` but allows you to attach additional data within a `block`. In this case we're adding additional diagnostics as a `diagnostics` tab.
 
-5. [Set the severity](/notify_severity)
+4. [Set the severity](/notify_severity)
 <br/>
-    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
\ No newline at end of file
+    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
diff --git a/example/padrino/config/boot.rb b/example/padrino/config/boot.rb
index 54e54347c..1af233fa7 100644
--- a/example/padrino/config/boot.rb
+++ b/example/padrino/config/boot.rb
@@ -38,6 +38,14 @@
 Padrino.before_load do
   Bugsnag.configure do |config|
     config.api_key = 'YOUR_API_KEY'
+
+    config.add_on_error(proc do |report|
+      report.add_tab(:user, {
+        username: 'bob-hoskins',
+        email: 'bugsnag@bugsnag.com',
+        registered_user: true
+      })
+    end)
   end
 end
 
diff --git a/example/rack/server.rb b/example/rack/server.rb
index c02887894..19674730e 100644
--- a/example/rack/server.rb
+++ b/example/rack/server.rb
@@ -5,9 +5,16 @@
 require 'bugsnag'
 require 'redcarpet'
 
-
 Bugsnag.configure do |config|
   config.api_key = 'YOUR_API_KEY'
+
+  config.add_on_error(proc do |report|
+    report.add_tab(:user, {
+      username: 'bob-hoskins',
+      email: 'bugsnag@bugsnag.com',
+      registered_user: true
+    })
+  end)
 end
 
 class BugsnagDemo
@@ -18,19 +25,6 @@ def call(env)
     when '/crash'
       raise RuntimeError.new('Bugsnag Rack demo says: It crashed! Go check ' +
         'bugsnag.com for a new notification!')
-    when '/crash_with_callback'
-      Bugsnag.before_notify_callbacks << proc { |report|
-        new_tab = {
-          message: 'Rack demo says: Everything is great',
-          code: 200
-        }
-        report.add_tab(:diagnostics, new_tab)
-      }
-
-      msg = 'Bugsnag Rack demo says: It crashed! But, due to the attached callback' +
-        ' the exception has meta information. Go check' +
-        ' bugsnag.com for a new notification (see the Diagnostics tab)!'
-      raise RuntimeError.new(msg)
     when '/notify'
       Bugsnag.notify(RuntimeError.new("Bugsnag Rack demo says: False alarm, your application didn't crash"))
 
@@ -39,21 +33,16 @@ def call(env)
         ' for a new notification.'
     when '/notify_data'
       error = RuntimeError.new("Bugsnag Rack demo says: False alarm, your application didn't crash")
-      Bugsnag.notify error do |report|
-        report.add_tab(:user, {
-          :username => "bob-hoskins",
-          :email => 'bugsnag@bugsnag.com',
-          :registered_user => true
-        })
+      Bugsnag.notify(error) do |report|
         report.add_tab(:diagnostics, {
-          :message => 'Rack demo says: Everything is great',
-          :code => 200
+          message: 'Padrino demo says: Everything is great',
+          code: 200
         })
       end
 
       text = "Bugsnag Rack demo says: It didn't crash! " +
         'But still go check <a href="https://bugsnag.com">https://bugsnag.com</a>' +
-        ' for a new notification. Check out the User tab for the meta data'
+        ' for a new notification. Check out the Diagnostics tab for the meta data'
     when '/notify_severity'
       msg = "Bugsnag Rack demo says: Look at the circle on the right side. It's different"
       error = RuntimeError.new(msg)
diff --git a/example/rack/templates/index.md b/example/rack/templates/index.md
index 52695efe2..6e111829b 100644
--- a/example/rack/templates/index.md
+++ b/example/rack/templates/index.md
@@ -8,18 +8,14 @@ While testing the examples open [your dashboard](https://app.bugsnag.com) in ord
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
 
-2. [Crash and use callbacks](/crash_with_callback)
-<br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
-
-3. [Notify](/notify)
+2. [Notify](/notify)
 <br/>
     Sends Bugsnag a report on demand using `bugsnag.notify`.  Allows details of handled errors or information to be sent to the Bugsnag dashboard without crashing your code.
 
-4. [Notify with data](/notify_data)
+3. [Notify with data](/notify_data)
 <br/>
-    Same as `notify` but allows you to attach additional data within a `block`, similar to the `before_notify_callbacks` example above.  In this case we're adding information about the user to go into the `user` tab, and additional diagnostics as a `diagnostics` tab.
+    Same as `notify` but allows you to attach additional data within a `block`. In this case we're adding additional diagnostics as a `diagnostics` tab.
 
-5. [Set the severity](/notify_severity)
+4. [Set the severity](/notify_severity)
 <br/>
-    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
\ No newline at end of file
+    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
diff --git a/example/rails-42/app/controllers/application_controller.rb b/example/rails-42/app/controllers/application_controller.rb
index 33f2701ff..6d100c60f 100644
--- a/example/rails-42/app/controllers/application_controller.rb
+++ b/example/rails-42/app/controllers/application_controller.rb
@@ -10,19 +10,6 @@ def crash
       'bugsnag.com for a new notification')
   end
 
-  def callback
-    Bugsnag.before_notify_callbacks << proc { |report|
-      new_tab = {
-        message: 'Rails v4.2 demo says: Everything is great',
-        code: 200
-      }
-      report.add_tab(:diagnostics, new_tab)
-    }
-    raise RuntimeError.new('Bugsnag Rails demo says: It crashed! But, due to the attached callback' +
-      ' the exception has meta information. Go check' +
-      ' bugsnag.com for a new notification (see the Diagnostics tab)!')
-  end
-
   def notify
     Bugsnag.notify(RuntimeError.new("Bugsnag Rails demo says: False alarm, your application didn't crash"))
     @text = "Bugsnag Rack demo says: It didn't crash! " +
@@ -32,20 +19,15 @@ def notify
 
   def data
     error = RuntimeError.new("Bugsnag Rails demo says: False alarm, your application didn't crash")
-    Bugsnag.notify error do |report|
-      report.add_tab(:user, {
-        :username => "bob-hoskins",
-        :email => 'bugsnag@bugsnag.com',
-        :registered_user => true
-      })
+    Bugsnag.notify(error) do |report|
       report.add_tab(:diagnostics, {
-        :message => 'Rails demo says: Everything is great',
-        :code => 200
+        message: 'Rails demo says: Everything is great',
+        code: 200
       })
     end
     @text = "Bugsnag Rails demo says: It didn't crash! " +
       'But still go check <a href="https://bugsnag.com">https://bugsnag.com</a>' +
-      ' for a new notification. Check out the User tab for the meta data'
+      ' for a new notification. Check out the Diagnostics tab for the meta data'
   end
 
   def severity
diff --git a/example/rails-42/app/views/index.md b/example/rails-42/app/views/index.md
index 4b37c029c..5595a3980 100644
--- a/example/rails-42/app/views/index.md
+++ b/example/rails-42/app/views/index.md
@@ -8,18 +8,14 @@ While testing the examples open [your dashboard](https://app.bugsnag.com) in ord
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
 
-2. [Crash and use callbacks](/crash_with_callback)
-<br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
-
-3. [Notify](/notify)
+2. [Notify](/notify)
 <br/>
     Sends Bugsnag a report on demand using `bugsnag.notify`.  Allows details of handled errors or information to be sent to the Bugsnag dashboard without crashing your code.
 
-4. [Notify with data](/notify_data)
+3. [Notify with data](/notify_data)
 <br/>
-    Same as `notify` but allows you to attach additional data within a `block`, similar to the `before_notify_callbacks` example above.  In this case we're adding information about the user to go into the `user` tab, and additional diagnostics as a `diagnostics` tab.
+    Same as `notify` but allows you to attach additional data within a `block`. In this case we're adding additional diagnostics as a `diagnostics` tab.
 
-5. [Set the severity](/notify_severity)
+4. [Set the severity](/notify_severity)
 <br/>
-    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
\ No newline at end of file
+    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
diff --git a/example/rails-42/config/initializers/bugsnag.rb b/example/rails-42/config/initializers/bugsnag.rb
index 58c57fbf6..2b02bc66a 100644
--- a/example/rails-42/config/initializers/bugsnag.rb
+++ b/example/rails-42/config/initializers/bugsnag.rb
@@ -1,3 +1,11 @@
 Bugsnag.configure do |config|
   config.api_key = "YOUR_API_KEY_HERE"
+
+  config.add_on_error(proc do |report|
+    report.add_tab(:user, {
+      username: 'bob-hoskins',
+      email: 'bugsnag@bugsnag.com',
+      registered_user: true
+    })
+  end)
 end
diff --git a/example/rails-42/config/routes.rb b/example/rails-42/config/routes.rb
index 9a3a1b9f0..70fe670cc 100644
--- a/example/rails-42/config/routes.rb
+++ b/example/rails-42/config/routes.rb
@@ -1,10 +1,8 @@
 Rails.application.routes.draw do
   root :to => 'application#index'
-  
+
   get 'crash' => 'application#crash'
-  get 'crash_with_callback' => 'application#callback'
   get 'notify' => 'application#notify'
   get 'notify_data' => 'application#data'
   get 'notify_severity' => 'application#severity'
-
 end
diff --git a/example/rails-51/README.md b/example/rails-51/README.md
index f4bd52f94..f3bf4fbca 100644
--- a/example/rails-51/README.md
+++ b/example/rails-51/README.md
@@ -132,5 +132,5 @@ Navigate to the `/resque` page and queue any of the examples using links provide
 To process the queues, run the `resque:work` task as stated in the example webpage. In order to process any of the queues on a single thread start the resque worker using the command:
 
 ```shell
-QUEUE=crash,callback,metadata bundle exec rake resque:work
-```
\ No newline at end of file
+QUEUE=crash,metadata bundle exec rake resque:work
+```
diff --git a/example/rails-51/app/controllers/application_controller.rb b/example/rails-51/app/controllers/application_controller.rb
index 33f2701ff..6d100c60f 100644
--- a/example/rails-51/app/controllers/application_controller.rb
+++ b/example/rails-51/app/controllers/application_controller.rb
@@ -10,19 +10,6 @@ def crash
       'bugsnag.com for a new notification')
   end
 
-  def callback
-    Bugsnag.before_notify_callbacks << proc { |report|
-      new_tab = {
-        message: 'Rails v4.2 demo says: Everything is great',
-        code: 200
-      }
-      report.add_tab(:diagnostics, new_tab)
-    }
-    raise RuntimeError.new('Bugsnag Rails demo says: It crashed! But, due to the attached callback' +
-      ' the exception has meta information. Go check' +
-      ' bugsnag.com for a new notification (see the Diagnostics tab)!')
-  end
-
   def notify
     Bugsnag.notify(RuntimeError.new("Bugsnag Rails demo says: False alarm, your application didn't crash"))
     @text = "Bugsnag Rack demo says: It didn't crash! " +
@@ -32,20 +19,15 @@ def notify
 
   def data
     error = RuntimeError.new("Bugsnag Rails demo says: False alarm, your application didn't crash")
-    Bugsnag.notify error do |report|
-      report.add_tab(:user, {
-        :username => "bob-hoskins",
-        :email => 'bugsnag@bugsnag.com',
-        :registered_user => true
-      })
+    Bugsnag.notify(error) do |report|
       report.add_tab(:diagnostics, {
-        :message => 'Rails demo says: Everything is great',
-        :code => 200
+        message: 'Rails demo says: Everything is great',
+        code: 200
       })
     end
     @text = "Bugsnag Rails demo says: It didn't crash! " +
       'But still go check <a href="https://bugsnag.com">https://bugsnag.com</a>' +
-      ' for a new notification. Check out the User tab for the meta data'
+      ' for a new notification. Check out the Diagnostics tab for the meta data'
   end
 
   def severity
diff --git a/example/rails-51/app/controllers/que_controller.rb b/example/rails-51/app/controllers/que_controller.rb
index a8de7ceb2..5b3db72b1 100644
--- a/example/rails-51/app/controllers/que_controller.rb
+++ b/example/rails-51/app/controllers/que_controller.rb
@@ -11,9 +11,4 @@ def crash
     QueCrash.enqueue
     @text = "Que has queued the crash task"
   end
-
-  def callbacks
-    QueCallback.enqueue
-    @text = "Que has queued the callbacks task"
-  end
 end
diff --git a/example/rails-51/app/controllers/resque_controller.rb b/example/rails-51/app/controllers/resque_controller.rb
index c0eb75595..420a13fb1 100644
--- a/example/rails-51/app/controllers/resque_controller.rb
+++ b/example/rails-51/app/controllers/resque_controller.rb
@@ -16,9 +16,4 @@ def metadata
     Resque.enqueue(ResqueWorkers::Metadata)
     @text = "The metadata task has been queued.  This can be run using the `QUEUE=metadata bundle exec rake resque:work` command"
   end
-
-  def callbacks
-    Resque.enqueue(ResqueWorkers::Callback)
-    @text = "The callback task has been queued.  This can be run using the `QUEUE=callback bundle exec rake resque:work` command"
-  end
 end
diff --git a/example/rails-51/app/controllers/sidekiq_controller.rb b/example/rails-51/app/controllers/sidekiq_controller.rb
index 2b814ab82..2e1ee5fa7 100644
--- a/example/rails-51/app/controllers/sidekiq_controller.rb
+++ b/example/rails-51/app/controllers/sidekiq_controller.rb
@@ -17,9 +17,4 @@ def metadata
     @text = "Sidekiq is performing a task that will notify an error with some metadata without crashing.
     Check out your dashboard!"
   end
-
-  def callbacks
-    SidekiqWorkers::CallbackWorker.perform_async
-    @text = "Sidekiq is performing a task that will crash, but registers a callback before this to attach additional metadata."
-  end
 end
diff --git a/example/rails-51/app/jobs/que_callback.rb b/example/rails-51/app/jobs/que_callback.rb
deleted file mode 100644
index 55032c9d0..000000000
--- a/example/rails-51/app/jobs/que_callback.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-class QueCallback < Que::Job
-  @run_at = proc { 5.seconds.from_now }
-
-  def run(options={})
-    Bugsnag.before_notify_callbacks << proc { |report|
-      new_tab = {
-        message: 'Que demo says: Everything is great',
-        code: 200
-      }
-      report.add_tab(:diagnostics, new_tab)
-    }
-    raise "Oh no"
-  end
-end
diff --git a/example/rails-51/app/views/index.md b/example/rails-51/app/views/index.md
index 5edbc9eb5..0f2db4363 100644
--- a/example/rails-51/app/views/index.md
+++ b/example/rails-51/app/views/index.md
@@ -14,18 +14,14 @@ While testing the examples open [your dashboard](https://app.bugsnag.com) in ord
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
 
-2. [Crash and use callbacks](/crash_with_callback)
-<br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
-
-3. [Notify](/notify)
+2. [Notify](/notify)
 <br/>
     Sends Bugsnag a report on demand using `bugsnag.notify`.  Allows details of handled errors or information to be sent to the Bugsnag dashboard without crashing your code.
 
-4. [Notify with data](/notify_data)
+3. [Notify with data](/notify_data)
 <br/>
-    Same as `notify` but allows you to attach additional data within a `block`, similar to the `before_notify_callbacks` example above.  In this case we're adding information about the user to go into the `user` tab, and additional diagnostics as a `diagnostics` tab.
+    Same as `notify` but allows you to attach additional data within a `block`. In this case we're adding additional diagnostics as a `diagnostics` tab.
 
-5. [Set the severity](/notify_severity)
+4. [Set the severity](/notify_severity)
 <br/>
-    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
\ No newline at end of file
+    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
diff --git a/example/rails-51/app/views/que.md b/example/rails-51/app/views/que.md
index 6272fd7b8..805bcb185 100644
--- a/example/rails-51/app/views/que.md
+++ b/example/rails-51/app/views/que.md
@@ -9,7 +9,3 @@ Make sure you have a PostgreSQL instance running that your test application can
 1. [Crash](/que/crash)
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
-
-2. [Crash and use callbacks](/que/crash_with_callback)
-<br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
\ No newline at end of file
diff --git a/example/rails-51/app/views/que/callbacks.html.erb b/example/rails-51/app/views/que/callbacks.html.erb
deleted file mode 100644
index aa2786a7d..000000000
--- a/example/rails-51/app/views/que/callbacks.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-<%= markdown(@text)%>
\ No newline at end of file
diff --git a/example/rails-51/app/views/resque.md b/example/rails-51/app/views/resque.md
index b4ed7b0a3..221a821a3 100644
--- a/example/rails-51/app/views/resque.md
+++ b/example/rails-51/app/views/resque.md
@@ -9,17 +9,13 @@ Make sure you have a Redis instance running that your test application can conne
 While each queue can be run individually, to run Resque so that it automatically executes each task, use:
 
 ```shell
-QUEUE=crash,callback,metadata bundle exec rake resque:work
+QUEUE=crash,metadata bundle exec rake resque:work
 ```
 
 1. [Crash](/resque/crash)
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
 
-2. [Crash and use callbacks](/resque/crash_with_callback)
+2. [Notify with data](/resque/notify_data)
 <br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
-
-3. [Notify with data](/resque/notify_data)
-<br/>
-    Same as `notify` but allows you to attach additional data within a `block`, similar to the `before_notify_callbacks` example above.  In this case we're adding information about the queue to go into the `queue` tab, and additional diagnostics as a `diagnostics` tab.
\ No newline at end of file
+    Same as `notify` but allows you to attach additional data within a `block`.  In this case we're adding information about the queue to go into the `queue` tab, and additional diagnostics as a `diagnostics` tab.
diff --git a/example/rails-51/app/views/resque/callbacks.html.erb b/example/rails-51/app/views/resque/callbacks.html.erb
deleted file mode 100644
index aa2786a7d..000000000
--- a/example/rails-51/app/views/resque/callbacks.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-<%= markdown(@text)%>
\ No newline at end of file
diff --git a/example/rails-51/app/views/sidekiq.md b/example/rails-51/app/views/sidekiq.md
index 65edd291b..ace8f64ac 100644
--- a/example/rails-51/app/views/sidekiq.md
+++ b/example/rails-51/app/views/sidekiq.md
@@ -10,10 +10,6 @@ Make sure you have a Redis instance running that your test application can conne
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
 
-2. [Crash and use callbacks](/sidekiq/crash_with_callback)
+2. [Notify with data](/sidekiq/notify_data)
 <br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
-
-3. [Notify with data](/sidekiq/notify_data)
-<br/>
-    Same as `notify` but allows you to attach additional data within a `block`, similar to the `before_notify_callbacks` example above.  In this case we're adding information about the function being called to go into the `function` tab, and additional diagnostics as a `diagnostics` tab.
\ No newline at end of file
+    Same as `notify` but allows you to attach additional data within a `block`.  In this case we're adding information about the function being called to go into the `function` tab, and additional diagnostics as a `diagnostics` tab.
diff --git a/example/rails-51/app/views/sidekiq/callbacks.html.erb b/example/rails-51/app/views/sidekiq/callbacks.html.erb
deleted file mode 100644
index aa2786a7d..000000000
--- a/example/rails-51/app/views/sidekiq/callbacks.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-<%= markdown(@text)%>
\ No newline at end of file
diff --git a/example/rails-51/app/workers/resque_workers.rb b/example/rails-51/app/workers/resque_workers.rb
index 327a5984e..9891db8bc 100644
--- a/example/rails-51/app/workers/resque_workers.rb
+++ b/example/rails-51/app/workers/resque_workers.rb
@@ -7,22 +7,6 @@ def self.perform
     end
   end
 
-  # Unhandled with callback Exception example
-  class Callback
-    @queue = :callback
-
-    def self.perform
-      Bugsnag.before_notify_callbacks << proc { |report|
-        new_tab = {
-          message: 'Resque demo says: Everything is great',
-          code: 200
-        }
-        report.add_tab(:diagnostics, new_tab)
-      }
-      raise Exception.new "Crashed - Check the Bugsnag dashboard for diagnostic data"
-    end
-  end
-
   # Handled example with additional data
   class Metadata
     @queue = :metadata
@@ -41,4 +25,4 @@ def self.perform
       puts "The Resque worker hasn't crashed, but it has sent a notification, with additional data to the dashboard"
     end
   end
-end
\ No newline at end of file
+end
diff --git a/example/rails-51/app/workers/sidekiq_workers.rb b/example/rails-51/app/workers/sidekiq_workers.rb
index 3aa06d2ce..8ced0c6a8 100644
--- a/example/rails-51/app/workers/sidekiq_workers.rb
+++ b/example/rails-51/app/workers/sidekiq_workers.rb
@@ -1,20 +1,4 @@
 module SidekiqWorkers
-  class CallbackWorker
-    include Sidekiq::Worker
-    sidekiq_options :retry => false
-
-    def perform
-      Bugsnag.before_notify_callbacks << proc { |report|
-        new_tab = {
-          message: 'Sidekiq demo says: Everything is great',
-          code: 200
-        }
-        report.add_tab(:diagnostics, new_tab)
-      }
-      raise Exception.new "Sidekiq crashed, but the callback added metadata - Check your Bugsnag dashboard"
-    end
-  end
-
   class CrashWorker
     include Sidekiq::Worker
     sidekiq_options :retry => false
diff --git a/example/rails-51/config/initializers/bugsnag.rb b/example/rails-51/config/initializers/bugsnag.rb
index 58c57fbf6..2b02bc66a 100644
--- a/example/rails-51/config/initializers/bugsnag.rb
+++ b/example/rails-51/config/initializers/bugsnag.rb
@@ -1,3 +1,11 @@
 Bugsnag.configure do |config|
   config.api_key = "YOUR_API_KEY_HERE"
+
+  config.add_on_error(proc do |report|
+    report.add_tab(:user, {
+      username: 'bob-hoskins',
+      email: 'bugsnag@bugsnag.com',
+      registered_user: true
+    })
+  end)
 end
diff --git a/example/rails-51/config/routes.rb b/example/rails-51/config/routes.rb
index 1bb035f6a..bdee80b99 100644
--- a/example/rails-51/config/routes.rb
+++ b/example/rails-51/config/routes.rb
@@ -2,7 +2,6 @@
   # Vanilla rails routing
   get '/', to: 'application#index'
   get '/crash', to: 'application#crash'
-  get '/crash_with_callback', to: 'application#callback'
   get '/notify', to: 'application#notify'
   get '/notify_data', to: 'application#data'
   get '/notify_severity', to: 'application#severity'
@@ -11,16 +10,13 @@
   get '/sidekiq', to: 'sidekiq#index'
   get '/sidekiq/crash', to: 'sidekiq#crash'
   get '/sidekiq/notify_data', to: 'sidekiq#metadata'
-  get '/sidekiq/crash_with_callback', to: 'sidekiq#callbacks'
 
   # Que routing
   get '/que', to: 'que#index'
   get '/que/crash', to: 'que#crash'
   get '/que/notify_data', to: 'que#metadata'
-  get '/que/crash_with_callback', to: 'que#callbacks'
 
   # Resque routing
   get '/resque', to: 'resque#index'
   get '/resque/crash', to: 'resque#crash'
-  get '/resque/crash_with_callback', to: 'resque#callbacks'
 end
diff --git a/example/rails-60/README.md b/example/rails-60/README.md
index 2eaa10f01..86b66e508 100644
--- a/example/rails-60/README.md
+++ b/example/rails-60/README.md
@@ -133,5 +133,5 @@ Navigate to the `/resque` page and queue any of the examples using links provide
 To process the queues, run the `resque:work` task as stated in the example webpage. In order to process any of the queues on a single thread start the resque worker using the command:
 
 ```shell
-QUEUE=crash,callback,metadata bundle exec rake resque:work
+QUEUE=crash,metadata bundle exec rake resque:work
 ```
diff --git a/example/rails-60/app/controllers/application_controller.rb b/example/rails-60/app/controllers/application_controller.rb
index 4be69567f..1f7a9f073 100644
--- a/example/rails-60/app/controllers/application_controller.rb
+++ b/example/rails-60/app/controllers/application_controller.rb
@@ -4,19 +4,6 @@ def crash
       'Go check bugsnag.com for a new notification'
   end
 
-  def callback
-    Bugsnag.before_notify_callbacks << proc { |report|
-      report.add_tab(:diagnostics, {
-        message: 'Rails v6.0 demo says: Everything is great',
-        code: 200
-      })
-    }
-
-    raise 'Bugsnag Rails demo says: It crashed! But, due to the attached callback' \
-      'the exception has meta information. Go check ' \
-      'bugsnag.com for a new notification (see the Diagnostics tab)!'
-  end
-
   def notify
     Bugsnag.notify(RuntimeError.new("Bugsnag Rails demo says: False alarm, your application didn't crash"))
   end
@@ -25,12 +12,6 @@ def data
     error = RuntimeError.new("Bugsnag Rails demo says: False alarm, your application didn't crash")
 
     Bugsnag.notify(error) do |report|
-      report.add_tab(:user, {
-        username: 'bob-hoskins',
-        email: 'bugsnag@bugsnag.com',
-        registered_user: true
-      })
-
       report.add_tab(:diagnostics, {
         message: 'Rails demo says: Everything is great',
         code: 200
diff --git a/example/rails-60/app/controllers/que_controller.rb b/example/rails-60/app/controllers/que_controller.rb
index eb7529196..00400cce5 100644
--- a/example/rails-60/app/controllers/que_controller.rb
+++ b/example/rails-60/app/controllers/que_controller.rb
@@ -1,5 +1,4 @@
 require './app/jobs/que_crash'
-require './app/jobs/que_callback'
 
 class QueController < ActionController::Base
   layout "application"
@@ -7,8 +6,4 @@ class QueController < ActionController::Base
   def crash
     QueCrash.enqueue
   end
-
-  def callbacks
-    QueCallback.enqueue
-  end
 end
diff --git a/example/rails-60/app/controllers/resque_controller.rb b/example/rails-60/app/controllers/resque_controller.rb
index 2bdea9196..8dc1d2534 100644
--- a/example/rails-60/app/controllers/resque_controller.rb
+++ b/example/rails-60/app/controllers/resque_controller.rb
@@ -8,8 +8,4 @@ def crash
   def metadata
     Resque.enqueue(ResqueWorkers::Metadata)
   end
-
-  def callbacks
-    Resque.enqueue(ResqueWorkers::Callback)
-  end
 end
diff --git a/example/rails-60/app/controllers/sidekiq_controller.rb b/example/rails-60/app/controllers/sidekiq_controller.rb
index b925be414..80135f81c 100644
--- a/example/rails-60/app/controllers/sidekiq_controller.rb
+++ b/example/rails-60/app/controllers/sidekiq_controller.rb
@@ -10,8 +10,4 @@ def crash
   def metadata
     SidekiqWorkers::MetadataWorker.perform_async
   end
-
-  def callbacks
-    SidekiqWorkers::CallbackWorker.perform_async
-  end
 end
diff --git a/example/rails-60/app/jobs/que_callback.rb b/example/rails-60/app/jobs/que_callback.rb
deleted file mode 100644
index 34efe9a48..000000000
--- a/example/rails-60/app/jobs/que_callback.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-class QueCallback < Que::Job
-  @run_at = proc { 5.seconds.from_now }
-
-  def run(_options = {})
-    Bugsnag.before_notify_callbacks << proc { |report|
-      report.add_tab(:diagnostics, {
-        message: 'Que demo says: Everything is great',
-        code: 200
-      })
-    }
-
-    raise 'Oh no!'
-  end
-end
diff --git a/example/rails-60/app/views/application/data.html.erb b/example/rails-60/app/views/application/data.html.erb
index 3c050280e..e79a6e076 100644
--- a/example/rails-60/app/views/application/data.html.erb
+++ b/example/rails-60/app/views/application/data.html.erb
@@ -5,6 +5,6 @@
     for a new notification.
 </p>
 
-<p>Check out the <code class="upper">user</code> tab to see the metadata.</p>
+<p>Check out the <code class="upper">diagnostics</code> tab to see the metadata.</p>
 
 <p><a href="/">&larr; return to examples</a></p>
diff --git a/example/rails-60/app/views/application/index.html.erb b/example/rails-60/app/views/application/index.html.erb
index 098615448..ce506e105 100644
--- a/example/rails-60/app/views/application/index.html.erb
+++ b/example/rails-60/app/views/application/index.html.erb
@@ -23,15 +23,6 @@
         </p>
     </li>
 
-    <li>
-        <p><a href="/crash_with_callback">Crash and use callbacks</a></p>
-        <p>
-            Raises an exception within the framework, but with additional data
-            attached to the report.  By registering a callback before the error
-            occurs useful data can be attached as a tab in the Bugsnag dashboard.
-        </p>
-    </li>
-
     <li>
         <p><a href="/notify">Notify</a></p>
         <p>
@@ -45,12 +36,10 @@
         <p><a href="/notify_data">Notify with data</a></p>
         <p>
             Same as <code>notify</code> but allows you to attach additional data
-            within a <code>block</code>, similar to the
-            <code>before_notify_callbacks</code> example above.
+            within a <code>block</code>.
         </p>
         <p>
-            In this case we're adding information about the user to go into the
-            <code>user</code> tab, and additional diagnostics as a
+            In this case we're adding additional diagnostics as a
             <code>diagnostics</code> tab.
         </p>
     </li>
diff --git a/example/rails-60/app/views/que/callbacks.html.erb b/example/rails-60/app/views/que/callbacks.html.erb
deleted file mode 100644
index d7e5095f1..000000000
--- a/example/rails-60/app/views/que/callbacks.html.erb
+++ /dev/null
@@ -1,8 +0,0 @@
-<h2>Que</h2>
-
-<p>
-    Que has queued the callbacks task, check
-    <a href="https://app.bugsnag.com">your Bugsnag dashboard</a> for the result!
-</p>
-
-<p><a href="/que">&larr; return to Que examples</a></p>
diff --git a/example/rails-60/app/views/que/index.html.erb b/example/rails-60/app/views/que/index.html.erb
index 2568880cc..f22da5a78 100644
--- a/example/rails-60/app/views/que/index.html.erb
+++ b/example/rails-60/app/views/que/index.html.erb
@@ -22,20 +22,6 @@
             Bugsnag dashboard.
         </p>
     </li>
-
-    <li>
-        <p><a href="/que/crash_with_callback">Crash and use callbacks</a></p>
-
-        <p>
-            Raises an exception within the framework, but with additional data
-            attached to the report.
-        </p>
-
-        <p>
-            By registering a callback before the error occurs useful data can
-            be attached as a tab in the Bugsnag dashboard.
-        </p>
-    </li>
 </ol>
 
 <p><a href="/">&larr; return home</a></p>
diff --git a/example/rails-60/app/views/resque/callbacks.html.erb b/example/rails-60/app/views/resque/callbacks.html.erb
deleted file mode 100644
index d730810d8..000000000
--- a/example/rails-60/app/views/resque/callbacks.html.erb
+++ /dev/null
@@ -1,17 +0,0 @@
-<h2>Resque</h2>
-
-<p>
-    The callback task has been queued.
-</p>
-
-<p>
-    This can be executed by running:
-</p>
-
-<pre class="shell">QUEUE=callback bundle exec rake resque:work</pre>
-
-<p>
-    Check <a href="https://app.bugsnag.com">your Bugsnag dashboard</a> for the result!
-</p>
-
-<p><a href="/resque">&larr; return to Resque examples</a></p>
diff --git a/example/rails-60/app/views/resque/index.html.erb b/example/rails-60/app/views/resque/index.html.erb
index 7f5e4d763..f43d18487 100644
--- a/example/rails-60/app/views/resque/index.html.erb
+++ b/example/rails-60/app/views/resque/index.html.erb
@@ -18,7 +18,7 @@
     automatically executes each task, use:
 </p>
 
-<pre class="shell">QUEUE=crash,callback,metadata bundle exec rake resque:work</pre>
+<pre class="shell">QUEUE=crash,metadata bundle exec rake resque:work</pre>
 
 <ol>
     <li>
@@ -30,27 +30,12 @@
         </p>
     </li>
 
-    <li>
-        <p><a href="/resque/crash_with_callback">Crash and use callbacks</a></p>
-
-        <p>
-            Raises an exception within the framework, but with additional data
-            attached to the report.
-        </p>
-
-        <p>
-            By registering a callback before the error occurs useful data can
-            be attached as a tab in the Bugsnag dashboard.
-        </p>
-    </li>
-
     <li>
         <p><a href="/resque/notify_data">Notify with data</a></p>
 
         <p>
             Same as <code>notify</code> but allows you to attach additional data
-            within a <code>block</code>, similar to the
-            "crash and use callbacks" example above.
+            within a <code>block</code>.
         </p>
 
         <p>
diff --git a/example/rails-60/app/views/sidekiq/callbacks.html.erb b/example/rails-60/app/views/sidekiq/callbacks.html.erb
deleted file mode 100644
index 14bb48406..000000000
--- a/example/rails-60/app/views/sidekiq/callbacks.html.erb
+++ /dev/null
@@ -1,12 +0,0 @@
-<h2>Sidekiq</h2>
-
-<p>
-    Sidekiq is performing a task that will crash, but registers a callback
-    before this to attach additional metadata.
-</p>
-
-<p>
-    Check <a href="https://app.bugsnag.com">your Bugsnag dashboard</a> for the result!
-</p>
-
-<p><a href="/sidekiq">&larr; return to Sidekiq examples</a></p>
diff --git a/example/rails-60/app/views/sidekiq/index.html.erb b/example/rails-60/app/views/sidekiq/index.html.erb
index 110b6746c..79313d0f3 100644
--- a/example/rails-60/app/views/sidekiq/index.html.erb
+++ b/example/rails-60/app/views/sidekiq/index.html.erb
@@ -20,23 +20,12 @@
         <p>Raises an error within the framework, generating a report in the Bugsnag dashboard.</p>
     </li>
 
-    <li>
-        <p><a href="/sidekiq/crash_with_callback">Crash and use callbacks</a></p>
-
-        <p>
-            Raises an exception within the framework, but with additional data
-            attached to the report. By registering a callback before the error
-            occurs useful data can be attached as a tab in the Bugsnag dashboard.
-        </p>
-    </li>
-
     <li>
         <p><a href="/sidekiq/notify_data">Notify with data</a></p>
 
         <p>
             Sends Bugsnag a report on demand using <code>bugsnag.notify</code>
-            and attaches additional data within a <code>block</code>, similar to
-            the "Crash and use callbacks" example above. In this case we're
+            and attaches additional data within a <code>block</code>. In this case we're
             adding information about the function being called to go into the
             <code class="upper">function</code> tab, and additional diagnostics
             as a <code class="upper">diagnostics</code> tab.
diff --git a/example/rails-60/app/workers/resque_workers.rb b/example/rails-60/app/workers/resque_workers.rb
index 1b0feee35..055cc1ad0 100644
--- a/example/rails-60/app/workers/resque_workers.rb
+++ b/example/rails-60/app/workers/resque_workers.rb
@@ -7,22 +7,6 @@ def self.perform
     end
   end
 
-  # Unhandled with callback Exception example
-  class Callback
-    @queue = :callback
-
-    def self.perform
-      Bugsnag.before_notify_callbacks << proc { |report|
-        report.add_tab(:diagnostics, {
-          message: 'Resque demo says: Everything is great',
-          code: 200
-        })
-      }
-
-      raise 'Crashed - Check the Bugsnag dashboard for diagnostic data'
-    end
-  end
-
   # Handled example with additional data
   class Metadata
     @queue = :metadata
diff --git a/example/rails-60/app/workers/sidekiq_workers.rb b/example/rails-60/app/workers/sidekiq_workers.rb
index 59bfddef1..7e78ed17b 100644
--- a/example/rails-60/app/workers/sidekiq_workers.rb
+++ b/example/rails-60/app/workers/sidekiq_workers.rb
@@ -1,21 +1,4 @@
 module SidekiqWorkers
-  class CallbackWorker
-    include Sidekiq::Worker
-
-    sidekiq_options :retry => false
-
-    def perform
-      Bugsnag.before_notify_callbacks << proc { |report|
-        report.add_tab(:diagnostics, {
-          message: 'Sidekiq demo says: Everything is great',
-          code: 200
-        })
-      }
-
-      raise 'Sidekiq crashed, but the callback added metadata - Check your Bugsnag dashboard'
-    end
-  end
-
   class CrashWorker
     include Sidekiq::Worker
     sidekiq_options :retry => false
diff --git a/example/rails-60/config/initializers/bugsnag.rb b/example/rails-60/config/initializers/bugsnag.rb
index 58c57fbf6..2b02bc66a 100644
--- a/example/rails-60/config/initializers/bugsnag.rb
+++ b/example/rails-60/config/initializers/bugsnag.rb
@@ -1,3 +1,11 @@
 Bugsnag.configure do |config|
   config.api_key = "YOUR_API_KEY_HERE"
+
+  config.add_on_error(proc do |report|
+    report.add_tab(:user, {
+      username: 'bob-hoskins',
+      email: 'bugsnag@bugsnag.com',
+      registered_user: true
+    })
+  end)
 end
diff --git a/example/rails-60/config/routes.rb b/example/rails-60/config/routes.rb
index b5f1747c3..8e1e16d07 100644
--- a/example/rails-60/config/routes.rb
+++ b/example/rails-60/config/routes.rb
@@ -2,7 +2,6 @@
   # Vanilla rails routing
   get '/', to: 'application#index'
   get '/crash', to: 'application#crash'
-  get '/crash_with_callback', to: 'application#callback'
   get '/notify', to: 'application#notify'
   get '/notify_data', to: 'application#data'
   get '/notify_severity', to: 'application#severity'
@@ -11,17 +10,14 @@
   get '/sidekiq', to: 'sidekiq#index'
   get '/sidekiq/crash', to: 'sidekiq#crash'
   get '/sidekiq/notify_data', to: 'sidekiq#metadata'
-  get '/sidekiq/crash_with_callback', to: 'sidekiq#callbacks'
 
   # Que routing
   get '/que', to: 'que#index'
   get '/que/crash', to: 'que#crash'
   get '/que/notify_data', to: 'que#metadata'
-  get '/que/crash_with_callback', to: 'que#callbacks'
 
   # Resque routing
   get '/resque', to: 'resque#index'
   get '/resque/crash', to: 'resque#crash'
-  get '/resque/crash_with_callback', to: 'resque#callbacks'
   get '/resque/notify_data', to: 'resque#metadata'
 end
diff --git a/example/rails-60/tmp/.keep b/example/rails-60/tmp/.keep
deleted file mode 100644
index e69de29bb..000000000
diff --git a/example/rails-60/tmp/pids/.keep b/example/rails-60/tmp/pids/.keep
deleted file mode 100644
index e69de29bb..000000000
diff --git a/example/sinatra/config.ru b/example/sinatra/config.ru
index 68efa300b..da677dabb 100644
--- a/example/sinatra/config.ru
+++ b/example/sinatra/config.ru
@@ -8,6 +8,14 @@ set :show_exceptions, false
 
 Bugsnag.configure do |config|
   config.api_key = 'YOUR_API_KEY'
+
+  config.add_on_error(proc do |report|
+    report.add_tab(:user, {
+      username: 'bob-hoskins',
+      email: 'bugsnag@bugsnag.com',
+      registered_user: true
+    })
+  end)
 end
 
 get '/' do
@@ -23,21 +31,6 @@ get '/crash' do
     'bugsnag.com for a new notification!')
 end
 
-get '/crash_with_callback' do
-  Bugsnag.before_notify_callbacks << proc { |notification|
-    new_tab = {
-      message: 'Sinatra demo says: Everything is great',
-      code: 200
-    }
-    notification.add_tab(:diagnostics, new_tab)
-  }
-
-  msg = 'Bugsnag Sinatra demo says: It crashed! But, due to the attached callback' +
-    ' the exception has meta information. Go check' +
-    ' bugsnag.com for a new notification (see the Diagnostics tab)!'
-  raise RuntimeError.new(msg)
-end
-
 get '/notify' do
   Bugsnag.notify(RuntimeError.new("Bugsnag Sinatra demo says: False alarm, your application didn't crash"))
 
@@ -48,21 +41,16 @@ end
 
 get '/notify_data' do
   error = RuntimeError.new("Bugsnag Sinatra demo says: False alarm, your application didn't crash")
-  Bugsnag.notify error do |report|
-    report.add_tab(:user, {
-      :username => "bob-hoskins",
-      :email => 'bugsnag@bugsnag.com',
-      :registered_user => true
-    })
+  Bugsnag.notify(error) do |report|
     report.add_tab(:diagnostics, {
-      :message => 'Sinatra demo says: Everything is great',
-      :code => 200
+      message: 'Sinatra demo says: Everything is great',
+      code: 200
     })
   end
 
   "Bugsnag Sinatra demo says: It didn't crash! " +
     'But still go check <a href="https://bugsnag.com">https://bugsnag.com</a>' +
-    ' for a new notification. Check out the User tab for the meta data'
+    ' for a new notification. Check out the Diagnostics tab for the meta data'
 end
 
 get '/notify_severity' do
diff --git a/example/sinatra/templates/index.md b/example/sinatra/templates/index.md
index 52695efe2..6e111829b 100644
--- a/example/sinatra/templates/index.md
+++ b/example/sinatra/templates/index.md
@@ -8,18 +8,14 @@ While testing the examples open [your dashboard](https://app.bugsnag.com) in ord
 <br/>
     Raises an error within the framework, generating a report in the Bugsnag dashboard.
 
-2. [Crash and use callbacks](/crash_with_callback)
-<br/>
-    Raises an exception within the framework, but with additional data attached to the report.  By registering a callback before the error occurs useful data can be attached as a tab in the Bugsnag dashboard.
-
-3. [Notify](/notify)
+2. [Notify](/notify)
 <br/>
     Sends Bugsnag a report on demand using `bugsnag.notify`.  Allows details of handled errors or information to be sent to the Bugsnag dashboard without crashing your code.
 
-4. [Notify with data](/notify_data)
+3. [Notify with data](/notify_data)
 <br/>
-    Same as `notify` but allows you to attach additional data within a `block`, similar to the `before_notify_callbacks` example above.  In this case we're adding information about the user to go into the `user` tab, and additional diagnostics as a `diagnostics` tab.
+    Same as `notify` but allows you to attach additional data within a `block`. In this case we're adding additional diagnostics as a `diagnostics` tab.
 
-5. [Set the severity](/notify_severity)
+4. [Set the severity](/notify_severity)
 <br/>
-    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.
\ No newline at end of file
+    This uses the same mechanism as adding meta-data, but allows you to set he `severity` when notifying Bugsnag of the error.  Valid severities are `error`, `warning`, and `info`.  Have a look on the dashboard to see the difference in these severities.

From f80f785ca3a5592ee6b2d105ad7b1a7afc974b6c Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 20 Jul 2020 10:52:12 +0100
Subject: [PATCH 24/30] Add plain Ruby Maze Runner tests for on_error

---
 .../initiators/handled_on_error.rb            | 10 +++++++
 .../initiators/unhandled_on_error.rb          | 11 +++++++
 .../initiators/handled_on_error.rb            | 29 +++++++++++++++++++
 .../initiators/unhandled_on_error.rb          | 26 +++++++++++++++++
 features/plain_features/add_tab.feature       |  8 ++++-
 features/plain_features/ignore_report.feature |  2 ++
 .../plain_features/report_api_key.feature     |  4 ++-
 .../plain_features/report_severity.feature    |  2 ++
 .../report_stack_frames.feature               |  4 +++
 features/plain_features/report_user.feature   |  8 ++++-
 10 files changed, 101 insertions(+), 3 deletions(-)
 create mode 100644 features/fixtures/plain/app/report_modification/initiators/handled_on_error.rb
 create mode 100644 features/fixtures/plain/app/report_modification/initiators/unhandled_on_error.rb
 create mode 100644 features/fixtures/plain/app/stack_frame_modification/initiators/handled_on_error.rb
 create mode 100644 features/fixtures/plain/app/stack_frame_modification/initiators/unhandled_on_error.rb

diff --git a/features/fixtures/plain/app/report_modification/initiators/handled_on_error.rb b/features/fixtures/plain/app/report_modification/initiators/handled_on_error.rb
new file mode 100644
index 000000000..16e8af4d2
--- /dev/null
+++ b/features/fixtures/plain/app/report_modification/initiators/handled_on_error.rb
@@ -0,0 +1,10 @@
+require 'bugsnag'
+require './app'
+
+configure_basics
+
+def run(callback)
+  Bugsnag.add_on_error(callback)
+
+  Bugsnag.notify(RuntimeError.new("Oh no"))
+end
diff --git a/features/fixtures/plain/app/report_modification/initiators/unhandled_on_error.rb b/features/fixtures/plain/app/report_modification/initiators/unhandled_on_error.rb
new file mode 100644
index 000000000..0b949961c
--- /dev/null
+++ b/features/fixtures/plain/app/report_modification/initiators/unhandled_on_error.rb
@@ -0,0 +1,11 @@
+require 'bugsnag'
+require './app'
+
+configure_basics
+add_at_exit
+
+def run(callback)
+  Bugsnag.add_on_error(callback)
+
+  raise RuntimeError.new "Oh no"
+end
diff --git a/features/fixtures/plain/app/stack_frame_modification/initiators/handled_on_error.rb b/features/fixtures/plain/app/stack_frame_modification/initiators/handled_on_error.rb
new file mode 100644
index 000000000..012c6fca8
--- /dev/null
+++ b/features/fixtures/plain/app/stack_frame_modification/initiators/handled_on_error.rb
@@ -0,0 +1,29 @@
+require 'bugsnag'
+require './app'
+
+configure_basics
+
+def run(callback)
+  Bugsnag.add_on_error(callback)
+  step_one
+end
+
+def step_one
+  step_two
+end
+
+def step_two
+  step_three
+end
+
+def step_three
+  crash
+end
+
+def crash
+  begin
+    "Test".insrt(-1, "!")
+  rescue Exception => e
+    Bugsnag.notify(e)
+  end
+end
diff --git a/features/fixtures/plain/app/stack_frame_modification/initiators/unhandled_on_error.rb b/features/fixtures/plain/app/stack_frame_modification/initiators/unhandled_on_error.rb
new file mode 100644
index 000000000..7f323503c
--- /dev/null
+++ b/features/fixtures/plain/app/stack_frame_modification/initiators/unhandled_on_error.rb
@@ -0,0 +1,26 @@
+require 'bugsnag'
+require './app'
+
+configure_basics
+add_at_exit
+
+def run(callback)
+  Bugsnag.add_on_error(callback)
+  step_one
+end
+
+def step_one
+  step_two
+end
+
+def step_two
+  step_three
+end
+
+def step_three
+  crash
+end
+
+def crash
+  raise RuntimeError.new "Oh no"
+end
diff --git a/features/plain_features/add_tab.feature b/features/plain_features/add_tab.feature
index 6dd380a9e..6e2218b8a 100644
--- a/features/plain_features/add_tab.feature
+++ b/features/plain_features/add_tab.feature
@@ -15,6 +15,8 @@ Scenario Outline: Metadata can be added to a report using add_tab
   | handled_before_notify   |
   | handled_block           |
   | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
 
 Scenario Outline: Metadata can be added to an existing tab using add_tab
   Given I set environment variable "CALLBACK_INITIATOR" to "<initiator>"
@@ -33,6 +35,8 @@ Scenario Outline: Metadata can be added to an existing tab using add_tab
   | handled_before_notify   |
   | handled_block           |
   | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
 
 Scenario Outline: Metadata can be overwritten using add_tab
   Given I set environment variable "CALLBACK_INITIATOR" to "<initiator>"
@@ -46,4 +50,6 @@ Scenario Outline: Metadata can be overwritten using add_tab
   | initiator               |
   | handled_before_notify   |
   | handled_block           |
-  | unhandled_before_notify |
\ No newline at end of file
+  | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
diff --git a/features/plain_features/ignore_report.feature b/features/plain_features/ignore_report.feature
index e0087bbc7..c5c837829 100644
--- a/features/plain_features/ignore_report.feature
+++ b/features/plain_features/ignore_report.feature
@@ -10,3 +10,5 @@ Scenario Outline: A reports severity can be modified
   | initiator               |
   | handled_before_notify   |
   | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
diff --git a/features/plain_features/report_api_key.feature b/features/plain_features/report_api_key.feature
index b4252071e..6821c6125 100644
--- a/features/plain_features/report_api_key.feature
+++ b/features/plain_features/report_api_key.feature
@@ -11,4 +11,6 @@ Scenario Outline: A report can have its api_key modified
   | initiator               |
   | handled_before_notify   |
   | handled_block           |
-  | unhandled_before_notify |
\ No newline at end of file
+  | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
diff --git a/features/plain_features/report_severity.feature b/features/plain_features/report_severity.feature
index 84e181ba3..c96dc258c 100644
--- a/features/plain_features/report_severity.feature
+++ b/features/plain_features/report_severity.feature
@@ -13,3 +13,5 @@ Scenario Outline: A reports severity can be modified
   | handled_before_notify   |
   | handled_block           |
   | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
diff --git a/features/plain_features/report_stack_frames.feature b/features/plain_features/report_stack_frames.feature
index 1cb3635de..ce34204e1 100644
--- a/features/plain_features/report_stack_frames.feature
+++ b/features/plain_features/report_stack_frames.feature
@@ -12,6 +12,8 @@ Scenario Outline: Stack frames can be removed
   | initiator               | lineNumber |
   | handled_before_notify   | 20         |
   | unhandled_before_notify | 21         |
+  | handled_on_error        | 20         |
+  | unhandled_on_error      | 21         |
 
 Scenario: Stack frames can be removed from a notified string
   Given I set environment variable "CALLBACK_INITIATOR" to "handled_block"
@@ -36,6 +38,8 @@ Scenario Outline: Stack frames can be marked as in project
   | initiator               |
   | handled_before_notify   |
   | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
 
 Scenario: Stack frames can be marked as in project with a handled string
   Given I set environment variable "CALLBACK_INITIATOR" to "handled_block"
diff --git a/features/plain_features/report_user.feature b/features/plain_features/report_user.feature
index 9e3919273..4f4a34fe5 100644
--- a/features/plain_features/report_user.feature
+++ b/features/plain_features/report_user.feature
@@ -14,6 +14,8 @@ Scenario Outline: A report can have a user name, email, and id set
   | handled_before_notify   |
   | handled_block           |
   | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
 
 Scenario Outline: A report can have custom info set
   Given I set environment variable "CALLBACK_INITIATOR" to "<initiator>"
@@ -30,6 +32,8 @@ Scenario Outline: A report can have custom info set
   | handled_before_notify   |
   | handled_block           |
   | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |
 
 Scenario Outline: A report can have its user info removed
   Given I set environment variable "CALLBACK_INITIATOR" to "<initiator>"
@@ -42,4 +46,6 @@ Scenario Outline: A report can have its user info removed
   | initiator               |
   | handled_before_notify   |
   | handled_block           |
-  | unhandled_before_notify |
\ No newline at end of file
+  | unhandled_before_notify |
+  | handled_on_error        |
+  | unhandled_on_error      |

From a943fc4eef7b1582a36532fe97a09078ebd7c91d Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 20 Jul 2020 11:51:08 +0100
Subject: [PATCH 25/30] Add Rails Maze Runner tests for on_error callbacks

---
 features/fixtures/docker-compose.yml          |  6 +++-
 .../rails3/app/config/initializers/bugsnag.rb |  8 +++++
 .../rails4/app/config/initializers/bugsnag.rb |  8 +++++
 .../rails5/app/config/initializers/bugsnag.rb |  8 +++++
 .../rails6/app/config/initializers/bugsnag.rb |  8 +++++
 features/rails_features/on_error.feature      | 29 +++++++++++++++++++
 6 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 features/rails_features/on_error.feature

diff --git a/features/fixtures/docker-compose.yml b/features/fixtures/docker-compose.yml
index 3352a421a..0357d5fb5 100644
--- a/features/fixtures/docker-compose.yml
+++ b/features/fixtures/docker-compose.yml
@@ -120,6 +120,7 @@ services:
       - BUGSNAG_TIMEOUT
       - CALLBACK_INITIATOR
       - SQL_ONLY_BREADCRUMBS
+      - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
     restart: "no"
 
@@ -155,6 +156,7 @@ services:
       - BUGSNAG_TIMEOUT
       - CALLBACK_INITIATOR
       - SQL_ONLY_BREADCRUMBS
+      - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
     restart: "no"
 
@@ -190,6 +192,7 @@ services:
       - BUGSNAG_TIMEOUT
       - CALLBACK_INITIATOR
       - SQL_ONLY_BREADCRUMBS
+      - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
     restart: "no"
 
@@ -225,6 +228,7 @@ services:
       - BUGSNAG_TIMEOUT
       - CALLBACK_INITIATOR
       - SQL_ONLY_BREADCRUMBS
+      - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
     restart: "no"
     networks:
@@ -296,4 +300,4 @@ services:
 
 networks:
   default:
-    name: ${NETWORK_NAME}
\ No newline at end of file
+    name: ${NETWORK_NAME}
diff --git a/features/fixtures/rails3/app/config/initializers/bugsnag.rb b/features/fixtures/rails3/app/config/initializers/bugsnag.rb
index b849c0995..8a1ac08ea 100644
--- a/features/fixtures/rails3/app/config/initializers/bugsnag.rb
+++ b/features/fixtures/rails3/app/config/initializers/bugsnag.rb
@@ -18,4 +18,12 @@
       breadcrumb.ignore! unless breadcrumb.meta_data[:event_name] == "sql.active_record" && breadcrumb.meta_data[:name] == "User Load"
     end
   end
+
+  if ENV["ADD_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.add_tab(:on_error, {
+        source: report.unhandled ? 'on_error unhandled' : 'on_error handled'
+      })
+    end)
+  end
 end
diff --git a/features/fixtures/rails4/app/config/initializers/bugsnag.rb b/features/fixtures/rails4/app/config/initializers/bugsnag.rb
index b849c0995..8a1ac08ea 100644
--- a/features/fixtures/rails4/app/config/initializers/bugsnag.rb
+++ b/features/fixtures/rails4/app/config/initializers/bugsnag.rb
@@ -18,4 +18,12 @@
       breadcrumb.ignore! unless breadcrumb.meta_data[:event_name] == "sql.active_record" && breadcrumb.meta_data[:name] == "User Load"
     end
   end
+
+  if ENV["ADD_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.add_tab(:on_error, {
+        source: report.unhandled ? 'on_error unhandled' : 'on_error handled'
+      })
+    end)
+  end
 end
diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb
index b849c0995..8a1ac08ea 100644
--- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb
+++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb
@@ -18,4 +18,12 @@
       breadcrumb.ignore! unless breadcrumb.meta_data[:event_name] == "sql.active_record" && breadcrumb.meta_data[:name] == "User Load"
     end
   end
+
+  if ENV["ADD_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.add_tab(:on_error, {
+        source: report.unhandled ? 'on_error unhandled' : 'on_error handled'
+      })
+    end)
+  end
 end
diff --git a/features/fixtures/rails6/app/config/initializers/bugsnag.rb b/features/fixtures/rails6/app/config/initializers/bugsnag.rb
index b849c0995..8a1ac08ea 100644
--- a/features/fixtures/rails6/app/config/initializers/bugsnag.rb
+++ b/features/fixtures/rails6/app/config/initializers/bugsnag.rb
@@ -18,4 +18,12 @@
       breadcrumb.ignore! unless breadcrumb.meta_data[:event_name] == "sql.active_record" && breadcrumb.meta_data[:name] == "User Load"
     end
   end
+
+  if ENV["ADD_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.add_tab(:on_error, {
+        source: report.unhandled ? 'on_error unhandled' : 'on_error handled'
+      })
+    end)
+  end
 end
diff --git a/features/rails_features/on_error.feature b/features/rails_features/on_error.feature
new file mode 100644
index 000000000..35166ade6
--- /dev/null
+++ b/features/rails_features/on_error.feature
@@ -0,0 +1,29 @@
+Feature: On error callbacks
+
+@rails3 @rails4 @rails5 @rails6
+Scenario: Rails on_error works on handled errors
+  Given I set environment variable "ADD_ON_ERROR" to "true"
+  And I start the rails service
+  When I navigate to the route "/handled/unthrown" on the rails app
+  And I wait to receive a request
+  Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier"
+  And the exception "errorClass" equals "RuntimeError"
+  And the exception "message" starts with "handled unthrown error"
+  And the event "unhandled" is false
+  And the event "app.type" equals "rails"
+  And the event "metaData.request.url" ends with "/handled/unthrown"
+  And the event "metaData.on_error.source" equals "on_error handled"
+
+@rails3 @rails4 @rails5 @rails6
+Scenario: Rails on_error works on unhandled errors
+  Given I set environment variable "ADD_ON_ERROR" to "true"
+  And I start the rails service
+  When I navigate to the route "/unhandled/error" on the rails app
+  And I wait to receive a request
+  Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier"
+  And the exception "errorClass" equals "NameError"
+  And the exception "message" starts with "undefined local variable or method `generate_unhandled_error' for #<UnhandledController"
+  And the event "unhandled" is true
+  And the event "app.type" equals "rails"
+  And the event "metaData.request.url" ends with "/unhandled/error"
+  And the event "metaData.on_error.source" equals "on_error unhandled"

From 89a9b62b95d4a7cf9457259505100ed9a89f6a1a Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Thu, 16 Jul 2020 16:14:52 +0100
Subject: [PATCH 26/30] Require 'concurrent' only when needed

This helps reduce the memory overhead of Bugsnag when session tracking
isn't enabled by ~4 MiB
---
 lib/bugsnag/session_tracker.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/bugsnag/session_tracker.rb b/lib/bugsnag/session_tracker.rb
index d57e227c7..65c0f98a5 100644
--- a/lib/bugsnag/session_tracker.rb
+++ b/lib/bugsnag/session_tracker.rb
@@ -1,11 +1,9 @@
 require 'thread'
 require 'time'
 require 'securerandom'
-require 'concurrent'
 
 module Bugsnag
   class SessionTracker
-
     THREAD_SESSION = "bugsnag_session"
     SESSION_PAYLOAD_VERSION = "1.0"
     MUTEX = Mutex.new
@@ -27,6 +25,8 @@ def self.get_current_session
     ##
     # Initializes the session tracker.
     def initialize
+      require 'concurrent'
+
       @session_counts = Concurrent::Hash.new(0)
     end
 
@@ -139,4 +139,4 @@ def deliver(session_payload)
       Bugsnag::Delivery[Bugsnag.configuration.delivery_method].deliver(Bugsnag.configuration.session_endpoint, payload, Bugsnag.configuration, options)
     end
   end
-end
\ No newline at end of file
+end

From a7c2c2bf08b08d418fb5a7b30dba72a041d3032f Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Thu, 16 Jul 2020 16:49:53 +0100
Subject: [PATCH 27/30] Create the session tracker after configure has run

Create the session tracker if sessions are enabled to avoid the overhead
of creating it on the first request

By requiring the concurrent gem in the session tracker's initialize
method, we shifted the overhead from startup to the first request.
This caused the first request to go from ~10ms to ~70ms (in the Sinatra
example app run locally), which is quite alot of overhead
---
 lib/bugsnag.rb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb
index 178ce0efc..7d215b3d6 100644
--- a/lib/bugsnag.rb
+++ b/lib/bugsnag.rb
@@ -48,6 +48,12 @@ class << self
     def configure(validate_api_key=true)
       yield(configuration) if block_given?
 
+      # Create the session tracker if sessions are enabled to avoid the overhead
+      # of creating it on the first request. We skip this if we're not validating
+      # the API key as we use this internally before the user's configure block
+      # has run, so we don't know if sessions are enabled yet.
+      session_tracker if validate_api_key && configuration.auto_capture_sessions
+
       check_key_valid if validate_api_key
       check_endpoint_setup
 

From 287c913fd1a1567f9795315704d8525111b6bbcc Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Thu, 23 Jul 2020 13:32:06 +0100
Subject: [PATCH 28/30] Fix typo in example Rails 6 app

---
 example/rails-60/app/views/resque/index.html.erb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/example/rails-60/app/views/resque/index.html.erb b/example/rails-60/app/views/resque/index.html.erb
index f43d18487..483e94910 100644
--- a/example/rails-60/app/views/resque/index.html.erb
+++ b/example/rails-60/app/views/resque/index.html.erb
@@ -1,6 +1,6 @@
-<h2>Rescue</h2>
+<h2>Resque</h2>
 
-<p>This route demonstrates how to use Bugsnag with Rescue.</p>
+<p>This route demonstrates how to use Bugsnag with Resque.</p>
 
 <p>
     While testing the examples open

From 6ab3d680212141ae83aee3a492fea07ca0364bbb Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Thu, 23 Jul 2020 17:20:26 +0100
Subject: [PATCH 29/30] Update changelog

---
 CHANGELOG.md | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 21e758dd4..4c416d366 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,13 +1,19 @@
 Changelog
 =========
 
-## TBD
+## 6.15.0 (27 July 2020)
 
 ### Enhancements
 
 * Add `on_error` callbacks to replace `before_notify_callbacks`
   | [#608](https://github.com/bugsnag/bugsnag-ruby/pull/608)
 
+* Improve performance when extracting code from files in stacktraces
+  | [#604](https://github.com/bugsnag/bugsnag-ruby/pull/604)
+
+* Reduce memory use when session tracking is disabled
+  | [#606](https://github.com/bugsnag/bugsnag-ruby/pull/606)
+
 ### Deprecated
 
 * `before_notify_callbacks` have been deprecated in favour of `on_error` and will be removed in the next major release

From a893ea6510bde5d75ad52acca7cbfa8898050927 Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Mon, 27 Jul 2020 13:44:37 +0100
Subject: [PATCH 30/30] Bump version

---
 VERSION | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/VERSION b/VERSION
index 68390495f..a3748c562 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-6.14.0
+6.15.0