Added retryIfValue and onRetryIfValue to support retry on returned value condition#55
Added retryIfValue and onRetryIfValue to support retry on returned value condition#55ScottMacDougall wants to merge 2 commits intogoogle:masterfrom
Conversation
| /// () => 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, |
| 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(); |
| 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?
| /// thrown, or if [retryIfValue] returns `true` for the value returned from [fn]. | ||
| /// | ||
| /// 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? |
| 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)
4 participants
Resolves #45