From 13f6fa65046100485c9ad055e473d9a2851511ec Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Thu, 30 Sep 2021 17:26:13 +0000 Subject: [PATCH 1/5] feat: implement basic retry support in runtime --- .../kotlin/runtime/retries/SdkRetryPolicy.kt | 41 ++++++++++++++++ .../runtime/retries/SdkRetryPolicyTest.kt | 19 ++++++++ docs/design/README.md | 1 + docs/design/retries.md | 47 +++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/SdkRetryPolicy.kt create mode 100644 aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/SdkRetryPolicyTest.kt create mode 100644 docs/design/retries.md diff --git a/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/SdkRetryPolicy.kt b/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/SdkRetryPolicy.kt new file mode 100644 index 00000000000..70742fc3277 --- /dev/null +++ b/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/SdkRetryPolicy.kt @@ -0,0 +1,41 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package aws.sdk.kotlin.runtime.retries + +import aws.sdk.kotlin.runtime.AwsServiceException +import aws.smithy.kotlin.runtime.retries.RetryDirective +import aws.smithy.kotlin.runtime.retries.RetryErrorType.* +import aws.smithy.kotlin.runtime.retries.impl.StandardRetryPolicy + +object SdkRetryPolicy : StandardRetryPolicy() { + internal val knownErrorTypes = mapOf( + "BandwidthLimitExceeded" to Throttling, + "EC2ThrottledException" to Throttling, + "LimitExceededException" to Throttling, + "PriorRequestNotComplete" to Throttling, + "ProvisionedThroughputExceededException" to Throttling, + "RequestLimitExceeded" to Throttling, + "RequestThrottled" to Throttling, + "RequestThrottledException" to Throttling, + "RequestTimeout" to Timeout, + "RequestTimeoutException" to Timeout, + "SlowDown" to Throttling, + "ThrottledException" to Throttling, + "Throttling" to Throttling, + "ThrottlingException" to Throttling, + "TooManyRequestsException" to Throttling, + "TransactionInProgressException" to Throttling, + ) + + override fun evaluateOtherExceptions(ex: Throwable): RetryDirective? = when (ex) { + is AwsServiceException -> evaluateAwsServiceException(ex) + else -> null + } + + private fun evaluateAwsServiceException(ex: AwsServiceException): RetryDirective? = with(ex.sdkErrorMetadata) { + knownErrorTypes[errorCode]?.let { RetryDirective.RetryError(it) } + } +} diff --git a/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/SdkRetryPolicyTest.kt b/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/SdkRetryPolicyTest.kt new file mode 100644 index 00000000000..63410871721 --- /dev/null +++ b/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/SdkRetryPolicyTest.kt @@ -0,0 +1,19 @@ +package aws.sdk.kotlin.runtime.retries + +import aws.sdk.kotlin.runtime.AwsErrorMetadata +import aws.sdk.kotlin.runtime.AwsServiceException +import aws.smithy.kotlin.runtime.retries.RetryDirective +import kotlin.test.Test +import kotlin.test.assertEquals + +class SdkRetryPolicyTest { + @Test + fun testAwsServiceExceptions() { + SdkRetryPolicy.knownErrorTypes.forEach { (errorCode, errorType) -> + val ex = AwsServiceException() + ex.sdkErrorMetadata.attributes[AwsErrorMetadata.ErrorCode] = errorCode + val result = SdkRetryPolicy.evaluate(Result.failure(ex)) + assertEquals(RetryDirective.RetryError(errorType), result) + } + } +} diff --git a/docs/design/README.md b/docs/design/README.md index 0cbcf0edbf5..565df41b8ed 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -9,3 +9,4 @@ These designs extend or augment the [Smithy Kotlin Designs](https://github.com/a ## Detailed sub-designs * [Endpoint resolution](endpoint-resolution.md) +* [SDK-specific Retries](retries.md) diff --git a/docs/design/retries.md b/docs/design/retries.md new file mode 100644 index 00000000000..4b118da6481 --- /dev/null +++ b/docs/design/retries.md @@ -0,0 +1,47 @@ +# SDK-specific Retry Design + +* **Type**: Design +* **Author(s)**: Ian Botsford + +# Abstract + +The AWS SDK for Kotlin uses a specialization of the generalized +[**smithy-kotlin** Retry Design](https://github.com/awslabs/smithy-kotlin/blob/main/docs/design/retries.md). This +document covers those specializations (but does not re-hash the generalized design). + +# SDK implementation + +The SDK uses the following customizations/specializations over the generalized +[**smithy-kotlin** Retry Design](https://github.com/awslabs/smithy-kotlin/blob/main/docs/design/retries.md): + +## Retry policy + +The generalized `StandardRetryPolicy` is subclassed to provide support for information only available in AWS-specific +exception types: + +```kotlin +object SdkRetryPolicy : StandardRetryPolicy() { + internal val knownErrorTypes = mapOf( + "BandwidthLimitExceeded" to Throttling, + "RequestTimeoutException" to Timeout, + "TooManyRequestsException" to Throttling, + … + ) + + override fun evaluateOtherExceptions(ex: Throwable): RetryDirective? = when (ex) { + is AwsServiceException -> evaluateAwsServiceException(ex) + else -> null + } + + private fun evaluateAwsServiceException(ex: AwsServiceException): RetryDirective? = with(ex.sdkErrorMetadata) { + knownErrorTypes[errorCode]?.let { RetryDirective.RetryError(it) } + } +} +``` + +This policy utilizes the error code provided in the exception to derive a retry directive based on a known list. This +list may grow/change over time. + +# Revision history + +* 9/27/2021 - Created From 90d5cc424ee2ff8edca5fa527653ea6f1d729b4c Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Mon, 4 Oct 2021 20:20:44 +0000 Subject: [PATCH 2/5] =?UTF-8?q?addressing=20PR=20feedback:=20SdkRetryPolic?= =?UTF-8?q?y=20=E2=86=92=20AwsDefaultRetryPolicy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../retries/{SdkRetryPolicy.kt => AwsDefaultRetryPolicy.kt} | 3 ++- .../{SdkRetryPolicyTest.kt => AwsDefaultRetryPolicyTest.kt} | 6 +++--- docs/design/retries.md | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) rename aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/{SdkRetryPolicy.kt => AwsDefaultRetryPolicy.kt} (93%) rename aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/{SdkRetryPolicyTest.kt => AwsDefaultRetryPolicyTest.kt} (72%) diff --git a/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/SdkRetryPolicy.kt b/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicy.kt similarity index 93% rename from aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/SdkRetryPolicy.kt rename to aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicy.kt index 70742fc3277..664f4be0ead 100644 --- a/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/SdkRetryPolicy.kt +++ b/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicy.kt @@ -10,10 +10,11 @@ import aws.smithy.kotlin.runtime.retries.RetryDirective import aws.smithy.kotlin.runtime.retries.RetryErrorType.* import aws.smithy.kotlin.runtime.retries.impl.StandardRetryPolicy -object SdkRetryPolicy : StandardRetryPolicy() { +public object AwsDefaultRetryPolicy : StandardRetryPolicy() { internal val knownErrorTypes = mapOf( "BandwidthLimitExceeded" to Throttling, "EC2ThrottledException" to Throttling, + "IDPCommunicationError" to Timeout, "LimitExceededException" to Throttling, "PriorRequestNotComplete" to Throttling, "ProvisionedThroughputExceededException" to Throttling, diff --git a/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/SdkRetryPolicyTest.kt b/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicyTest.kt similarity index 72% rename from aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/SdkRetryPolicyTest.kt rename to aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicyTest.kt index 63410871721..e19167c20a1 100644 --- a/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/SdkRetryPolicyTest.kt +++ b/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicyTest.kt @@ -6,13 +6,13 @@ import aws.smithy.kotlin.runtime.retries.RetryDirective import kotlin.test.Test import kotlin.test.assertEquals -class SdkRetryPolicyTest { +class AwsDefaultRetryPolicyTest { @Test fun testAwsServiceExceptions() { - SdkRetryPolicy.knownErrorTypes.forEach { (errorCode, errorType) -> + AwsDefaultRetryPolicy.knownErrorTypes.forEach { (errorCode, errorType) -> val ex = AwsServiceException() ex.sdkErrorMetadata.attributes[AwsErrorMetadata.ErrorCode] = errorCode - val result = SdkRetryPolicy.evaluate(Result.failure(ex)) + val result = AwsDefaultRetryPolicy.evaluate(Result.failure(ex)) assertEquals(RetryDirective.RetryError(errorType), result) } } diff --git a/docs/design/retries.md b/docs/design/retries.md index 4b118da6481..55f7d7ca1f2 100644 --- a/docs/design/retries.md +++ b/docs/design/retries.md @@ -20,7 +20,7 @@ The generalized `StandardRetryPolicy` is subclassed to provide support for infor exception types: ```kotlin -object SdkRetryPolicy : StandardRetryPolicy() { +object AwsDefaultRetryPolicy : StandardRetryPolicy() { internal val knownErrorTypes = mapOf( "BandwidthLimitExceeded" to Throttling, "RequestTimeoutException" to Timeout, From f2e65f3de1b075122ed2946e30e6a54b91b457f6 Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Tue, 5 Oct 2021 17:59:40 +0000 Subject: [PATCH 3/5] addressing PR feedback: adding handling of HTTP status code to retry policy --- .../retries/AwsDefaultRetryPolicyTest.kt | 19 ---------- .../http}/retries/AwsDefaultRetryPolicy.kt | 18 ++++++++-- .../http/retries/AwsDefaultRetryPolicyTest.kt | 36 +++++++++++++++++++ 3 files changed, 52 insertions(+), 21 deletions(-) delete mode 100644 aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicyTest.kt rename aws-runtime/{aws-core/common/src/aws/sdk/kotlin/runtime => protocols/http/common/src/aws/sdk/kotlin/runtime/http}/retries/AwsDefaultRetryPolicy.kt (71%) create mode 100644 aws-runtime/protocols/http/common/test/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicyTest.kt diff --git a/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicyTest.kt b/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicyTest.kt deleted file mode 100644 index e19167c20a1..00000000000 --- a/aws-runtime/aws-core/common/test/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicyTest.kt +++ /dev/null @@ -1,19 +0,0 @@ -package aws.sdk.kotlin.runtime.retries - -import aws.sdk.kotlin.runtime.AwsErrorMetadata -import aws.sdk.kotlin.runtime.AwsServiceException -import aws.smithy.kotlin.runtime.retries.RetryDirective -import kotlin.test.Test -import kotlin.test.assertEquals - -class AwsDefaultRetryPolicyTest { - @Test - fun testAwsServiceExceptions() { - AwsDefaultRetryPolicy.knownErrorTypes.forEach { (errorCode, errorType) -> - val ex = AwsServiceException() - ex.sdkErrorMetadata.attributes[AwsErrorMetadata.ErrorCode] = errorCode - val result = AwsDefaultRetryPolicy.evaluate(Result.failure(ex)) - assertEquals(RetryDirective.RetryError(errorType), result) - } - } -} diff --git a/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicy.kt b/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt similarity index 71% rename from aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicy.kt rename to aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt index 664f4be0ead..48c42926d5d 100644 --- a/aws-runtime/aws-core/common/src/aws/sdk/kotlin/runtime/retries/AwsDefaultRetryPolicy.kt +++ b/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt @@ -3,9 +3,12 @@ * SPDX-License-Identifier: Apache-2.0. */ -package aws.sdk.kotlin.runtime.retries +package aws.sdk.kotlin.runtime.http.retries +import aws.sdk.kotlin.runtime.AwsErrorMetadata import aws.sdk.kotlin.runtime.AwsServiceException +import aws.smithy.kotlin.runtime.ServiceErrorMetadata +import aws.smithy.kotlin.runtime.http.response.HttpResponse import aws.smithy.kotlin.runtime.retries.RetryDirective import aws.smithy.kotlin.runtime.retries.RetryErrorType.* import aws.smithy.kotlin.runtime.retries.impl.StandardRetryPolicy @@ -31,12 +34,23 @@ public object AwsDefaultRetryPolicy : StandardRetryPolicy() { "TransactionInProgressException" to Throttling, ) + internal val knownStatusCodes = mapOf( + 500 to Throttling, + 502 to Throttling, + 503 to Throttling, + 504 to Throttling, + ) + override fun evaluateOtherExceptions(ex: Throwable): RetryDirective? = when (ex) { is AwsServiceException -> evaluateAwsServiceException(ex) else -> null } private fun evaluateAwsServiceException(ex: AwsServiceException): RetryDirective? = with(ex.sdkErrorMetadata) { - knownErrorTypes[errorCode]?.let { RetryDirective.RetryError(it) } + (knownErrorTypes[errorCode] ?: knownStatusCodes[statusCode]) + ?.let { RetryDirective.RetryError(it) } } + + private val ServiceErrorMetadata.statusCode: Int? + get() = (protocolResponse as? HttpResponse)?.status?.value } diff --git a/aws-runtime/protocols/http/common/test/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicyTest.kt b/aws-runtime/protocols/http/common/test/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicyTest.kt new file mode 100644 index 00000000000..4adfc4b2ca4 --- /dev/null +++ b/aws-runtime/protocols/http/common/test/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicyTest.kt @@ -0,0 +1,36 @@ +package aws.sdk.kotlin.runtime.http.retries + +import aws.sdk.kotlin.runtime.AwsErrorMetadata +import aws.sdk.kotlin.runtime.AwsServiceException +import aws.smithy.kotlin.runtime.ServiceErrorMetadata +import aws.smithy.kotlin.runtime.http.Headers +import aws.smithy.kotlin.runtime.http.HttpBody +import aws.smithy.kotlin.runtime.http.HttpStatusCode +import aws.smithy.kotlin.runtime.http.response.HttpResponse +import aws.smithy.kotlin.runtime.retries.RetryDirective +import kotlin.test.Test +import kotlin.test.assertEquals + +class AwsDefaultRetryPolicyTest { + @Test + fun testErrorsByErrorCode() { + AwsDefaultRetryPolicy.knownErrorTypes.forEach { (errorCode, errorType) -> + val ex = AwsServiceException() + ex.sdkErrorMetadata.attributes[AwsErrorMetadata.ErrorCode] = errorCode + val result = AwsDefaultRetryPolicy.evaluate(Result.failure(ex)) + assertEquals(RetryDirective.RetryError(errorType), result) + } + } + + @Test + fun testErrorsByStatusCode() { + AwsDefaultRetryPolicy.knownStatusCodes.forEach { (statusCode, errorType) -> + val modeledStatusCode = HttpStatusCode.fromValue(statusCode) + val response = HttpResponse(modeledStatusCode, Headers.Empty, HttpBody.Empty) + val ex = AwsServiceException() + ex.sdkErrorMetadata.attributes[ServiceErrorMetadata.ProtocolResponse] = response + val result = AwsDefaultRetryPolicy.evaluate(Result.failure(ex)) + assertEquals(RetryDirective.RetryError(errorType), result) + } + } +} From 4416e3f2c1cc31331a89addcf72e722cf93828f2 Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Tue, 5 Oct 2021 18:05:19 +0000 Subject: [PATCH 4/5] linting --- .../aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt b/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt index 48c42926d5d..dbf21750765 100644 --- a/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt +++ b/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt @@ -5,7 +5,6 @@ package aws.sdk.kotlin.runtime.http.retries -import aws.sdk.kotlin.runtime.AwsErrorMetadata import aws.sdk.kotlin.runtime.AwsServiceException import aws.smithy.kotlin.runtime.ServiceErrorMetadata import aws.smithy.kotlin.runtime.http.response.HttpResponse From a18cca0f4348437339e0b88daf7560a00d1552f8 Mon Sep 17 00:00:00 2001 From: Ian Smith Botsford Date: Tue, 5 Oct 2021 18:42:13 +0000 Subject: [PATCH 5/5] addressing PR feedback: fixing incorrect status code mappings --- .../kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt b/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt index dbf21750765..4e3a32978bb 100644 --- a/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt +++ b/aws-runtime/protocols/http/common/src/aws/sdk/kotlin/runtime/http/retries/AwsDefaultRetryPolicy.kt @@ -34,10 +34,10 @@ public object AwsDefaultRetryPolicy : StandardRetryPolicy() { ) internal val knownStatusCodes = mapOf( - 500 to Throttling, - 502 to Throttling, - 503 to Throttling, - 504 to Throttling, + 500 to Timeout, + 502 to Timeout, + 503 to Timeout, + 504 to Timeout, ) override fun evaluateOtherExceptions(ex: Throwable): RetryDirective? = when (ex) {