Skip to content

Conversation

@drown0315
Copy link

if retries is exhausted, the result of invoking the [orElse] function is returned.
If [orElse] is omitted, it defaults to throwing a [RetryExhaustedException]
when retries is exhausted.

reference by issues #167

Copy link
Member

@jonasfj jonasfj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change, even if orElse is omitted.

Is it really better to get a retries exhausted exception, rather than throwing the last exception that was caught?

We'd need to investigate of breaking this package would affect many users.

).retry(fn, retryIf: retryIf, onRetry: onRetry);
).retry(fn, retryIf: retryIf, onRetry: onRetry, orElse: orElse);

/// throw [RetryExhaustedException] when retries is exhausted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalize

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change, even if orElse is omitted.

Is it really better to get a retries exhausted exception, rather than throwing the last exception that was caught?

We'd need to investigate of breaking this package would affect many users.

I think the reason you need RetryExhaustException is that when you need to do subsequent processing for a retry failure, you need to repeatedly use the logic in retryIf to determine if the exception is a failed retry.

Maybe not needing RetryExhaustException, orElse to handle only the retryIf exceptions is a better way?

But here the RetryExhaustException throwing should rely on retryIf, a little embarrassing😅.

/// thrown. To retry on an [Error], the error must be caught and _rethrown_
/// as an [Exception].
///
/// if retries is exhausted, the result of invoking the [orElse] function is returned.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalize

@drown0315 drown0315 closed this Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants