resque-retry icon indicating copy to clipboard operation
resque-retry copied to clipboard

retry_criteria_check is not triggered if @retry_exceptions and @fatal_exceptions are unset

Open oehlschl opened this issue 8 years ago • 3 comments

retry_criteria_check methods are not triggered if both @retry_exceptions and @fatal_exceptions are nil per lines:

https://github.com/lantins/resque-retry/blob/v1.5.0/lib/resque/plugins/retry.rb#L275 https://github.com/lantins/resque-retry/blob/v1.5.0/lib/resque/plugins/retry.rb#L199

This case seems like a common one (defining a criteria check method without whitelisting or blacklisting any exception classes), and the behavior is not at all intuitive: the only constraint I define is not called because I didn't define others. For now, setting @retry_exceptions = [] is a sufficient workaround to fail the nil check without introducing unintended consequences, but retry_exceptions should not be required like this.

Never calls retry_criteria_check, always re-retries (up to limit):

  extend Resque::Plugins::Retry

  @retry_limit = 3
  @retry_delay = 60
  retry_criteria_check do |exception, *args|
    exception.is_a?(Mandrill::Error) && exception.message.include?('504 Gateway Time-out')
  end

Works as expected, calling retry_criteria_check logic:

  extend Resque::Plugins::Retry

  @retry_limit = 3
  @retry_delay = 60
  @retry_exceptions = []
  retry_criteria_check do |exception, *args|
    exception.is_a?(Mandrill::Error) && exception.message.include?('504 Gateway Time-out')
  end

oehlschl avatar Jan 10 '17 18:01 oehlschl

Just ran into this myself. From the README I was expecting to be able to not specify @retry_exceptions and then have any exception run through my retry_criteria_check.

davidcpell avatar May 09 '18 14:05 davidcpell

Update: I added @retry_exceptions = [x, y] and the retry_criteria_check still seems not to be running at all. I put some debug logs in there and am not getting them.

davidcpell avatar May 09 '18 18:05 davidcpell

If someone would like to submit a PR to buff up the retry_criteria_valid? method (e.g. change the ordering to support this case) I'd be happy to review / merge if appropriate. I would not be opposed to short-circuiting if that solves the problem.

jzaleski avatar Jun 06 '18 01:06 jzaleski