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...

• MustModify: On review, one issue I see is that retrying from simultaneous processes would cause confusing logs.
• Jonah: Aren't the self. prefixes superfluous, fire being an instance method. also, where is retry defined?
• MustModify: I've found self to be a useful reminder to me that it's a class method. But they are superfluous with respect to the compiler. retry is a Ruby keyword.
• Jonah: Ah, I wasn't aware of retry. Re: self, I'm struggling to see how this would be a class method. Aren't you creating Retryer instances? Doesn't each individual instance have its own tries count, eg?
• MustModify: My mistake. What I meant was that it's an instance method as opposed to a variable. Really wish I could edit that to avoid my looking stupid, but I can see it would make the comments incongruous.

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.