diff --git a/packages/aws_common/lib/src/exception/aws_http_exception.dart b/packages/aws_common/lib/src/exception/aws_http_exception.dart index 4918468b232..d8e800d2d99 100644 --- a/packages/aws_common/lib/src/exception/aws_http_exception.dart +++ b/packages/aws_common/lib/src/exception/aws_http_exception.dart @@ -15,10 +15,37 @@ class AWSHttpException implements Exception { if (underlyingException is AWSHttpException) { return underlyingException; } - return AWSHttpException._(request.method, request.uri, underlyingException); + return AWSHttpException._( + request.method, + request.uri, + underlyingException, + false, + ); } - const AWSHttpException._(this.method, this.uri, this.underlyingException); + /// Creates an exception for a transport-level failure that is safe to retry. + factory AWSHttpException.retryable( + AWSBaseHttpRequest request, [ + Object? underlyingException, + ]) { + // Anti-double-wrap guard; callers pass the raw transport error. + if (underlyingException is AWSHttpException) { + return underlyingException; + } + return AWSHttpException._( + request.method, + request.uri, + underlyingException, + true, + ); + } + + const AWSHttpException._( + this.method, + this.uri, + this.underlyingException, + this.retryable, + ); /// The method of the HTTP operation which was in progress. final AWSHttpMethod method; @@ -29,6 +56,9 @@ class AWSHttpException implements Exception { /// The exception which triggered this exception being thrown. final Object? underlyingException; + /// Whether this is a transport-level failure that is safe to retry. + final bool retryable; + @override String toString() => '${method.value} $uri failed' diff --git a/packages/aws_common/lib/src/http/aws_http_client_io.dart b/packages/aws_common/lib/src/http/aws_http_client_io.dart index 5b39ffb31c1..07bd721ccdb 100644 --- a/packages/aws_common/lib/src/http/aws_http_client_io.dart +++ b/packages/aws_common/lib/src/http/aws_http_client_io.dart @@ -11,6 +11,22 @@ import 'package:http2/http2.dart'; import 'package:meta/meta.dart'; import 'package:stream_transform/stream_transform.dart'; +/// Whether [error] is a transient transport-level failure that is safe to +/// retry. +@visibleForTesting +bool isRetryableTransportError(Object error) => + error is SocketException || + error is HttpException || + error is TimeoutException || + error is TransportException; + +AWSHttpException _transportException( + AWSBaseHttpRequest request, + Object error, +) => isRetryableTransportError(error) + ? AWSHttpException.retryable(request, error) + : AWSHttpException(request, error); + /// {@template aws_common.http.http_client_impl} /// The platform-specific implementation of [AWSHttpClient]. /// @@ -351,14 +367,14 @@ class AWSHttpClientImpl extends AWSHttpClient { logger.debug('Error in stream: $error'); if (!gotHeaders.isCompleted) { gotHeaders.completeError( - AWSHttpException(request, error), + _transportException(request, error), stackTrace, ); return; } if (!bodyController.isClosed) { bodyController - ..addError(AWSHttpException(request, error), stackTrace) + ..addError(_transportException(request, error), stackTrace) ..close(); } }, @@ -528,7 +544,7 @@ class AWSHttpClientImpl extends AWSHttpClient { ), ); }).catchError((Object e, StackTrace st) { - completer.completeError(AWSHttpException(request, e), st); + completer.completeError(_transportException(request, e), st); }); return operation; diff --git a/packages/aws_common/test/http/retryable_transport_test.dart b/packages/aws_common/test/http/retryable_transport_test.dart new file mode 100644 index 00000000000..271f7ee11c7 --- /dev/null +++ b/packages/aws_common/test/http/retryable_transport_test.dart @@ -0,0 +1,58 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +@TestOn('vm') +library; + +import 'dart:async'; +import 'dart:io'; + +import 'package:aws_common/aws_common.dart'; +import 'package:aws_common/src/http/aws_http_client_io.dart' + show isRetryableTransportError; +import 'package:http2/http2.dart'; +import 'package:test/test.dart'; + +void main() { + group('AWSHttpClient transport failures', () { + test('flags a failed connection as a retryable AWSHttpException', () async { + // Bind then release a port so the connection is guaranteed to fail. + final socket = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0); + final port = socket.port; + await socket.close(); + + final client = AWSHttpClient(); + addTearDown(client.close); + final request = AWSHttpRequest.get(Uri.parse('http://127.0.0.1:$port/')); + await expectLater( + client.send(request).response, + throwsA( + isA().having( + (e) => e.retryable, + 'retryable', + isTrue, + ), + ), + ); + }); + }); + + group('isRetryableTransportError', () { + test('transport-level failures are retryable', () { + expect(isRetryableTransportError(const SocketException('reset')), isTrue); + expect(isRetryableTransportError(const HttpException('closed')), isTrue); + expect(isRetryableTransportError(TimeoutException('timed out')), isTrue); + expect(isRetryableTransportError(TransportException('h2')), isTrue); + expect( + isRetryableTransportError(StreamTransportException('h2 stream')), + isTrue, + ); + }); + + test('non-transport failures are not retryable', () { + expect(isRetryableTransportError(const FormatException('bad')), isFalse); + expect(isRetryableTransportError(StateError('bad')), isFalse); + expect(isRetryableTransportError(ArgumentError('bad')), isFalse); + }); + }); +} diff --git a/packages/smithy/smithy_aws/lib/src/http/retry/aws_retryer.dart b/packages/smithy/smithy_aws/lib/src/http/retry/aws_retryer.dart index 23fce1affb7..f55b1115a37 100644 --- a/packages/smithy/smithy_aws/lib/src/http/retry/aws_retryer.dart +++ b/packages/smithy/smithy_aws/lib/src/http/retry/aws_retryer.dart @@ -96,6 +96,10 @@ class AWSRetryer implements Retryer { if (exception is TimeoutException) { return true; } + // Transport-level failures are flagged retryable by the HTTP client. + if (exception is AWSHttpException && exception.retryable) { + return true; + } if (exception is! SmithyException) { return false; } diff --git a/packages/smithy/smithy_aws/test/http/aws_retryer_test.dart b/packages/smithy/smithy_aws/test/http/aws_retryer_test.dart index f4b4910a6d2..4e8ad539ba2 100644 --- a/packages/smithy/smithy_aws/test/http/aws_retryer_test.dart +++ b/packages/smithy/smithy_aws/test/http/aws_retryer_test.dart @@ -174,6 +174,56 @@ void main() { }); } }); + + group('isRetryable', () { + final retryer = AWSRetryer(); + final request = AWSHttpRequest.get(Uri.parse('https://example.com')); + + test('retries transport-level AWSHttpException (retryable flag)', () { + expect( + retryer.isRetryable(AWSHttpException.retryable(request)), + isTrue, + ); + }); + + test('does not retry a non-transport AWSHttpException', () { + expect(retryer.isRetryable(AWSHttpException(request)), isFalse); + }); + + test('retries TimeoutException', () { + expect(retryer.isRetryable(TimeoutException('slow')), isTrue); + }); + + test('does not retry non-transport, non-Smithy exceptions', () { + expect(retryer.isRetryable(const FormatException('bad')), isFalse); + }); + }); + + group('retry recovery', () { + test('recovers after a transient AWSHttpException (retries then ' + 'succeeds)', () async { + await runZoned(() async { + // exponentialBase: 0 => zero backoff, so the test is fast. + final retryer = AWSRetryer(exponentialBase: 0); + final request = AWSHttpRequest.get(Uri.parse('https://example.com')); + var attempts = 0; + final result = await retryer.retry(() { + attempts++; + final completer = CancelableCompleter(); + if (attempts < 2) { + completer.completeError( + AWSHttpException.retryable(request, Exception('reset')), + ); + } else { + completer.complete(42); + } + return completer.operation; + }).valueOrCancellation(); + expect(attempts, 2, reason: 'should retry once, then succeed'); + expect(result, 42, reason: 'should return the retried result'); + }, zoneValues: {AWSConfigValue.maxAttempts: 3}); + }); + }); }); }