Xuanwo/backon

Pick a more appropriate name for `with_error_fn`?

wfxr opened this issue ยท 4 comments

wfxr commented

@Xuanwo This crate is awesome! I never thought of a retry mechanism that could be implemented in such an elegant way. ๐Ÿ‘

However the function name with_error_fn is somewhat ambiguous for me. I cannot guess what it is used for without reading the comments.

How about renaming it to continue_if or break_if (on the contrary), following the naming convention in std::ops::ControlFlow:

let content = fetch
    .retry(ExponentialBackoff::default())
    .continue_if(|e| e.to_string() != "EOF") // continue if error is not "EOF"
    .await?;

v.s.

let content = fetch
    .retry(ExponentialBackoff::default())
    .break_if(|e| e.to_string() == "EOF") // stop retrying when error is "EOF"
    .await?;

Personally I think break_if is a good choice. What do you think of it?

Sorry for the late response. This idea is quick and cool.

How about when?

let content = fetch
    .retry(ExponentialBackoff::default())
    .when(|e| e.to_string() == "EOF") // If error is retryable, retry it.
    .await?;
wfxr commented

How about when?

More concise! Can I help with this change?

Can I help with this change?

Of course! Welcome, go for this!

Although backon is a 0.0.x version, keeping API compatible is still an excellent choice for our users. Please consider adding a #[deprecate] message for with_error_fn instead of removing it directly.

wfxr commented

Although backon is a 0.0.x version, keeping API compatible is still an excellent choice for our users. Please consider adding a #[deprecate] message for with_error_fn instead of removing it directly.

Good advice, thanks!