From d145e2605e1c28b5403d39778d122a48aa6f9b0b Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 25 Aug 2021 13:34:09 +0100 Subject: [PATCH 1/5] 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 794a0e41..53cf52f0 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 b8c7baa4..58a523a8 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 2/5] 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 502895cf..560dcfb0 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 5ff05ef7..b85e7f9f 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 c82d5052..bcef91c2 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 5caa960a..fddece6b 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 6825ccf2..2fe6e365 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 16cc588d..094350d2 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 ba6d6ca3..379ff832 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 453da2b2..6adcdd7b 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 be214b3d..efc186e6 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 57847efa..d100acf2 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 bfd2cc40..6139ae86 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 952d2337..c75ac1f7 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 3/5] 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 49e677b6..96da7fd6 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 4/5] 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 96da7fd6..f1cba934 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 53cf52f0..0aa59952 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 58a523a8..14f36dcc 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 5/5] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d96d824..48f5943d 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