ihrwein / backoff

Exponential backoff and retry for Rust.
https://github.com/ihrwein/backoff
Apache License 2.0
306 stars 37 forks source link

Re-consider the default of `ExponentialBackOff::max_elapsed_time` #39

Open thomaseizinger opened 3 years ago

thomaseizinger commented 3 years ago

We've ran into a surprising behaviour where backoff didn't retry a failed Future even though the error was clearly mapped to backoff::Error::Transient.

After some digging, we discovered that the default value of max_elapsed_time is 15 minutes and in our case, the Future only failed several hours in. For context, our Future first establishes a websocket connection and then reads values from it in a loop. This connection can fail at any time in which case we would like to re-establish the connection (but only if it failed for certain reasons), hence our use of backoff.

I would like to suggest to either remove max_elapsed_time entirely or at least set it to None by default. It is very surprising behaviour that the total runtime of a Future influences whether or not it will actually be retried once it completes with an error.

Thoughts?

let4be commented 3 years ago

-exactly- the same problem! @thomaseizinger thanks so much! probably saved me half day worth of digging :)

Default value for max_elapsed_time definitely should be None