avast/retry-go

Proposal: make default unit nanosecond

zaquestion opened this issue · 4 comments

zaquestion commented

Its somewhat non-intuitive when setting a delay which takes a time.Duration that the units are in microseconds. Its easy to write code like this and think its valid.

       err = retry.Do(func() error {
               return foo()
       }, retry.Attempts(3), retry.Delay(time.Second))
codyaray commented

+💯

This is a huge bug in the usability of this library.

I just spent a couple hours tracking down why my app was appearing to just hang. My configuration of retry.Attempts(5), retry.Delay(1*time.Minute) was calling time.Sleep for 16h40m0s.

See https://play.golang.org/p/hqx9XFcPATj

A time.Duration is represented in nanoseconds. The fact that this library multiplies by microseconds by default completely breaks the standard time math in golang.

JaSei commented

Good point, thank you... What you mean about change API to delay and unit merge to one attribute delay which will be parsed by time.ParseDuration ?

codyaray commented

I think the api should just take a delay parameter of type time.Duration and there shouldn't be a separate unit field.

  • 👍1