Closed dlchen closed 1 year ago
I'd really love if we can make this configurable with a default value equal to the current static value (100) That way we don't change behavior and we can use a minor version when releasing this changes WDYT?
I'd really love if we can make this configurable with a default value equal to the current static value (100) That way we don't change behavior and we can use a minor version when releasing this changes WDYT?
@mindhells please review my attempt at this 7845a5a thanks!
I'd really love if we can make this configurable with a default value equal to the current static value (100) That way we don't change behavior and we can use a minor version when releasing this changes WDYT?
@mindhells please review my attempt at this 7845a5a thanks!
tests got broken, not sure why 🤔 , would you have a look?
I'd really love if we can make this configurable with a default value equal to the current static value (100) That way we don't change behavior and we can use a minor version when releasing this changes WDYT?
@mindhells please review my attempt at this 7845a5a thanks!
tests got broken, not sure why 🤔 , would you have a look?
Looks like error
was being passed into the retryDelay
function for some reason. I removed it and the tests are passing now.
I'd really love if we can make this configurable with a default value equal to the current static value (100) That way we don't change behavior and we can use a minor version when releasing this changes WDYT?
@mindhells please review my attempt at this 7845a5a thanks!
tests got broken, not sure why 🤔 , would you have a look?
Looks like
error
was being passed into theretryDelay
function for some reason. I removed it and the tests are passing now.
Removing the error parameter from the retryDelay callback would break the library API (see Readme)
I'd really love if we can make this configurable with a default value equal to the current static value (100) That way we don't change behavior and we can use a minor version when releasing this changes WDYT?
@mindhells please review my attempt at this 7845a5a thanks!
tests got broken, not sure why 🤔 , would you have a look?
Looks like
error
was being passed into theretryDelay
function for some reason. I removed it and the tests are passing now.Removing the error parameter from the retryDelay callback would break the library API (see Readme)
Gotcha. Well it's kind of awkward since the error is not the first param of the callback. In the name of backwards compatibility this is the best I could come up with. Lmk your thoughts.
I'd really love if we can make this configurable with a default value equal to the current static value (100) That way we don't change behavior and we can use a minor version when releasing this changes WDYT?
@mindhells please review my attempt at this 7845a5a thanks!
tests got broken, not sure why 🤔 , would you have a look?
Looks like
error
was being passed into theretryDelay
function for some reason. I removed it and the tests are passing now.Removing the error parameter from the retryDelay callback would break the library API (see Readme)
Gotcha. Well it's kind of awkward since the error is not the first param of the callback. In the name of backwards compatibility this is the best I could come up with. Lmk your thoughts.
LGTM, thanks a lot!
Issue
Current implementation of
exponentialDelay
results in retries based on the order of 100ms – i.e. 100ms, 200ms, 400ms, 800ms, etc. This is in contrast with the Exponential Backoff link found in the README which suggests 1s, 2s, 4s, 8s, etc.Solution
Add
delayFactor
param which can be passed 1000. Defaults to 100 as to keep the current behavior.Related