Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry request on timeouts #159

Merged
merged 1 commit into from Nov 14, 2019
Merged

Conversation

MarkusHarmsen
Copy link
Contributor

Using this awesome gem, we discovered that sometimes API calls fail due to timeout/load related errors on Tesla's API side.

We especially recorded:

  • 408 Request Timeout
  • 502 Bad Gateway

in maybe 1 in 100 calls.

This pull requests enables the Faraday retry feature and therefore retries any request up to 3 times with a pause of 0.5 seconds between each trial (increasing with a backoff factor 2), if the response code was one of the before mentioned codes.

lib/tesla_api/client.rb Outdated Show resolved Hide resolved
@timdorr
Copy link
Owner

timdorr commented Nov 13, 2019

I think this should be configurable so that users can turn it off if it doesn't suit their use-case. I handle these right now on my end so I can have more granular control over the recovery mechanism (immediate retry vs. backing out of some work completely).

@MarkusHarmsen
Copy link
Contributor Author

What do you think about making this off by default and allowing to configure (i.e. passing) Faraday's Retry options directly in the client initialization if needed?

Kind of a drawback would be the "leaked" Faraday dependency though.

@timdorr
Copy link
Owner

timdorr commented Nov 14, 2019

I'm fine with folks knowing about Faraday. It's not a secret or anything.

@MarkusHarmsen
Copy link
Contributor Author

Ok, updated the pull request.

@timdorr
Copy link
Owner

timdorr commented Nov 14, 2019

Cool. Thanks!

@timdorr timdorr merged commit 0638965 into timdorr:master Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants