Refactor error handling by removing the reliance on the @errors ivar

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
 

src/raven/client.cr

@@ -34,19 +34,19 @@ 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
       transport.send_feedback(event_id, data)
     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

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,79 +373,103 @@ 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
                             when Exception.class then klass >= ex.class
                             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

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

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