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

Add exponential backoff retries for HTTP requests #693

Closed
EverlastingBugstopper opened this issue Jul 27, 2021 · 3 comments · Fixed by #727
Closed

Add exponential backoff retries for HTTP requests #693

EverlastingBugstopper opened this issue Jul 27, 2021 · 3 comments · Fixed by #727
Assignees
Labels
feature 🎉 new commands, flags, functionality, and improved error messages 💅 polish
Milestone

Comments

@EverlastingBugstopper
Copy link
Contributor

Description

  • What does it do?
    • On any HTTP request that we think might resolve on a retry (maybe we should default to 500 type errors and perhaps specific RoverClientErrors) we should have Rover retry the request. We need to be careful not to just retry everything though, we need to be explicit about the exact scenarios in which retries are desired.
  • What are the inputs/outputs?
    • I'm thinking we either implement retries by default, with a way to opt-out. Maybe there's a --no-retry flag that we add for this one on any command that uses a Client. We only use one Client in Rover due to recent refactors which should make this change very straightforward!
  • Prior art
  • What is your use case for this?
    • Sometimes HTTP requests fail! Most of the time those 500-type errors will resolve themselves if you just do a retry. Would make Rover more resilient to blips with Apollo Studio and blips on user's GraphQL servers themselves.
@EverlastingBugstopper EverlastingBugstopper added feature 🎉 new commands, flags, functionality, and improved error messages needs design ✏️ triage issues and PRs that need to be triaged 💅 polish size/small labels Jul 27, 2021
@EverlastingBugstopper EverlastingBugstopper added this to the v0.2.0 milestone Aug 4, 2021
@EverlastingBugstopper
Copy link
Contributor Author

I'm having a hard time integrating the backoff error API with our own, so I'm going to try using the retry crate instead.

@EverlastingBugstopper
Copy link
Contributor Author

no dice on retry... might have to just do some looping manually. the problem i'm running into is the error types returned by these libraries don't give me an easy way to return a Result<T, RoverClientError> and I can't seem to map things properly.

@EverlastingBugstopper
Copy link
Contributor Author

I think my main issue here was trying to integrate this into the existing codebase. I've split out some of what I want into this example repo that should help me integrate the logic into Rover directly. I also played around with httpmock which seems very promising for expanding some of our test coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎉 new commands, flags, functionality, and improved error messages 💅 polish
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants