Option to Ignore Specific Types of Errors

[hwrdprkns]

Hi,

I was wondering if you would accept a pull request which allowed the option to ignore specific types of errors. Right now it seems like the only option is to rescue from within the block.

Let me know if you have any questions or comments!

[kamui]

Do you have a specific use case you can provide as an example? I believe someone asked this before and I was against the idea because listing the exceptions you want to retry is more explicit and less error prone. All sorts of exceptions can be raised that you don't expect, so it's better, in general, to list the ones you want to retry, than to retry everything you think is fine, except a small subset.

[hwrdprkns]

Sure, something like the following:

Retriable.retriable do
  begin
    Curl.get("http://www.myapp.com/#{someUserId}")
  rescue 404NotFound
    puts "I probably shouldn't retry here"
    puts "If the user didn't exist the first time it won't exist the second time"
  end
end

I would love to do

Retriable.retriable :except [404NotFound] { Curl.get("http://www.myapp.com/#{someUserId}") }

[kamui]

The problem with that is a lot of different exceptions could be raised that you don't want to retry. What if you forgot to require curl? Then you'd get a undefined constant Curl, or what if Curl's api changed and #get was deprecated, you'd get a method not found, etc.. and all these exceptions would get retried and that's undesirable.

In your example, it's actually very specific when you'd want to retry that request. You'd probably want to retry on 408 timeout, 500, 503, 504. A very small subset. 400 bad request means something's wrong with your request, retrying wont fix it, same with 401 unauthorized, 501 not implemented means the server doesnt support that route, etc. Most of the errors possible are cases where retrying would not fix the request, where the request is wrong or the server will not respond to that request.

[hwrdprkns]

Right, but that was already an issue if I had done Retriable.retriable { someCodeThatRaises }. I guess I'm just for allowing another option to make the lib a bit more flexible. It just seems like this library would be ripe to support the first case instead of :

Retriable.retriable on: [someRareException], except: [404NotFound, NoMethodError, NameError] do { someCodeThatRaises }

over

allExceptions = [aBunchInThe500Range, aBunchInThe400Range]
Retriable.retriable on: allExceptions { someCodeThatRaises }

It seems like in some cases looking up all the exceptions that you want to retry instead of just a couple that you want to ignore seems like a valid use case for this lib.

[kamui]

In your top example, 404NotFound, NoMethodError, NameError would all have to be subclasses of someRareException then no? Otherwise, I'm not sure why you're excluding them since you specified a specific exception to retry on.

[shideneyu]

I agree with @hwrdprkns . Being to be able to have a black list on top of a white list is very important.

@kamui , I see your point, but what if the top exception got 10000 sub-exceptions children ? Having to maintain the white list is a headache. That's exactly what happens for Mechanize::Error and Mechanize::ResponseCodeError: 404.

I want to handle every exceptions of Mechanize, except the ResponseCodeError: 404 but I don't want to have a huge white list.

This would be perfect:

Retriable.retriable on: [someRareException], except: [404NotFound, NoMethodError, NameError] do { someCodeThatRaises }

`