From 7bea4f07409b0aa59857ac09155f5e0bee41b862 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 11 Aug 2021 16:15:25 +0100 Subject: [PATCH 01/63] Add Bugsnag::BreadcrumbType module This will eventually replace the breadcrumb types in the Bugsnag::Breadcrumbs module --- lib/bugsnag.rb | 3 ++- lib/bugsnag/breadcrumb_type.rb | 14 ++++++++++++++ lib/bugsnag/breadcrumbs/breadcrumb.rb | 2 +- lib/bugsnag/breadcrumbs/breadcrumbs.rb | 1 + lib/bugsnag/configuration.rb | 2 +- spec/breadcrumb_type_spec.rb | 23 +++++++++++++++++++++++ 6 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 lib/bugsnag/breadcrumb_type.rb create mode 100644 spec/breadcrumb_type_spec.rb diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index b654dfb78..81d6b160f 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -28,6 +28,7 @@ require "bugsnag/middleware/classify_error" require "bugsnag/middleware/delayed_job" +require "bugsnag/breadcrumb_type" require "bugsnag/breadcrumbs/validator" require "bugsnag/breadcrumbs/breadcrumb" require "bugsnag/breadcrumbs/breadcrumbs" @@ -237,7 +238,7 @@ def load_integration(integration) # # @param name [String] the main breadcrumb name/message # @param meta_data [Hash] String, Numeric, or Boolean meta data to attach - # @param type [String] the breadcrumb type, from Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES + # @param type [String] the breadcrumb type, see {Bugsnag::BreadcrumbType} # @param auto [Symbol] set to :auto if the breadcrumb is automatically created # @return [void] def leave_breadcrumb(name, meta_data={}, type=Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, auto=:manual) diff --git a/lib/bugsnag/breadcrumb_type.rb b/lib/bugsnag/breadcrumb_type.rb new file mode 100644 index 000000000..f8a103ccc --- /dev/null +++ b/lib/bugsnag/breadcrumb_type.rb @@ -0,0 +1,14 @@ +require "bugsnag/breadcrumbs/breadcrumbs" + +module Bugsnag + module BreadcrumbType + ERROR = Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE + LOG = Bugsnag::Breadcrumbs::LOG_BREADCRUMB_TYPE + MANUAL = Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE + NAVIGATION = Bugsnag::Breadcrumbs::NAVIGATION_BREADCRUMB_TYPE + PROCESS = Bugsnag::Breadcrumbs::PROCESS_BREADCRUMB_TYPE + REQUEST = Bugsnag::Breadcrumbs::REQUEST_BREADCRUMB_TYPE + STATE = Bugsnag::Breadcrumbs::STATE_BREADCRUMB_TYPE + USER = Bugsnag::Breadcrumbs::USER_BREADCRUMB_TYPE + end +end diff --git a/lib/bugsnag/breadcrumbs/breadcrumb.rb b/lib/bugsnag/breadcrumbs/breadcrumb.rb index 90d9533c9..129ab568d 100644 --- a/lib/bugsnag/breadcrumbs/breadcrumb.rb +++ b/lib/bugsnag/breadcrumbs/breadcrumb.rb @@ -23,7 +23,7 @@ class Breadcrumb # @api private # # @param name [String] the breadcrumb name - # @param type [String] the breadcrumb type from Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES + # @param type [String] the breadcrumb type from Bugsnag::BreadcrumbType # @param meta_data [Hash, nil] a hash containing strings, numbers, or booleans, or nil # @param auto [Symbol] set to `:auto` if the breadcrumb is automatically generated def initialize(name, type, meta_data, auto) diff --git a/lib/bugsnag/breadcrumbs/breadcrumbs.rb b/lib/bugsnag/breadcrumbs/breadcrumbs.rb index 69506d29d..b92b325cf 100644 --- a/lib/bugsnag/breadcrumbs/breadcrumbs.rb +++ b/lib/bugsnag/breadcrumbs/breadcrumbs.rb @@ -1,4 +1,5 @@ module Bugsnag::Breadcrumbs + # @deprecated Use {Bugsnag::BreadcrumbType} instead VALID_BREADCRUMB_TYPES = [ ERROR_BREADCRUMB_TYPE = "error", MANUAL_BREADCRUMB_TYPE = "manual", diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index ea8abc1ec..4a211185c 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -125,7 +125,7 @@ class Configuration attr_reader :enable_sessions # A list of strings indicating allowable automatic breadcrumb types - # @see Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES + # @see Bugsnag::BreadcrumbType # @return [Array] attr_accessor :enabled_automatic_breadcrumb_types diff --git a/spec/breadcrumb_type_spec.rb b/spec/breadcrumb_type_spec.rb new file mode 100644 index 000000000..e1d75c747 --- /dev/null +++ b/spec/breadcrumb_type_spec.rb @@ -0,0 +1,23 @@ +require "spec_helper" + +require "bugsnag/breadcrumb_type" +require "bugsnag/breadcrumbs/breadcrumbs" + +describe Bugsnag::BreadcrumbType do + it "contains constants equivalent to the breadcrumb types defined in Bugsnag::Breadcrumbs" do + expect(Bugsnag::BreadcrumbType::ERROR).to eq(Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::LOG).to eq(Bugsnag::Breadcrumbs::LOG_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::MANUAL).to eq(Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::NAVIGATION).to eq(Bugsnag::Breadcrumbs::NAVIGATION_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::PROCESS).to eq(Bugsnag::Breadcrumbs::PROCESS_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::REQUEST).to eq(Bugsnag::Breadcrumbs::REQUEST_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::STATE).to eq(Bugsnag::Breadcrumbs::STATE_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::USER).to eq(Bugsnag::Breadcrumbs::USER_BREADCRUMB_TYPE) + end + + it "defines the same number of breadcrumb type constants" do + old_types = Bugsnag::Breadcrumbs.constants.select { |type| type.to_s.end_with?("_BREADCRUMB_TYPE") } + + expect(Bugsnag::BreadcrumbType.constants.length).to eq(old_types.length) + end +end From 5eff8c0d8bc492667af98b6e9ccf1a7a69fb3b04 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 11 Aug 2021 16:22:42 +0100 Subject: [PATCH 02/63] Add Configuration#enabled_release_stages This will eventually replace notify_release_stages entirely, but for now exists as an alias --- lib/bugsnag/configuration.rb | 19 +++++++++++++++++++ spec/report_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 4a211185c..fca1722e0 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -24,6 +24,7 @@ class Configuration attr_accessor :release_stage # A list of which release stages should cause notifications to be sent + # @deprecated Use {#enabled_release_stages} instead # @return [Array, nil] attr_accessor :notify_release_stages @@ -503,6 +504,24 @@ def remove_on_error(callback) middleware.remove(callback) end + # TODO: "enabled_release_stages" can be a simple attr_accessor when it + # replaces "notify_release_stages" + # NOTE: this is not an alias as YARD doesn't allow documenting the non-alias + # as deprecated without also marking the alias as deprecated + + # A list of which release stages should cause notifications to be sent + # @!attribute enabled_release_stages + # @return [Array, nil] + def enabled_release_stages + @notify_release_stages + end + + # @param release_stages [Array, nil] + # @return [void] + def enabled_release_stages=(release_stages) + @notify_release_stages = release_stages + end + private attr_writer :scopes_to_filter diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 06edff8bb..dce06d828 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -504,6 +504,26 @@ def gloops } end + it "respects the enabled_release_stages setting by not sending in development" do + Bugsnag.configuration.enabled_release_stages = ["production"] + Bugsnag.configuration.release_stage = "development" + + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).not_to have_sent_notification + end + + it "respects the enabled_release_stages setting when set" do + Bugsnag.configuration.release_stage = "development" + Bugsnag.configuration.enabled_release_stages = ["development"] + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).to(have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["exceptions"].length).to eq(1) + }) + end + it "respects the notify_release_stages setting by not sending in development" do Bugsnag.configuration.notify_release_stages = ["production"] Bugsnag.configuration.release_stage = "development" From afa45992bd0f13602ec7d5b66c6f4528abc364c6 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 11 Aug 2021 16:36:30 +0100 Subject: [PATCH 03/63] Add message & metadata to replace name & meta_data --- lib/bugsnag/breadcrumbs/breadcrumb.rb | 33 +++++++++++++++++++++++++++ spec/breadcrumbs/breadcrumb_spec.rb | 26 +++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/lib/bugsnag/breadcrumbs/breadcrumb.rb b/lib/bugsnag/breadcrumbs/breadcrumb.rb index 129ab568d..a5ed2e16b 100644 --- a/lib/bugsnag/breadcrumbs/breadcrumb.rb +++ b/lib/bugsnag/breadcrumbs/breadcrumb.rb @@ -1,11 +1,13 @@ module Bugsnag::Breadcrumbs class Breadcrumb + # @deprecated Use {#message} instead # @return [String] the breadcrumb name attr_accessor :name # @return [String] the breadcrumb type attr_accessor :type + # @deprecated Use {#metadata} instead # @return [Hash, nil] metadata hash containing strings, numbers, or booleans, or nil attr_accessor :meta_data @@ -72,5 +74,36 @@ def to_h :timestamp => @timestamp.iso8601(3) } end + + # TODO: "message" and "metadata" can be simple attr_accessors when they + # replace "name" and "meta_data" + # NOTE: these are not aliases as YARD doesn't allow documenting the non-alias + # as deprecated without also marking the alias as deprecated + + # The breadcrumb message + # @!attribute message + # @return [String] + def message + @name + end + + # @param message [String] + # @return [void] + def message=(message) + @name = message + end + + # A Hash containing arbitrary metadata associated with this breadcrumb + # @!attribute metadata + # @return [Hash, nil] + def metadata + @meta_data + end + + # @param metadata [Hash, nil] + # @return [void] + def metadata=(metadata) + @meta_data = metadata + end end end diff --git a/spec/breadcrumbs/breadcrumb_spec.rb b/spec/breadcrumbs/breadcrumb_spec.rb index eb79f2c0f..b4bd43019 100644 --- a/spec/breadcrumbs/breadcrumb_spec.rb +++ b/spec/breadcrumbs/breadcrumb_spec.rb @@ -10,6 +10,19 @@ breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new("my message", nil, nil, nil) expect(breadcrumb.name).to eq("my message") + expect(breadcrumb.message).to eq("my message") + end + + it "can be accessed as #message" do + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new("my message", nil, nil, nil) + + breadcrumb.message = "my other message" + expect(breadcrumb.message).to eq("my other message") + expect(breadcrumb.name).to eq("my other message") + + breadcrumb.name = "another message" + expect(breadcrumb.message).to eq("another message") + expect(breadcrumb.name).to eq("another message") end end @@ -26,6 +39,19 @@ breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new(nil, nil, {:a => 1, :b => 2}, nil) expect(breadcrumb.meta_data).to eq({:a => 1, :b => 2}) + expect(breadcrumb.metadata).to eq({:a => 1, :b => 2}) + end + + it "can be accessed as #metadata" do + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new(nil, nil, { a: 1, b: 2 }, nil) + + breadcrumb.metadata = { c: 3 } + expect(breadcrumb.meta_data).to eq({ c: 3 }) + expect(breadcrumb.metadata).to eq({ c: 3 }) + + breadcrumb.meta_data = { d: 4 } + expect(breadcrumb.meta_data).to eq({ d: 4 }) + expect(breadcrumb.metadata).to eq({ d: 4 }) end end From 63050e4e37d5b0c3c72a73f69c1e31d6fee01b28 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 11 Aug 2021 16:43:55 +0100 Subject: [PATCH 04/63] Add Configuration#enabled_breadcrumb_types --- lib/bugsnag/configuration.rb | 21 ++++++++++++++++++--- spec/configuration_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index fca1722e0..b9fd20851 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -126,6 +126,7 @@ class Configuration attr_reader :enable_sessions # A list of strings indicating allowable automatic breadcrumb types + # @deprecated Use {#enabled_breadcrumb_types} instead # @see Bugsnag::BreadcrumbType # @return [Array] attr_accessor :enabled_automatic_breadcrumb_types @@ -504,9 +505,9 @@ def remove_on_error(callback) middleware.remove(callback) end - # TODO: "enabled_release_stages" can be a simple attr_accessor when it - # replaces "notify_release_stages" - # NOTE: this is not an alias as YARD doesn't allow documenting the non-alias + # TODO: These methods can be a simple attr_accessor when they replace the + # methods they are aliasing + # NOTE: they are not aliases as YARD doesn't allow documenting the non-alias # as deprecated without also marking the alias as deprecated # A list of which release stages should cause notifications to be sent @@ -522,6 +523,20 @@ def enabled_release_stages=(release_stages) @notify_release_stages = release_stages end + # A list of breadcrumb types that Bugsnag will collect automatically + # @!attribute enabled_breadcrumb_types + # @see Bugsnag::BreadcrumbType + # @return [Array] + def enabled_breadcrumb_types + @enabled_automatic_breadcrumb_types + end + + # @param breadcrumb_types [Array] + # @return [void] + def enabled_breadcrumb_types=(breadcrumb_types) + @enabled_automatic_breadcrumb_types = breadcrumb_types + end + private attr_writer :scopes_to_filter diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index b4c8acd1c..260d8cdc6 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -443,6 +443,29 @@ def debug(name, &block) end end + describe "#enabled_breadcrumb_types" do + it "defaults to Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES" do + expect(subject.enabled_breadcrumb_types).to eq(Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES) + end + + it "is an editable array" do + subject.enabled_breadcrumb_types << "Some custom type" + expect(subject.enabled_breadcrumb_types).to include("Some custom type") + end + + it "shares a backing array with 'enabled_automatic_breadcrumb_types'" do + expect(subject.enabled_breadcrumb_types).to be(subject.enabled_automatic_breadcrumb_types) + + subject.enabled_breadcrumb_types = [1, 2, 3] + expect(subject.enabled_breadcrumb_types).to eq([1, 2, 3]) + expect(subject.enabled_automatic_breadcrumb_types).to eq([1, 2, 3]) + + subject.enabled_automatic_breadcrumb_types = [4, 5, 6] + expect(subject.enabled_breadcrumb_types).to eq([4, 5, 6]) + expect(subject.enabled_automatic_breadcrumb_types).to eq([4, 5, 6]) + end + end + describe "#before_breadcrumb_callbacks" do it "initially returns an empty array" do expect(subject.before_breadcrumb_callbacks).to eq([]) From 4ff542e03ec39c67e185978e4296245455bf49ed Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 12 Aug 2021 09:55:27 +0100 Subject: [PATCH 05/63] Add metadata to Report to replace meta_data --- lib/bugsnag/report.rb | 14 ++++++++++++++ spec/report_spec.rb | 21 +++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 9fcb0ee86..fa2d8591d 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -72,6 +72,7 @@ class Report attr_accessor :grouping_hash # Arbitrary metadata attached to this report + # @deprecated Use {#metadata} instead # @return [Hash] attr_accessor :meta_data @@ -257,6 +258,19 @@ def summary end end + # A Hash containing arbitrary metadata + # @!attribute metadata + # @return [Hash] + def metadata + @meta_data + end + + # @param metadata [Hash] + # @return [void] + def metadata=(metadata) + @meta_data = metadata + end + private def generate_exception_list diff --git a/spec/report_spec.rb b/spec/report_spec.rb index dce06d828..784e66367 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -199,15 +199,27 @@ def gloops data: "also here" } }) + + report.metadata.merge!({ + some_other_tab: { + info: true, + data: "very true" + } + }) end - expect(Bugsnag).to have_sent_notification{ |payload, headers| + expect(Bugsnag).to(have_sent_notification{ |payload, headers| event = get_event_from_payload(payload) expect(event["metaData"]["some_tab"]).to eq( "info" => "here", "data" => "also here" ) - } + + expect(event["metaData"]["some_other_tab"]).to eq( + "info" => true, + "data" => "very true" + ) + }) end it "accepts meta data from an exception that mixes in Bugsnag::MetaData" do @@ -644,7 +656,8 @@ def gloops :user_secret => "key" } }) - report.meta_data.merge!({ + + report.metadata.merge!({ :session => { :"warden.user.user.key" => "1234", :"warden.user.foobar.key" => "1234", @@ -693,7 +706,7 @@ def gloops it "filters params from all payload hashes if they are added to meta_data_filters as regex" do Bugsnag.configuration.meta_data_filters << /other_data/ Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| - report.meta_data.merge!({:request => {:params => {:password => "1234", :other_password => "123456", :other_data => "123456"}}}) + report.metadata.merge!({:request => {:params => {:password => "1234", :other_password => "123456", :other_data => "123456"}}}) end expect(Bugsnag).to have_sent_notification{ |payload, headers| From 8fc889461baeaefe53a41ca585e1106accb783e8 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 12 Aug 2021 10:10:37 +0100 Subject: [PATCH 06/63] Add Event as an alias of the Report class --- lib/bugsnag.rb | 1 + lib/bugsnag/event.rb | 7 ++ spec/report_spec.rb | 201 ++++++++++++++++++++++--------------------- 3 files changed, 111 insertions(+), 98 deletions(-) create mode 100644 lib/bugsnag/event.rb diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 81d6b160f..f4b1121ad 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -5,6 +5,7 @@ require "bugsnag/configuration" require "bugsnag/meta_data" require "bugsnag/report" +require "bugsnag/event" require "bugsnag/cleaner" require "bugsnag/helpers" require "bugsnag/session_tracker" diff --git a/lib/bugsnag/event.rb b/lib/bugsnag/event.rb new file mode 100644 index 000000000..af42bb814 --- /dev/null +++ b/lib/bugsnag/event.rb @@ -0,0 +1,7 @@ +require "bugsnag/report" + +module Bugsnag + # For now Event is just an alias of Report. This points to the same object so + # any changes to Report will also affect Event + Event = Report +end diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 784e66367..0316babc6 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -27,8 +27,111 @@ def gloops end end +shared_examples "Report or Event tests" do |class_to_test| + it "#headers should return the correct request headers" do + fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) + expect(Time).to receive(:now).and_return(fake_now) + + report_or_event = class_to_test.new( + BugsnagTestException.new("It crashed"), + Bugsnag.configuration + ) + + expect(report_or_event.headers).to eq({ + "Bugsnag-Api-Key" => "c9d60ae4c7e70c4b6c4ebd3e8056d2b8", + "Bugsnag-Payload-Version" => "4.0", + # This matches the time we stubbed earlier (fake_now) + "Bugsnag-Sent-At" => "2020-01-02T03:04:05.123Z" + }) + end + + describe "#summary" do + it "provides a hash of the name, message, and severity" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + expect(report_or_event.summary).to eq({ + :error_class => "ZeroDivisionError", + :message => "divided by 0", + :severity => "warning" + }) + end + end + + it "handles strings" do + report_or_event = class_to_test.new("test string", Bugsnag.configuration) + + expect(report_or_event.summary).to eq({ + :error_class => "RuntimeError", + :message => "test string", + :severity => "warning" + }) + end + + it "handles error edge cases" do + report_or_event = class_to_test.new(Timeout::Error, Bugsnag.configuration) + + expect(report_or_event.summary).to eq({ + :error_class => "Timeout::Error", + :message => "Timeout::Error", + :severity => "warning" + }) + end + + it "handles empty exceptions" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + report_or_event.exceptions = [] + + expect(report_or_event.summary).to eq({ + :error_class => "Unknown", + :severity => "warning" + }) + end + end + + it "handles removed exceptions" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + report_or_event.exceptions = nil + + expect(report_or_event.summary).to eq({ + :error_class => "Unknown", + :severity => "warning" + }) + end + end + + it "handles exceptions being replaced" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + report_or_event.exceptions = "no one should ever do this" + + expect(report_or_event.summary).to eq({ + :error_class => "Unknown", + :severity => "warning" + }) + end + end + end +end + # rubocop:disable Metrics/BlockLength describe Bugsnag::Report do + include_examples("Report or Event tests", Bugsnag::Report) + include_examples("Report or Event tests", Bugsnag::Event) + it "should contain an api_key if one is set" do Bugsnag.notify(BugsnagTestException.new("It crashed")) @@ -92,23 +195,6 @@ def gloops } end - it "#headers should return the correct request headers" do - fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).and_return(fake_now) - - report = Bugsnag::Report.new( - BugsnagTestException.new("It crashed"), - Bugsnag.configuration - ) - - expect(report.headers).to eq({ - "Bugsnag-Api-Key" => "c9d60ae4c7e70c4b6c4ebd3e8056d2b8", - "Bugsnag-Payload-Version" => "4.0", - # This matches the time we stubbed earlier (fake_now) - "Bugsnag-Sent-At" => "2020-01-02T03:04:05.123Z" - }) - end - it "has the right exception class" do Bugsnag.notify(BugsnagTestException.new("It crashed")) @@ -1519,87 +1605,6 @@ def to_s end end - describe "#summary" do - it "provides a hash of the name, message, and severity" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - expect(report.summary).to eq({ - :error_class => "ZeroDivisionError", - :message => "divided by 0", - :severity => "warning" - }) - end - end - - it "handles strings" do - report = Bugsnag::Report.new("test string", Bugsnag.configuration) - - expect(report.summary).to eq({ - :error_class => "RuntimeError", - :message => "test string", - :severity => "warning" - }) - end - - it "handles error edge cases" do - report = Bugsnag::Report.new(Timeout::Error, Bugsnag.configuration) - - expect(report.summary).to eq({ - :error_class => "Timeout::Error", - :message => "Timeout::Error", - :severity => "warning" - }) - end - - it "handles empty exceptions" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - report.exceptions = [] - - expect(report.summary).to eq({ - :error_class => "Unknown", - :severity => "warning" - }) - end - end - - it "handles removed exceptions" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - report.exceptions = nil - - expect(report.summary).to eq({ - :error_class => "Unknown", - :severity => "warning" - }) - end - end - - it "handles exceptions being replaced" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - report.exceptions = "no one should ever do this" - - expect(report.summary).to eq({ - :error_class => "Unknown", - :severity => "warning" - }) - end - end - end - if defined?(JRUBY_VERSION) it "works with java.lang.Throwables" do From 1154aaac1662dc3d75001beb51717bfeeb9efeef Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 12 Aug 2021 14:38:49 +0100 Subject: [PATCH 07/63] Add Configuration#auto_track_sessions --- lib/bugsnag/configuration.rb | 14 ++++++++++++++ spec/configuration_spec.rb | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index b9fd20851..9594f75a6 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -105,6 +105,7 @@ class Configuration attr_accessor :discard_classes # Whether Bugsnag should automatically record sessions + # @deprecated Use {#auto_track_sessions} instead # @return [Boolean] attr_accessor :auto_capture_sessions @@ -537,6 +538,19 @@ def enabled_breadcrumb_types=(breadcrumb_types) @enabled_automatic_breadcrumb_types = breadcrumb_types end + # Whether sessions should be tracked automatically + # @!attribute auto_track_sessions + # @return [Boolean] + def auto_track_sessions + @auto_capture_sessions + end + + # @param track_sessions [Boolean] + # @return [void] + def auto_track_sessions=(track_sessions) + @auto_capture_sessions = track_sessions + end + private attr_writer :scopes_to_filter diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 260d8cdc6..fc81b84a3 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -87,6 +87,22 @@ end end + describe "#auto_track_sessions" do + it "defaults to true" do + expect(subject.auto_track_sessions).to eq(true) + end + + it "shares a backing boolean with 'auto_capture_sessions'" do + subject.auto_track_sessions = false + expect(subject.auto_track_sessions).to eq(false) + expect(subject.auto_capture_sessions).to eq(false) + + subject.auto_capture_sessions = true + expect(subject.auto_track_sessions).to eq(true) + expect(subject.auto_capture_sessions).to eq(true) + end + end + describe "#enable_sessions" do it "defaults to true" do expect(subject.enable_sessions).to eq(true) From 9cf9d768c6b8d01490bff86cf34ba9ff08245f9f Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 12 Aug 2021 14:44:41 +0100 Subject: [PATCH 08/63] Use the new property names in some MR tests By using them in the Rails 5 app, but not the Rails 4 & 6 apps, we can be more confident that the aliases work exactly as the old names did --- features/fixtures/plain/app/app.rb | 2 +- features/fixtures/rails5/app/config/initializers/bugsnag.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/features/fixtures/plain/app/app.rb b/features/fixtures/plain/app/app.rb index 37524117e..a0c3b3383 100644 --- a/features/fixtures/plain/app/app.rb +++ b/features/fixtures/plain/app/app.rb @@ -16,7 +16,7 @@ def configure_using_environment conf.auto_notify = ENV["BUGSNAG_AUTO_NOTIFY"] != "false" conf.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" conf.meta_data_filters << ENV["BUGSNAG_META_DATA_FILTERS"] if ENV.include? "BUGSNAG_META_DATA_FILTERS" - conf.notify_release_stages = [ENV["BUGSNAG_NOTIFY_RELEASE_STAGE"]] if ENV.include? "BUGSNAG_NOTIFY_RELEASE_STAGE" + conf.enabled_release_stages = [ENV["BUGSNAG_NOTIFY_RELEASE_STAGE"]] if ENV.include? "BUGSNAG_NOTIFY_RELEASE_STAGE" conf.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" conf.proxy_host = ENV["BUGSNAG_PROXY_HOST"] if ENV.include? "BUGSNAG_PROXY_HOST" conf.proxy_password = ENV["BUGSNAG_PROXY_PASSWORD"] if ENV.include? "BUGSNAG_PROXY_PASSWORD" diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb index baa9c5d3f..3c876f138 100644 --- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb @@ -7,14 +7,14 @@ config.auto_notify = ENV["BUGSNAG_AUTO_NOTIFY"] != "false" config.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" config.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" - config.auto_capture_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" + config.auto_track_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" config.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" config.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] == "true" config.meta_data_filters << 'filtered_parameter' if ENV["SQL_ONLY_BREADCRUMBS"] == "true" config.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.ignore! unless breadcrumb.meta_data[:event_name] == "sql.active_record" && breadcrumb.meta_data[:name] == "User Load" + breadcrumb.ignore! unless breadcrumb.metadata[:event_name] == "sql.active_record" && breadcrumb.metadata[:name] == "User Load" end end From c987ca7da9e86393f26b50234b20b8b665479ad3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 12 Aug 2021 15:38:27 +0100 Subject: [PATCH 09/63] Update changelog --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce18a806..bb01054f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,20 @@ Changelog ========= +## TBD + +### Deprecated + +* For consistency with Bugsnag notifiers for other languages, a number of methods have been deprecated in this release. The old options will be removed in the next major version | [#676](https://github.com/bugsnag/bugsnag-ruby/pull/676) + * The `notify_release_stages` configuration option has been deprecated in favour of `enabled_release_stages` + * The `auto_capture_sessions` and `track_sessions` configuration options have been deprecated in favour of `auto_track_sessions` + * The `Report` class has been deprecated in favour of the `Event` class + * The `Report#meta_data` attribute has been deprecated in favour of `Report#metadata` + * The `Breadcrumb#meta_data` attribute has been deprecated in favour of `Breadcrumb#metadata` + * The `Breadcrumb#name` attribute has been deprecated in favour of `Breadcrumb#message` + * The breadcrumb type constants in the `Bugsnag::Breadcrumbs` module has been deprecated in favour of the constants available in the `Bugsnag::BreadcrumbType` module + For example, `Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE` is now available as `Bugsnag::BreadcrumbType::ERROR` + ## v6.22.1 (11 August 2021) ### Fixes From 44c0494997b9cec5a1391c31fa2c23c63e4c2620 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 12 Aug 2021 17:11:23 +0000 Subject: [PATCH 10/63] Update CHANGELOG.md Co-authored-by: Delisa --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb01054f3..fc8579837 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Changelog * The `notify_release_stages` configuration option has been deprecated in favour of `enabled_release_stages` * The `auto_capture_sessions` and `track_sessions` configuration options have been deprecated in favour of `auto_track_sessions` * The `Report` class has been deprecated in favour of the `Event` class - * The `Report#meta_data` attribute has been deprecated in favour of `Report#metadata` + * The `Report#meta_data` attribute has been deprecated in favour of `Event#metadata` * The `Breadcrumb#meta_data` attribute has been deprecated in favour of `Breadcrumb#metadata` * The `Breadcrumb#name` attribute has been deprecated in favour of `Breadcrumb#message` * The breadcrumb type constants in the `Bugsnag::Breadcrumbs` module has been deprecated in favour of the constants available in the `Bugsnag::BreadcrumbType` module From e31e40832f9d7965f1461429ceec555dbba42005 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 16 Aug 2021 14:40:08 +0100 Subject: [PATCH 11/63] Don't start a session in a disabled release stage --- lib/bugsnag/session_tracker.rb | 8 ++------ spec/session_tracker_spec.rb | 22 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/bugsnag/session_tracker.rb b/lib/bugsnag/session_tracker.rb index 65c0f98a5..9a02bc008 100644 --- a/lib/bugsnag/session_tracker.rb +++ b/lib/bugsnag/session_tracker.rb @@ -35,7 +35,8 @@ def initialize # # This allows Bugsnag to track error rates for a release. def start_session - return unless Bugsnag.configuration.enable_sessions + return unless Bugsnag.configuration.enable_sessions && Bugsnag.configuration.should_notify_release_stage? + start_delivery_thread start_time = Time.now().utc().strftime('%Y-%m-%dT%H:%M:00') new_session = { @@ -106,11 +107,6 @@ def deliver(session_payload) return end - if !Bugsnag.configuration.should_notify_release_stage? - Bugsnag.configuration.debug("Not delivering sessions due to notify_release_stages :#{Bugsnag.configuration.notify_release_stages.inspect}") - return - end - body = { :notifier => { :name => Bugsnag::Report::NOTIFIER_NAME, diff --git a/spec/session_tracker_spec.rb b/spec/session_tracker_spec.rb index e208494c8..291262081 100644 --- a/spec/session_tracker_spec.rb +++ b/spec/session_tracker_spec.rb @@ -16,7 +16,7 @@ res.status = 202 res.body = "OK\n" end - Thread.new{ server.start } + Thread.new { server.start } end before(:each) do @@ -78,6 +78,26 @@ expect(Bugsnag.session_tracker.session_counts.size).to eq(0) end + it 'will not create sessions if the release stage is not enabled' do + Bugsnag.configure do |config| + config.enabled_release_stages = ['abc'] + config.release_stage = 'xyz' + end + + expect(Bugsnag.configuration.enable_sessions).to eq(true) + expect(Bugsnag.session_tracker.session_counts.size).to eq(0) + + Bugsnag.start_session + + expect(Bugsnag.session_tracker.session_counts.size).to eq(0) + + Bugsnag.configuration.release_stage = 'abc' + + Bugsnag.start_session + + expect(Bugsnag.session_tracker.session_counts.size).to eq(1) + end + it 'sends sessions when send_sessions is called' do Bugsnag.configure do |conf| conf.auto_capture_sessions = true From 86a9aa6ca18e9adeeff25b93831a388596459184 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 16 Aug 2021 16:36:58 +0100 Subject: [PATCH 12/63] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc8579837..5c48d967f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ Changelog ## TBD +### Fixes + +* Avoid starting session delivery thread when the current release stage is not enabled + | [#677](https://github.com/bugsnag/bugsnag-ruby/pull/677) + ### Deprecated * For consistency with Bugsnag notifiers for other languages, a number of methods have been deprecated in this release. The old options will be removed in the next major version | [#676](https://github.com/bugsnag/bugsnag-ruby/pull/676) From 3ad02fde77af8dae43a8fcfdf0ab66245a0498c0 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 18 Aug 2021 09:21:28 +0100 Subject: [PATCH 13/63] Deliver sessions every 10 seconds --- lib/bugsnag/session_tracker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bugsnag/session_tracker.rb b/lib/bugsnag/session_tracker.rb index 9a02bc008..810816f0d 100644 --- a/lib/bugsnag/session_tracker.rb +++ b/lib/bugsnag/session_tracker.rb @@ -84,7 +84,7 @@ def start_delivery_thread end end end - @delivery_thread = Concurrent::TimerTask.execute(execution_interval: 30) do + @delivery_thread = Concurrent::TimerTask.execute(execution_interval: 10) do if @session_counts.size > 0 send_sessions end From abe01c7096b900e81e8e29dad50eeda72bf95239 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 18 Aug 2021 09:22:41 +0100 Subject: [PATCH 14/63] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c48d967f..5f1387282 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ Changelog ## TBD +### Enhancements + +* Sessions will now be delivered every 10 seconds, instead of every 30 seconds + | [#680](https://github.com/bugsnag/bugsnag-ruby/pull/680) + ### Fixes * Avoid starting session delivery thread when the current release stage is not enabled From 9bf9dd80229b72986f6654786afea10f97a4057f Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 18 Aug 2021 12:14:50 +0100 Subject: [PATCH 15/63] Refactor logging tests to use a real Logger --- spec/configuration_spec.rb | 125 +++++++++++++------------------------ 1 file changed, 43 insertions(+), 82 deletions(-) diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index fc81b84a3..598a90994 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -246,40 +246,21 @@ end describe "logger" do - class TestLogger - attr_accessor :logs - - def initialize - @logs = [] - end - - def log(level, name, &block) - message = block.call - @logs << { - :level => level, - :name => name, - :message => message - } + before do + @output = StringIO.new + @formatter = proc do |severity, _datetime, progname, message| + "#{progname} #{severity}: #{message}" end - def info(name, &block) - log('info', name, &block) - end + logger = Logger.new(@output) + logger.formatter = @formatter - def warn(name, &block) - log('warning', name, &block) - end - - def debug(name, &block) - log('debug', name, &block) - end + Bugsnag.configuration.logger = logger end - before do - @logger = TestLogger.new - Bugsnag.configure do |bugsnag| - bugsnag.logger = @logger - end + def output_lines + @output.rewind # always read from the start of output + @output.readlines.map(&:chomp) # old rubies don't support `readlines(chomp: true)` end context "using configure" do @@ -287,35 +268,31 @@ def debug(name, &block) Bugsnag.configuration.api_key = nil Bugsnag.instance_variable_set("@key_warning", nil) ENV['BUGSNAG_API_KEY'] = nil - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty end context "API key is not specified" do it "skips logging a warning if validate_api_key is false" do Bugsnag.configure(false) - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty end it "logs a warning by default" do Bugsnag.configure - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "No valid API key has been set, notifications will not be sent" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq( + '[Bugsnag] WARN: No valid API key has been set, notifications will not be sent' + ) end it "logs a warning if validate_api_key is true" do Bugsnag.configure(true) - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "No valid API key has been set, notifications will not be sent" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq( + '[Bugsnag] WARN: No valid API key has been set, notifications will not be sent' + ) end end @@ -324,64 +301,48 @@ def debug(name, &block) Bugsnag.configure do |config| config.api_key = 'd57a2472bd130ac0ab0f52715bbdc600' end - expect(@logger.logs.size).to eq(0) + + expect(output_lines).to be_empty end it "logs a warning if the configured API key is invalid" do Bugsnag.configure do |config| config.api_key = 'WARNING: not a real key' end - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "No valid API key has been set, notifications will not be sent" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq( + '[Bugsnag] WARN: No valid API key has been set, notifications will not be sent' + ) end end end it "should log info messages to the set logger" do - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty + Bugsnag.configuration.info("Info message") - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "info", - :name => "[Bugsnag]", - :message => "Info message" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] INFO: Info message') end it "should log warning messages to the set logger" do - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty + Bugsnag.configuration.warn("Warning message") - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "Warning message" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] WARN: Warning message') end it "should log debug messages to the set logger" do - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty + Bugsnag.configuration.debug("Debug message") - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "debug", - :name => "[Bugsnag]", - :message => "Debug message" - }) - end - after do - Bugsnag.configure do |bugsnag| - bugsnag.logger = Logger.new(StringIO.new) - end + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] DEBUG: Debug message') end end From 7b4b57c46bdc9c6b085327b07bf4cc4e96083047 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 18 Aug 2021 12:15:49 +0100 Subject: [PATCH 16/63] Add Configuration#error to log error messages --- lib/bugsnag/configuration.rb | 12 ++++++++++-- spec/configuration_spec.rb | 9 +++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 9594f75a6..5c90227a7 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -392,15 +392,23 @@ def info(message) ## # Logs a warning level message # - # @param (see info) + # @param message [String, #to_s] The message to log def warn(message) logger.warn(PROG_NAME) { message } end + ## + # Logs an error level message + # + # @param message [String, #to_s] The message to log + def error(message) + logger.error(PROG_NAME) { message } + end + ## # Logs a debug level message # - # @param (see info) + # @param message [String, #to_s] The message to log def debug(message) logger.debug(PROG_NAME) { message } end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 598a90994..cabe2f615 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -336,6 +336,15 @@ def output_lines expect(output_lines.first).to eq('[Bugsnag] WARN: Warning message') end + it "should log error messages to the set logger" do + expect(output_lines).to be_empty + + Bugsnag.configuration.error("Error message") + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] ERROR: Error message') + end + it "should log debug messages to the set logger" do expect(output_lines).to be_empty From 52f67e73460195194f93d43d8cd806ed4c4cf8d2 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 18 Aug 2021 12:16:11 +0100 Subject: [PATCH 17/63] Log delivery failures at error level This should help to differentiate errors that prevent things from being delivered from errors that we can continue from, e.g. errors raised in middleware --- lib/bugsnag/delivery/synchronous.rb | 4 ++-- lib/bugsnag/delivery/thread_queue.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/bugsnag/delivery/synchronous.rb b/lib/bugsnag/delivery/synchronous.rb index b5444ce5b..b0da7fde5 100644 --- a/lib/bugsnag/delivery/synchronous.rb +++ b/lib/bugsnag/delivery/synchronous.rb @@ -18,8 +18,8 @@ def deliver(url, body, configuration, options={}) # KLUDGE: Since we don't re-raise http exceptions, this breaks rspec raise if e.class.to_s == "RSpec::Expectations::ExpectationNotMetError" - configuration.warn("Notification to #{url} failed, #{e.inspect}") - configuration.warn(e.backtrace) + configuration.error("Unable to send information to Bugsnag (#{url}), #{e.inspect}") + configuration.error(e.backtrace) end end diff --git a/lib/bugsnag/delivery/thread_queue.rb b/lib/bugsnag/delivery/thread_queue.rb index 50423cedb..2b82b2212 100644 --- a/lib/bugsnag/delivery/thread_queue.rb +++ b/lib/bugsnag/delivery/thread_queue.rb @@ -31,8 +31,8 @@ def serialize_and_deliver(url, get_payload, configuration, options={}) begin payload = get_payload.call rescue StandardError => e - configuration.warn("Notification to #{url} failed, #{e.inspect}") - configuration.warn(e.backtrace) + configuration.error("Unable to send information to Bugsnag (#{url}), #{e.inspect}") + configuration.error(e.backtrace) end Synchronous.deliver(url, payload, configuration, options) unless payload.nil? From 693a6662b4a294ce9013641938a2985fde2481f6 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 18 Aug 2021 12:18:50 +0100 Subject: [PATCH 18/63] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f1387282..737d0307d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ Changelog * Sessions will now be delivered every 10 seconds, instead of every 30 seconds | [#680](https://github.com/bugsnag/bugsnag-ruby/pull/680) +* Log errors that prevent delivery at `ERROR` level + | [#681](https://github.com/bugsnag/bugsnag-ruby/pull/681) ### Fixes From a1daac7c8e60ca626fdf2fbe224d359d74ec74d7 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 19 Aug 2021 13:42:05 +0100 Subject: [PATCH 19/63] Add tests for a job that does not raise --- .../app/app/jobs/working_job.rb | 23 +++++++++++ features/rails_features/integrations.feature | 40 +++++++++++++++++-- 2 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 features/fixtures/rails_integrations/app/app/jobs/working_job.rb diff --git a/features/fixtures/rails_integrations/app/app/jobs/working_job.rb b/features/fixtures/rails_integrations/app/app/jobs/working_job.rb new file mode 100644 index 000000000..3a191832c --- /dev/null +++ b/features/fixtures/rails_integrations/app/app/jobs/working_job.rb @@ -0,0 +1,23 @@ +class WorkingJob < ApplicationJob + self.queue_adapter = ENV['ACTIVE_JOB_QUEUE_ADAPTER'].to_sym + + def perform + do_stuff + + more_stuff + + success! + end + + def do_stuff + 'beep boop' + end + + def more_stuff + 'boop beep' + end + + def success! + 'yay' + end +end diff --git a/features/rails_features/integrations.feature b/features/rails_features/integrations.feature index 093d7ab72..6743a0bc6 100644 --- a/features/rails_features/integrations.feature +++ b/features/rails_features/integrations.feature @@ -128,7 +128,7 @@ Scenario: Sidekiq And the event "metaData.sidekiq.queue" equals "default" @rails_integrations -Scenario: Using Sidekiq as the Active Job queue adapter +Scenario: Using Sidekiq as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "sidekiq" And I run "bundle exec sidekiq" in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -150,7 +150,7 @@ Scenario: Using Sidekiq as the Active Job queue adapter And the event "metaData.sidekiq.queue" equals "default" @rails_integrations -Scenario: Using Rescue as the Active Job queue adapter +Scenario: Using Rescue as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "resque" And I run "bundle exec rake resque:work" in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -171,7 +171,7 @@ Scenario: Using Rescue as the Active Job queue adapter And the event "metaData.payload.args.0.queue_name" equals "default" @rails_integrations -Scenario: Using Que as the Active Job queue adapter +Scenario: Using Que as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "que" And I run "bundle exec que -q default ./config/environment.rb" in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -192,7 +192,7 @@ Scenario: Using Que as the Active Job queue adapter And the event "metaData.job.queue" equals "default" @rails_integrations -Scenario: Using Delayed Job as the Active Job queue adapter +Scenario: Using Delayed Job as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "delayed_job" And I run the "jobs:work" rake task in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -211,3 +211,35 @@ Scenario: Using Delayed Job as the Active Job queue adapter And the event "metaData.job.payload.arguments.0" equals 1 And the event "metaData.job.payload.arguments.1.yes" is true And the event "metaData.job.queue" equals "default" + +@rails_integrations +Scenario: Using Sidekiq as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "sidekiq" + And I run "bundle exec sidekiq" in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests + +@rails_integrations +Scenario: Using Rescue as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "resque" + And I run "bundle exec rake resque:work" in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests + +@rails_integrations +Scenario: Using Que as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "que" + And I run "bundle exec que -q default ./config/environment.rb" in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests + +@rails_integrations +Scenario: Using Delayed Job as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "delayed_job" + And I run the "jobs:work" rake task in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests From f4602fffbc37efd152962b813f02bb79afd1e181 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 19 Aug 2021 13:43:18 +0100 Subject: [PATCH 20/63] Allow rails_integration fixture to be run manually --- features/fixtures/docker-compose.yml | 2 ++ features/fixtures/rails_integrations/app/config/application.rb | 2 +- .../rails_integrations/app/config/environments/development.rb | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/features/fixtures/docker-compose.yml b/features/fixtures/docker-compose.yml index 953329ba2..0b6957592 100644 --- a/features/fixtures/docker-compose.yml +++ b/features/fixtures/docker-compose.yml @@ -261,6 +261,8 @@ services: - RUN_AT_EXIT_HOOKS - ACTIVE_JOB_QUEUE_ADAPTER restart: "no" + ports: + - "3000:3000" sidekiq: build: diff --git a/features/fixtures/rails_integrations/app/config/application.rb b/features/fixtures/rails_integrations/app/config/application.rb index 66b2dd2d0..f28492e96 100644 --- a/features/fixtures/rails_integrations/app/config/application.rb +++ b/features/fixtures/rails_integrations/app/config/application.rb @@ -32,7 +32,7 @@ class Application < Rails::Application # Only loads a smaller set of middleware suitable for API only apps. # Middleware like session, flash, cookies can be added back manually. # Skip views, helpers and assets when generating a new resource. - config.api_only = true + # config.api_only = true config.autoload_paths << Rails.root.join('app/workers') end diff --git a/features/fixtures/rails_integrations/app/config/environments/development.rb b/features/fixtures/rails_integrations/app/config/environments/development.rb index e0f5c3996..11cc198eb 100644 --- a/features/fixtures/rails_integrations/app/config/environments/development.rb +++ b/features/fixtures/rails_integrations/app/config/environments/development.rb @@ -44,4 +44,6 @@ # Use an evented file watcher to asynchronously detect changes in source code, # routes, locales, etc. This feature depends on the listen gem. config.file_watcher = ActiveSupport::EventedFileUpdateChecker + + config.hosts = [] end From 448fb374759f11fd803c80300c8feac52b927cd3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 19 Aug 2021 13:43:53 +0100 Subject: [PATCH 21/63] Add routes & a controller to queue jobs --- .../app/app/controllers/job_controller.rb | 13 +++++++++++++ .../rails_integrations/app/config/routes.rb | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 features/fixtures/rails_integrations/app/app/controllers/job_controller.rb diff --git a/features/fixtures/rails_integrations/app/app/controllers/job_controller.rb b/features/fixtures/rails_integrations/app/app/controllers/job_controller.rb new file mode 100644 index 000000000..a480afd92 --- /dev/null +++ b/features/fixtures/rails_integrations/app/app/controllers/job_controller.rb @@ -0,0 +1,13 @@ +class JobController < ApplicationController + def working + WorkingJob.perform_later + + render json: { result: 'queued WorkingJob!' } + end + + def unhandled + UnhandledJob.perform_later + + render json: { result: 'queued UnhandledJob!' } + end +end diff --git a/features/fixtures/rails_integrations/app/config/routes.rb b/features/fixtures/rails_integrations/app/config/routes.rb index c06383a17..6ffc2be67 100644 --- a/features/fixtures/rails_integrations/app/config/routes.rb +++ b/features/fixtures/rails_integrations/app/config/routes.rb @@ -1,3 +1,4 @@ Rails.application.routes.draw do - # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html + get '/job/working', 'job#working' + get '/job/unhandled', 'job#unhandled' end From 1084ebb9cbad3e059390b160faed45abdd71a79e Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 19 Aug 2021 16:04:35 +0100 Subject: [PATCH 22/63] Add missing deprecation to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 737d0307d..e5000aca8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Changelog * For consistency with Bugsnag notifiers for other languages, a number of methods have been deprecated in this release. The old options will be removed in the next major version | [#676](https://github.com/bugsnag/bugsnag-ruby/pull/676) * The `notify_release_stages` configuration option has been deprecated in favour of `enabled_release_stages` * The `auto_capture_sessions` and `track_sessions` configuration options have been deprecated in favour of `auto_track_sessions` + * The `enabled_automatic_breadcrumb_types` configuration option has been deprecated in favour of `enabled_breadcrumb_types` * The `Report` class has been deprecated in favour of the `Event` class * The `Report#meta_data` attribute has been deprecated in favour of `Event#metadata` * The `Breadcrumb#meta_data` attribute has been deprecated in favour of `Breadcrumb#metadata` From f4030f9954558ec759820b855795da9ce04ea736 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 20 Aug 2021 13:11:14 +0100 Subject: [PATCH 23/63] Add OnBreadcrumbCallbackList class This manages and calls any registered on_breadcrumb callbacks --- .../on_breadcrumb_callback_list.rb | 44 ++++ .../on_breadcrumb_callback_list_spec.rb | 231 ++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb create mode 100644 spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb diff --git a/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb b/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb new file mode 100644 index 000000000..5f345fffa --- /dev/null +++ b/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb @@ -0,0 +1,44 @@ +require "set" + +module Bugsnag::Breadcrumbs + class OnBreadcrumbCallbackList + def initialize + @callbacks = Set.new + @mutex = Mutex.new + end + + ## + # @param callback [Proc, Method, #call] + # @return [void] + def add(callback) + @mutex.synchronize do + @callbacks.add(callback) + end + end + + ## + # @param callback [Proc, Method, #call] + # @return [void] + def remove(callback) + @mutex.synchronize do + @callbacks.delete(callback) + end + end + + ## + # @param breadcrumb [Breadcrumb] + # @return [void] + def call(breadcrumb) + @callbacks.each do |callback| + should_continue = callback.call(breadcrumb) + + # only stop if should_continue is explicity 'false' to allow callbacks + # to return 'nil' + if should_continue == false + breadcrumb.ignore! + break + end + end + end + end +end diff --git a/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb b/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb new file mode 100644 index 000000000..87ab1f335 --- /dev/null +++ b/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb @@ -0,0 +1,231 @@ +require "spec_helper" + +require "bugsnag/breadcrumbs/on_breadcrumb_callback_list" + +RSpec.describe Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList do + it "can add callbacks to its list" do + callback = spy('callback') + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list.add(callback) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(callback).to have_received(:call).with(breadcrumb) + expect(breadcrumb.ignore?).to be(false) + end + + it "can remove callbacks to its list" do + callback = spy('callback') + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list.add(callback) + list.remove(callback) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(callback).not_to have_received(:call) + expect(breadcrumb.ignore?).to be(false) + end + + it "won't remove a callback that isn't the same instance" do + callback1 = spy('callback1') + callback2 = spy('callback2') + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + # note: adding callback1 but removing callback2 + list.add(callback1) + list.remove(callback2) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(callback1).to have_received(:call).with(breadcrumb) + expect(callback2).not_to have_received(:call) + expect(breadcrumb.ignore?).to be(false) + end + + it "calls callbacks in the order they were added" do + calls = [] + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(proc { calls << 1 }) + list.add(proc { calls << 2 }) + list.add(proc { calls << 3 }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(calls).to eq([1, 2, 3]) + expect(breadcrumb.ignore?).to be(false) + end + + it "ignores the breadcrumb if a callback returns false" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(proc { false }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.ignore?).to be(true) + end + + it "does not ignore the breadcrumb if a callback returns nil" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(proc { nil }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.ignore?).to be(false) + end + + it "supports Method objects as callbacks" do + class ArbitraryClassMethod + def self.arbitrary_name(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class ArbitraryInstanceMethod + def arbitrary_name(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(ArbitraryClassMethod.method(:arbitrary_name)) + + xyz = ArbitraryInstanceMethod.new + list.add(xyz.method(:arbitrary_name)) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({ abc: 123, xyz: 789 }) + end + + it "allows removing Method objects as callbacks" do + class ArbitraryClassMethod + def self.arbitrary_name(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class ArbitraryInstanceMethod + def arbitrary_name(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(ArbitraryClassMethod.method(:arbitrary_name)) + list.remove(ArbitraryClassMethod.method(:arbitrary_name)) + + xyz = ArbitraryInstanceMethod.new + list.add(xyz.method(:arbitrary_name)) + list.remove(xyz.method(:arbitrary_name)) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({}) + end + + it "supports arbitrary objects that respond to #call as callbacks" do + class RespondsToCallAsClassMethod + def self.call(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class RespondsToCallAsInstanceMethod + def call(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(RespondsToCallAsClassMethod) + list.add(RespondsToCallAsInstanceMethod.new) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({ abc: 123, xyz: 789 }) + end + + it "supports removing arbitrary objects that respond to #call as callbacks" do + class RespondsToCallAsClassMethod + def self.call(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class RespondsToCallAsInstanceMethod + def call(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(RespondsToCallAsClassMethod) + list.remove(RespondsToCallAsClassMethod) + + instance = RespondsToCallAsInstanceMethod.new + list.add(instance) + list.remove(instance) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({}) + end + + it "works when accessed concurrently" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + + list.add(proc do |breadcrumb| + breadcrumb.metadata[:numbers] = [] + end) + + NUMBER_OF_THREADS = 20 + + threads = NUMBER_OF_THREADS.times.map do |i| + Thread.new do + list.add(proc do |breadcrumb| + breadcrumb.metadata[:numbers].push(i) + end) + end + end + + threads.shuffle.each(&:join) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + list.call(breadcrumb) + + # sort the numbers as they will be out of order but that doesn't matter as + # long as every number is present + expect(breadcrumb.metadata[:numbers].sort).to eq((0...NUMBER_OF_THREADS).to_a) + end +end From c4de39f6a56b0f690c8adff1eee9a1159ae7bb86 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 20 Aug 2021 13:19:18 +0100 Subject: [PATCH 24/63] Handle errors raised in on_breadcrumb callbacks --- .../on_breadcrumb_callback_list.rb | 10 ++- .../on_breadcrumb_callback_list_spec.rb | 67 ++++++++++++++++--- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb b/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb index 5f345fffa..df7cc5add 100644 --- a/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb +++ b/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb @@ -2,9 +2,10 @@ module Bugsnag::Breadcrumbs class OnBreadcrumbCallbackList - def initialize + def initialize(configuration) @callbacks = Set.new @mutex = Mutex.new + @configuration = configuration end ## @@ -30,7 +31,12 @@ def remove(callback) # @return [void] def call(breadcrumb) @callbacks.each do |callback| - should_continue = callback.call(breadcrumb) + begin + should_continue = callback.call(breadcrumb) + rescue StandardError => e + @configuration.warn("Error occurred in on_breadcrumb callback: '#{e}'") + @configuration.warn("on_breadcrumb callback stacktrace: #{e.backtrace.inspect}") + end # only stop if should_continue is explicity 'false' to allow callbacks # to return 'nil' diff --git a/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb b/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb index 87ab1f335..6011124ab 100644 --- a/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb +++ b/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb @@ -6,7 +6,7 @@ it "can add callbacks to its list" do callback = spy('callback') - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(callback) breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) @@ -20,7 +20,7 @@ it "can remove callbacks to its list" do callback = spy('callback') - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(callback) list.remove(callback) @@ -36,7 +36,7 @@ callback1 = spy('callback1') callback2 = spy('callback2') - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) # note: adding callback1 but removing callback2 list.add(callback1) @@ -54,7 +54,7 @@ it "calls callbacks in the order they were added" do calls = [] - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(proc { calls << 1 }) list.add(proc { calls << 2 }) @@ -69,7 +69,7 @@ end it "ignores the breadcrumb if a callback returns false" do - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(proc { false }) @@ -81,7 +81,7 @@ end it "does not ignore the breadcrumb if a callback returns nil" do - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(proc { nil }) @@ -105,7 +105,7 @@ def arbitrary_name(breadcrumb) end end - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(ArbitraryClassMethod.method(:arbitrary_name)) @@ -132,7 +132,7 @@ def arbitrary_name(breadcrumb) end end - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(ArbitraryClassMethod.method(:arbitrary_name)) list.remove(ArbitraryClassMethod.method(:arbitrary_name)) @@ -161,7 +161,7 @@ def call(breadcrumb) end end - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(RespondsToCallAsClassMethod) list.add(RespondsToCallAsInstanceMethod.new) @@ -186,7 +186,7 @@ def call(breadcrumb) end end - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(RespondsToCallAsClassMethod) list.remove(RespondsToCallAsClassMethod) @@ -203,7 +203,7 @@ def call(breadcrumb) end it "works when accessed concurrently" do - list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) list.add(proc do |breadcrumb| breadcrumb.metadata[:numbers] = [] @@ -228,4 +228,49 @@ def call(breadcrumb) # long as every number is present expect(breadcrumb.metadata[:numbers].sort).to eq((0...NUMBER_OF_THREADS).to_a) end + + it "logs errors thrown in callbacks" do + logger = spy('logger') + Bugsnag.configuration.logger = logger + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + error = RuntimeError.new('Oh no!') + + list.add(proc { raise error }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.ignore?).to be(false) + + message_index = 0 + expected_messages = [ + /^Error occurred in on_breadcrumb callback: 'Oh no!'$/, + /^on_breadcrumb 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 "calls subsequent callbacks after an error is raised" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + calls = [] + + list.add(proc { calls << 1 }) + list.add(proc { calls << 2 }) + list.add(proc { raise 'ab' }) + list.add(proc { calls << 4 }) + list.add(proc { calls << 5 }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(calls).to eq([1, 2, 4, 5]) + expect(breadcrumb.ignore?).to be(false) + end end From 21f459bae6240340853979d191f97fd0cd9dc182 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 20 Aug 2021 13:43:17 +0100 Subject: [PATCH 25/63] Move before_breadcrumb tests to a new describe --- spec/bugsnag_spec.rb | 156 ++++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 77 deletions(-) diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 15a16065a..dcd373c0b 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -323,97 +323,99 @@ module Kernel }) end - it "runs callbacks before leaving" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.meta_data = { - :callback => true - } - end - Bugsnag.leave_breadcrumb("TestName") - expect(breadcrumbs.to_a.size).to eq(1) - expect(breadcrumbs.first.to_h).to match({ - :name => "TestName", - :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, - :metaData => { - :callback => true - }, - :timestamp => match(timestamp_regex) - }) - end - - it "validates after callbacks" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.meta_data = { - :int => 1, - :array => [1, 2, 3], - :hash => { - :a => 1, - :b => 2 + describe "before_breadcrumb_callbacks" do + it "runs callbacks before leaving" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.meta_data = { + :callback => true } - } - breadcrumb.type = "Not a real type" - breadcrumb.name = "123123123123123123123123123123456456456456456" + end + Bugsnag.leave_breadcrumb("TestName") + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + :name => "TestName", + :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, + :metaData => { + :callback => true + }, + :timestamp => match(timestamp_regex) + }) end - Bugsnag.leave_breadcrumb("TestName") - - expect(breadcrumbs.to_a.size).to eq(1) - expect(breadcrumbs.first.to_h).to match({ - :name => "123123123123123123123123123123456456456456456", - :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, - :metaData => { - :int => 1, - :array => [1, 2, 3], - :hash => { - :a => 1, - :b => 2 + it "validates after callbacks" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.meta_data = { + :int => 1, + :array => [1, 2, 3], + :hash => { + :a => 1, + :b => 2 + } } - }, - :timestamp => match(timestamp_regex) - }) - end + breadcrumb.type = "Not a real type" + breadcrumb.name = "123123123123123123123123123123456456456456456" + end - it "doesn't add when ignored by the validator" do - Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] - Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) - expect(breadcrumbs.to_a.size).to eq(0) - end + Bugsnag.leave_breadcrumb("TestName") + + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + :name => "123123123123123123123123123123456456456456456", + :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, + :metaData => { + :int => 1, + :array => [1, 2, 3], + :hash => { + :a => 1, + :b => 2 + } + }, + :timestamp => match(timestamp_regex) + }) + end - it "doesn't add if ignored in a callback" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.ignore! + it "doesn't add when ignored by the validator" do + Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) + expect(breadcrumbs.to_a.size).to eq(0) end - Bugsnag.leave_breadcrumb("TestName") - expect(breadcrumbs.to_a.size).to eq(0) - end - it "doesn't add when ignored after the callbacks" do - Bugsnag.configuration.enabled_automatic_breadcrumb_types = [ - Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE - ] - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.type = Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE + it "doesn't add if ignored in a callback" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.ignore! + end + Bugsnag.leave_breadcrumb("TestName") + expect(breadcrumbs.to_a.size).to eq(0) end - Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, :auto) - expect(breadcrumbs.to_a.size).to eq(0) - end - it "doesn't call callbacks if ignored early" do - Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - fail "This shouldn't be called" + it "doesn't add when ignored after the callbacks" do + Bugsnag.configuration.enabled_automatic_breadcrumb_types = [ + Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE + ] + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.type = Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE + end + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, :auto) + expect(breadcrumbs.to_a.size).to eq(0) end - Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) - end - it "doesn't continue to call callbacks if ignored in them" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.ignore! + it "doesn't call callbacks if ignored early" do + Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + fail "This shouldn't be called" + end + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) end - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - fail "This shouldn't be called" + + it "doesn't continue to call callbacks if ignored in them" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.ignore! + end + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + fail "This shouldn't be called" + end + Bugsnag.leave_breadcrumb("TestName") end - Bugsnag.leave_breadcrumb("TestName") end end From 99d7729fd82ea94fa8b57eb2b747afb5bae8475e Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 20 Aug 2021 13:52:39 +0100 Subject: [PATCH 26/63] Add add and remove on_breadcrumb methods --- lib/bugsnag.rb | 27 +++++++++++++++++++++++++++ lib/bugsnag/configuration.rb | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index f4b1121ad..e8d7ff1a6 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -295,6 +295,33 @@ def remove_on_error(callback) configuration.remove_on_error(callback) end + ## + # Add the given callback to the list of on_breadcrumb callbacks + # + # The on_breadcrumb callbacks will be called when a breadcrumb is left and + # are passed the {Breadcrumbs::Breadcrumb Breadcrumb} object + # + # Returning false from an on_breadcrumb callback will cause the breadcrumb + # to be ignored and will prevent any remaining callbacks from being called + # + # @param callback [Proc, Method, #call] + # @return [void] + def add_on_breadcrumb(callback) + configuration.add_on_breadcrumb(callback) + end + + ## + # Remove the given callback from the list of on_breadcrumb callbacks + # + # Note that this must be the same instance that was passed to + # {add_on_breadcrumb}, otherwise it will not be removed + # + # @param callback [Proc, Method, #call] + # @return [void] + def remove_on_breadcrumb(callback) + configuration.remove_on_breadcrumb(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 9594f75a6..438f20b65 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -12,6 +12,7 @@ require "bugsnag/middleware/breadcrumbs" require "bugsnag/utility/circular_buffer" require "bugsnag/breadcrumbs/breadcrumbs" +require "bugsnag/breadcrumbs/on_breadcrumb_callback_list" module Bugsnag class Configuration @@ -148,6 +149,11 @@ class Configuration # @return [Array] attr_reader :scopes_to_filter + # Expose on_breadcrumb_callbacks internally for Bugsnag.leave_breadcrumb + # @api private + # @return [Breadcrumbs::OnBreadcrumbCallbackList] + attr_reader :on_breadcrumb_callbacks + API_KEY_REGEX = /[0-9a-f]{32}/i THREAD_LOCAL_NAME = "bugsnag_req_data" @@ -196,6 +202,7 @@ def initialize # All valid breadcrumb types should be allowable initially self.enabled_automatic_breadcrumb_types = Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES.dup self.before_breadcrumb_callbacks = [] + @on_breadcrumb_callbacks = Breadcrumbs::OnBreadcrumbCallbackList.new(self) # Store max_breadcrumbs here instead of outputting breadcrumbs.max_items # to avoid infinite recursion when creating breadcrumb buffer @@ -506,6 +513,33 @@ def remove_on_error(callback) middleware.remove(callback) end + ## + # Add the given callback to the list of on_breadcrumb callbacks + # + # The on_breadcrumb callbacks will be called when a breadcrumb is left and + # are passed the {Breadcrumbs::Breadcrumb Breadcrumb} object + # + # Returning false from an on_breadcrumb callback will cause the breadcrumb + # to be ignored and will prevent any remaining callbacks from being called + # + # @param callback [Proc, Method, #call] + # @return [void] + def add_on_breadcrumb(callback) + @on_breadcrumb_callbacks.add(callback) + end + + ## + # Remove the given callback from the list of on_breadcrumb callbacks + # + # Note that this must be the same instance that was passed to + # {add_on_breadcrumb}, otherwise it will not be removed + # + # @param callback [Proc, Method, #call] + # @return [void] + def remove_on_breadcrumb(callback) + @on_breadcrumb_callbacks.remove(callback) + end + # TODO: These methods can be a simple attr_accessor when they replace the # methods they are aliasing # NOTE: they are not aliases as YARD doesn't allow documenting the non-alias From 056d110009a89812ad573dedb9c31f072b0a4499 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 20 Aug 2021 13:53:19 +0100 Subject: [PATCH 27/63] Call on_breadcrumb callbacks in leave_breadcrumb --- lib/bugsnag.rb | 6 ++- spec/bugsnag_spec.rb | 89 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index e8d7ff1a6..23ba1029e 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -252,7 +252,7 @@ def leave_breadcrumb(name, meta_data={}, type=Bugsnag::Breadcrumbs::MANUAL_BREAD # Skip if it's already invalid return if breadcrumb.ignore? - # Run callbacks + # Run before_breadcrumb_callbacks configuration.before_breadcrumb_callbacks.each do |c| c.arity > 0 ? c.call(breadcrumb) : c.call break if breadcrumb.ignore? @@ -261,6 +261,10 @@ def leave_breadcrumb(name, meta_data={}, type=Bugsnag::Breadcrumbs::MANUAL_BREAD # Return early if ignored return if breadcrumb.ignore? + # Run on_breadcrumb callbacks + configuration.on_breadcrumb_callbacks.call(breadcrumb) + return if breadcrumb.ignore? + # Validate again in case of callback alteration validator.validate(breadcrumb) diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index dcd373c0b..a09520c83 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -417,6 +417,95 @@ module Kernel Bugsnag.leave_breadcrumb("TestName") end end + + describe "on_breadcrumb callbacks" do + it "runs callbacks when a breadcrumb is left" do + Bugsnag.add_on_breadcrumb(proc do |breadcrumb| + breadcrumb.metadata = { callback: true } + end) + + Bugsnag.leave_breadcrumb("TestName") + + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + name: "TestName", + type: Bugsnag::BreadcrumbType::MANUAL, + metaData: { callback: true }, + timestamp: match(timestamp_regex) + }) + end + + it "validates any changes made in a callback" do + Bugsnag.add_on_breadcrumb(proc do |breadcrumb| + breadcrumb.metadata = { abc: 123, xyz: { a: 1, b: 2 } } + + breadcrumb.type = "Not a real type" + breadcrumb.name = "123123123123123123123123123123456456456456456" + end) + + Bugsnag.leave_breadcrumb("TestName") + + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + name: "123123123123123123123123123123456456456456456", + type: Bugsnag::BreadcrumbType::MANUAL, + metaData: { abc: 123, xyz: { a: 1, b: 2 } }, + timestamp: match(timestamp_regex) + }) + end + + it "doesn't add the breadcrumb when ignored due to enabled_breadcrumb_types" do + Bugsnag.configure do |config| + config.enabled_breadcrumb_types = [Bugsnag::BreadcrumbType::MANUAL] + + config.add_on_breadcrumb(proc do |breadcrumb| + breadcrumb.type = Bugsnag::BreadcrumbType::ERROR + end) + end + + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::BreadcrumbType::MANUAL, :auto) + + expect(breadcrumbs.to_a).to be_empty + end + + it "stops calling callbacks if the breadcrumb is ignored in them" do + callback1 = spy('callback1') + callback2 = spy('callback2') + + Bugsnag.configure do |config| + config.add_on_breadcrumb(callback1) + config.add_on_breadcrumb(proc { false }) + config.add_on_breadcrumb(callback2) + end + + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::BreadcrumbType::ERROR, :auto) + + expect(callback1).to have_received(:call) + expect(callback2).not_to have_received(:call) + end + + it "continues calling callbacks after a callback raises" do + callback1 = spy('callback1') + callback2 = spy('callback2') + + Bugsnag.configure do |config| + config.add_on_breadcrumb(callback1) + config.add_on_breadcrumb(proc { raise 'uh oh' }) + config.add_on_breadcrumb(callback2) + end + + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::BreadcrumbType::ERROR, :auto) + + expect(callback1).to have_received(:call) + expect(callback2).to have_received(:call) + expect(breadcrumbs.to_a.first.to_h).to match({ + name: "TestName", + type: Bugsnag::BreadcrumbType::ERROR, + metaData: {}, + timestamp: match(timestamp_regex) + }) + end + end end describe "request headers" do From 21f20fc7ba16d31513d9afd95c067850c64bd951 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 19 Aug 2021 13:49:21 +0100 Subject: [PATCH 28/63] Add script to run the integrations fixture --- features/fixtures/run-ruby-integrations | 120 ++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100755 features/fixtures/run-ruby-integrations diff --git a/features/fixtures/run-ruby-integrations b/features/fixtures/run-ruby-integrations new file mode 100755 index 000000000..46a8aea47 --- /dev/null +++ b/features/fixtures/run-ruby-integrations @@ -0,0 +1,120 @@ +#!/usr/bin/env ruby + +require "socket" +require "timeout" +require "pathname" + +DOCKER_DIRECTORY = Pathname.new(__dir__) +ROOT_DIRECTORY = DOCKER_DIRECTORY + "../.." +FIXTURE_DIRECTORY = DOCKER_DIRECTORY + "rails_integrations" + +raise "Fixture directory does not exist at: '#{FIXTURE_DIRECTORY}'" unless FIXTURE_DIRECTORY.exist? + +QUEUE_LIBRARY_COMMANDS = { + sidekiq: 'sidekiq', + resque: 'rake resque:work', + que: 'que --log-level debug --queue-name "*" ./config/environment.rb', + delayed_job: 'rake jobs:work', +} + +QUEUE_LIBRARY = ARGV.fetch(0, :sidekiq).to_sym + +raise "Invalid queue libarary '#{QUEUE_LIBRARY}'" unless QUEUE_LIBRARY_COMMANDS.key?(QUEUE_LIBRARY) + +def wait_for_port(port, max_attempts: 60, seconds_between_attempts: 1) + is_open = false + attempts = 0 + + until is_open || attempts > max_attempts + begin + attempts += 1 + + # add a timeout as sometimes TCPSocket will wait for ages before realising + # it can't connect - this is a local port so should be """instant""" + Timeout.timeout(2) do + TCPSocket.new("127.0.0.1", port).close + + # success! + is_open = true + end + rescue Timeout::Error, Errno::ECONNREFUSED, Errno::EHOSTUNREACH + # ignore timeouts and errors from the port being closed + + # wait between attempts to give the port some time to open + sleep(seconds_between_attempts) + end + end + + raise "Port #{port} not open in time!" unless is_open +end + +def run_in_shell(command, env: {}, background: false) + puts "running '#{command}' with env: #{env}, background: #{background}" + + if background + spawn(env, command) + else + system(env, command, exception: true) + end +end + +def run_docker_command(command, env: {}, **kwargs) + default_env = { "NETWORK_NAME" => "notwerk-norm", "ACTIVE_JOB_QUEUE_ADAPTER" => QUEUE_LIBRARY.to_s } + + run_in_shell(command, env: default_env.merge(env), **kwargs) +end + +# ensure we clean up after ourselves on exit +at_exit do + temp_bugsnag_lib = FIXTURE_DIRECTORY + "temp-bugsnag-lib" + temp_bugsnag_lib.rmtree if temp_bugsnag_lib.exist? + + # stop the docker compose stack + Dir.chdir(FIXTURE_DIRECTORY) do + run_docker_command("docker-compose down") + end +end + +# build the bugsnag gem and move it to the fixture directory +Dir.chdir(ROOT_DIRECTORY) do + run_in_shell("gem build bugsnag.gemspec -o bugsnag.gem") + run_in_shell("mv bugsnag.gem #{FIXTURE_DIRECTORY}") +end + +Dir.chdir(FIXTURE_DIRECTORY) do + # unpack the gem into the directory the dockerfile expects + run_in_shell("gem unpack bugsnag.gem --target temp-bugsnag-lib") + run_in_shell("rm bugsnag.gem") + + rails_pid = run_docker_command( + "docker-compose up --build rails_integrations", + env: { "RUBY_TEST_VERSION" => "2.7" }, + background: true + ) + + # wait for the container to finish building & starting + wait_for_port(3000) + + # setup and migrate the database + run_docker_command("docker-compose run rails_integrations bundle exec rake db:prepare") + run_docker_command("docker-compose run rails_integrations bundle exec rake db:migrate") + + # run the queue library in the background (not using '--detach' so we can see the logs) + queue_library_pid = run_docker_command( + "docker-compose run rails_integrations bundle exec #{QUEUE_LIBRARY_COMMANDS[QUEUE_LIBRARY]}", + background: true + ) + + # give the queue library some time to start before we print stuff, otherwise + # we'll print before the library does + sleep(5) + + puts "Everything is running!" + + # this will wait forever as the queue libraries won't exit on their own - quit with Ctrl+C + Process.wait(queue_library_pid) + + # the queue library has exited (because of Ctrl+C) so tell rails to stop too, + # otherwise you'll need to Ctrl+C twice and no one has time for that + Process.kill("TERM", rails_pid) +end From 0fb28b2cb21e5d36a63750cb0f1c6f681c2032c3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 23 Aug 2021 10:27:51 +0100 Subject: [PATCH 29/63] Add run-rails-integrations to testing doc --- TESTING.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/TESTING.md b/TESTING.md index 28afd94ff..e10187cc3 100644 --- a/TESTING.md +++ b/TESTING.md @@ -79,3 +79,24 @@ RUBY_TEST_VERSION=2.6 RAILS_VERSION=6 docker-compose run --use-aliases ruby-maze ``` In order to avoid running flakey or unfinished tests, the tag `"not @wip"` can be added to the tags option. This is recommended for all CI runs. If a tag is already specified, this should be added using the `and` keyword, e.g. `--tags "@rails6 and not @wip"` + +## Manually testing queue libraries + +To help manually test queue libraries and Active Job with various queue adapters, you can use [the `run-ruby-integrations` script](./features/fixtures/run-ruby-integrations). This takes care of installing your local copy of Bugsnag, booting Rails, setting up the database and booting the queue library + +As with the end-to-end tests, only Bugsnag employees can run this script as it relies on the same private resources + +The script will default to booting Sidekiq: + +``` +# run the rails_integrations fixture with sidekiq +$ ./features/fixtures/run-rails-integrations +``` + +The script can also run Resque, Que or Delayed Job: + +``` +$ ./features/fixtures/run-rails-integrations resque +$ ./features/fixtures/run-rails-integrations que +$ ./features/fixtures/run-rails-integrations delayed_job +``` From 7545b012fa835200d412120e8350c982cc98d27c Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 23 Aug 2021 11:20:17 +0100 Subject: [PATCH 30/63] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f1387282..30618f796 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ Changelog * Sessions will now be delivered every 10 seconds, instead of every 30 seconds | [#680](https://github.com/bugsnag/bugsnag-ruby/pull/680) +* Add `on_breadcrumb` callbacks to replace `before_breadcrumb_callbacks` + | [#686](https://github.com/bugsnag/bugsnag-ruby/pull/686) + ### Fixes * Avoid starting session delivery thread when the current release stage is not enabled @@ -15,6 +18,7 @@ Changelog ### Deprecated +* `before_breadcrumb_callbacks` have been deprecated in favour of `on_breadcrumb` callbacks and will be removed in the next major release * For consistency with Bugsnag notifiers for other languages, a number of methods have been deprecated in this release. The old options will be removed in the next major version | [#676](https://github.com/bugsnag/bugsnag-ruby/pull/676) * The `notify_release_stages` configuration option has been deprecated in favour of `enabled_release_stages` * The `auto_capture_sessions` and `track_sessions` configuration options have been deprecated in favour of `auto_track_sessions` From 4fd0a9d61750a89453905b5ce150fa0421f8aed3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 11:46:30 +0100 Subject: [PATCH 31/63] Allow setting context on Configuration --- lib/bugsnag/configuration.rb | 4 ++++ spec/configuration_spec.rb | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 29eef8f78..49e677b6f 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -145,6 +145,10 @@ class Configuration # @return [Regexp] attr_accessor :vendor_path + # The default context for all future events + # @return [String, nil] + attr_accessor :context + # @api private # @return [Array] attr_reader :scopes_to_filter diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index cabe2f615..98b95957c 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -61,6 +61,17 @@ end end + describe "context" do + it "should default to nil" do + expect(subject.context).to be_nil + end + + it "should be settable" do + subject.context = "test" + expect(subject.context).to eq("test") + end + end + describe "#notify_endpoint" do it "defaults to DEFAULT_NOTIFY_ENDPOINT" do expect(subject.notify_endpoint).to eq(Bugsnag::Configuration::DEFAULT_NOTIFY_ENDPOINT) From 49699c8cecb16a6858a86424e9f6680709d4546b Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 11:46:38 +0100 Subject: [PATCH 32/63] Use the Configuration context for new events --- lib/bugsnag/report.rb | 1 + spec/report_spec.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index fa2d8591d..794a0e418 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -118,6 +118,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.app_type = configuration.app_type self.app_version = configuration.app_version self.breadcrumbs = [] + self.context = configuration.context self.delivery_method = configuration.delivery_method self.hostname = configuration.hostname self.runtime_versions = configuration.runtime_versions.dup diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 0316babc6..b8c7baa4f 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -568,6 +568,34 @@ def gloops } end + it "uses the context from Configuration, if set" do + Bugsnag.configure do |config| + config.context = "example context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("example context") + }) + end + + it "allows overriding the context from Configuration" do + Bugsnag.configure do |config| + config.context = "example context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.context = "different context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("different context") + }) + end + it "accepts a user_id in overrides" do Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| report.user = {id: 'test_user'} From 7c64262959b1b9dc8155380b5aed4901e2d1bfa5 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 13:14:55 +0100 Subject: [PATCH 33/63] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ff3baa07..8d96d8245 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,10 @@ Changelog | [#680](https://github.com/bugsnag/bugsnag-ruby/pull/680) * Log errors that prevent delivery at `ERROR` level | [#681](https://github.com/bugsnag/bugsnag-ruby/pull/681) - * Add `on_breadcrumb` callbacks to replace `before_breadcrumb_callbacks` | [#686](https://github.com/bugsnag/bugsnag-ruby/pull/686) +* Add `context` attribute to configuration, which will be used as the default context for events + | [#687](https://github.com/bugsnag/bugsnag-ruby/pull/687) ### Fixes From d145e2605e1c28b5403d39778d122a48aa6f9b0b Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 13:34:09 +0100 Subject: [PATCH 34/63] Allow setting an 'automatic_context' on events --- lib/bugsnag/report.rb | 21 +++++++++++++++++---- spec/report_spec.rb | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 794a0e418..53cf52f07 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -45,10 +45,6 @@ class Report # @return [Configuration] attr_accessor :configuration - # Additional context for this report - # @return [String, nil] - attr_accessor :context - # The delivery method that will be used for this report # @see Configuration#delivery_method # @return [Symbol] @@ -129,6 +125,23 @@ def initialize(exception, passed_configuration, auto_notify=false) self.user = {} end + ## + # Additional context for this report + # @!attribute context + # @return [String, nil] + def context + @context || @automatic_context + end + + attr_writer :context + + ## + # Context set automatically by Bugsnag uses this attribute, which prevents + # it from overwriting the user-supplied context + # @api private + # @return [String, nil] + attr_accessor :automatic_context + ## # Add a new metadata tab to this notification. # diff --git a/spec/report_spec.rb b/spec/report_spec.rb index b8c7baa4f..58a523a8d 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -568,6 +568,48 @@ def gloops } end + it "uses automatic context if no other context has been set" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.automatic_context = "automatic context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("automatic context") + }) + end + + it "uses Configuration context even if the automatic context has been set" do + Bugsnag.configure do |config| + config.context = "configuration context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.automatic_context = "automatic context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("configuration context") + }) + end + + it "uses overridden context even if the automatic context has been set" do + Bugsnag.configure do |config| + config.context = "configuration context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.context = "overridden context" + report.automatic_context = "automatic context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("overridden context") + }) + end + it "uses the context from Configuration, if set" do Bugsnag.configure do |config| config.context = "example context" From c1b43ee90da112b9eb7c422582702ac331d40e96 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 13:40:06 +0100 Subject: [PATCH 35/63] Set automatic context in integrations & middleware --- lib/bugsnag/integrations/resque.rb | 2 +- lib/bugsnag/middleware/active_job.rb | 2 +- lib/bugsnag/middleware/delayed_job.rb | 2 +- lib/bugsnag/middleware/exception_meta_data.rb | 2 ++ lib/bugsnag/middleware/rack_request.rb | 4 ++-- lib/bugsnag/middleware/rails3_request.rb | 4 ++-- lib/bugsnag/middleware/rake.rb | 2 +- lib/bugsnag/middleware/sidekiq.rb | 2 +- lib/bugsnag/tasks/bugsnag.rake | 2 +- spec/integrations/rack_spec.rb | 4 ++-- spec/integrations/rake_spec.rb | 4 ++-- spec/integrations/resque_spec.rb | 2 +- 12 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/bugsnag/integrations/resque.rb b/lib/bugsnag/integrations/resque.rb index 502895cf3..560dcfb05 100644 --- a/lib/bugsnag/integrations/resque.rb +++ b/lib/bugsnag/integrations/resque.rb @@ -56,7 +56,7 @@ def save context = "#{class_name}@#{queue}" report.meta_data.merge!({ context: context, payload: metadata }) - report.context = context + report.automatic_context = context end end end diff --git a/lib/bugsnag/middleware/active_job.rb b/lib/bugsnag/middleware/active_job.rb index 5ff05ef74..b85e7f9f9 100644 --- a/lib/bugsnag/middleware/active_job.rb +++ b/lib/bugsnag/middleware/active_job.rb @@ -9,7 +9,7 @@ def call(report) if data report.add_tab(:active_job, data) - report.context = "#{data[:job_name]}@#{data[:queue]}" + report.automatic_context = "#{data[:job_name]}@#{data[:queue]}" end @bugsnag.call(report) diff --git a/lib/bugsnag/middleware/delayed_job.rb b/lib/bugsnag/middleware/delayed_job.rb index c82d50528..bcef91c25 100644 --- a/lib/bugsnag/middleware/delayed_job.rb +++ b/lib/bugsnag/middleware/delayed_job.rb @@ -30,7 +30,7 @@ def call(report) payload_data = construct_job_payload(job.payload_object) context = get_context(payload_data, job_data[:active_job]) - report.context = context unless context.nil? + report.automatic_context = context unless context.nil? job_data[:payload] = payload_data end diff --git a/lib/bugsnag/middleware/exception_meta_data.rb b/lib/bugsnag/middleware/exception_meta_data.rb index 5caa960a0..fddece6bc 100644 --- a/lib/bugsnag/middleware/exception_meta_data.rb +++ b/lib/bugsnag/middleware/exception_meta_data.rb @@ -16,6 +16,8 @@ def call(report) if exception.respond_to?(:bugsnag_context) context = exception.bugsnag_context + # note: this should set 'context' not 'automatic_context' as it's a + # user-supplied value report.context = context if context.is_a?(String) end diff --git a/lib/bugsnag/middleware/rack_request.rb b/lib/bugsnag/middleware/rack_request.rb index 6825ccf21..2fe6e365e 100644 --- a/lib/bugsnag/middleware/rack_request.rb +++ b/lib/bugsnag/middleware/rack_request.rb @@ -18,8 +18,8 @@ def call(report) client_ip = request.ip.to_s rescue SPOOF session = env["rack.session"] - # Set the context - report.context = "#{request.request_method} #{request.path}" + # Set the automatic context + report.automatic_context = "#{request.request_method} #{request.path}" # Set a sensible default for user_id report.user["id"] = request.ip diff --git a/lib/bugsnag/middleware/rails3_request.rb b/lib/bugsnag/middleware/rails3_request.rb index 16cc588d3..094350d27 100644 --- a/lib/bugsnag/middleware/rails3_request.rb +++ b/lib/bugsnag/middleware/rails3_request.rb @@ -15,8 +15,8 @@ def call(report) client_ip = env["action_dispatch.remote_ip"].to_s rescue SPOOF if params - # Set the context - report.context = "#{params[:controller]}##{params[:action]}" + # Set the automatic context + report.automatic_context = "#{params[:controller]}##{params[:action]}" # Augment the request tab report.add_tab(:request, { diff --git a/lib/bugsnag/middleware/rake.rb b/lib/bugsnag/middleware/rake.rb index ba6d6ca3b..379ff832f 100644 --- a/lib/bugsnag/middleware/rake.rb +++ b/lib/bugsnag/middleware/rake.rb @@ -16,7 +16,7 @@ def call(report) :arguments => task.arg_description }) - report.context ||= task.name + report.automatic_context ||= task.name end @bugsnag.call(report) diff --git a/lib/bugsnag/middleware/sidekiq.rb b/lib/bugsnag/middleware/sidekiq.rb index 453da2b2d..6adcdd7b7 100644 --- a/lib/bugsnag/middleware/sidekiq.rb +++ b/lib/bugsnag/middleware/sidekiq.rb @@ -10,7 +10,7 @@ def call(report) sidekiq = report.request_data[:sidekiq] if sidekiq report.add_tab(:sidekiq, sidekiq) - report.context ||= "#{sidekiq[:msg]['wrapped'] || sidekiq[:msg]['class']}@#{sidekiq[:msg]['queue']}" + report.automatic_context ||= "#{sidekiq[:msg]['wrapped'] || sidekiq[:msg]['class']}@#{sidekiq[:msg]['queue']}" end @bugsnag.call(report) end diff --git a/lib/bugsnag/tasks/bugsnag.rake b/lib/bugsnag/tasks/bugsnag.rake index be214b3da..efc186e6f 100644 --- a/lib/bugsnag/tasks/bugsnag.rake +++ b/lib/bugsnag/tasks/bugsnag.rake @@ -7,7 +7,7 @@ namespace :bugsnag do raise RuntimeError.new("Bugsnag test exception") rescue => e Bugsnag.notify(e) do |report| - report.context = "rake#test_exception" + report.automatic_context = "rake#test_exception" end end end diff --git a/spec/integrations/rack_spec.rb b/spec/integrations/rack_spec.rb index 57847efa4..d100acf2f 100644 --- a/spec/integrations/rack_spec.rb +++ b/spec/integrations/rack_spec.rb @@ -124,7 +124,7 @@ class Request allow(report).to receive(:request_data).and_return({ :rack_env => rack_env }) - expect(report).to receive(:context=).with("TEST /TEST_PATH") + expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH") expect(report).to receive(:user).and_return({}) config = Bugsnag.configuration @@ -185,7 +185,7 @@ class Request allow(report).to receive(:request_data).and_return({ :rack_env => rack_env }) - expect(report).to receive(:context=).with("TEST /TEST_PATH") + expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH") expect(report).to receive(:user).and_return({}) config = Bugsnag.configuration diff --git a/spec/integrations/rake_spec.rb b/spec/integrations/rake_spec.rb index bfd2cc40b..6139ae86c 100644 --- a/spec/integrations/rake_spec.rb +++ b/spec/integrations/rake_spec.rb @@ -23,8 +23,8 @@ :description => "TEST_COMMENT", :arguments => "TEST_ARGS" }) - expect(report).to receive(:context).with(no_args) - expect(report).to receive(:context=).with("TEST_NAME") + expect(report).to receive(:automatic_context).with(no_args) + expect(report).to receive(:automatic_context=).with("TEST_NAME") expect(callback).to receive(:call).with(report) diff --git a/spec/integrations/resque_spec.rb b/spec/integrations/resque_spec.rb index 952d2337f..c75ac1f75 100644 --- a/spec/integrations/resque_spec.rb +++ b/spec/integrations/resque_spec.rb @@ -85,7 +85,7 @@ def require(path) "class" => "class" } }) - expect(report).to receive(:context=).with(expected_context) + expect(report).to receive(:automatic_context=).with(expected_context) expect(Bugsnag).to receive(:notify).with(exception, true).and_yield(report) resque.save end From 8f0f031b8e97ba28d76107ebe1e5494f876518ae Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 13:46:00 +0100 Subject: [PATCH 36/63] Add note for the configuration context precedence --- lib/bugsnag/configuration.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 49e677b6f..96da7fd66 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -146,6 +146,7 @@ class Configuration attr_accessor :vendor_path # The default context for all future events + # Setting this will disable automatic context setting # @return [String, nil] attr_accessor :context From 1b3c6f4ade73e9419d39847cbae579ae2f75d5fb Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 14:30:12 +0100 Subject: [PATCH 37/63] Use the configuration context even if it's 'nil' This allows users to opt out of automatic context setting without having to set their own context for all events --- lib/bugsnag/configuration.rb | 12 ++++++++++++ lib/bugsnag/report.rb | 6 ++++-- spec/report_spec.rb | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 96da7fd66..f1cba9340 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -553,6 +553,18 @@ def remove_on_breadcrumb(callback) @on_breadcrumb_callbacks.remove(callback) end + ## + # Has the context been explicitly set? + # + # This is necessary to differentiate between the context not being set and + # the context being set to 'nil' explicitly + # + # @api private + # @return [Boolean] + def context_set? + defined?(@context) != nil + end + # TODO: These methods can be a simple attr_accessor when they replace the # methods they are aliasing # NOTE: they are not aliases as YARD doesn't allow documenting the non-alias diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 53cf52f07..0aa59952a 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -114,7 +114,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.app_type = configuration.app_type self.app_version = configuration.app_version self.breadcrumbs = [] - self.context = configuration.context + self.context = configuration.context if configuration.context_set? self.delivery_method = configuration.delivery_method self.hostname = configuration.hostname self.runtime_versions = configuration.runtime_versions.dup @@ -130,7 +130,9 @@ def initialize(exception, passed_configuration, auto_notify=false) # @!attribute context # @return [String, nil] def context - @context || @automatic_context + return @context if defined?(@context) + + @automatic_context end attr_writer :context diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 58a523a8d..14f36dcc2 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -610,6 +610,21 @@ def gloops }) end + it "uses overridden context even it is set to 'nil'" do + Bugsnag.configure do |config| + config.context = nil + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.automatic_context = "automatic context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to be_nil + }) + end + it "uses the context from Configuration, if set" do Bugsnag.configure do |config| config.context = "example context" From ba1e8771f982fb340d2323249c2f4c2f8930ab83 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 14:44:36 +0100 Subject: [PATCH 38/63] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d96d8245..48f5943d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,9 @@ Changelog | [#681](https://github.com/bugsnag/bugsnag-ruby/pull/681) * Add `on_breadcrumb` callbacks to replace `before_breadcrumb_callbacks` | [#686](https://github.com/bugsnag/bugsnag-ruby/pull/686) -* Add `context` attribute to configuration, which will be used as the default context for events +* Add `context` attribute to configuration, which will be used as the default context for events. Using this option will disable automatic context setting | [#687](https://github.com/bugsnag/bugsnag-ruby/pull/687) + | [#688](https://github.com/bugsnag/bugsnag-ruby/pull/688) ### Fixes From 36a0fa4b70d12d03ae7b0bd531419129199b2308 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 17:05:55 +0100 Subject: [PATCH 39/63] Add Bugsnag#breadcrumbs --- CHANGELOG.md | 2 ++ lib/bugsnag.rb | 11 +++++++++++ lib/bugsnag/configuration.rb | 7 +++++-- spec/bugsnag_spec.rb | 6 ++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48f5943d8..f84c53307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ Changelog * Add `context` attribute to configuration, which will be used as the default context for events. Using this option will disable automatic context setting | [#687](https://github.com/bugsnag/bugsnag-ruby/pull/687) | [#688](https://github.com/bugsnag/bugsnag-ruby/pull/688) +* Add `Bugsnag#breadcrumbs` getter to fetch the current list of breadcrumbs + | [#689](https://github.com/bugsnag/bugsnag-ruby/pull/689) ### Fixes diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 23ba1029e..6c47e3f5e 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -326,6 +326,17 @@ def remove_on_breadcrumb(callback) configuration.remove_on_breadcrumb(callback) end + ## + # Returns the current list of breadcrumbs + # + # This is a per-thread circular buffer, containing at most 'max_breadcrumbs' + # breadcrumbs + # + # @return [Bugsnag::Utility::CircularBuffer] + def breadcrumbs + configuration.breadcrumbs + 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 f1cba9340..b7468cc33 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -449,9 +449,12 @@ def max_breadcrumbs=(new_max_breadcrumbs) end ## - # Returns the breadcrumb circular buffer + # Returns the current list of breadcrumbs # - # @return [Bugsnag::Utility::CircularBuffer] a thread based circular buffer containing breadcrumbs + # This is a per-thread circular buffer, containing at most 'max_breadcrumbs' + # breadcrumbs + # + # @return [Bugsnag::Utility::CircularBuffer] def breadcrumbs request_data[:breadcrumbs] ||= Bugsnag::Utility::CircularBuffer.new(@max_breadcrumbs) end diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index a09520c83..b95c220a2 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -521,4 +521,10 @@ module Kernel } end end + + describe "#breadcrumbs" do + it "returns the configuration's breadcrumb buffer" do + expect(Bugsnag.breadcrumbs).to be(Bugsnag.configuration.breadcrumbs) + end + end end From 5968696c06b0011e6d50c0cf9ffb5d3443c8035d Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 18:00:04 +0100 Subject: [PATCH 40/63] Add 'time' to device metadata --- lib/bugsnag/report.rb | 6 +++++- spec/bugsnag_spec.rb | 2 +- spec/report_spec.rb | 17 ++++++++++++++--- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 0aa59952a..b63c472f9 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -102,6 +102,9 @@ class Report ## # Initializes a new report from an exception. def initialize(exception, passed_configuration, auto_notify=false) + # store the creation time for use as device.time + @created_at = Time.now.utc.iso8601(3) + @should_ignore = false @unhandled = auto_notify @@ -192,7 +195,8 @@ def as_json context: context, device: { hostname: hostname, - runtimeVersions: runtime_versions + runtimeVersions: runtime_versions, + time: @created_at }, exceptions: exceptions, groupingHash: grouping_hash, diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index b95c220a2..7bd5dd7f7 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -511,7 +511,7 @@ module Kernel describe "request headers" do it "Bugsnag-Sent-At should use the current time" do fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).at_most(5).times.and_return(fake_now) + expect(Time).to receive(:now).at_most(6).times.and_return(fake_now) Bugsnag.notify(BugsnagTestException.new("It crashed")) diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 14f36dcc2..9375924bf 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -30,7 +30,7 @@ def gloops shared_examples "Report or Event tests" do |class_to_test| it "#headers should return the correct request headers" do fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).and_return(fake_now) + expect(Time).to receive(:now).twice.and_return(fake_now) report_or_event = class_to_test.new( BugsnagTestException.new("It crashed"), @@ -1709,15 +1709,26 @@ def to_s end it 'includes device data when notify is called' do + fake_device_time = Time.gm(2020, 1, 2, 3, 4, 5, 123456) + fake_sent_at = Time.gm(2021, 1, 2, 3, 4, 5, 123456) + expect(Time).to receive(:now).at_least(:twice).and_return(fake_device_time, fake_sent_at) + Bugsnag.configuration.hostname = 'test-host' Bugsnag.configuration.runtime_versions["ruby"] = '9.9.9' Bugsnag.notify(BugsnagTestException.new("It crashed")) - expect(Bugsnag).to have_sent_notification{ |payload, headers| + expect(Bugsnag).to(have_sent_notification { |payload, headers| event = payload["events"][0] expect(event["device"]["hostname"]).to eq('test-host') expect(event["device"]["runtimeVersions"]["ruby"]).to eq('9.9.9') - } + # This matches the time we stubbed earlier (fake_device_time) + expect(event["device"]["time"]).to eq("2020-01-02T03:04:05.123Z") + + expect(headers["Bugsnag-Api-Key"]).to eq("c9d60ae4c7e70c4b6c4ebd3e8056d2b8") + expect(headers["Bugsnag-Payload-Version"]).to eq("4.0") + # This matches the time we stubbed earlier (fake_sent_at) + expect(headers["Bugsnag-Sent-At"]).to eq("2021-01-02T03:04:05.123Z") + }) end end # rubocop:enable Metrics/BlockLength From 3964c37140cd7d34adee97490603faa392d35405 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 18:02:26 +0100 Subject: [PATCH 41/63] Add some Maze Runner checks for device time --- features/plain_features/handled_errors.feature | 5 ++++- features/plain_features/unhandled_errors.feature | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/features/plain_features/handled_errors.feature b/features/plain_features/handled_errors.feature index 88e98a7ff..ff97e32e9 100644 --- a/features/plain_features/handled_errors.feature +++ b/features/plain_features/handled_errors.feature @@ -7,6 +7,7 @@ Scenario: A rescued exception sends a report And the event "unhandled" is false And the event "severity" equals "warning" And the event "severityReason.type" equals "handledException" + And the event "device.time" is a timestamp And the exception "errorClass" equals "RuntimeError" And the "file" of stack frame 0 equals "/usr/src/app/handled/notify_exception.rb" And the "lineNumber" of stack frame 0 equals 6 @@ -18,6 +19,7 @@ Scenario: A notified string sends a report And the event "unhandled" is false And the event "severity" equals "warning" And the event "severityReason.type" equals "handledException" + And the event "device.time" is a timestamp And the exception "errorClass" equals "RuntimeError" And the "file" of the top non-bugsnag stackframe equals "/usr/src/app/handled/notify_string.rb" And the "lineNumber" of the top non-bugsnag stackframe equals 8 @@ -39,4 +41,5 @@ Scenario: A handled error can attach metadata in a block And the "lineNumber" of stack frame 0 equals 6 And the event "metaData.account.id" equals "1234abcd" And the event "metaData.account.name" equals "Acme Co" - And the event "metaData.account.support" is true \ No newline at end of file + And the event "metaData.account.support" is true + And the event "device.time" is a timestamp diff --git a/features/plain_features/unhandled_errors.feature b/features/plain_features/unhandled_errors.feature index 2b227dc73..6bca5787f 100644 --- a/features/plain_features/unhandled_errors.feature +++ b/features/plain_features/unhandled_errors.feature @@ -7,6 +7,7 @@ Scenario Outline: An unhandled error sends a report And the event "unhandled" is true And the event "severity" equals "error" And the event "severityReason.type" equals "unhandledException" + And the event "device.time" is a timestamp And the exception "errorClass" equals "" And the "file" of stack frame 0 equals "/usr/src/app/unhandled/.rb" And the "lineNumber" of stack frame 0 equals From 45736b6d01b50e655400d1d87d5c84d935768622 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 26 Aug 2021 10:33:11 +0100 Subject: [PATCH 42/63] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f84c53307..3560db3e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ Changelog | [#688](https://github.com/bugsnag/bugsnag-ruby/pull/688) * Add `Bugsnag#breadcrumbs` getter to fetch the current list of breadcrumbs | [#689](https://github.com/bugsnag/bugsnag-ruby/pull/689) +* Add `time` (an ISO8601 string in UTC) to `device` metadata + | [#690](https://github.com/bugsnag/bugsnag-ruby/pull/690) ### Fixes From 8b64784a7845f125eb474d68c4358c9fbfec9fc3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 3 Sep 2021 16:24:45 +0100 Subject: [PATCH 43/63] Add 'errors' to Report/Event class --- lib/bugsnag/error.rb | 9 ++++++ lib/bugsnag/report.rb | 18 ++++++++++++ spec/report_spec.rb | 67 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 lib/bugsnag/error.rb diff --git a/lib/bugsnag/error.rb b/lib/bugsnag/error.rb new file mode 100644 index 000000000..581b4adea --- /dev/null +++ b/lib/bugsnag/error.rb @@ -0,0 +1,9 @@ +module Bugsnag + # @!attribute error_class + # @return [String] the error's class name + # @!attribute error_message + # @return [String] the error's message + # @!attribute type + # @return [String] the type of error (always "ruby") + Error = Struct.new(:error_class, :error_message, :type) +end diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index b63c472f9..9c8be25e3 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -1,8 +1,10 @@ require "json" require "pathname" +require "bugsnag/error" require "bugsnag/stacktrace" module Bugsnag + # rubocop:todo Metrics/ClassLength class Report NOTIFIER_NAME = "Ruby Bugsnag Notifier" NOTIFIER_VERSION = Bugsnag::VERSION @@ -19,6 +21,9 @@ class Report CURRENT_PAYLOAD_VERSION = "4.0" + # @api private + ERROR_TYPE = "ruby".freeze + # Whether this report is for a handled or unhandled error # @return [Boolean] attr_reader :unhandled @@ -51,6 +56,7 @@ class Report attr_accessor :delivery_method # The list of exceptions in this report + # @deprecated Use {#errors} instead # @return [Array] attr_accessor :exceptions @@ -99,6 +105,10 @@ class Report # @return [Hash] attr_accessor :user + # A list of errors in this report + # @return [Array] + attr_reader :errors + ## # Initializes a new report from an exception. def initialize(exception, passed_configuration, auto_notify=false) @@ -112,6 +122,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.raw_exceptions = generate_raw_exceptions(exception) self.exceptions = generate_exception_list + @errors = generate_error_list self.api_key = configuration.api_key self.app_type = configuration.app_type @@ -303,6 +314,12 @@ def generate_exception_list end end + def generate_error_list + exceptions.map do |exception| + Error.new(exception[:errorClass], exception[:message], ERROR_TYPE) + end + end + def error_class(exception) # The "Class" check is for some strange exceptions like Timeout::Error # which throw the error class instead of an instance @@ -345,4 +362,5 @@ def generate_raw_exceptions(exception) exceptions end end + # rubocop:enable Metrics/ClassLength end diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 9375924bf..0db481aab 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -125,6 +125,73 @@ def gloops end end end + + describe "#errors" do + it "has required hash keys" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.errors).to eq( + [Bugsnag::Error.new("RuntimeError", "example error", "ruby")] + ) + end + + it "includes errors that caused the top-most exception" do + begin + begin + raise "one" + rescue + Ruby21Exception.raise!("two") + end + rescue => exception + end + + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.errors).to eq( + [ + Bugsnag::Error.new("Ruby21Exception", "two", "ruby"), + Bugsnag::Error.new("RuntimeError", "one", "ruby") + ] + ) + end + + it "cannot be assigned to" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report).not_to respond_to(:errors=) + end + + it "can be mutated" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + report.errors.push("haha") + report.errors.push("haha 2") + report.errors.pop + + expect(report.errors).to eq( + [ + Bugsnag::Error.new("RuntimeError", "example error", "ruby"), + "haha" + ] + ) + end + + it "contains mutable data" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + report.errors.first.error_class = "haha" + report.errors.first.error_message = "ahah" + report.errors.first.type = "aahh" + + expect(report.errors).to eq( + [Bugsnag::Error.new("haha", "ahah", "aahh")] + ) + end + end end # rubocop:disable Metrics/BlockLength From 6cb175e5efcc800abce3a7924fb94b698d914599 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 6 Sep 2021 13:23:12 +0100 Subject: [PATCH 44/63] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3560db3e3..4dcf959ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ Changelog | [#689](https://github.com/bugsnag/bugsnag-ruby/pull/689) * Add `time` (an ISO8601 string in UTC) to `device` metadata | [#690](https://github.com/bugsnag/bugsnag-ruby/pull/690) +* Add `errors` to `Report`/`Event` containing an array of `Error` objects. The `Error` object contains `error_class`, `error_message` and `type` (always "ruby") + | [#691](https://github.com/bugsnag/bugsnag-ruby/pull/691) ### Fixes @@ -37,6 +39,7 @@ Changelog * The `Breadcrumb#name` attribute has been deprecated in favour of `Breadcrumb#message` * The breadcrumb type constants in the `Bugsnag::Breadcrumbs` module has been deprecated in favour of the constants available in the `Bugsnag::BreadcrumbType` module For example, `Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE` is now available as `Bugsnag::BreadcrumbType::ERROR` +* `Report#exceptions` has been deprecated in favour of the new `errors` property ## v6.22.1 (11 August 2021) From d45a084e9f3782893809902003e12eba94d560d4 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 6 Sep 2021 14:20:00 +0100 Subject: [PATCH 45/63] Add 'original_error' attribute This stores the original Exception instance directly. This is different to 'raw_exceptions' because 'raw_exceptions' traverses 'cause' chains to build an array of Exception objects and attempts to normalise non-Exception objects into Exceptions --- lib/bugsnag/report.rb | 6 ++++++ spec/report_spec.rb | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 9c8be25e3..ca608d24f 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -79,6 +79,7 @@ class Report attr_accessor :meta_data # The raw Exception instances for this report + # @deprecated Use {#original_error} instead # @see #exceptions # @return [Array] attr_accessor :raw_exceptions @@ -109,6 +110,10 @@ class Report # @return [Array] attr_reader :errors + # The Exception instance this report was created for + # @return [Exception] + attr_reader :original_error + ## # Initializes a new report from an exception. def initialize(exception, passed_configuration, auto_notify=false) @@ -120,6 +125,7 @@ def initialize(exception, passed_configuration, auto_notify=false) self.configuration = passed_configuration + @original_error = exception self.raw_exceptions = generate_raw_exceptions(exception) self.exceptions = generate_exception_list @errors = generate_error_list diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 0db481aab..afdb4ab53 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -192,6 +192,13 @@ def gloops ) end end + + it "has a reference to the original error" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.original_error).to be(exception) + end end # rubocop:disable Metrics/BlockLength From 46c9a84d069bc7c78b6417c014b7d084535bce2e Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 6 Sep 2021 14:27:47 +0100 Subject: [PATCH 46/63] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dcf959ee..24f7f75b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ Changelog | [#690](https://github.com/bugsnag/bugsnag-ruby/pull/690) * Add `errors` to `Report`/`Event` containing an array of `Error` objects. The `Error` object contains `error_class`, `error_message` and `type` (always "ruby") | [#691](https://github.com/bugsnag/bugsnag-ruby/pull/691) +* Add `original_error` to `Report`/`Event` containing the original Exception instance + | [#692](https://github.com/bugsnag/bugsnag-ruby/pull/692) ### Fixes @@ -40,6 +42,7 @@ Changelog * The breadcrumb type constants in the `Bugsnag::Breadcrumbs` module has been deprecated in favour of the constants available in the `Bugsnag::BreadcrumbType` module For example, `Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE` is now available as `Bugsnag::BreadcrumbType::ERROR` * `Report#exceptions` has been deprecated in favour of the new `errors` property +* `Report#raw_exceptions` has been deprecated in favour of the new `original_error` property ## v6.22.1 (11 August 2021) From 8bc55580216da66355b89c547362eef94d95ce25 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 6 Sep 2021 15:53:42 +0100 Subject: [PATCH 47/63] Refactor Rack spec to use a real Report instance --- spec/integrations/rack_spec.rb | 167 ++++++++++++++------------------- 1 file changed, 72 insertions(+), 95 deletions(-) diff --git a/spec/integrations/rack_spec.rb b/spec/integrations/rack_spec.rb index d100acf2f..2b94f9d83 100644 --- a/spec/integrations/rack_spec.rb +++ b/spec/integrations/rack_spec.rb @@ -92,135 +92,112 @@ class Request end end + after do + Object.send(:remove_const, :Rack) if @mocked_rack + end + it "correctly redacts from url and referer any value indicated by meta_data_filters" do - callback = double rack_env = { - :env => true, - :HTTP_REFERER => "https://bugsnag.com/about?email=hello@world.com&another_param=thing", - "rack.session" => { - :session => true - } + env: true, + HTTP_REFERER: "https://bugsnag.com/about?email=hello@world.com&another_param=thing", + "rack.session" => { session: true } } rack_request = double - rack_params = { - :param => 'test' - } allow(rack_request).to receive_messages( - :params => rack_params, - :ip => "rack_ip", - :request_method => "TEST", - :path => "/TEST_PATH", - :scheme => "http", - :host => "test_host", - :port => 80, - :referer => "https://bugsnag.com/about?email=hello@world.com&another_param=thing", - :fullpath => "/TEST_PATH?email=hello@world.com&another_param=thing" + params: { param: 'test' }, + ip: "rack_ip", + request_method: "TEST", + path: "/TEST_PATH", + scheme: "http", + host: "test_host", + port: 80, + referer: "https://bugsnag.com/about?email=hello@world.com&another_param=thing", + fullpath: "/TEST_PATH?email=hello@world.com&another_param=thing" ) + expect(::Rack::Request).to receive(:new).with(rack_env).and_return(rack_request) - # modify rack_env to include redacted referer - report = double("Bugsnag::Report") - allow(report).to receive(:request_data).and_return({ - :rack_env => rack_env - }) - expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH") - expect(report).to receive(:user).and_return({}) - - config = Bugsnag.configuration - config.send_environment = true - config.meta_data_filters = ['email'] - - allow(report).to receive(:configuration).and_return(config) - expect(report).to receive(:add_tab).once.with(:request, { - :url => "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing", - :httpMethod => "TEST", - :params => rack_params, - :referer => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing", - :clientIp => "rack_ip", - :headers => { - "Referer" => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing" - } - }) - # rack_env["HTTP_REFERER"] = "https://bugsnag.com/about?email=[FILTERED]&another_param=thing" - expect(report).to receive(:add_tab).once.with(:environment, rack_env) - expect(report).to receive(:add_tab).once.with(:session, { - :session => true - }) + Bugsnag.configure do |config| + config.send_environment = true + config.meta_data_filters = ['email'] + config.request_data[:rack_env] = rack_env + end + report = Bugsnag::Report.new(RuntimeError.new('abc'), Bugsnag.configuration) + + callback = double expect(callback).to receive(:call).with(report) middleware = Bugsnag::Middleware::RackRequest.new(callback) middleware.call(report) + + expect(report.metadata[:request]).to eq({ + url: "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing", + httpMethod: "TEST", + params: { param: 'test' }, + referer: "https://bugsnag.com/about?email=[FILTERED]&another_param=thing", + clientIp: "rack_ip", + headers: { + "Referer" => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing" + } + }) + + expect(report.metadata[:environment]).to eq(rack_env) + expect(report.metadata[:session]).to eq({ session: true }) end it "correctly extracts data from rack middleware" do - callback = double rack_env = { - :env => true, - :HTTP_test_key => "test_key", - "rack.session" => { - :session => true - } + env: true, + HTTP_test_key: "test_key", + "rack.session" => { session: true } } rack_request = double - rack_params = { - :param => 'test' - } allow(rack_request).to receive_messages( - :params => rack_params, - :ip => "rack_ip", - :request_method => "TEST", - :path => "/TEST_PATH", - :scheme => "http", - :host => "test_host", - :port => 80, - :referer => "referer", - :fullpath => "/TEST_PATH" + params: { param: 'test' }, + ip: "rack_ip", + request_method: "TEST", + path: "/TEST_PATH", + scheme: "http", + host: "test_host", + port: 80, + referer: "referer", + fullpath: "/TEST_PATH" ) + expect(Rack::Request).to receive(:new).with(rack_env).and_return(rack_request) - report = double("Bugsnag::Report") - allow(report).to receive(:request_data).and_return({ - :rack_env => rack_env - }) - expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH") - expect(report).to receive(:user).and_return({}) - - config = Bugsnag.configuration - config.send_environment = true - config.meta_data_filters = [] - - allow(report).to receive(:configuration).and_return(config) - expect(report).to receive(:add_tab).once.with(:environment, rack_env) - expect(report).to receive(:add_tab).once.with(:request, { - :url => "http://test_host/TEST_PATH", - :httpMethod => "TEST", - :params => rack_params, - :referer => "referer", - :clientIp => "rack_ip", - :headers => { - "Test-Key" => "test_key" - } - }) - expect(report).to receive(:add_tab).once.with(:session, { - :session => true - }) + Bugsnag.configure do |config| + config.send_environment = true + config.meta_data_filters = [] + config.request_data[:rack_env] = rack_env + end + + report = Bugsnag::Report.new(RuntimeError.new('abc'), Bugsnag.configuration) + callback = double expect(callback).to receive(:call).with(report) middleware = Bugsnag::Middleware::RackRequest.new(callback) middleware.call(report) - end - after do - Object.send(:remove_const, :Rack) if @mocked_rack - end + expect(report.metadata[:request]).to eq({ + url: "http://test_host/TEST_PATH", + httpMethod: "TEST", + params: { param: 'test' }, + referer: "referer", + clientIp: "rack_ip", + headers: { "Test-Key" => "test_key" } + }) + expect(report.metadata[:environment]).to eq(rack_env) + expect(report.metadata[:session]).to eq({ session: true }) + end end - it "don't mess with middlewares list on each req" do + it "doesn't change the middleware list on each request" do app = lambda { |env| ['200', {}, ['']] } Bugsnag::Rack.new(app) From 2467be33dc993b6b99ddffa869f6cea17f580ad7 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 7 Sep 2021 10:00:10 +0100 Subject: [PATCH 48/63] Add Report#request to fetch HTTP request data This currently reads from metadata to avoid a breaking change, however this will be extracted from metadata as it's a top-level field in the API --- lib/bugsnag/report.rb | 8 ++++++++ spec/integrations/rack_spec.rb | 6 ++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index ca608d24f..c1c021274 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -308,6 +308,14 @@ def metadata=(metadata) @meta_data = metadata end + ## + # Data from the current HTTP request. May be nil if no data has been recorded + # + # @return [Hash, nil] + def request + @meta_data[:request] + end + private def generate_exception_list diff --git a/spec/integrations/rack_spec.rb b/spec/integrations/rack_spec.rb index 2b94f9d83..b23e5fedb 100644 --- a/spec/integrations/rack_spec.rb +++ b/spec/integrations/rack_spec.rb @@ -132,7 +132,7 @@ class Request middleware = Bugsnag::Middleware::RackRequest.new(callback) middleware.call(report) - expect(report.metadata[:request]).to eq({ + expect(report.request).to eq({ url: "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing", httpMethod: "TEST", params: { param: 'test' }, @@ -143,6 +143,7 @@ class Request } }) + expect(report.metadata[:request]).to be(report.request) expect(report.metadata[:environment]).to eq(rack_env) expect(report.metadata[:session]).to eq({ session: true }) end @@ -183,7 +184,7 @@ class Request middleware = Bugsnag::Middleware::RackRequest.new(callback) middleware.call(report) - expect(report.metadata[:request]).to eq({ + expect(report.request).to eq({ url: "http://test_host/TEST_PATH", httpMethod: "TEST", params: { param: 'test' }, @@ -192,6 +193,7 @@ class Request headers: { "Test-Key" => "test_key" } }) + expect(report.metadata[:request]).to be(report.request) expect(report.metadata[:environment]).to eq(rack_env) expect(report.metadata[:session]).to eq({ session: true }) end From 87b39ac37ecc28f2538fb2f1c4145813103cdb80 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 7 Sep 2021 10:13:34 +0100 Subject: [PATCH 49/63] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24f7f75b1..449f5d25a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ Changelog | [#691](https://github.com/bugsnag/bugsnag-ruby/pull/691) * Add `original_error` to `Report`/`Event` containing the original Exception instance | [#692](https://github.com/bugsnag/bugsnag-ruby/pull/692) +* Add `request` to `Report`/`Event` containing HTTP request metadata + | [#693](https://github.com/bugsnag/bugsnag-ruby/pull/693) ### Fixes @@ -43,6 +45,7 @@ Changelog For example, `Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE` is now available as `Bugsnag::BreadcrumbType::ERROR` * `Report#exceptions` has been deprecated in favour of the new `errors` property * `Report#raw_exceptions` has been deprecated in favour of the new `original_error` property +* Accessing request data via `Report#metadata` has been deprecated in favour of using the new `request` property. Request data will be moved out of metadata in the next major version ## v6.22.1 (11 August 2021) From d10d22e3c8f33ecd7fe0518362f464f3df719d97 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 7 Sep 2021 10:30:38 +0100 Subject: [PATCH 50/63] Add a MR test for all request data --- features/rails_features/request.feature | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 features/rails_features/request.feature diff --git a/features/rails_features/request.feature b/features/rails_features/request.feature new file mode 100644 index 000000000..6bbed3a1d --- /dev/null +++ b/features/rails_features/request.feature @@ -0,0 +1,25 @@ +Feature: Request data + +@rails3 @rails4 @rails5 @rails6 +Scenario: Request data is collected automatically + Given I start the rails service + When I navigate to the route "/unhandled/error?a=123&b=456" 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 event "unhandled" is true + And the exception "errorClass" equals "NameError" + And the exception "message" starts with "undefined local variable or method `generate_unhandled_error' for # Date: Tue, 7 Sep 2021 10:40:57 +0100 Subject: [PATCH 51/63] Add MR test showing request data can be mutated --- features/fixtures/docker-compose.yml | 4 +++ .../rails3/app/config/initializers/bugsnag.rb | 7 +++++ .../rails4/app/config/initializers/bugsnag.rb | 7 +++++ .../rails5/app/config/initializers/bugsnag.rb | 7 +++++ .../rails6/app/config/initializers/bugsnag.rb | 7 +++++ features/rails_features/request.feature | 27 +++++++++++++++++++ 6 files changed, 59 insertions(+) diff --git a/features/fixtures/docker-compose.yml b/features/fixtures/docker-compose.yml index 0b6957592..ccb7f6fb0 100644 --- a/features/fixtures/docker-compose.yml +++ b/features/fixtures/docker-compose.yml @@ -129,6 +129,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" rails4: @@ -165,6 +166,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" rails5: @@ -201,6 +203,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" rails6: @@ -237,6 +240,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" networks: default: diff --git a/features/fixtures/rails3/app/config/initializers/bugsnag.rb b/features/fixtures/rails3/app/config/initializers/bugsnag.rb index baa9c5d3f..ca5b93ca7 100644 --- a/features/fixtures/rails3/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails3/app/config/initializers/bugsnag.rb @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/fixtures/rails4/app/config/initializers/bugsnag.rb b/features/fixtures/rails4/app/config/initializers/bugsnag.rb index baa9c5d3f..ca5b93ca7 100644 --- a/features/fixtures/rails4/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails4/app/config/initializers/bugsnag.rb @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb index 3c876f138..258d13b59 100644 --- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/fixtures/rails6/app/config/initializers/bugsnag.rb b/features/fixtures/rails6/app/config/initializers/bugsnag.rb index baa9c5d3f..ca5b93ca7 100644 --- a/features/fixtures/rails6/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails6/app/config/initializers/bugsnag.rb @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/rails_features/request.feature b/features/rails_features/request.feature index 6bbed3a1d..46e735ea0 100644 --- a/features/rails_features/request.feature +++ b/features/rails_features/request.feature @@ -23,3 +23,30 @@ Scenario: Request data is collected automatically And the event "metaData.request.referer" is null And the event "metaData.request.requestId" is not null And the event "metaData.request.url" ends with "/unhandled/error?a=123&b=456" + +@rails3 @rails4 @rails5 @rails6 +Scenario: Request data can be modified in callbacks + Given I set environment variable "ADD_REQUEST_ON_ERROR" to "true" + And I start the rails service + When I navigate to the route "/unhandled/error?a=123&b=456" 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 event "unhandled" is true + And the exception "errorClass" equals "NameError" + And the exception "message" starts with "undefined local variable or method `generate_unhandled_error' for # Date: Thu, 9 Sep 2021 09:40:43 +0100 Subject: [PATCH 52/63] Add MetadataDelegate utility class This class handles the various ways metadata can be updated so this logic can be reused in multiple classes --- lib/bugsnag/utility/metadata_delegate.rb | 102 ++++++++++++++ spec/utility/metadata_delegate_spec.rb | 163 +++++++++++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 lib/bugsnag/utility/metadata_delegate.rb create mode 100644 spec/utility/metadata_delegate_spec.rb diff --git a/lib/bugsnag/utility/metadata_delegate.rb b/lib/bugsnag/utility/metadata_delegate.rb new file mode 100644 index 000000000..8b43c545d --- /dev/null +++ b/lib/bugsnag/utility/metadata_delegate.rb @@ -0,0 +1,102 @@ +module Bugsnag::Utility + # @api private + class MetadataDelegate + # nil is a valid metadata value, so we need a sentinel object so we can tell + # if the value parameter has been provided + NOT_PROVIDED = Object.new + + ## + # Add values to metadata + # + # @overload add_metadata(metadata, section, data) + # Merges data into the given section of metadata + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # @param data [Hash] + # + # @overload add_metadata(metadata, section, key, value) + # Sets key to value in the given section of metadata. If the value is nil + # the key will be deleted + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # @param key [String, Symbol] + # @param value + # + # @return [void] + def add_metadata(metadata, section, key_or_data, value = NOT_PROVIDED) + case value + when NOT_PROVIDED + merge_metadata(metadata, section, key_or_data) + when nil + clear_metadata(metadata, section, key_or_data) + else + overwrite_metadata(metadata, section, key_or_data, value) + end + end + + ## + # Clear values from metadata + # + # @overload clear_metadata(metadata, section) + # Clears the given section of metadata + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # + # @overload clear_metadata(metadata, section, key) + # Clears the key in the given section of metadata + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # @param key [String, Symbol] + # + # @return [void] + def clear_metadata(metadata, section, key = nil) + if key.nil? + metadata.delete(section) + elsif metadata[section] + metadata[section].delete(key) + end + end + + private + + ## + # Merge new metadata into the existing metadata + # + # Any keys with a 'nil' value in the new metadata will be deleted from the + # existing metadata + # + # @param existing_metadata [Hash] + # @param section [String, Symbol] + # @param new_metadata [Hash] + # @return [void] + def merge_metadata(existing_metadata, section, new_metadata) + return unless new_metadata.is_a?(Hash) + + existing_metadata[section] ||= {} + data = existing_metadata[section] + + new_metadata.each do |key, value| + if value.nil? + data.delete(key) + else + data[key] = value + end + end + end + + ## + # Overwrite the value in metadata's section & key + # + # @param metadata [Hash] + # @param section [String, Symbol] + # @param key [String, Symbol] + # @param value + # @return [void] + def overwrite_metadata(metadata, section, key, value) + return unless key.is_a?(String) || key.is_a?(Symbol) + + metadata[section] ||= {} + metadata[section][key] = value + end + end +end diff --git a/spec/utility/metadata_delegate_spec.rb b/spec/utility/metadata_delegate_spec.rb new file mode 100644 index 000000000..0d60a10fc --- /dev/null +++ b/spec/utility/metadata_delegate_spec.rb @@ -0,0 +1,163 @@ +require 'spec_helper' +require 'bugsnag/utility/metadata_delegate' + +RSpec.describe Bugsnag::Utility::MetadataDelegate do + context "#add_metadata" do + context "with 'section', 'key' and 'value'" do + it "adds the given key/value pair to the section" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = {} + delegate.add_metadata(metadata, :abc, :xyz, "Hello!") + + expect(metadata).to eq({ abc: { xyz: "Hello!" } }) + end + + it "merges the new key/value pair with existing data in the section" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = { abc: { a: 1, b: 2, c: 3 } } + delegate.add_metadata(metadata, :abc, :xyz, "Hello!") + + expect(metadata).to eq({ abc: { a: 1, b: 2, c: 3, xyz: "Hello!" } }) + end + + it "replaces existing metadata if the 'key' already exists" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = { abc: { xyz: "Hello!" } } + delegate.add_metadata(metadata, :abc, :xyz, "Goodbye!") + + expect(metadata).to eq({ abc: { xyz: "Goodbye!" } }) + end + + it "removes the key if 'value' is nil" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = { abc: { xyz: "Hello!" } } + delegate.add_metadata(metadata, :abc, :xyz, nil) + + expect(metadata).to eq({ abc: {} }) + end + end + + context "with 'section' and 'data'" do + it "adds the data to the given section" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = {} + delegate.add_metadata(metadata, :xyz, { abc: "Hello!" }) + + expect(metadata).to eq({ xyz: { abc: "Hello!" } }) + end + + it "merges the new data with any existing data in the section" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = { xyz: { x: 1, y: 2, z: 3 } } + delegate.add_metadata(metadata, :xyz, { abc: "Hello!" }) + + expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "Hello!" } }) + end + + it "does not deep merge conflicting data in the section" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = { xyz: { x: { a: 1, b: 2 } } } + delegate.add_metadata(metadata, :xyz, { x: { c: 3 } }) + + # if this was a deep merge, metadata[:xyz][:x] would be { a: 1, b: 2, c: 3 } + expect(metadata).to eq({ xyz: { x: { c: 3 } } }) + end + + it "replaces existing keys in the section" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = { xyz: { x: 0, z: "yes", abc: "??" } } + delegate.add_metadata(metadata, :xyz, { x: 1, y: 2, z: 3 }) + + expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "??" } }) + end + + it "removes keys that have a value of 'nil'" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = { xyz: { x: 0, z: "yes", abc: "??" } } + delegate.add_metadata(metadata, :xyz, { x: nil, y: 2, z: 3 }) + + expect(metadata).to eq({ xyz: { y: 2, z: 3, abc: "??" } }) + end + end + + context "with bad parameters" do + it "does nothing if called with a 'key' but no 'value'" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = {} + delegate.add_metadata(metadata, :a, :b) + + expect(metadata).to be_empty + end + + it "does nothing if called with a Hash 'key' and a value" do + delegate = Bugsnag::Utility::MetadataDelegate.new + + metadata = {} + delegate.add_metadata(metadata, :a, { b: 1 }, 123) + + expect(metadata).to be_empty + end + end + end + + context "#clear_metadata" do + context "with 'section'" do + it "removes the given section from metadata" do + delegate = Bugsnag::Utility::MetadataDelegate.new + metadata = { some: "data", goes: "here" } + + delegate.clear_metadata(metadata, :some) + + expect(metadata).to eq({ goes: "here" }) + end + + it "does nothing if the section does not exist" do + delegate = Bugsnag::Utility::MetadataDelegate.new + metadata = { some: "data", goes: "here" } + + delegate.clear_metadata(metadata, :does_not_exist) + + expect(metadata).to eq({ some: "data", goes: "here" }) + end + end + + context "with 'section' and 'key'" do + it "removes the given 'key' from 'section'" do + delegate = Bugsnag::Utility::MetadataDelegate.new + metadata = { some: { data: { goes: "here" }, also: "there" } } + + delegate.clear_metadata(metadata, :some, :data) + + expect(metadata).to eq({ some: { also: "there" } }) + end + + it "does nothing if the section does not exist" do + delegate = Bugsnag::Utility::MetadataDelegate.new + metadata = { some: { data: { goes: "here" }, also: "there" } } + + delegate.clear_metadata(metadata, :nope, :data) + + expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) + end + + it "does nothing if the key does not exist in the section" do + delegate = Bugsnag::Utility::MetadataDelegate.new + metadata = { some: { data: { goes: "here" }, also: "there" } } + + delegate.clear_metadata(metadata, :some, :nah) + + expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) + end + end + end +end From aa0f7dc56f03056ad57100e5a835308898296172 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 9 Sep 2021 11:57:07 +0100 Subject: [PATCH 53/63] Move metadata delegate tests into shared examples This will let us re-use them in other tests, to ensure all classes support every use-case of the metadata delegate --- spec/support/shared_examples_for_metadata.rb | 157 ++++++++++++++++++ spec/utility/metadata_delegate_spec.rb | 164 +------------------ 2 files changed, 163 insertions(+), 158 deletions(-) create mode 100644 spec/support/shared_examples_for_metadata.rb diff --git a/spec/support/shared_examples_for_metadata.rb b/spec/support/shared_examples_for_metadata.rb new file mode 100644 index 000000000..eb869abe7 --- /dev/null +++ b/spec/support/shared_examples_for_metadata.rb @@ -0,0 +1,157 @@ +require 'spec_helper' + +# shared examples for #add_metadata and #clear_metadata +# use by providing two functions - one that implements add_metadata and one +# that implements clear_metadata, e.g. +# +# RSpec.describe SomeClass do +# include_examples( +# 'metadata delegate', +# ->(metadata, *args) { SomeClass.new(metadata).add_metadata(*args) }, +# ->(metadata, *args) { SomeClass.new(metadata).clear_metadata(*args) } +# ) +# end +RSpec.shared_examples 'metadata delegate' do |add_metadata, clear_metadata| + context "#add_metadata" do + context "with 'section', 'key' and 'value'" do + it "adds the given key/value pair to the section" do + metadata = {} + + add_metadata.call(metadata, :abc, :xyz, "Hello!") + + expect(metadata).to eq({ abc: { xyz: "Hello!" } }) + end + + it "merges the new key/value pair with existing data in the section" do + metadata = { abc: { a: 1, b: 2, c: 3 } } + + add_metadata.call(metadata, :abc, :xyz, "Hello!") + + expect(metadata).to eq({ abc: { a: 1, b: 2, c: 3, xyz: "Hello!" } }) + end + + it "replaces existing metadata if the 'key' already exists" do + metadata = { abc: { xyz: "Hello!" } } + + add_metadata.call(metadata, :abc, :xyz, "Goodbye!") + + expect(metadata).to eq({ abc: { xyz: "Goodbye!" } }) + end + + it "removes the key if 'value' is nil" do + metadata = { abc: { xyz: "Hello!" } } + + add_metadata.call(metadata, :abc, :xyz, nil) + + expect(metadata).to eq({ abc: {} }) + end + end + + context "with 'section' and 'data'" do + it "adds the data to the given section" do + metadata = {} + + add_metadata.call(metadata, :xyz, { abc: "Hello!" }) + + expect(metadata).to eq({ xyz: { abc: "Hello!" } }) + end + + it "merges the new data with any existing data in the section" do + metadata = { xyz: { x: 1, y: 2, z: 3 } } + + add_metadata.call(metadata, :xyz, { abc: "Hello!" }) + + expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "Hello!" } }) + end + + it "does not deep merge conflicting data in the section" do + metadata = { xyz: { x: { a: 1, b: 2 } } } + + add_metadata.call(metadata, :xyz, { x: { c: 3 } }) + + # if this was a deep merge, metadata[:xyz][:x] would be { a: 1, b: 2, c: 3 } + expect(metadata).to eq({ xyz: { x: { c: 3 } } }) + end + + it "replaces existing keys in the section" do + metadata = { xyz: { x: 0, z: "yes", abc: "??" } } + + add_metadata.call(metadata, :xyz, { x: 1, y: 2, z: 3 }) + + expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "??" } }) + end + + it "removes keys that have a value of 'nil'" do + metadata = { xyz: { x: 0, z: "yes", abc: "??" } } + + add_metadata.call(metadata, :xyz, { x: nil, y: 2, z: 3 }) + + expect(metadata).to eq({ xyz: { y: 2, z: 3, abc: "??" } }) + end + end + + context "with bad parameters" do + it "does nothing if called with a 'key' but no 'value'" do + metadata = {} + + add_metadata.call(metadata, :a, :b) + + expect(metadata).to be_empty + end + + it "does nothing if called with a Hash 'key' and a value" do + metadata = {} + + add_metadata.call(metadata, :a, { b: 1 }, 123) + + expect(metadata).to be_empty + end + end + end + + context "#clear_metadata" do + context "with 'section'" do + it "removes the given section from metadata" do + metadata = { some: "data", goes: "here" } + + clear_metadata.call(metadata, :some) + + expect(metadata).to eq({ goes: "here" }) + end + + it "does nothing if the section does not exist" do + metadata = { some: "data", goes: "here" } + + clear_metadata.call(metadata, :does_not_exist) + + expect(metadata).to eq({ some: "data", goes: "here" }) + end + end + + context "with 'section' and 'key'" do + it "removes the given 'key' from 'section'" do + metadata = { some: { data: { goes: "here" }, also: "there" } } + + clear_metadata.call(metadata, :some, :data) + + expect(metadata).to eq({ some: { also: "there" } }) + end + + it "does nothing if the section does not exist" do + metadata = { some: { data: { goes: "here" }, also: "there" } } + + clear_metadata.call(metadata, :nope, :data) + + expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) + end + + it "does nothing if the key does not exist in the section" do + metadata = { some: { data: { goes: "here" }, also: "there" } } + + clear_metadata.call(metadata, :some, :nah) + + expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) + end + end + end +end diff --git a/spec/utility/metadata_delegate_spec.rb b/spec/utility/metadata_delegate_spec.rb index 0d60a10fc..f08edcdff 100644 --- a/spec/utility/metadata_delegate_spec.rb +++ b/spec/utility/metadata_delegate_spec.rb @@ -1,163 +1,11 @@ require 'spec_helper' require 'bugsnag/utility/metadata_delegate' +require 'support/shared_examples_for_metadata' RSpec.describe Bugsnag::Utility::MetadataDelegate do - context "#add_metadata" do - context "with 'section', 'key' and 'value'" do - it "adds the given key/value pair to the section" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = {} - delegate.add_metadata(metadata, :abc, :xyz, "Hello!") - - expect(metadata).to eq({ abc: { xyz: "Hello!" } }) - end - - it "merges the new key/value pair with existing data in the section" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = { abc: { a: 1, b: 2, c: 3 } } - delegate.add_metadata(metadata, :abc, :xyz, "Hello!") - - expect(metadata).to eq({ abc: { a: 1, b: 2, c: 3, xyz: "Hello!" } }) - end - - it "replaces existing metadata if the 'key' already exists" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = { abc: { xyz: "Hello!" } } - delegate.add_metadata(metadata, :abc, :xyz, "Goodbye!") - - expect(metadata).to eq({ abc: { xyz: "Goodbye!" } }) - end - - it "removes the key if 'value' is nil" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = { abc: { xyz: "Hello!" } } - delegate.add_metadata(metadata, :abc, :xyz, nil) - - expect(metadata).to eq({ abc: {} }) - end - end - - context "with 'section' and 'data'" do - it "adds the data to the given section" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = {} - delegate.add_metadata(metadata, :xyz, { abc: "Hello!" }) - - expect(metadata).to eq({ xyz: { abc: "Hello!" } }) - end - - it "merges the new data with any existing data in the section" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = { xyz: { x: 1, y: 2, z: 3 } } - delegate.add_metadata(metadata, :xyz, { abc: "Hello!" }) - - expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "Hello!" } }) - end - - it "does not deep merge conflicting data in the section" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = { xyz: { x: { a: 1, b: 2 } } } - delegate.add_metadata(metadata, :xyz, { x: { c: 3 } }) - - # if this was a deep merge, metadata[:xyz][:x] would be { a: 1, b: 2, c: 3 } - expect(metadata).to eq({ xyz: { x: { c: 3 } } }) - end - - it "replaces existing keys in the section" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = { xyz: { x: 0, z: "yes", abc: "??" } } - delegate.add_metadata(metadata, :xyz, { x: 1, y: 2, z: 3 }) - - expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "??" } }) - end - - it "removes keys that have a value of 'nil'" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = { xyz: { x: 0, z: "yes", abc: "??" } } - delegate.add_metadata(metadata, :xyz, { x: nil, y: 2, z: 3 }) - - expect(metadata).to eq({ xyz: { y: 2, z: 3, abc: "??" } }) - end - end - - context "with bad parameters" do - it "does nothing if called with a 'key' but no 'value'" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = {} - delegate.add_metadata(metadata, :a, :b) - - expect(metadata).to be_empty - end - - it "does nothing if called with a Hash 'key' and a value" do - delegate = Bugsnag::Utility::MetadataDelegate.new - - metadata = {} - delegate.add_metadata(metadata, :a, { b: 1 }, 123) - - expect(metadata).to be_empty - end - end - end - - context "#clear_metadata" do - context "with 'section'" do - it "removes the given section from metadata" do - delegate = Bugsnag::Utility::MetadataDelegate.new - metadata = { some: "data", goes: "here" } - - delegate.clear_metadata(metadata, :some) - - expect(metadata).to eq({ goes: "here" }) - end - - it "does nothing if the section does not exist" do - delegate = Bugsnag::Utility::MetadataDelegate.new - metadata = { some: "data", goes: "here" } - - delegate.clear_metadata(metadata, :does_not_exist) - - expect(metadata).to eq({ some: "data", goes: "here" }) - end - end - - context "with 'section' and 'key'" do - it "removes the given 'key' from 'section'" do - delegate = Bugsnag::Utility::MetadataDelegate.new - metadata = { some: { data: { goes: "here" }, also: "there" } } - - delegate.clear_metadata(metadata, :some, :data) - - expect(metadata).to eq({ some: { also: "there" } }) - end - - it "does nothing if the section does not exist" do - delegate = Bugsnag::Utility::MetadataDelegate.new - metadata = { some: { data: { goes: "here" }, also: "there" } } - - delegate.clear_metadata(metadata, :nope, :data) - - expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) - end - - it "does nothing if the key does not exist in the section" do - delegate = Bugsnag::Utility::MetadataDelegate.new - metadata = { some: { data: { goes: "here" }, also: "there" } } - - delegate.clear_metadata(metadata, :some, :nah) - - expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) - end - end - end + include_examples( + 'metadata delegate', + Bugsnag::Utility::MetadataDelegate.new.method(:add_metadata), + Bugsnag::Utility::MetadataDelegate.new.method(:clear_metadata) + ) end From 4f8d6f90a14f041d1c05b3520af92d173fc3a293 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 9 Sep 2021 11:58:05 +0100 Subject: [PATCH 54/63] Add #add_metadata and #clear_metadata methods --- lib/bugsnag.rb | 2 ++ lib/bugsnag/report.rb | 39 +++++++++++++++++++++++++ spec/report_spec.rb | 67 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 6c47e3f5e..f19d50192 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -34,6 +34,8 @@ require "bugsnag/breadcrumbs/breadcrumb" require "bugsnag/breadcrumbs/breadcrumbs" +require "bugsnag/utility/metadata_delegate" + # rubocop:todo Metrics/ModuleLength module Bugsnag LOCK = Mutex.new diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index c1c021274..d0fe53346 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -143,6 +143,8 @@ def initialize(exception, passed_configuration, auto_notify=false) self.severity = auto_notify ? "error" : "warning" self.severity_reason = auto_notify ? {:type => UNHANDLED_EXCEPTION} : {:type => HANDLED_EXCEPTION} self.user = {} + + @metadata_delegate = Utility::MetadataDelegate.new end ## @@ -316,6 +318,43 @@ def request @meta_data[:request] end + ## + # Add values to metadata + # + # @overload add_metadata(section, data) + # Merges data into the given section of metadata + # @param section [String, Symbol] + # @param data [Hash] + # + # @overload add_metadata(section, key, value) + # Sets key to value in the given section of metadata. If the value is nil + # the key will be deleted + # @param section [String, Symbol] + # @param key [String, Symbol] + # @param value + # + # @return [void] + def add_metadata(section, key_or_data, *args) + @metadata_delegate.add_metadata(@meta_data, section, key_or_data, *args) + end + + ## + # Clear values from metadata + # + # @overload clear_metadata(section) + # Clears the given section of metadata + # @param section [String, Symbol] + # + # @overload clear_metadata(section, key) + # Clears the key in the given section of metadata + # @param section [String, Symbol] + # @param key [String, Symbol] + # + # @return [void] + def clear_metadata(section, *args) + @metadata_delegate.clear_metadata(@meta_data, section, *args) + end + private def generate_exception_list diff --git a/spec/report_spec.rb b/spec/report_spec.rb index afdb4ab53..173f1d2c7 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -2,6 +2,7 @@ require_relative './spec_helper' require 'securerandom' require 'ostruct' +require 'support/shared_examples_for_metadata' module ActiveRecord; class RecordNotFound < RuntimeError; end; end class NestedException < StandardError; attr_accessor :original_exception; end @@ -28,6 +29,24 @@ def gloops end shared_examples "Report or Event tests" do |class_to_test| + context "metadata" do + include_examples( + "metadata delegate", + lambda do |metadata, *args| + report = class_to_test.new(RuntimeError.new, Bugsnag.configuration) + report.metadata = metadata + + report.add_metadata(*args) + end, + lambda do |metadata, *args| + report = class_to_test.new(RuntimeError.new, Bugsnag.configuration) + report.metadata = metadata + + report.clear_metadata(*args) + end + ) + end + it "#headers should return the correct request headers" do fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) expect(Time).to receive(:now).twice.and_return(fake_now) @@ -349,7 +368,53 @@ def gloops end end - # TODO: nested context + it "metadata added with 'add_metadata' ends up in the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.add_metadata( + :some_tab, + { info: "here", data: "also here" } + ) + + report.add_metadata(:some_other_tab, :info, true) + report.add_metadata(:some_other_tab, :data, "very true") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["metaData"]).to eq({ + "some_tab" => { + "info" => "here", + "data" => "also here" + }, + "some_other_tab" => { + "info" => true, + "data" => "very true" + } + }) + }) + end + + it "metadata removed with 'clear_metadata' does not end up in the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.add_metadata( + :some_tab, + { info: "here", data: "also here" } + ) + + report.add_metadata(:some_other_tab, :info, true) + report.add_metadata(:some_other_tab, :data, "very true") + + report.clear_metadata(:some_tab) + report.clear_metadata(:some_other_tab, :info) + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["metaData"]).to eq({ + "some_other_tab" => { "data" => "very true" } + }) + }) + end it "accepts tabs in overrides and adds them to metaData" do Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| From cb0e92648ec3cf13b1066061bfc8b4c4a58011e6 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 9 Sep 2021 13:55:21 +0100 Subject: [PATCH 55/63] Deprecated add_tab & remove_tab These can be replaced with add/clear metadata --- lib/bugsnag/report.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index d0fe53346..a18a7bd5d 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -174,6 +174,8 @@ def context # exists, this will be merged with the existing values. If a Hash is not # given, the value will be placed into the 'custom' tab # @return [void] + # + # @deprecated Use {#add_metadata} instead def add_tab(name, value) return if name.nil? @@ -192,6 +194,8 @@ def add_tab(name, value) # # @param name [String] # @return [void] + # + # @deprecated Use {#clear_metadata} instead def remove_tab(name) return if name.nil? From 17bdb425d5eb2bf997453a17613555cf19b1a5de Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 9 Sep 2021 13:58:14 +0100 Subject: [PATCH 56/63] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 449f5d25a..4b88ef31c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ Changelog | [#692](https://github.com/bugsnag/bugsnag-ruby/pull/692) * Add `request` to `Report`/`Event` containing HTTP request metadata | [#693](https://github.com/bugsnag/bugsnag-ruby/pull/693) +* Add `add_metadata` and `clear_metadata` to `Report`/`Event` + | [#694](https://github.com/bugsnag/bugsnag-ruby/pull/694) ### Fixes @@ -46,6 +48,7 @@ Changelog * `Report#exceptions` has been deprecated in favour of the new `errors` property * `Report#raw_exceptions` has been deprecated in favour of the new `original_error` property * Accessing request data via `Report#metadata` has been deprecated in favour of using the new `request` property. Request data will be moved out of metadata in the next major version +* The `Report#add_tab` and `Report#remove_tab` methods have been deprecated in favour of the new `add_metadata` and `clear_metadata` methods ## v6.22.1 (11 August 2021) From 1074a63c540ab74348365220fc9f5ddd372526ad Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 10 Sep 2021 11:25:50 +0100 Subject: [PATCH 57/63] Add the stacktrace to each Error --- lib/bugsnag/error.rb | 4 +- lib/bugsnag/report.rb | 7 ++- spec/report_spec.rb | 115 +++++++++++++++++++++++++++++++++++------- 3 files changed, 105 insertions(+), 21 deletions(-) diff --git a/lib/bugsnag/error.rb b/lib/bugsnag/error.rb index 581b4adea..2181de6f8 100644 --- a/lib/bugsnag/error.rb +++ b/lib/bugsnag/error.rb @@ -3,7 +3,9 @@ module Bugsnag # @return [String] the error's class name # @!attribute error_message # @return [String] the error's message + # @!attribute stacktrace + # @return [Hash] the error's processed stacktrace # @!attribute type # @return [String] the type of error (always "ruby") - Error = Struct.new(:error_class, :error_message, :type) + Error = Struct.new(:error_class, :error_message, :stacktrace, :type) end diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index a18a7bd5d..b6063cc57 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -373,7 +373,12 @@ def generate_exception_list def generate_error_list exceptions.map do |exception| - Error.new(exception[:errorClass], exception[:message], ERROR_TYPE) + Error.new( + exception[:errorClass], + exception[:message], + exception[:stacktrace], + ERROR_TYPE + ) end end diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 173f1d2c7..5a18a7a33 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -146,13 +146,18 @@ def gloops end describe "#errors" do - it "has required hash keys" do + it "has required attributes" do exception = RuntimeError.new("example error") report = class_to_test.new(exception, Bugsnag.configuration) - expect(report.errors).to eq( - [Bugsnag::Error.new("RuntimeError", "example error", "ruby")] - ) + expect(report.errors.length).to eq(1) + + error = report.errors.first + + expect(error).to respond_to(:error_class) + expect(error).to respond_to(:error_message) + expect(error).to respond_to(:type) + expect(error).to respond_to(:stacktrace) end it "includes errors that caused the top-most exception" do @@ -167,12 +172,21 @@ def gloops report = class_to_test.new(exception, Bugsnag.configuration) - expect(report.errors).to eq( - [ - Bugsnag::Error.new("Ruby21Exception", "two", "ruby"), - Bugsnag::Error.new("RuntimeError", "one", "ruby") - ] - ) + expect(report.errors.length).to eq(2) + + expect(report.errors[0].stacktrace).not_to be_empty + expect(report.errors[0]).to have_attributes({ + error_class: "Ruby21Exception", + error_message: "two", + type: "ruby" + }) + + expect(report.errors[1].stacktrace).not_to be_empty + expect(report.errors[1]).to have_attributes({ + error_class: "RuntimeError", + error_message: "one", + type: "ruby" + }) end it "cannot be assigned to" do @@ -190,25 +204,88 @@ def gloops report.errors.push("haha 2") report.errors.pop - expect(report.errors).to eq( - [ - Bugsnag::Error.new("RuntimeError", "example error", "ruby"), - "haha" - ] - ) + expect(report.errors.length).to eq(2) + + expect(report.errors.first.stacktrace).not_to be_empty + expect(report.errors.first).to have_attributes({ + error_class: "RuntimeError", + error_message: "example error", + type: "ruby" + }) + + expect(report.errors[1]).to eq("haha") end it "contains mutable data" do exception = RuntimeError.new("example error") report = class_to_test.new(exception, Bugsnag.configuration) + expect(report.errors.length).to eq(1) + report.errors.first.error_class = "haha" report.errors.first.error_message = "ahah" report.errors.first.type = "aahh" - expect(report.errors).to eq( - [Bugsnag::Error.new("haha", "ahah", "aahh")] - ) + expect(report.errors.first.stacktrace).not_to be_empty + expect(report.errors.first).to have_attributes({ + error_class: "haha", + error_message: "ahah", + type: "aahh" + }) + end + + it "shares the stacktrace with #exceptions" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.errors.length).to eq(1) + expect(report.exceptions.length).to eq(1) + + error = report.errors.first + exception = report.exceptions.first + + expect(error.stacktrace).not_to be_empty + expect(error.stacktrace).to all(have_key(:lineNumber)) + expect(error.stacktrace).to all(have_key(:file)) + expect(error.stacktrace).to all(have_key(:method)) + expect(error.stacktrace).to all(have_key(:code)) + + expect(error.stacktrace).to be(exception[:stacktrace]) + end + + it "mutating the stacktrace affects the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + expect(report.errors.length).to eq(1) + + error = report.errors.first + + error.stacktrace.clear + error.stacktrace[0] = { + lineNumber: 123, + file: "/dev/null", + method: "do_nothing", + code: "yes, lots" + } + + # reassiging the stacktrace will not affect the payload, until + # 'report#errors' replaces 'report#exceptions' entirely + error.stacktrace = [] + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + exception = get_exception_from_payload(payload) + + expect(exception["stacktrace"]).to eq( + [ + { + "lineNumber" => 123, + "file" => "/dev/null", + "method" => "do_nothing", + "code" => "yes, lots" + } + ] + ) + }) end end From 9f3a75f916f323b4dd204d9df4a39deaf7f338b1 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 10 Sep 2021 12:12:36 +0100 Subject: [PATCH 58/63] Refactor Error class to make stacktrace readonly --- lib/bugsnag/error.rb | 32 +++++++++++++++++++++++--------- lib/bugsnag/report.rb | 10 +--------- spec/report_spec.rb | 9 +++++---- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/lib/bugsnag/error.rb b/lib/bugsnag/error.rb index 2181de6f8..d6e429edd 100644 --- a/lib/bugsnag/error.rb +++ b/lib/bugsnag/error.rb @@ -1,11 +1,25 @@ module Bugsnag - # @!attribute error_class - # @return [String] the error's class name - # @!attribute error_message - # @return [String] the error's message - # @!attribute stacktrace - # @return [Hash] the error's processed stacktrace - # @!attribute type - # @return [String] the type of error (always "ruby") - Error = Struct.new(:error_class, :error_message, :stacktrace, :type) + class Error + # @return [String] the error's class name + attr_accessor :error_class + + # @return [String] the error's message + attr_accessor :error_message + + # @return [Hash] the error's processed stacktrace + attr_reader :stacktrace + + # @return [String] the type of error (always "ruby") + attr_accessor :type + + # @api private + TYPE = "ruby".freeze + + def initialize(error_class, error_message, stacktrace) + @error_class = error_class + @error_message = error_message + @stacktrace = stacktrace + @type = TYPE + end + end end diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index b6063cc57..843540e1f 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -21,9 +21,6 @@ class Report CURRENT_PAYLOAD_VERSION = "4.0" - # @api private - ERROR_TYPE = "ruby".freeze - # Whether this report is for a handled or unhandled error # @return [Boolean] attr_reader :unhandled @@ -373,12 +370,7 @@ def generate_exception_list def generate_error_list exceptions.map do |exception| - Error.new( - exception[:errorClass], - exception[:message], - exception[:stacktrace], - ERROR_TYPE - ) + Error.new(exception[:errorClass], exception[:message], exception[:stacktrace]) end end diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 5a18a7a33..da638f323 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -158,6 +158,11 @@ def gloops expect(error).to respond_to(:error_message) expect(error).to respond_to(:type) expect(error).to respond_to(:stacktrace) + + expect(error).to respond_to(:error_class=) + expect(error).to respond_to(:error_message=) + expect(error).to respond_to(:type=) + expect(error).not_to respond_to(:stacktrace=) end it "includes errors that caused the top-most exception" do @@ -266,10 +271,6 @@ def gloops method: "do_nothing", code: "yes, lots" } - - # reassiging the stacktrace will not affect the payload, until - # 'report#errors' replaces 'report#exceptions' entirely - error.stacktrace = [] end expect(Bugsnag).to(have_sent_notification { |payload, _headers| From 6e63b627b7a34f780df2ecb33daede59a05b1d24 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 10 Sep 2021 16:26:23 +0100 Subject: [PATCH 59/63] Add Report#set_user --- lib/bugsnag/report.rb | 18 +++++++++ spec/report_spec.rb | 90 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 97 insertions(+), 11 deletions(-) diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index a18a7bd5d..d8b283415 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -359,6 +359,24 @@ def clear_metadata(section, *args) @metadata_delegate.clear_metadata(@meta_data, section, *args) end + ## + # Set information about the current user + # + # Additional user fields can be added as metadata in a "user" section + # + # Setting a field to 'nil' will remove it from the user data + # + # @param id [String, nil] + # @param email [String, nil] + # @param name [String, nil] + # @return [void] + def set_user(id = nil, email = nil, name = nil) + new_user = { id: id, email: email, name: name } + new_user.reject! { |key, value| value.nil? } + + @user = new_user + end + private def generate_exception_list diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 173f1d2c7..85b5f77fe 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -792,17 +792,6 @@ def gloops }) end - it "accepts a user_id in overrides" do - Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| - report.user = {id: 'test_user'} - end - - expect(Bugsnag).to have_sent_notification{ |payload, headers| - event = get_event_from_payload(payload) - expect(event["user"]["id"]).to eq("test_user") - } - end - it "does not send an automatic notification if auto_notify is false" do Bugsnag.configure do |config| config.auto_notify = false @@ -1869,5 +1858,84 @@ def to_s expect(headers["Bugsnag-Sent-At"]).to eq("2021-01-02T03:04:05.123Z") }) end + + context "#user" do + it "accepts an arbitrary user hash" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.user = { id: "test_user", abc: "xyz" } + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("test_user") + expect(event["user"]["abc"]).to eq("xyz") + }) + end + + it "set_user will set the three supported fields" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123", "abc.xyz@example.com", "abc xyz") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]["email"]).to eq("abc.xyz@example.com") + expect(event["user"]["name"]).to eq("abc xyz") + }) + end + + it "set_user will not set fields that are 'nil'" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123", nil, "abc xyz") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]).not_to have_key("email") + expect(event["user"]["name"]).to eq("abc xyz") + }) + end + + it "set_user will unset all fields if passed no parameters" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.user = { id: "nope", email: "nah@example.com", name: "yes", other: "stuff" } + + report.set_user + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]).to be_empty + }) + end + + it "set_user can be passed only an ID" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]).not_to have_key("email") + expect(event["user"]).not_to have_key("name") + }) + end + + it "set_user can be passed only an ID and email" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123", "123@example.com") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]["email"]).to eq("123@example.com") + expect(event["user"]).not_to have_key("name") + }) + end + end end # rubocop:enable Metrics/BlockLength From 91ff9c7a9cde97321576a24ccffd38b1385ae5fc Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 10 Sep 2021 16:50:01 +0100 Subject: [PATCH 60/63] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b88ef31c..273c71b45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ Changelog | [#693](https://github.com/bugsnag/bugsnag-ruby/pull/693) * Add `add_metadata` and `clear_metadata` to `Report`/`Event` | [#694](https://github.com/bugsnag/bugsnag-ruby/pull/694) +* Add `set_user` to `Report`/`Event` + | [#695](https://github.com/bugsnag/bugsnag-ruby/pull/695) ### Fixes From ca54356f169674bd36efe47ac76d224ca5242bb1 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 14 Sep 2021 14:37:02 +0100 Subject: [PATCH 61/63] Add stacktrace to Error object summary --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 273c71b45..ce6f34ca2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ Changelog | [#689](https://github.com/bugsnag/bugsnag-ruby/pull/689) * Add `time` (an ISO8601 string in UTC) to `device` metadata | [#690](https://github.com/bugsnag/bugsnag-ruby/pull/690) -* Add `errors` to `Report`/`Event` containing an array of `Error` objects. The `Error` object contains `error_class`, `error_message` and `type` (always "ruby") +* Add `errors` to `Report`/`Event` containing an array of `Error` objects. The `Error` object contains `error_class`, `error_message`, `stacktrace` and `type` (always "ruby") | [#691](https://github.com/bugsnag/bugsnag-ruby/pull/691) * Add `original_error` to `Report`/`Event` containing the original Exception instance | [#692](https://github.com/bugsnag/bugsnag-ruby/pull/692) From 2089377b16c4de6874217fe3200c7885f24e5402 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 17 Sep 2021 11:47:11 +0100 Subject: [PATCH 62/63] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce6f34ca2..a78b81907 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ Changelog ========= -## TBD +## v6.23.0 (21 September 2021) ### Enhancements From 00460bf8146ab010a4536d5223f7f251a639bbb5 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 17 Sep 2021 11:47:31 +0100 Subject: [PATCH 63/63] Bump version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 261151bf6..0b31cc63b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.22.1 +6.23.0