From c23e18a5dea71e281ac80de6159295fb9068cce8 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 30 Sep 2021 09:33:29 +0100 Subject: [PATCH 1/6] Add EndpointConfiguration class --- lib/bugsnag/endpoint_configuration.rb | 11 +++++++++++ spec/endpoint_configuration_spec.rb | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 lib/bugsnag/endpoint_configuration.rb create mode 100644 spec/endpoint_configuration_spec.rb diff --git a/lib/bugsnag/endpoint_configuration.rb b/lib/bugsnag/endpoint_configuration.rb new file mode 100644 index 00000000..244bc361 --- /dev/null +++ b/lib/bugsnag/endpoint_configuration.rb @@ -0,0 +1,11 @@ +module Bugsnag + class EndpointConfiguration + attr_reader :notify + attr_reader :sessions + + def initialize(notify, sessions) + @notify = notify + @sessions = sessions + end + end +end diff --git a/spec/endpoint_configuration_spec.rb b/spec/endpoint_configuration_spec.rb new file mode 100644 index 00000000..81aacebd --- /dev/null +++ b/spec/endpoint_configuration_spec.rb @@ -0,0 +1,19 @@ +require "spec_helper" + +require "bugsnag/endpoint_configuration" + +describe Bugsnag::EndpointConfiguration do + it "has notify & session URLs" do + configuration = Bugsnag::EndpointConfiguration.new("notify", "session") + + expect(configuration.notify).to eq("notify") + expect(configuration.sessions).to eq("session") + end + + it "is immutable" do + configuration = Bugsnag::EndpointConfiguration.new("notify", "session") + + expect(configuration).not_to respond_to(:notify=) + expect(configuration).not_to respond_to(:session=) + end +end From d31ed0127a6fc240929d7232d2637b1d8a312181 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 30 Sep 2021 09:33:50 +0100 Subject: [PATCH 2/6] Add validation for EndpointConfiguration --- lib/bugsnag/endpoint_validator.rb | 80 ++++++++++++++++++++++ spec/endpoint_validator_spec.rb | 106 ++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 lib/bugsnag/endpoint_validator.rb create mode 100644 spec/endpoint_validator_spec.rb diff --git a/lib/bugsnag/endpoint_validator.rb b/lib/bugsnag/endpoint_validator.rb new file mode 100644 index 00000000..b2957784 --- /dev/null +++ b/lib/bugsnag/endpoint_validator.rb @@ -0,0 +1,80 @@ +module Bugsnag + # @api private + class EndpointValidator + def self.validate(endpoints) + # ensure we have an EndpointConfiguration object + return Result.missing_urls unless endpoints.is_a?(EndpointConfiguration) + + # check for missing URLs + return Result.missing_urls if endpoints.notify.nil? && endpoints.sessions.nil? + return Result.missing_notify if endpoints.notify.nil? + return Result.missing_session if endpoints.sessions.nil? + + # check for empty URLs + return Result.invalid_urls if endpoints.notify.empty? && endpoints.sessions.empty? + return Result.invalid_notify if endpoints.notify.empty? + return Result.invalid_session if endpoints.sessions.empty? + + Result.valid + end + + # @api private + class Result + # rubocop:disable Layout/LineLength + MISSING_URLS = "Invalid configuration. endpoints must be set with both a notify and session URL. Bugsnag will not send any requests.".freeze + MISSING_NOTIFY_URL = "Invalid configuration. endpoints.notify cannot be set without also setting endpoints.sessions. Sessions will not be sent to Bugsnag.".freeze + MISSING_SESSION_URL = "Invalid configuration. endpoints.sessions cannot be set without also setting endpoints.notify. Bugsnag will not send any requests.".freeze + + INVALID_URLS = "Invalid configuration. endpoints should be valid URLs, got empty strings. Bugsnag will not send any requests.".freeze + INVALID_NOTIFY_URL = "Invalid configuration. endpoints.notify should be a valid URL, got empty string. Sessions will not be sent to Bugsnag.".freeze + INVALID_SESSION_URL = "Invalid configuration. endpoints.sessions should be a valid URL, got empty string. Bugsnag will not send any requests.".freeze + # rubocop:enable Layout/LineLength + + attr_reader :reason + + def initialize(valid, keep_events_enabled_for_backwards_compatibility = true, reason = nil) + @valid = valid + @keep_events_enabled_for_backwards_compatibility = keep_events_enabled_for_backwards_compatibility + @reason = reason + end + + def valid? + @valid + end + + def keep_events_enabled_for_backwards_compatibility? + @keep_events_enabled_for_backwards_compatibility + end + + # factory functions + + def self.valid + new(true) + end + + def self.missing_urls + new(false, false, MISSING_URLS) + end + + def self.missing_notify + new(false, false, MISSING_NOTIFY_URL) + end + + def self.missing_session + new(false, true, MISSING_SESSION_URL) + end + + def self.invalid_urls + new(false, false, INVALID_URLS) + end + + def self.invalid_notify + new(false, false, INVALID_NOTIFY_URL) + end + + def self.invalid_session + new(false, true, INVALID_SESSION_URL) + end + end + end +end diff --git a/spec/endpoint_validator_spec.rb b/spec/endpoint_validator_spec.rb new file mode 100644 index 00000000..351cb39b --- /dev/null +++ b/spec/endpoint_validator_spec.rb @@ -0,0 +1,106 @@ +require "spec_helper" + +require "bugsnag/endpoint_configuration" +require "bugsnag/endpoint_validator" + +describe Bugsnag::EndpointValidator do + describe "#validate" do + it "returns an invalid result if given nil" do + result = Bugsnag::EndpointValidator.validate(nil) + + expect(result).to have_attributes({ + valid?: false, + keep_events_enabled_for_backwards_compatibility?: false, + reason: Bugsnag::EndpointValidator::Result::MISSING_URLS, + }) + end + + it "returns an invalid result if no URL is set" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new(nil, nil) + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: false, + keep_events_enabled_for_backwards_compatibility?: false, + reason: Bugsnag::EndpointValidator::Result::MISSING_URLS, + }) + end + + it "returns an invalid result if notify URL is not set" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new(nil, "sessions.example.com") + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: false, + keep_events_enabled_for_backwards_compatibility?: false, + reason: Bugsnag::EndpointValidator::Result::MISSING_NOTIFY_URL, + }) + end + + it "returns an invalid result if session URL is not set" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new("notify.example.com", nil) + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: false, + keep_events_enabled_for_backwards_compatibility?: true, + reason: Bugsnag::EndpointValidator::Result::MISSING_SESSION_URL, + }) + end + + it "returns an invalid result if both URLs are empty" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new("", "") + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: false, + keep_events_enabled_for_backwards_compatibility?: false, + reason: Bugsnag::EndpointValidator::Result::INVALID_URLS, + }) + end + + it "returns an invalid result if notify URL is empty" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new("", "session.example.com") + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: false, + keep_events_enabled_for_backwards_compatibility?: false, + reason: Bugsnag::EndpointValidator::Result::INVALID_NOTIFY_URL, + }) + end + + it "returns an invalid result if session URL is empty" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new("notify.example.com", "") + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: false, + keep_events_enabled_for_backwards_compatibility?: true, + reason: Bugsnag::EndpointValidator::Result::INVALID_SESSION_URL, + }) + end + + it "returns a valid result when given two valid URLs" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new("notify.example.com", "session.example.com") + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: true, + keep_events_enabled_for_backwards_compatibility?: true, + reason: nil, + }) + end + + it "returns a valid result when given two non-empty strings" do + endpoint_configuration = Bugsnag::EndpointConfiguration.new("a b c", "x y z") + result = Bugsnag::EndpointValidator.validate(endpoint_configuration) + + expect(result).to have_attributes({ + valid?: true, + keep_events_enabled_for_backwards_compatibility?: true, + reason: nil, + }) + end + end +end From aa84d9f887fd561853226e9ba6d06ebde6c17211 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 30 Sep 2021 10:03:02 +0100 Subject: [PATCH 3/6] Use the new endpoint configuration --- lib/bugsnag/configuration.rb | 70 +++++++++--- spec/bugsnag_spec.rb | 6 +- spec/configuration_spec.rb | 206 ++++++++++++++++++++++++++--------- 3 files changed, 211 insertions(+), 71 deletions(-) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 48958da5..ff775dd4 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -13,6 +13,8 @@ require "bugsnag/utility/circular_buffer" require "bugsnag/breadcrumbs/breadcrumbs" require "bugsnag/breadcrumbs/on_breadcrumb_callback_list" +require "bugsnag/endpoint_configuration" +require "bugsnag/endpoint_validator" module Bugsnag class Configuration @@ -114,16 +116,17 @@ class Configuration # @return [Set] attr_accessor :ignore_classes - # The URL error notifications will be delivered to - # @return [String] - attr_reader :notify_endpoint - alias :endpoint :notify_endpoint + # The URLs to send events and sessions to + # @return [EndpointConfiguration] + attr_reader :endpoints - # The URL session notifications will be delivered to - # @return [String] - attr_reader :session_endpoint + # Whether events will be delivered + # @api private + # @return [Boolean] + attr_reader :enable_events # Whether sessions will be delivered + # @api private # @return [Boolean] attr_reader :enable_sessions @@ -217,9 +220,9 @@ def initialize # to avoid infinite recursion when creating breadcrumb buffer @max_breadcrumbs = DEFAULT_MAX_BREADCRUMBS - # These are set exclusively using the "set_endpoints" method - @notify_endpoint = DEFAULT_NOTIFY_ENDPOINT - @session_endpoint = DEFAULT_SESSION_ENDPOINT + @endpoints = EndpointConfiguration.new(DEFAULT_NOTIFY_ENDPOINT, DEFAULT_SESSION_ENDPOINT) + + @enable_events = true @enable_sessions = true @metadata = {} @@ -466,26 +469,44 @@ def breadcrumbs request_data[:breadcrumbs] ||= Bugsnag::Utility::CircularBuffer.new(@max_breadcrumbs) end + # The URL error notifications will be delivered to + # @!attribute notify_endpoint + # @return [String] + # @deprecated Use {#endpoints} instead + def notify_endpoint + @endpoints.notify + end + + alias :endpoint :notify_endpoint + # Sets the notification endpoint # - # @deprecated Use {#set_endpoints} instead + # @deprecated Use {#endpoints} instead # # @param new_notify_endpoint [String] The URL to deliver error notifications to # @return [void] def endpoint=(new_notify_endpoint) - warn("The 'endpoint' configuration option is deprecated. The 'set_endpoints' method should be used instead") + warn("The 'endpoint' configuration option is deprecated. Set both endpoints with the 'endpoints=' method instead") set_endpoints(new_notify_endpoint, session_endpoint) # Pass the existing session_endpoint through so it doesn't get overwritten end + # The URL session notifications will be delivered to + # @!attribute session_endpoint + # @return [String] + # @deprecated Use {#endpoints} instead + def session_endpoint + @endpoints.sessions + end + ## # Sets the sessions endpoint # - # @deprecated Use {#set_endpoints} instead + # @deprecated Use {#endpoints} instead # # @param new_session_endpoint [String] The URL to deliver session notifications to # @return [void] def session_endpoint=(new_session_endpoint) - warn("The 'session_endpoint' configuration option is deprecated. The 'set_endpoints' method should be used instead") + warn("The 'session_endpoint' configuration option is deprecated. Set both endpoints with the 'endpoints=' method instead") set_endpoints(notify_endpoint, new_session_endpoint) # Pass the existing notify_endpoint through so it doesn't get overwritten end @@ -495,9 +516,26 @@ def session_endpoint=(new_session_endpoint) # @param new_notify_endpoint [String] The URL to deliver error notifications to # @param new_session_endpoint [String] The URL to deliver session notifications to # @return [void] + # @deprecated Use {#endpoints} instead def set_endpoints(new_notify_endpoint, new_session_endpoint) - @notify_endpoint = new_notify_endpoint - @session_endpoint = new_session_endpoint + self.endpoints = EndpointConfiguration.new(new_notify_endpoint, new_session_endpoint) + end + + def endpoints=(endpoint_configuration) + result = EndpointValidator.validate(endpoint_configuration) + + if result.valid? + @enable_events = true + @enable_sessions = true + else + warn(result.reason) + + @enable_events = result.keep_events_enabled_for_backwards_compatibility? + @enable_sessions = false + end + + # use the given endpoints even if they are invalid + @endpoints = endpoint_configuration end ## diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index e1b4e482..4eb91378 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -121,6 +121,7 @@ end it "warns and disables sessions if a notify endpoint is set without a session endpoint" do + expect(Bugsnag.configuration).to receive(:warn).with(Bugsnag::EndpointValidator::Result::MISSING_SESSION_URL) expect(Bugsnag.configuration).to receive(:warn).with("The session endpoint has not been set, all further session capturing will be disabled") expect(Bugsnag.configuration).to receive(:disable_sessions) Bugsnag.configuration.set_endpoints(custom_notify_endpoint, nil) @@ -133,9 +134,10 @@ end it "is called after the configuration block has returned" do - expect(Bugsnag.configuration).to receive(:warn).with("The 'endpoint' configuration option is deprecated. The 'set_endpoints' method should be used instead").once - expect(Bugsnag.configuration).to receive(:warn).with("The 'session_endpoint' configuration option is deprecated. The 'set_endpoints' method should be used instead").once + expect(Bugsnag.configuration).to receive(:warn).with("The 'endpoint' configuration option is deprecated. Set both endpoints with the 'endpoints=' method instead").once + expect(Bugsnag.configuration).to receive(:warn).with("The 'session_endpoint' configuration option is deprecated. Set both endpoints with the 'endpoints=' method instead").once expect(Bugsnag.configuration).not_to receive(:warn).with("The session endpoint has not been set, all further session capturing will be disabled") + Bugsnag.configure do |configuration| configuration.endpoint = custom_notify_endpoint configuration.session_endpoint = custom_session_endpoint diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index c04b708a..df0567ea 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -73,26 +73,6 @@ end end - describe "#notify_endpoint" do - it "defaults to DEFAULT_NOTIFY_ENDPOINT" do - expect(subject.notify_endpoint).to eq(Bugsnag::Configuration::DEFAULT_NOTIFY_ENDPOINT) - end - - it "is readonly" do - expect{ subject.notify_endpoint = "My Custom Url" }.to raise_error(NoMethodError) - end - - it "is the same as endpoint" do - expect(subject.notify_endpoint).to equal(subject.endpoint) - end - end - - describe "#session_endpoint" do - it "defaults to DEFAULT_SESSION_ENDPOINT" do - expect(subject.session_endpoint).to eq(Bugsnag::Configuration::DEFAULT_SESSION_ENDPOINT) - end - end - describe "#auto_capture_sessions" do it "defaults to true" do expect(subject.auto_capture_sessions).to eq(true) @@ -125,47 +105,167 @@ end end - describe "#endpoint=" do - let(:custom_notify_endpoint) { "My custom notify endpoint" } - let(:session_endpoint) { "My session endpoint" } - it "calls #warn with a deprecation notice" do - allow(subject).to receive(:set_endpoints) - expect(subject).to receive(:warn).with("The 'endpoint' configuration option is deprecated. The 'set_endpoints' method should be used instead") - subject.endpoint = custom_notify_endpoint + describe "endpoint configuration" do + describe "#notify_endpoint" do + it "defaults to DEFAULT_NOTIFY_ENDPOINT" do + expect(subject.notify_endpoint).to eq(Bugsnag::Configuration::DEFAULT_NOTIFY_ENDPOINT) + end + + it "is readonly" do + expect{ subject.notify_endpoint = "My Custom Url" }.to raise_error(NoMethodError) + end + + it "is the same as endpoint" do + expect(subject.notify_endpoint).to equal(subject.endpoint) + end + end + + describe "#session_endpoint" do + it "defaults to DEFAULT_SESSION_ENDPOINT" do + expect(subject.session_endpoint).to eq(Bugsnag::Configuration::DEFAULT_SESSION_ENDPOINT) + end end - it "calls #set_endpoints with the new notify_endpoint and existing session endpoint" do - allow(subject).to receive(:session_endpoint).and_return(session_endpoint) - allow(subject).to receive(:warn) - expect(subject).to receive(:set_endpoints).with(custom_notify_endpoint, session_endpoint) - subject.endpoint = custom_notify_endpoint + describe "#endpoint=" do + let(:custom_notify_endpoint) { "My custom notify endpoint" } + let(:session_endpoint) { "My session endpoint" } + it "calls #warn with a deprecation notice" do + allow(subject).to receive(:set_endpoints) + expect(subject).to receive(:warn).with("The 'endpoint' configuration option is deprecated. Set both endpoints with the 'endpoints=' method instead") + subject.endpoint = custom_notify_endpoint + end + + it "calls #set_endpoints with the new notify_endpoint and existing session endpoint" do + allow(subject).to receive(:session_endpoint).and_return(session_endpoint) + allow(subject).to receive(:warn) + expect(subject).to receive(:set_endpoints).with(custom_notify_endpoint, session_endpoint) + subject.endpoint = custom_notify_endpoint + end end - end - describe "#session_endpoint=" do - let(:notify_endpoint) { "My notify endpoint" } - let(:custom_session_endpoint) { "My custom session endpoint" } - it "calls #warn with a deprecation notice" do - allow(subject).to receive(:set_endpoints) - expect(subject).to receive(:warn).with("The 'session_endpoint' configuration option is deprecated. The 'set_endpoints' method should be used instead") - subject.session_endpoint = custom_session_endpoint + describe "#session_endpoint=" do + let(:notify_endpoint) { "My notify endpoint" } + let(:custom_session_endpoint) { "My custom session endpoint" } + it "calls #warn with a deprecation notice" do + allow(subject).to receive(:set_endpoints) + expect(subject).to receive(:warn).with("The 'session_endpoint' configuration option is deprecated. Set both endpoints with the 'endpoints=' method instead") + subject.session_endpoint = custom_session_endpoint + end + + it "calls #set_endpoints with the existing notify_endpoint and new session endpoint" do + allow(subject).to receive(:notify_endpoint).and_return(notify_endpoint) + allow(subject).to receive(:warn) + expect(subject).to receive(:set_endpoints).with(notify_endpoint, custom_session_endpoint) + subject.session_endpoint = custom_session_endpoint + end end - it "calls #set_endpoints with the existing notify_endpoint and new session endpoint" do - allow(subject).to receive(:notify_endpoint).and_return(notify_endpoint) - allow(subject).to receive(:warn) - expect(subject).to receive(:set_endpoints).with(notify_endpoint, custom_session_endpoint) - subject.session_endpoint = custom_session_endpoint + describe "#set_endpoints" do + let(:custom_notify_endpoint) { "My custom notify endpoint" } + let(:custom_session_endpoint) { "My custom session endpoint" } + it "set notify_endpoint and session_endpoint" do + subject.set_endpoints(custom_notify_endpoint, custom_session_endpoint) + expect(subject.notify_endpoint).to eq(custom_notify_endpoint) + expect(subject.session_endpoint).to eq(custom_session_endpoint) + end + end + + describe "#endpoints" do + it "defaults to 'DEFAULT_NOTIFY_ENDPOINT' & 'DEFAULT_SESSION_ENDPOINT'" do + config = Bugsnag::Configuration.new + + expect(config.endpoints.notify).to eq(Bugsnag::Configuration::DEFAULT_NOTIFY_ENDPOINT) + expect(config.endpoints.sessions).to eq(Bugsnag::Configuration::DEFAULT_SESSION_ENDPOINT) + end end - end - describe "#set_endpoints" do - let(:custom_notify_endpoint) { "My custom notify endpoint" } - let(:custom_session_endpoint) { "My custom session endpoint" } - it "set notify_endpoint and session_endpoint" do - subject.set_endpoints(custom_notify_endpoint, custom_session_endpoint) - expect(subject.notify_endpoint).to eq(custom_notify_endpoint) - expect(subject.session_endpoint).to eq(custom_session_endpoint) + describe "#endpoints=" do + it "can be set with an EndpointConfiguration" do + config = Bugsnag::Configuration.new + expect(config).not_to receive(:warn) + + config.endpoints = Bugsnag::EndpointConfiguration.new("notify.example.com", "sessions.example.com") + + expect(config.endpoints.notify).to eq("notify.example.com") + expect(config.endpoints.sessions).to eq("sessions.example.com") + end + + it "warns and disables all sending if the no URLs are given" do + config = Bugsnag::Configuration.new + expect(config).to receive(:warn).with(Bugsnag::EndpointValidator::Result::MISSING_URLS) + + config.endpoints = Bugsnag::EndpointConfiguration.new(nil, nil) + + expect(config.endpoints.notify).to be_nil + expect(config.endpoints.sessions).to be_nil + + expect(config.enable_events).to be(false) + expect(config.enable_sessions).to be(false) + end + + # TODO: this behaviour exists for backwards compatibilitiy + # ideally we should not send events in this case + it "warns and disables sessions if only notify URL is given" do + config = Bugsnag::Configuration.new + expect(config).to receive(:warn).with(Bugsnag::EndpointValidator::Result::MISSING_SESSION_URL) + + config.endpoints = Bugsnag::EndpointConfiguration.new("notify.example.com", nil) + + expect(config.endpoints.notify).to eq("notify.example.com") + expect(config.endpoints.sessions).to be_nil + + expect(config.enable_events).to be(true) + expect(config.enable_sessions).to be(false) + end + + it "warns and disables events and sessions if only session URL is given" do + config = Bugsnag::Configuration.new + expect(config).to receive(:warn).with(Bugsnag::EndpointValidator::Result::MISSING_NOTIFY_URL) + + config.endpoints = Bugsnag::EndpointConfiguration.new(nil, "sessions.example.com") + + expect(config.endpoints.notify).to be_nil + expect(config.endpoints.sessions).to eq("sessions.example.com") + + expect(config.enable_events).to be(false) + expect(config.enable_sessions).to be(false) + end + + it "re-enables sessions if valid URLs are given after only giving notify URL" do + config = Bugsnag::Configuration.new + expect(config).to receive(:warn).with(Bugsnag::EndpointValidator::Result::MISSING_SESSION_URL) + + config.endpoints = Bugsnag::EndpointConfiguration.new("notify.example.com", nil) + + expect(config.enable_events).to be(true) + expect(config.enable_sessions).to be(false) + + config.endpoints = Bugsnag::EndpointConfiguration.new("notify.example.com", "sessions.example.com") + + expect(config.endpoints.notify).to eq("notify.example.com") + expect(config.endpoints.sessions).to eq("sessions.example.com") + + expect(config.enable_events).to be(true) + expect(config.enable_sessions).to be(true) + end + + it "re-enables events and sessions if valid URLs are given after only giving session URL" do + config = Bugsnag::Configuration.new + expect(config).to receive(:warn).with(Bugsnag::EndpointValidator::Result::MISSING_NOTIFY_URL) + + config.endpoints = Bugsnag::EndpointConfiguration.new(nil, "sessions.example.com") + + expect(config.enable_events).to be(false) + expect(config.enable_sessions).to be(false) + + config.endpoints = Bugsnag::EndpointConfiguration.new("notify.example.com", "sessions.example.com") + + expect(config.endpoints.notify).to eq("notify.example.com") + expect(config.endpoints.sessions).to eq("sessions.example.com") + + expect(config.enable_events).to be(true) + expect(config.enable_sessions).to be(true) + end end end From 46f9d7c49b427a413cbb3670f12bc58f79eae59f Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 30 Sep 2021 11:51:37 +0100 Subject: [PATCH 4/6] Disable sending events if endpoints are invalid --- lib/bugsnag.rb | 2 ++ spec/bugsnag_spec.rb | 66 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 4e48c1a6..a2786c97 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -406,6 +406,8 @@ def clear_metadata(section, *args) private def should_deliver_notification?(exception, auto_notify) + return false unless configuration.enable_events + reason = abort_reason(exception, auto_notify) configuration.debug(reason) unless reason.nil? reason.nil? diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 4eb91378..1ab82baf 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -145,6 +145,72 @@ end end + describe "endpoint configuration" do + it "does not send events when both endpoints are invalid" do + Bugsnag.configuration.endpoints = {} + + expect(Bugsnag.configuration).not_to receive(:debug) + expect(Bugsnag.configuration).not_to receive(:info) + expect(Bugsnag.configuration).not_to receive(:warn) + expect(Bugsnag.configuration).not_to receive(:error) + + Bugsnag.notify(RuntimeError.new("abc")) + + expect(Bugsnag).not_to have_sent_notification + end + + it "does not send sessions when both endpoints are invalid" do + Bugsnag.configuration.endpoints = {} + + expect(Bugsnag.configuration).not_to receive(:debug) + expect(Bugsnag.configuration).not_to receive(:info) + expect(Bugsnag.configuration).not_to receive(:warn) + expect(Bugsnag.configuration).not_to receive(:error) + + Bugsnag.start_session + + expect(Bugsnag).not_to have_sent_sessions + end + + it "does not send events or sessions when the notify endpoint is invalid" do + Bugsnag.configuration.endpoints = { sessions: "sessions.example.com" } + + expect(Bugsnag.configuration).not_to receive(:debug) + expect(Bugsnag.configuration).not_to receive(:info) + expect(Bugsnag.configuration).not_to receive(:warn) + expect(Bugsnag.configuration).not_to receive(:error) + + Bugsnag.notify(RuntimeError.new("abc")) + Bugsnag.start_session + + expect(Bugsnag).not_to have_sent_notification + expect(Bugsnag).not_to have_sent_sessions + end + + it "does not send sessions when the session endpoint is invalid" do + Bugsnag.configuration.endpoints = Bugsnag::EndpointConfiguration.new("http://notify.example.com", nil) + + expect(Bugsnag.configuration).to receive(:debug).with("Request to http://notify.example.com completed, status: 200").once + expect(Bugsnag.configuration).to receive(:info).with("Notifying http://notify.example.com of RuntimeError").once + expect(Bugsnag.configuration).not_to receive(:warn) + expect(Bugsnag.configuration).not_to receive(:error) + + stub_request(:post, "http://notify.example.com/") + + Bugsnag.notify(RuntimeError.new("abc")) + Bugsnag.start_session + + expect(Bugsnag).to(have_requested(:post, "http://notify.example.com/").with do |request| + payload = JSON.parse(request.body) + exception = get_exception_from_payload(payload) + + expect(exception["message"]).to eq("abc") + end) + + expect(Bugsnag).not_to have_sent_sessions + end + end + describe "add_exit_handler" do before do From e6fa88633c9996681e5d5d5d8c5f1c575243b563 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 30 Sep 2021 14:20:25 +0100 Subject: [PATCH 5/6] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e15a75ea..34ca9aea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,13 @@ Changelog | [#699](https://github.com/bugsnag/bugsnag-ruby/pull/699) * Add `cookies`, `body` and `httpVersion` to the automatically captured request data for Rack apps | [#700](https://github.com/bugsnag/bugsnag-ruby/pull/700) +* Add `Configuration#endpoints` for reading the notify and sessions endpoints and `Configuration#endpoints=` for setting them + | [#701](https://github.com/bugsnag/bugsnag-ruby/pull/701) ### Deprecated * In the next major release, `params` will only contain query string parameters. Currently it also contains the request body for form data requests, but this is deprecated in favour of the new `body` property +* The `Configuration#set_endpoints` method is now deprecated in favour of `Configuration#endpoints=` ## v6.23.0 (21 September 2021) From ef3c82e53b650739c265cab66e22ad69f84ebfff Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 1 Oct 2021 14:16:57 +0100 Subject: [PATCH 6/6] Correct missing URL error messages --- lib/bugsnag/endpoint_validator.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/bugsnag/endpoint_validator.rb b/lib/bugsnag/endpoint_validator.rb index b2957784..be6aaba2 100644 --- a/lib/bugsnag/endpoint_validator.rb +++ b/lib/bugsnag/endpoint_validator.rb @@ -22,12 +22,12 @@ def self.validate(endpoints) class Result # rubocop:disable Layout/LineLength MISSING_URLS = "Invalid configuration. endpoints must be set with both a notify and session URL. Bugsnag will not send any requests.".freeze - MISSING_NOTIFY_URL = "Invalid configuration. endpoints.notify cannot be set without also setting endpoints.sessions. Sessions will not be sent to Bugsnag.".freeze - MISSING_SESSION_URL = "Invalid configuration. endpoints.sessions cannot be set without also setting endpoints.notify. Bugsnag will not send any requests.".freeze + MISSING_NOTIFY_URL = "Invalid configuration. endpoints.sessions cannot be set without also setting endpoints.notify. Bugsnag will not send any requests.".freeze + MISSING_SESSION_URL = "Invalid configuration. endpoints.notify cannot be set without also setting endpoints.sessions. Bugsnag will not send any sessions.".freeze INVALID_URLS = "Invalid configuration. endpoints should be valid URLs, got empty strings. Bugsnag will not send any requests.".freeze - INVALID_NOTIFY_URL = "Invalid configuration. endpoints.notify should be a valid URL, got empty string. Sessions will not be sent to Bugsnag.".freeze - INVALID_SESSION_URL = "Invalid configuration. endpoints.sessions should be a valid URL, got empty string. Bugsnag will not send any requests.".freeze + INVALID_NOTIFY_URL = "Invalid configuration. endpoints.notify should be a valid URL, got empty string. Bugsnag will not send any requests.".freeze + INVALID_SESSION_URL = "Invalid configuration. endpoints.sessions should be a valid URL, got empty string. Bugsnag will not send any sessions.".freeze # rubocop:enable Layout/LineLength attr_reader :reason