feat: add delayFactor param to exponentialDelay

[dlchen]

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

• addresses Increasing exponentialDelay's interval constant #87
• updated (not-stale) dupe of Increase exponentialDelay interval constant to 1 sec #111

[ryparker]

Made some updates to satisfy the latest Google recommendations for exponential backoff.

export function exponentialDelay(retryNumber = 0) {
  const randomMs = Math.random() * 1000; // 0 to 1000 milliseconds
  const exponentialDelay = Math.pow(2, retryNumber) * 1000 + randomMs;
  const maxBackoff = 64 * 1000; // 64 seconds
  const delay = Math.min(exponentialDelay, maxBackoff);
  const deadline = 10 * 60 * 1000; // 10 minutes
  if (delay > deadline) {
    throw new Error(
      `Exponential retry delay exhausted after ${
        deadline / 1000 / 60
      } minutes (${retryNumber} retries).`,
    );
  }
  return delay;
}

We'd probably want to allow the user to configure the maxBackoff and deadline to their liking.

[dlchen]

This is a good idea but deserves its own PR IMO.

[mindhells]

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?

[dlchen]

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!

[mindhells]

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?

[dlchen]

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.

[mindhells]

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.

Removing the error parameter from the retryDelay callback would break the library API (see Readme)

[dlchen]

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.

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.

[mindhells]

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.

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!