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

onRetryAttempt is not awaited / custom back off strategy #452

Closed
simllll opened this issue Oct 22, 2021 · 5 comments · Fixed by #498
Closed

onRetryAttempt is not awaited / custom back off strategy #452

simllll opened this issue Oct 22, 2021 · 5 comments · Fixed by #498
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@simllll
Copy link

simllll commented Oct 22, 2021

Even though in the documentation it's stated that onRetryAttempt can return a promise,
the promise is not awaited:
see

onRetryAttempt?: (err: GaxiosError) => Promise<void> | void;

and
config.onRetryAttempt(err);

My challenge is to build a custom retry handler, which checks for retry-after headers and so on..
I see that adding await would maybe change some behaviour for end-users.. so I would instead vote for a property that allows to override the default expoenitial back off handler? Would that be an option? I can prepare a PR in this case.

@simllll
Copy link
Author

simllll commented Oct 22, 2021

Bascially it would be just here:

const backoff = new Promise(resolve => {

to chagne this line to:

const backoff = config.backoffHandler && config.backoffHandler(err) ||  new Promise(resolve => {
    setTimeout(resolve, delay);
  });

and backoffHandler is an optional function that returns a Promise

@simllll simllll changed the title onRetryAttempt is not awaited onRetryAttempt is not awaited / custom back off strategy Oct 22, 2021
@bcoe bcoe added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Oct 22, 2021
@bcoe
Copy link
Contributor

bcoe commented Oct 22, 2021

@simllll thank you for the bug report.

@chandlervdw
Copy link

@simllll would this allow for utilizing an exponential backoff retry strategy?

@simllll
Copy link
Author

simllll commented Jan 20, 2022

Yes, in this case you can do whatever you want to do... Including exponential back off

@bcoe bcoe added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Apr 11, 2022
@bcoe
Copy link
Contributor

bcoe commented Apr 11, 2022

@simllll switched to feature request from bug, since this is requesting an addition to functionality.

Would you be interested in submitting a patch for this functionality?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants