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

ThrottlingRetryPolicy only retries once #394

Closed
joheredi opened this issue Jul 7, 2020 · 2 comments
Closed

ThrottlingRetryPolicy only retries once #394

joheredi opened this issue Jul 7, 2020 · 2 comments
Labels

Comments

@joheredi
Copy link
Member

joheredi commented Jul 7, 2020

See Azure/azure-sdk-for-js/issues/7989 for details

FWIW, the API answers a 429 response which the ThrottlingRetryPolicy handles once with a retry, but when that also returns a 429, it doesn't try again. The API says Retry-After 17, but the request is still rejected with a 429 and another Retry-After 17. I wish the API would return a time that is more likely to work out. That is, I would have liked to see the API drive the retries, so that we could easily burst under 100 requests and throttle back based on meaningful responses from the API. Instead, I'm going to have to use p-throttle to proactively throttle and spread 100 requests out over 5 minutes.

@jeremymeng
Copy link
Member

We could retry more on 429. Some design questions to consider

  • should we provide an option to specify the max retry count
  • what value should be the default for max retry count
    • infinite/max integer value, or some small number

jeremymeng added a commit to jeremymeng/ms-rest-js that referenced this issue Feb 1, 2021
up to a limit in ThrottlingRetryPolicy.

- Remove `_handleResponse` callback that is used for testing. Use `sinon`
instead to verify that `retry()` has been called.

Addresses Azure#394
jeremymeng added a commit to jeremymeng/ms-rest-js that referenced this issue Feb 2, 2021
up to a limit in ThrottlingRetryPolicy.

- Remove `_handleResponse` callback that is used for testing. Use `sinon`
instead to verify that `retry()` has been called.

Addresses Azure#394
@jeremymeng
Copy link
Member

Fixed by #417. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants