Merge pull request #5 from lumoslabs/improve_exception_message_handling

Expand retry options

lib/pester.rb

@@ -26,25 +26,25 @@ def self.retry_with_exponential_backoff(options = {}, &block)
   # Options:
   #   retry_error_classes       - A single or array of exceptions to retry on. Thrown exceptions not in this list
   #                               (including parent/sub-classes) will be reraised
+  #   retry_error_messages      - A single or array of exception messages to retry on.  If only this options is passed,
+  #                               any exception with a message containing one of these strings will be retried.  If this
+  #                               option is passed along with retry_error_classes, retry will only happen when both the
+  #                               class and the message match the exception.  Strings and regexes are both permitted.
   #   reraise_error_classes     - A single or array of exceptions to always re-raiseon. Thrown exceptions not in
   #                               this list (including parent/sub-classes) will be retried
   #   max_attempts              - Max number of attempts to retry
   #   delay_interval            - Second interval by which successive attempts will be incremented. A value of 2
   #                               passed to retry_with_backoff will retry first after 2 seconds, then 4, then 6, et al.
   #   on_retry                  - A Proc to be called on each successive failure, before the next retry
   #   on_max_attempts_exceeded  - A Proc to be called when attempt_num >= max_attempts - 1
-  #   message                   - String or regex to look for in thrown exception messages. Matches will trigger retry
-  #                               logic, non-matches will cause the exception to be reraised
+  #   logger                    - Where to log the output
   #
   # Usage:
-  #   retry_action do
+  #   retry_action(retry_error_classes: [Mysql2::Error]) do
   #     puts 'trying to remove a directory'
   #     FileUtils.rm_r(directory)
   #   end
-  #
-  #  retryable(error_classes: Mysql2::Error, message: /^Lost connection to MySQL server/, max_attempts: 2) do
-  #    ActiveRecord::Base.connection.execute("LONG MYSQL STATEMENT")
-  #  end
+
   def self.retry_action(opts = {}, &block)
     merge_defaults(opts)
     if opts[:retry_error_classes] && opts[:reraise_error_classes]
@@ -53,16 +53,10 @@ def self.retry_action(opts = {}, &block)
 
     opts[:max_attempts].times do |attempt_num|
       begin
-        result = yield block
-        return result
+        return yield(block)
       rescue => e
-        class_reraise = opts[:retry_error_classes] && !opts[:retry_error_classes].include?(e.class)
-        reraise_error = opts[:reraise_error_classes] && opts[:reraise_error_classes].include?(e.class)
-        message_reraise = opts[:message] && !e.message[opts[:message]]
-
-        if class_reraise || message_reraise || reraise_error
-          match_type = class_reraise ? 'class' : 'message'
-          opts[:logger].warn("Reraising exception from inside retry_action because provided #{match_type} was not matched.")
+        unless should_retry?(e, opts)
+          opts[:logger].warn('Reraising exception from inside retry_action.')
           raise
         end
 
@@ -72,6 +66,7 @@ def self.retry_action(opts = {}, &block)
           opts[:logger].warn("Failure encountered: #{e}, backing off and trying again #{attempts_left} more times. Trace: #{trace}")
           opts[:on_retry].call(attempt_num, opts[:delay_interval])
         else
+          # Careful here because you will get back the return value of the on_max_attempts_exceeded proc!
           return opts[:on_max_attempts_exceeded].call(opts[:logger], opts[:max_attempts], e)
         end
       end
@@ -80,8 +75,29 @@ def self.retry_action(opts = {}, &block)
 
   private
 
+  def self.should_retry?(e, opts = {})
+    retry_error_classes = opts[:retry_error_classes]
+    retry_error_messages = opts[:retry_error_messages]
+    reraise_error_classes = opts[:reraise_error_classes]
+
+    if retry_error_classes
+      if retry_error_messages
+        retry_error_classes.include?(e.class) && retry_error_messages.any? { |m| e.message[m] }
+      else
+        retry_error_classes.include?(e.class)
+      end
+    elsif retry_error_messages
+      retry_error_messages.any? { |m| e.message[m] }
+    elsif reraise_error_classes && reraise_error_classes.include?(e.class)
+      false
+    else
+      true
+    end
+  end
+
   def self.merge_defaults(opts)
     opts[:retry_error_classes]      = opts[:retry_error_classes] ? Array(opts[:retry_error_classes]) : nil
+    opts[:retry_error_messages]     = opts[:retry_error_messages] ? Array(opts[:retry_error_messages]) : nil
     opts[:reraise_error_classes]    = opts[:reraise_error_classes] ? Array(opts[:reraise_error_classes]) : nil
     opts[:max_attempts]             ||= 4
     opts[:delay_interval]           ||= 30

lib/pester/version.rb

@@ -1,3 +1,3 @@
 module Pester
-  VERSION = '0.1.0'
+  VERSION = '0.1.1'
 end

spec/pester_spec.rb

@@ -55,7 +55,6 @@ class UnmatchedError < RuntimeError; end
     let(:actual_error_message) { matching_error_message }
 
     it_has_behavior "doesn't raise an error"
-
     it_has_behavior 'returns and succeeds'
   end
 end
@@ -66,7 +65,6 @@ class UnmatchedError < RuntimeError; end
     let(:actual_error_message) { non_matching_error_message }
 
     it_has_behavior "doesn't raise an error"
-
     it_has_behavior 'returns and succeeds'
   end
 
@@ -88,7 +86,6 @@ class UnmatchedError < RuntimeError; end
     let(:options) { { delay_interval: 0, logger: null_logger } }
 
     it_has_behavior "doesn't raise an error"
-
     it_has_behavior 'returns and succeeds'
   end
 
@@ -97,7 +94,6 @@ class UnmatchedError < RuntimeError; end
     let(:options) { { max_attempts: 3, logger: null_logger } }
 
     it_has_behavior "doesn't raise an error"
-
     it_has_behavior 'returns and succeeds'
   end
 
@@ -111,17 +107,34 @@ class UnmatchedError < RuntimeError; end
       it_has_behavior 'raises an error'
     end
 
-    context 'with on_max_attempts_exceeded specified (which does not raise)' do
-      let(:do_nothing_proc) { proc {} }
+    context 'with on_max_attempts_exceeded proc specified' do
       let(:options) do
         {
           max_attempts: max_attempts,
-          on_max_attempts_exceeded: do_nothing_proc,
+          on_max_attempts_exceeded: proc_to_call,
           logger: null_logger
         }
       end
 
-      it_has_behavior "doesn't raise an error"
+      context 'which does not do anything' do
+        let(:proc_to_call) { proc {} }
+        it_has_behavior "doesn't raise an error"
+      end
+
+      context 'which reraises' do
+        let(:proc_to_call) { Behaviors::WarnAndReraise }
+        it_has_behavior 'raises an error'
+      end
+
+      context 'which returns a value' do
+        let(:return_value) { 'return_value' }
+        let(:proc_to_call) { proc { return_value } }
+        it_has_behavior "doesn't raise an error"
+
+        it 'should return the result of the proc' do
+          expect(Pester.retry_action(options) { action }).to eq(return_value)
+        end
+      end
     end
   end
 
@@ -138,7 +151,7 @@ class UnmatchedError < RuntimeError; end
       let(:options) do
         {
           retry_error_classes: expected_error_classes,
-          message: /^Lost connection to MySQL server/,
+          retry_error_messages: /^Lost connection to MySQL server/,
           max_attempts: 10,
           logger: null_logger
         }