diff --git a/.changes/0645c929-bdfe-4d07-a131-b0b447422fff.json b/.changes/0645c929-bdfe-4d07-a131-b0b447422fff.json new file mode 100644 index 000000000..c931e4ee4 --- /dev/null +++ b/.changes/0645c929-bdfe-4d07-a131-b0b447422fff.json @@ -0,0 +1,5 @@ +{ + "id": "0645c929-bdfe-4d07-a131-b0b447422fff", + "type": "bugfix", + "description": "Treat all IO errors in OkHttp & CRT engines as retryable (e.g., socket errors, DNS lookup failures, etc.)" +} diff --git a/runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtil.kt b/runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtil.kt index 6fb6513c5..e969d0856 100644 --- a/runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtil.kt +++ b/runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtil.kt @@ -118,7 +118,11 @@ internal inline fun mapCrtException(block: () -> T): T = try { block() } catch (ex: CrtRuntimeException) { - throw HttpException(fmtCrtErrorMessage(ex.errorCode), errorCode = mapCrtErrorCode(ex.errorCode)) + throw HttpException( + message = fmtCrtErrorMessage(ex.errorCode), + errorCode = mapCrtErrorCode(ex.errorName), + retryable = isRetryable(ex.errorName), + ) } internal fun fmtCrtErrorMessage(errorCode: Int): String { @@ -133,7 +137,7 @@ internal fun fmtCrtErrorMessage(errorCode: Int): String { // See: // IO Errors: https://github.com/awslabs/aws-c-io/blob/v0.13.19/include/aws/io/io.h#L89 // HTTP Errors: https://github.com/awslabs/aws-c-http/blob/v0.7.6/include/aws/http/http.h#L15 -internal fun mapCrtErrorCode(code: Int): HttpErrorCode = when (CRT.errorName(code)) { +private fun mapCrtErrorCode(errorName: String?) = when (errorName) { "AWS_IO_SOCKET_TIMEOUT" -> HttpErrorCode.SOCKET_TIMEOUT "AWS_ERROR_HTTP_UNSUPPORTED_PROTOCOL" -> HttpErrorCode.PROTOCOL_NEGOTIATION_ERROR "AWS_IO_SOCKET_NOT_CONNECTED" -> HttpErrorCode.CONNECT_TIMEOUT @@ -143,6 +147,27 @@ internal fun mapCrtErrorCode(code: Int): HttpErrorCode = when (CRT.errorName(cod else -> HttpErrorCode.SDK_UNKNOWN } +internal fun mapCrtErrorCode(code: Int) = mapCrtErrorCode(CRT.errorName(code)) + +internal fun isRetryable(errorName: String?) = errorName?.let { + when { + // All IO errors are retryable + it.startsWith("AWS_IO_") || it.startsWith("AWS_ERROR_IO_") -> true + + // Any connection closure is retryable + it in connectionClosedErrors -> true + + // All proxy errors are retryable + it.startsWith("AWS_ERROR_HTTP_PROXY_") -> true + + // Any connection manager issues are retryable + it.startsWith("AWS_ERROR_HTTP_CONNECTION_MANAGER_") -> true + + // Any other errors are not retryable + else -> false + } +} ?: false // Unknown error codes are not retryable + private val tlsNegotiationErrors = setOf( "AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE", "AWS_IO_TLS_ERROR_NOT_NEGOTIATED", @@ -152,6 +177,7 @@ private val tlsNegotiationErrors = setOf( private val connectionClosedErrors = setOf( "AWS_ERROR_HTTP_CONNECTION_CLOSED", + "AWS_ERROR_HTTP_SERVER_CLOSED", "AWS_IO_BROKEN_PIPE", "AWS_IO_SOCKET_CLOSED", ) diff --git a/runtime/protocol/http-client-engines/http-client-engine-crt/common/test/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtilTest.kt b/runtime/protocol/http-client-engines/http-client-engine-crt/common/test/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtilTest.kt new file mode 100644 index 000000000..007f8ba5e --- /dev/null +++ b/runtime/protocol/http-client-engines/http-client-engine-crt/common/test/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtilTest.kt @@ -0,0 +1,46 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +package aws.smithy.kotlin.runtime.http.engine.crt + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class RequestUtilTest { + @Test + fun testExceptionRetryability() { + setOf( + // All IO errors are retryable + "AWS_IO_SOCKET_NETWORK_DOWN", + "AWS_ERROR_IO_OPERATION_CANCELLED", + "AWS_IO_DNS_NO_ADDRESS_FOR_HOST", + + // Any connection closure is retryable + "AWS_ERROR_HTTP_CONNECTION_CLOSED", + "AWS_ERROR_HTTP_SERVER_CLOSED", + "AWS_IO_BROKEN_PIPE", + + // All proxy errors are retryable + "AWS_ERROR_HTTP_PROXY_CONNECT_FAILED", + "AWS_ERROR_HTTP_PROXY_STRATEGY_NTLM_CHALLENGE_TOKEN_MISSING", + "AWS_ERROR_HTTP_PROXY_STRATEGY_TOKEN_RETRIEVAL_FAILURE", + + // Any connection manager issues are retryable + "AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE", + "AWS_ERROR_HTTP_CONNECTION_MANAGER_VENDED_CONNECTION_UNDERFLOW", + "AWS_ERROR_HTTP_CONNECTION_MANAGER_SHUTTING_DOWN", + ).forEach { name -> assertTrue(isRetryable(name), "Expected $name to be retryable!") } + + setOf( + // Any other errors are not retryable + "AWS_ERROR_HTTP_INVALID_METHOD", + "AWS_ERROR_PKCS11_CKR_CANCEL", + "AWS_ERROR_PEM_MALFORMED", + + // Unknown error codes are not retryable + null, + ).forEach { name -> assertFalse(isRetryable(name), "Expected $name to not be retryable!") } + } +} diff --git a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpUtils.kt b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpUtils.kt index 46f121ea7..5b793a33c 100644 --- a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpUtils.kt +++ b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpUtils.kt @@ -208,10 +208,9 @@ internal inline fun mapOkHttpExceptions(block: () -> T): T = try { block() } catch (ex: IOException) { - throw HttpException(ex, ex.errCode(), ex.isRetryable()) + throw HttpException(ex, ex.errCode(), retryable = true) // All IOExceptions are retryable } -private fun Exception.isRetryable(): Boolean = isCauseOrSuppressed() || isConnectionClosedException() private fun Exception.errCode(): HttpErrorCode = when { isConnectTimeoutException() -> HttpErrorCode.CONNECT_TIMEOUT isConnectionClosedException() -> HttpErrorCode.CONNECTION_CLOSED diff --git a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpExceptionTest.kt b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpExceptionTest.kt index fee0a7d7c..f69463892 100644 --- a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpExceptionTest.kt +++ b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpExceptionTest.kt @@ -7,6 +7,7 @@ package aws.smithy.kotlin.runtime.http.engine.okhttp import aws.smithy.kotlin.runtime.http.HttpErrorCode import aws.smithy.kotlin.runtime.http.HttpException +import org.junit.jupiter.api.Assertions.assertTrue import java.io.EOFException import java.io.IOException import java.net.ConnectException @@ -20,22 +21,21 @@ class OkHttpExceptionTest { data class ExceptionTest( val ex: Exception, val expectedError: HttpErrorCode, - val expectedRetryable: Boolean = false, ) @Test fun testMapExceptions() { val tests = listOf( ExceptionTest(IOException("unknown"), expectedError = HttpErrorCode.SDK_UNKNOWN), - ExceptionTest(IOException(SocketTimeoutException("connect timeout")), expectedError = HttpErrorCode.CONNECT_TIMEOUT, true), - ExceptionTest(IOException().apply { addSuppressed(SocketTimeoutException("connect timeout")) }, expectedError = HttpErrorCode.CONNECT_TIMEOUT, true), + ExceptionTest(IOException(SocketTimeoutException("connect timeout")), expectedError = HttpErrorCode.CONNECT_TIMEOUT), + ExceptionTest(IOException().apply { addSuppressed(SocketTimeoutException("connect timeout")) }, expectedError = HttpErrorCode.CONNECT_TIMEOUT), ExceptionTest(IOException(SocketTimeoutException("read timeout")), expectedError = HttpErrorCode.SOCKET_TIMEOUT), ExceptionTest(IOException().apply { addSuppressed(SocketTimeoutException("read timeout")) }, expectedError = HttpErrorCode.SOCKET_TIMEOUT), ExceptionTest(SocketTimeoutException("read timeout"), expectedError = HttpErrorCode.SOCKET_TIMEOUT), ExceptionTest(IOException(SSLHandshakeException("negotiate error")), expectedError = HttpErrorCode.TLS_NEGOTIATION_ERROR), - ExceptionTest(ConnectException("test connect error"), expectedError = HttpErrorCode.SDK_UNKNOWN, true), + ExceptionTest(ConnectException("test connect error"), expectedError = HttpErrorCode.SDK_UNKNOWN), // see https://github.com/awslabs/aws-sdk-kotlin/issues/905 - ExceptionTest(IOException("unexpected end of stream on https://test.aws.com", EOFException("\\n not found: limit=0 content=...")), expectedError = HttpErrorCode.CONNECTION_CLOSED, expectedRetryable = true), + ExceptionTest(IOException("unexpected end of stream on https://test.aws.com", EOFException("\\n not found: limit=0 content=...")), expectedError = HttpErrorCode.CONNECTION_CLOSED), ) for (test in tests) { @@ -46,7 +46,7 @@ class OkHttpExceptionTest { } assertEquals(test.expectedError, ex.errorCode, "ex=$ex; ${ex.suppressedExceptions}; cause=${ex.cause}; causeSuppressed=${ex.cause?.suppressedExceptions}") - assertEquals(test.expectedRetryable, ex.sdkErrorMetadata.isRetryable) + assertTrue(ex.sdkErrorMetadata.isRetryable) } } }