Added retryIfValue and onRetryIfValue to support retry on returned value condition #55
Conversation
| @@ -44,6 +44,7 @@ final _rand = math.Random(); | |||
| /// () => http.get('https://google.com').timeout(Duration(seconds: 5)), | |||
| /// // Retry on SocketException or TimeoutException | |||
| /// retryIf: (e) => e is SocketException || e is TimeoutException, | |||
| /// retryIfValue: (v) => v == 500, | |||
| }) async { | ||
| int attempt = 0; | ||
| // ignore: literal_only_boolean_expressions | ||
| while (true) { | ||
| attempt++; // first invocation is the first attempt | ||
| try { | ||
| return await fn(); | ||
| var value = await fn(); |
There was a problem hiding this comment.
| var value = await fn(); | |
| final value = await fn(); |
| @@ -122,13 +123,22 @@ class RetryOptions { | |||
| FutureOr<T> Function() fn, { | |||
| FutureOr<bool> Function(Exception) retryIf, | |||
| FutureOr<void> Function(Exception) onRetry, | |||
| FutureOr<bool> Function(dynamic) retryIfValue, | |||
| FutureOr<void> Function(dynamic) onRetryIfValue, | |||
There was a problem hiding this comment.
| FutureOr<void> Function(dynamic) onRetryIfValue, | |
| FutureOr<void> Function(T) onRetryValue, |
Why the if here? onRetry doesn't have that...
Also why are these dynamic, isn't T a better type? Will that work with void?
| /// | ||
| /// At every retry the [onRetry] function will be called (if given). The | ||
| /// At every retry the [onRetry] and [onRetryIfValue] functions will be called (if given). The |
There was a problem hiding this comment.
Shouldn't be an OR, either onRetry is called or onRetryValue is called?
And don't we need to explain which is called when...
|
Any news ? |
|
Wouldn't Additionally, are there any plans on merging this PR? |
| }) async { | ||
| int attempt = 0; | ||
| // ignore: literal_only_boolean_expressions | ||
| while (true) { | ||
| attempt++; // first invocation is the first attempt | ||
| try { | ||
| return await fn(); | ||
| var value = await fn(); | ||
| if (retryIfValue == null || !(await retryIfValue(value))) { |
There was a problem hiding this comment.
| if (retryIfValue == null || !(await retryIfValue(value))) { | |
| if (attempt >= maxAttempts || retryIfValue == null || !(await retryIfValue(value))) { |
Shouldn't there also be a maxAttempts check?
Otherwise, the users would have to count by themselves to not exceed the attempts if the value/result repeatedly triggers a retry. (e.g. HTTP 503 for longer than 30 seconds)
Resolves #45