diff --git a/spec/raven/configuration_spec.cr b/spec/raven/configuration_spec.cr index cfd45dc..f89966e 100644 --- a/spec/raven/configuration_spec.cr +++ b/spec/raven/configuration_spec.cr @@ -81,6 +81,7 @@ describe Raven::Configuration do configuration.environments = %w(test) configuration.capture_allowed?.should be_true + configuration.capture_allowed!.should be_nil end end @@ -90,7 +91,8 @@ describe Raven::Configuration do configuration.environments = %w(not_test) configuration.capture_allowed?.should be_false - configuration.errors.should eq(["Not configured to send/capture in environment 'test'"]) + ex = configuration.capture_allowed!.should_not be_nil + ex.errors.should eq(["Not configured to send/capture in environment 'test'"]) end end end @@ -135,8 +137,11 @@ describe Raven::Configuration do configuration.should_capture = ->(obj : Exception | String) { obj != "don't send me" } configuration.capture_allowed?("don't send me").should be_false - configuration.errors.should eq(["#should_capture returned false"]) + ex = configuration.capture_allowed!("don't send me").should_not be_nil + ex.errors.should eq(["#should_capture returned false"]) + configuration.capture_allowed?("send me").should be_true + configuration.capture_allowed!("send me").should be_nil end end end @@ -147,7 +152,8 @@ describe Raven::Configuration do configuration.dsn = "dummy://trololo" configuration.capture_allowed?.should be_false - configuration.errors.should eq([ + ex = configuration.capture_allowed!.should_not be_nil + ex.errors.should eq([ "No :public_key specified", "No :project_id specified", ]) @@ -162,7 +168,8 @@ describe Raven::Configuration do configuration.random = RandomSampleFail.new configuration.capture_allowed?.should be_false - configuration.errors.should eq(["Excluded by random sample"]) + ex = configuration.capture_allowed!.should_not be_nil + ex.errors.should eq(["Excluded by random sample"]) end end diff --git a/src/raven/client.cr b/src/raven/client.cr index bbf5fd6..d3d98e3 100644 --- a/src/raven/client.cr +++ b/src/raven/client.cr @@ -34,9 +34,9 @@ module Raven end def send_feedback(event_id : String, data : Hash) - unless configuration.valid? + if ex = configuration.validate! Log.debug { - "Client#send_feedback with event id '#{event_id}' failed: #{configuration.error_messages}" + "Client#send_feedback with event id '#{event_id}' failed: #{ex.error_messages}" } return false end @@ -44,9 +44,9 @@ module Raven end def send_event(event : Event | Event::HashType, hint : Event::Hint? = nil) - unless configuration.valid? + if ex = configuration.validate! Log.debug { - "Client#send_event with event '#{event}' failed: #{configuration.error_messages}" + "Client#send_event with event '#{event}' failed: #{ex.error_messages}" } return false end diff --git a/src/raven/configuration.cr b/src/raven/configuration.cr index 1613747..9c1ab2e 100644 --- a/src/raven/configuration.cr +++ b/src/raven/configuration.cr @@ -3,6 +3,21 @@ require "json" module Raven class Configuration + class ValidationError < Exception + getter errors : Array(String) + + def initialize(@errors) + end + + def error_messages : String + errors + .map_with_index do |e, i| + i > 0 ? e.downcase : e + end + .join(", ") + end + end + # Array of required properties needed to be set, before # `Configuration` is considered valid. REQUIRED_OPTIONS = %i(host public_key project_id) @@ -242,9 +257,6 @@ module Raven # :ditto: property before_send : Proc(Event, Event::Hint?, Event?)? - # Errors object - an `Array` containing error messages. - getter errors = [] of String - def initialize @current_environment = current_environment_from_env @exclude_loggers = ["#{Log.source}.*"] @@ -303,7 +315,7 @@ module Raven self.dsn = URI.parse(value) end - def detect_release : String? + private def detect_release : String? detect_release_from_env || detect_release_from_git || detect_release_from_capistrano || @@ -338,8 +350,7 @@ module Raven end private def heroku_dyno_name - return unless running_on_heroku? - ENV["DYNO"]? + ENV["DYNO"]? if running_on_heroku? end # Try to resolve the hostname to an FQDN, but fall back to whatever @@ -362,47 +373,95 @@ module Raven end end - def capture_allowed? - @errors = [] of String - valid? && - capture_in_current_environment? && - sample_allowed? + protected def validate + %w[].tap do |errors| + if dsn + {% for key in REQUIRED_OPTIONS %} + unless self.{{ key.id }} + errors << "No {{ key }} specified" + end + {% end %} + else + errors << "DSN not set" + end + end + end + + def validate! : ValidationError? + errors = validate + ValidationError.new(errors) unless errors.empty? + end + + def valid? : Bool + validate.empty? + end + + protected def capture_allowed + validate.tap do |errors| + next unless errors.empty? + + unless capture_in_current_environment? + errors << "Not configured to send/capture in environment '#{@current_environment}'" + end + unless sample_allowed? + errors << "Excluded by random sample" + end + end + end + + def capture_allowed! : ValidationError? + errors = capture_allowed + ValidationError.new(errors) unless errors.empty? end - def capture_allowed?(message_or_ex) - @errors = [] of String - capture_allowed? && - !raven_error?(message_or_ex) && - !excluded_exception?(message_or_ex) && - capture_allowed_by_callback?(message_or_ex) + def capture_allowed? : Bool + capture_allowed.empty? + end + + protected def capture_allowed(message_or_ex) + capture_allowed.tap do |errors| + next unless errors.empty? + + if raven_error?(message_or_ex) + errors << "Refusing to capture Raven error: #{message_or_ex.inspect}" + end + if excluded_exception?(message_or_ex) + errors << "User excluded error: #{message_or_ex.inspect}" + end + unless capture_allowed_by_callback?(message_or_ex) + errors << "#should_capture returned false" + end + end + end + + def capture_allowed!(message_or_ex) : ValidationError? + errors = capture_allowed(message_or_ex) + ValidationError.new(errors) unless errors.empty? + end + + def capture_allowed?(message_or_ex) : Bool + capture_allowed(message_or_ex).empty? end private def capture_in_current_environment? - return true if environments.empty? || environments.includes?(@current_environment) - @errors << "Not configured to send/capture in environment '#{@current_environment}'" - false + environments.empty? || environments.includes?(@current_environment) end private def capture_allowed_by_callback?(message_or_ex) - return true if !should_capture || should_capture.try &.call(message_or_ex) - @errors << "#should_capture returned false" - false + !should_capture || should_capture.try &.call(message_or_ex) end private def sample_allowed? return true if sample_rate == 1.0 - return true unless random.rand >= sample_rate - @errors << "Excluded by random sample" + return true if random.rand < sample_rate false end - def raven_error?(message_or_ex) - return false unless message_or_ex.is_a?(Raven::Error) - @errors << "Refusing to capture Raven error: #{message_or_ex.inspect}" - true + private def raven_error?(message_or_ex) + message_or_ex.is_a?(Raven::Error) end - def excluded_exception?(ex) + private def excluded_exception?(ex) return false unless ex.is_a?(Exception) return false unless excluded_exceptions.any? do |klass| case klass @@ -410,31 +469,7 @@ module Raven when String then klass == ex.class.name end end - @errors << "User excluded error: #{ex.inspect}" true end - - def valid? - valid = true - if dsn - {% for key in REQUIRED_OPTIONS %} - unless self.{{ key.id }} - valid = false - @errors << "No {{ key }} specified" - end - {% end %} - else - valid = false - @errors << "DSN not set" - end - valid - end - - def error_messages : String - errors = @errors.map_with_index do |e, i| - i > 0 ? e.downcase : e - end - errors.join(", ") - end end end diff --git a/src/raven/instance.cr b/src/raven/instance.cr index 68b8f81..f583e87 100644 --- a/src/raven/instance.cr +++ b/src/raven/instance.cr @@ -48,12 +48,12 @@ module Raven # Tell the log that the client is good to go. def report_status return if configuration.silence_ready? - if configuration.capture_allowed? - Log.info { "Raven #{VERSION} ready to catch errors" } - else + if ex = configuration.capture_allowed! Log.info { - "Raven #{VERSION} configured not to capture errors: #{configuration.error_messages}" + "Raven #{VERSION} configured not to capture errors: #{ex.error_messages}" } + else + Log.info { "Raven #{VERSION} ready to catch errors" } end end @@ -128,9 +128,9 @@ module Raven # end # ``` def capture(obj : Exception | String, **options, &) - unless configuration.capture_allowed?(obj) + if ex = configuration.capture_allowed!(obj) Log.debug { - "'#{obj}' excluded from capture: #{configuration.error_messages}" + "'#{obj}' excluded from capture: #{ex.error_messages}" } return false end diff --git a/src/raven/transports/http.cr b/src/raven/transports/http.cr index 529c2a3..d753306 100644 --- a/src/raven/transports/http.cr +++ b/src/raven/transports/http.cr @@ -52,9 +52,9 @@ module Raven end def send_event(auth_header, data, **options) - unless configuration.capture_allowed? - Log.debug { "Event not sent: #{configuration.error_messages}" } - return + if ex = configuration.capture_allowed! + Log.debug { "Event not sent: #{ex.error_messages}" } + return false end project_id = configuration.project_id