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

Question re: max retry delay #164

Closed
padamstx opened this issue Aug 20, 2021 · 3 comments
Closed

Question re: max retry delay #164

padamstx opened this issue Aug 20, 2021 · 3 comments
Labels
question Further information is requested

Comments

@padamstx
Copy link

padamstx commented Aug 20, 2021

Hi,
I work in the IBM Cloud Developer Experience group and we're considering the use of this package in our Node SDK "core" library (an enabling layer that is a dependency of our various IBM Cloud Node SDKs).

In studying the package a bit, it looks like there is perhaps one small requirement that we have that might not currently be supported. We would want to use the exponential backoff policy, but also be able to cap the delay time by some user-configurable maximum delay time. It's possible I missed something, but it doesn't appear that the package currently supports this.

Assuming that's true, would you be open to accepting a PR which introduces a new optional RetryConfig field perhaps named something like maxRetryDelay that would be used such that, if specified, it serves as the max delay computed by this section of onBackoffPromise:

      if (config.backoffType === 'linear') {
        delay = config.currentRetryAttempt! * 1000;
      } else if (config.backoffType === 'static') {
        delay = config.retryDelay!;
      } else {
        delay = ((Math.pow(2, config.currentRetryAttempt!) - 1) / 2) * 1000;
      }

Thanks!

@padamstx padamstx changed the title Question re: max retry wait time Question re: max retry dealy Aug 20, 2021
@padamstx padamstx changed the title Question re: max retry dealy Question re: max retry deala Aug 20, 2021
@padamstx padamstx changed the title Question re: max retry deala Question re: max retry delay Aug 20, 2021
@JustinBeckwith JustinBeckwith added the question Further information is requested label Aug 21, 2021
@JustinBeckwith
Copy link
Owner

That sounds great! Happy to accept a PR.

@padamstx
Copy link
Author

padamstx commented Aug 23, 2021

@JustinBeckwith Thanks Justin, we'll get a PR submitted for this. I'm thinking that this new maxRetryDelay value would only apply to the linear and exponential backoff cases (it wouldn't affect the static scenario). Do you agree?

@JustinBeckwith
Copy link
Owner

Looks like this was applied and shipped already here :) #165

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

No branches or pull requests

2 participants