Providing methods as an array of strings doesn't trigger retries

[doutatsu]

EDIT:

After investing this in detail, the issue is actually with providing methods as an array of strings, which is explained here: #26 (comment)

---

I might be understanding the functionality, but I've noticed my requests weren't being retried. My configuration is as follows, where my understanding would be that any of the specified exceptions will be retried. Alternatively, if status code 429 is seen, it will retry the requests using provided interval. Otherwise, it would use a custom interval.

{"max"=>4.0,
 "methods"=>["get", "post"],
 "interval"=>0.05,
  "exceptions"=>["Faraday::RetriableResponse", "Faraday::TimeoutError", "Faraday::ConnectionFailed"],
  "backoff_factor"=>2.0,
  "retry_statuses"=>[429],
  "interval_randomness"=>0.5
},

I've noticed that it was actually not working - when I removed everything but retry_statuses, the retries started working as expected again. Can both of these fields not be mixed? Or is it something else overriding the 429 status here?

[doutatsu]

Alright, this doesn't seem to be the case - I've tried using only statuses and exceptions. Reading the documentation, I also see that retry_statuses just raises RetriableResponse, which gets picked up by exceptions array. So it must be something else

[doutatsu]

I've found what is the root cause, which feels like a bug - if methods are specified as an array of strings, not symbols, nothing gets retried. I was storing my configuration in JSONB, hence why the methods were in string format. If I convert it into symbols, retry logic works as expected, without any changes

[iMacTia]

Hi @doutatsu, you're correct, the methods option expects an array of symbols as shown in the README example.
If you pass an array of strings instead, it won't work.

Now, I appreciate we could make faraday-retry smarter and deal with both cases, but then we'd need to also deal with uppercase/lowercase strings, which would make the implementation more complicated.
My suggestion would be to do exactly what you did, set the methods config to be a list of symbols as intended.

I'm sorry though this is wasn't clear and you had to spend some time figuring this out, so I'd instead be interested to find ways of improving the documentation so that people won't need to go through the same surprise in future.
Would you be able to share some feedback as to where you looked for how to use the methods options and where you would have expected we mentioned the list was supposed to be of lowercase symbols?

[doutatsu]

I was hoping it would work, considering exceptions allows you to provide both strings and actual classes. To figure it out, I just dug into https://github.com/lostisland/faraday-retry/blob/main/lib/faraday/retry/middleware.rb, but it wasn't obvious until i started removing options and adding them one by one.

The minimum improvement would be to update this:

To actually be explicit about the symbol requirement. Just like you specify it could be anything for exceptions:

I would the opposite and say something like: Methods can only be given as symbols or something like that.