connor4312 / cockatiel

🐦 A resilience and fault-handling library. Supports Backoffs, Retries, Circuit Breakers, Timeouts, Bulkhead Isolation, and Fallbacks.
MIT License
1.59k stars 50 forks source link

The first delay of DelegateBackoff and ExponentialBackoff is always zero. #30

Closed yowu closed 4 years ago

yowu commented 4 years ago

Let's take look at an example

 // use a ConstantBackoff
 const constDelayRetry = Policy.handleAll()
    .retry()
    .attempts(3)
    .backoff(new ConstantBackoff(10, 3));

  constDelayRetry.onRetry(({ delay }) => console.log('delay for next call: ', delay));
  await constDelayRetry.execute(() => {
    throw new Error('exception');
  });

// use a DelegateBackoff
  const delegatedDelayPolicy = Policy.handleAll()
    .retry()
    .attempts(3)
    .backoff(new DelegateBackoff(() => 10));

  delegatedDelayPolicy.onRetry(({ delay }) => console.log('delay for next call: ', delay));
  await delegatedDelayPolicy.execute(() => {
    throw new Error('exception');
  });

The output looks like

delay for next call:  10
delay for next call:  10
delay for next call:  10
delay for next call:  0
delay for next call:  10
delay for next call:  10

Conceptually, I would expect all delays are 10, but unfortunately, the first delay of DelegateBackoff is always 0. Looks like the ExponentialBackoff has the same problem.

But Polly doesn't has such problem. It looks like RetryPolicy.execute should adjust the logic to backoff.next() first then do the delay.

connor4312 commented 4 years ago

Thanks for the issue, I would be happy to take a PR, otherwise I can fix this in a couple days.

connor4312 commented 4 years ago

Fixed in the linked PR