Skip to content

Commit

Permalink
Merge pull request #369 from Workiva/try_increasing_timeout_on_retry
Browse files Browse the repository at this point in the history
CT-1375 Add the ability to increase the timeout on retries ✅
  • Loading branch information
rmconsole7-wk authored Jun 28, 2022
2 parents a3dccf8 + 0c35751 commit 833c869
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 2 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ package-lock.json

# Examples
/example/http/cross_origin_file_transfer/files/

#IDEs
.idea
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## [4.1.0](https://github.com/Workiva/w_transport/compare/4.0.8...4.1.0)

- **Improvement:** `AutoRetryConfig` gained the `increaseTimeoutOnRetry` flag.
This allows timeouts to increase as retry attempts increase.

## [4.0.8](https://github.com/Workiva/w_transport/compare/4.0.7...4.0.8)

- Update the changelog
Expand Down
35 changes: 35 additions & 0 deletions lib/src/http/auto_retry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
// limitations under the License.

import 'dart:async';
import 'dart:math';

import 'package:meta/meta.dart';
import 'package:w_transport/src/constants.dart' show v3Deprecation;

import 'package:w_transport/src/http/base_request.dart';
Expand Down Expand Up @@ -81,6 +83,18 @@ class AutoRetryConfig {
/// total - the first request and 2 retries.
int maxRetries = 2;

/// When `true`, timeouts on retries will be increased by multiplying [numAttempts] by the
/// [timeoutThreshold].
///
/// For example, if [timeoutThreshold] is set to 10s:
/// - On the original request, [numAttempts] is one, so this request will have a timeout of 10s
/// - On the first retry attempt, [numAttempts] is two, so this retry will have a timeout of 20s.
/// - On the second retry attempt, [numAttempts] is three, so this retry will have a timeout of 30s.
/// - ... and so on up to either 60 seconds, or [timeoutThreshold], whichever is greater.
///
/// See [timeoutThreshold].
bool increaseTimeoutOnRetry = false;

/// A custom [test] function that decides whether or not a request should be
/// retried. It will be called with:
///
Expand Down Expand Up @@ -154,6 +168,27 @@ class RequestAutoRetry extends AutoRetryConfig {
if (request is MultipartRequest && request.files.isNotEmpty) return false;
return true;
}

/// timeoutThreshold will not move beyond 60s or the [_request.timeoutThreshold], whichever is greater, when [increaseTimeoutOnRetry] is true.
Duration get timeoutThreshold {
if (increaseTimeoutOnRetry) {
return getRetryTimeoutThreshold(_request.timeoutThreshold, numAttempts);
}

return _request.timeoutThreshold;
}
}

@visibleForTesting
Duration getRetryTimeoutThreshold(Duration timeoutThreshold, int numAttempts) {
if (numAttempts <= 0) return timeoutThreshold;
var threshold = timeoutThreshold * numAttempts;
var maxTimeout = Duration(seconds: max(timeoutThreshold.inSeconds, 60));

if (threshold < maxTimeout)
return threshold;
else
return maxTimeout;
}

/// Representation of the back-off method to use when retrying requests. A fixed
Expand Down
5 changes: 3 additions & 2 deletions lib/src/http/common/request.dart
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,9 @@ abstract class CommonRequest extends Object
abortRequest();
}
isTimedOut = true;

_timeoutError = TimeoutException(
'Request took too long to complete.', timeoutThreshold);
'Request took too long to complete.', autoRetry.timeoutThreshold);
_timeoutCompleter.complete();
}

Expand Down Expand Up @@ -769,7 +770,7 @@ abstract class CommonRequest extends Object
// Enforce a timeout threshold if set.
Timer timeout;
if (timeoutThreshold != null) {
timeout = Timer(timeoutThreshold, _timeoutRequest);
timeout = Timer(autoRetry.timeoutThreshold, _timeoutRequest);
}

// Attempt to fetch the response.
Expand Down
41 changes: 41 additions & 0 deletions test/unit/auto_retry_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2015 Workiva Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

@TestOn('vm || browser')

import 'package:test/test.dart';
import 'package:w_transport/src/http/auto_retry.dart';

void main() {
group('getRetryTimeoutThreshold', () {
test('timeoutThreshold is less than 60', () {
expect(getRetryTimeoutThreshold(Duration(seconds: 30), 0),
Duration(seconds: 30));
expect(getRetryTimeoutThreshold(Duration(seconds: 30), 1),
Duration(seconds: 30));
expect(getRetryTimeoutThreshold(Duration(seconds: 30), 2),
Duration(seconds: 60));
expect(getRetryTimeoutThreshold(Duration(seconds: 30), 3),
Duration(seconds: 60));
});
test('timeoutThreshold is more than 60', () {
expect(getRetryTimeoutThreshold(Duration(seconds: 90), 0),
Duration(seconds: 90));
expect(getRetryTimeoutThreshold(Duration(seconds: 90), 1),
Duration(seconds: 90));
expect(getRetryTimeoutThreshold(Duration(seconds: 90), 2),
Duration(seconds: 90));
});
});
}
25 changes: 25 additions & 0 deletions test/unit/http/request_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,31 @@ void _runAutoRetryTestSuiteFor(String name,
expect(request.autoRetry.numAttempts, equals(2));
});

test(
'request timeout should be retried by default increaseTimeoutOnRetry',
() async {
// 1st and 2nd request = hangs until timeout, 3nd request succeeds
int c = 0;
MockTransports.http.when(requestUri, (request) async {
if (++c <= 2) {
await Future.delayed(Duration(seconds: 1));
}
// The timeout should be 75ms by now, so even with this delay, should succeed.
await Future.delayed(Duration(milliseconds: 60));
return MockResponse.ok();
}, method: 'GET');

final request = requestFactory();
request.timeoutThreshold = Duration(milliseconds: 25);
request.autoRetry
..enabled = true
..maxRetries = 2
..increaseTimeoutOnRetry = true;

await request.get(uri: requestUri);
expect(request.autoRetry.numAttempts, equals(3));
});

test('request timeout should not be retried if disabled', () async {
// 1st request = hangs until timeout
MockTransports.http.when(requestUri, (request) async {
Expand Down

0 comments on commit 833c869

Please sign in to comment.