Ruby class to encapsulate Retrying (for instance, with email)

Question

One of my clients sends quite a few emails a day ... I would think over 500. They are mostly reports to clients or employees. The volume has become sufficient that intermittent issues with gmail (like the occasional and annoying "please try again later" error, or "too many logins" error) are causing missed reports and headaches. We also have some issues with the MapQuest API and this might help.

So I'm writing this class to encapsulate the process of the immediate retry (as opposed to the "give it back to Resque to try again later" process.)

Dependencies:

• Uses my gem Valuable which makes modeling easier.

Feedback welcome.

class Retryer < Valuable
  has_value :attempts, :default => 3
  has_value :logger, default: lambda { Rails.logger }

  has_value :tries, :default => 0

  has_collection :reraise, :default => [NotImplementedError]

  def fire
    raise "You must provide a block of code to try" unless block_given?

    begin
      self.tries += 1
      yield
    rescue Timeout::Error => err
      logger && logger.warn("    - Timeout!")
      sleep(0.5)
      retry if self.tries < self.attempts
    rescue => err
      if reraise.include? err.class
        logger && logger.warn("    - try #{self.tries} of #{attempts} failed and will be reraised: #{err}")
        raise err
      else
        logger && logger.warn("    - try #{self.tries} of #{attempts} failed with #{err}")
        retry if self.tries < self.attempts
      end
    end
  end
end

One thing I'd like to add is the option to run another proc/block if all attempts raise. For instance, notify Rollbar. I imagine it working somewhat like js promises. But I would only do it if I could figure out a readable interface...

Answer 1

Other than the cosmetic suggestion we discussed about removing the unnecessary self prefixes, my central critique is that you're mixing concerns in this code.

Currently, you have specific logging strategies and even logging messages hardcoded in the retryer. Which will make it hard to use the class in other contexts.

The retryer should do just one thing: retry X number of times and return a result.

Exactly what you want the return values to be is up to you (you could even make a RetryResult class), but one simple option would be an array of results from each attempt: true would be the final array value if it eventually succeeds; and the array would rescued errors from each failed attempt.

The calling code could then decide how to process that information: logging it, sending emails, ignoring it, etc.