Skip to content

Commit a04c2cb

Browse files
aajtoddaajtodd
authored
feat(rt): retry HTTP exceptions (SRA) (#829)
1 parent d59d21e commit a04c2cb

File tree

17 files changed

+298
-45
lines changed

17 files changed

+298
-45
lines changed

‎.changes/389efd23-6868-4e0d-b392-fca03d358270.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"id": "389efd23-6868-4e0d-b392-fca03d358270",
3+
"type": "feature",
4+
"description": "Add support for retrying transient HTTP errors. `RetryErrorType.Timeout` was renamed to `RetryErrorType.Transient`."
5+
}

‎runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/CrtHttpEngine.kt‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ package aws.smithy.kotlin.runtime.http.engine.crt
77

88
import aws.sdk.kotlin.crt.http.*
99
import aws.sdk.kotlin.crt.io.*
10-
import aws.smithy.kotlin.runtime.ClientException
1110
import aws.smithy.kotlin.runtime.crt.SdkDefaultIO
11+
import aws.smithy.kotlin.runtime.http.HttpErrorCode
12+
import aws.smithy.kotlin.runtime.http.HttpException
1213
import aws.smithy.kotlin.runtime.http.engine.HttpClientEngine
1314
import aws.smithy.kotlin.runtime.http.engine.HttpClientEngineBase
1415
import aws.smithy.kotlin.runtime.http.engine.ProxyConfig
@@ -94,7 +95,7 @@ public class CrtHttpEngine(public val config: CrtHttpEngineConfig) : HttpClientE
9495
// handler)
9596
val conn = withTimeoutOrNull(config.connectionAcquireTimeout) {
9697
manager.acquireConnection()
97-
} ?: throw ClientException("timed out waiting for an HTTP connection to be acquired from the pool")
98+
} ?: throw HttpException("timed out waiting for an HTTP connection to be acquired from the pool", errorCode = HttpErrorCode.CONNECTION_ACQUIRE_TIMEOUT)
9899
logger.trace { "Acquired connection ${conn.id}" }
99100

100101
val respHandler = SdkStreamResponseHandler(conn, callContext)
@@ -107,8 +108,11 @@ public class CrtHttpEngine(public val config: CrtHttpEngineConfig) : HttpClientE
107108
val reqTime = Instant.now()
108109
val engineRequest = request.toCrtRequest(callContext)
109110

110-
val stream = conn.makeRequest(engineRequest, respHandler)
111-
stream.activate()
111+
val stream = mapCrtException {
112+
conn.makeRequest(engineRequest, respHandler).also { stream ->
113+
stream.activate()
114+
}
115+
}
112116

113117
if (request.isChunked) {
114118
withContext(SdkDispatchers.IO) {

‎runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtil.kt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
package aws.smithy.kotlin.runtime.http.engine.crt
77

8+
import aws.sdk.kotlin.crt.CRT
9+
import aws.sdk.kotlin.crt.CrtRuntimeException
810
import aws.sdk.kotlin.crt.http.HeadersBuilder
911
import aws.sdk.kotlin.crt.http.HttpRequestBodyStream
1012
import aws.sdk.kotlin.crt.http.HttpStream
@@ -14,6 +16,8 @@ import aws.sdk.kotlin.crt.io.UserInfo
1416
import aws.smithy.kotlin.runtime.crt.ReadChannelBodyStream
1517
import aws.smithy.kotlin.runtime.crt.SdkSourceBodyStream
1618
import aws.smithy.kotlin.runtime.http.HttpBody
19+
import aws.smithy.kotlin.runtime.http.HttpErrorCode
20+
import aws.smithy.kotlin.runtime.http.HttpException
1721
import aws.smithy.kotlin.runtime.http.request.HttpRequest
1822
import aws.smithy.kotlin.runtime.io.SdkBuffer
1923
import aws.smithy.kotlin.runtime.io.buffer
@@ -109,3 +113,45 @@ internal suspend fun HttpStream.sendChunkedBody(body: HttpBody) {
109113
else -> error("sendChunkedBody should not be called for non-chunked body types")
110114
}
111115
}
116+
117+
internal inline fun <T> mapCrtException(block: () -> T): T =
118+
try {
119+
block()
120+
} catch (ex: CrtRuntimeException) {
121+
throw HttpException(fmtCrtErrorMessage(ex.errorCode), errorCode = mapCrtErrorCode(ex.errorCode))
122+
}
123+
124+
internal fun fmtCrtErrorMessage(errorCode: Int): String {
125+
val errorDescription = CRT.errorString(errorCode)
126+
val errName = CRT.errorName(errorCode)
127+
return "$errName: $errorDescription; crtErrorCode=$errorCode"
128+
}
129+
130+
// do this by name rather than error code as it's difficult to map error codes on JVM side and would be prone to breaking
131+
// if new errors are added to the various aws-c-* lib enum blocks.
132+
//
133+
// See:
134+
// IO Errors: https://github.com/awslabs/aws-c-io/blob/v0.13.19/include/aws/io/io.h#L89
135+
// HTTP Errors: https://github.com/awslabs/aws-c-http/blob/v0.7.6/include/aws/http/http.h#L15
136+
internal fun mapCrtErrorCode(code: Int): HttpErrorCode = when (CRT.errorName(code)) {
137+
"AWS_IO_SOCKET_TIMEOUT" -> HttpErrorCode.SOCKET_TIMEOUT
138+
"AWS_ERROR_HTTP_UNSUPPORTED_PROTOCOL" -> HttpErrorCode.PROTOCOL_NEGOTIATION_ERROR
139+
"AWS_IO_SOCKET_NOT_CONNECTED" -> HttpErrorCode.CONNECT_TIMEOUT
140+
"AWS_IO_TLS_NEGOTIATION_TIMEOUT" -> HttpErrorCode.TLS_NEGOTIATION_TIMEOUT
141+
in tlsNegotiationErrors -> HttpErrorCode.TLS_NEGOTIATION_ERROR
142+
in connectionClosedErrors -> HttpErrorCode.CONNECTION_CLOSED
143+
else -> HttpErrorCode.SDK_UNKNOWN
144+
}
145+
146+
private val tlsNegotiationErrors = setOf(
147+
"AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE",
148+
"AWS_IO_TLS_ERROR_NOT_NEGOTIATED",
149+
"AWS_IO_TLS_DIGEST_ALGORITHM_UNSUPPORTED",
150+
"AWS_IO_TLS_SIGNATURE_ALGORITHM_UNSUPPORTED",
151+
)
152+
153+
private val connectionClosedErrors = setOf(
154+
"AWS_ERROR_HTTP_CONNECTION_CLOSED",
155+
"AWS_IO_BROKEN_PIPE",
156+
"AWS_IO_SOCKET_CLOSED",
157+
)

‎runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/SdkStreamResponseHandler.kt‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55

66
package aws.smithy.kotlin.runtime.http.engine.crt
77

8-
import aws.sdk.kotlin.crt.CRT
98
import aws.sdk.kotlin.crt.http.*
109
import aws.sdk.kotlin.crt.io.Buffer
1110
import aws.smithy.kotlin.runtime.http.*
1211
import aws.smithy.kotlin.runtime.http.HeadersBuilder
12+
import aws.smithy.kotlin.runtime.http.HttpException
1313
import aws.smithy.kotlin.runtime.http.response.HttpResponse
1414
import aws.smithy.kotlin.runtime.io.SdkBuffer
1515
import aws.smithy.kotlin.runtime.io.SdkByteChannel
@@ -85,15 +85,17 @@ internal class SdkStreamResponseHandler(
8585
val ch = SdkByteChannel(true)
8686
val writerContext = callContext + callContext.derivedName("response-body-writer")
8787
val job = GlobalScope.launch(writerContext) {
88-
for (buffer in bodyChan) {
89-
val wc = buffer.size.toInt()
90-
ch.write(buffer)
91-
// increment window
92-
onDataConsumed(wc)
88+
val result = runCatching {
89+
for (buffer in bodyChan) {
90+
val wc = buffer.size.toInt()
91+
ch.write(buffer)
92+
// increment window
93+
onDataConsumed(wc)
94+
}
9395
}
9496

9597
// immediately close when done to signal end of body stream
96-
ch.close()
98+
ch.close(result.exceptionOrNull())
9799
}
98100

99101
job.invokeOnCompletion { cause ->
@@ -171,10 +173,9 @@ internal class SdkStreamResponseHandler(
171173

172174
// close the body channel
173175
if (errorCode != 0) {
174-
val errorDescription = CRT.errorString(errorCode)
175-
val ex = CancellationException("CrtHttpEngine::response failed: ec=$errorCode; description=$errorDescription")
176+
val ex = HttpException(fmtCrtErrorMessage(errorCode), errorCode = mapCrtErrorCode(errorCode))
176177
responseReady.close(ex)
177-
bodyChan.cancel(ex)
178+
bodyChan.close(ex)
178179
} else {
179180
// closing the channel to indicate no more data will be sent
180181
bodyChan.close()

‎runtime/protocol/http-client-engines/http-client-engine-crt/common/test/aws/smithy/kotlin/runtime/http/engine/crt/SdkStreamResponseHandlerTest.kt‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,13 @@ package aws.smithy.kotlin.runtime.http.engine.crt
88
import aws.sdk.kotlin.crt.http.*
99
import aws.sdk.kotlin.crt.io.byteArrayBuffer
1010
import aws.smithy.kotlin.runtime.http.HttpBody
11+
import aws.smithy.kotlin.runtime.http.HttpErrorCode
12+
import aws.smithy.kotlin.runtime.http.HttpException
1113
import aws.smithy.kotlin.runtime.http.HttpStatusCode
1214
import aws.smithy.kotlin.runtime.io.SdkSink
1315
import aws.smithy.kotlin.runtime.io.readAll
1416
import aws.smithy.kotlin.runtime.io.readToBuffer
1517
import io.kotest.matchers.string.shouldContain
16-
import kotlinx.coroutines.CancellationException
1718
import kotlinx.coroutines.ExperimentalCoroutinesApi
1819
import kotlinx.coroutines.launch
1920
import kotlinx.coroutines.test.runTest
@@ -170,9 +171,11 @@ class SdkStreamResponseHandlerTest {
170171
assertEquals(data.length.toLong(), resp.body.contentLength)
171172
val respChan = (resp.body as HttpBody.ChannelContent).readFrom()
172173

173-
assertTrue(respChan.isClosedForWrite)
174-
assertFailsWith<CancellationException> {
174+
val ex = assertFailsWith<HttpException> {
175175
respChan.readAll(SdkSink.blackhole())
176-
}.message.shouldContain("CrtHttpEngine::response failed: ec=$socketClosedEc")
176+
}
177+
178+
ex.message.shouldContain("socket is closed.; crtErrorCode=$socketClosedEc")
179+
assertEquals(HttpErrorCode.CONNECTION_CLOSED, ex.errorCode)
177180
}
178181
}

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class OkHttpEngine(
3939

4040
val engineRequest = request.toOkHttpRequest(context, callContext)
4141
val engineCall = client.newCall(engineRequest)
42-
val engineResponse = engineCall.executeAsync()
42+
val engineResponse = mapOkHttpExceptions { engineCall.executeAsync() }
4343

4444
callContext.job.invokeOnCompletion {
4545
engineResponse.body.close()
@@ -69,9 +69,9 @@ private fun OkHttpEngineConfig.buildClient(): OkHttpClient {
6969
followRedirects(false)
7070
followSslRedirects(false)
7171

72-
// see: https://github.com/ktorio/ktor/issues/1708#issuecomment-609988128
73-
// TODO disable this once better transient exception handling is in place
74-
retryOnConnectionFailure(true)
72+
// Transient connection errors are handled by retry strategy (exceptions are wrapped and marked retryable
73+
// appropriately internally). We don't want inner retry logic that inflates the number of retries.
74+
retryOnConnectionFailure(false)
7575

7676
connectTimeout(config.connectTimeout.toJavaDuration())
7777
readTimeout(config.socketReadTimeout.toJavaDuration())

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpUtils.kt‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import okhttp3.Route
2525
import okhttp3.internal.http.HttpMethod
2626
import java.io.IOException
2727
import java.net.*
28+
import javax.net.ssl.SSLHandshakeException
2829
import kotlin.coroutines.CoroutineContext
2930
import aws.smithy.kotlin.runtime.http.engine.ProxySelector as SdkProxySelector
3031
import okhttp3.Request as OkHttpRequest
@@ -189,3 +190,27 @@ private fun URI.toUrl(): Url {
189190
fragment = uri.fragment?.takeIf { it.isNotBlank() }
190191
}
191192
}
193+
194+
internal inline fun<T> mapOkHttpExceptions(block: () -> T): T =
195+
try {
196+
block()
197+
} catch (ex: IOException) {
198+
throw HttpException(ex, ex.errCode(), ex.isRetryable())
199+
}
200+
201+
private fun Exception.isRetryable(): Boolean = isCauseOrSuppressed<ConnectException>()
202+
private fun Exception.errCode(): HttpErrorCode = when {
203+
isConnectTimeoutException() -> HttpErrorCode.CONNECT_TIMEOUT
204+
isCauseOrSuppressed<SocketTimeoutException>() -> HttpErrorCode.SOCKET_TIMEOUT
205+
isCauseOrSuppressed<SSLHandshakeException>() -> HttpErrorCode.TLS_NEGOTIATION_ERROR
206+
else -> HttpErrorCode.SDK_UNKNOWN
207+
}
208+
209+
private fun Exception.isConnectTimeoutException(): Boolean =
210+
findCauseOrSuppressed<SocketTimeoutException>()?.message?.contains("connect", ignoreCase = true) == true
211+
private inline fun <reified T> Exception.isCauseOrSuppressed(): Boolean = findCauseOrSuppressed<T>() != null
212+
private inline fun <reified T> Exception.findCauseOrSuppressed(): T? {
213+
if (this is T) return this
214+
if (cause is T) return cause as T
215+
return suppressedExceptions.firstOrNull { it is T } as? T
216+
}

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpExceptionTest.kt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package aws.smithy.kotlin.runtime.http.engine.okhttp
7+
8+
import aws.smithy.kotlin.runtime.http.HttpErrorCode
9+
import aws.smithy.kotlin.runtime.http.HttpException
10+
import java.io.IOException
11+
import java.net.ConnectException
12+
import java.net.SocketTimeoutException
13+
import javax.net.ssl.SSLHandshakeException
14+
import kotlin.test.Test
15+
import kotlin.test.assertEquals
16+
import kotlin.test.assertFailsWith
17+
18+
class OkHttpExceptionTest {
19+
data class ExceptionTest(
20+
val ex: Exception,
21+
val expectedError: HttpErrorCode,
22+
val expectedRetryable: Boolean = false,
23+
)
24+
25+
@Test
26+
fun testMapExceptions() {
27+
val tests = listOf(
28+
ExceptionTest(IOException("unknown"), expectedError = HttpErrorCode.SDK_UNKNOWN),
29+
ExceptionTest(IOException(SocketTimeoutException("connect timeout")), expectedError = HttpErrorCode.CONNECT_TIMEOUT, true),
30+
ExceptionTest(IOException().apply { addSuppressed(SocketTimeoutException("connect timeout")) }, expectedError = HttpErrorCode.CONNECT_TIMEOUT, true),
31+
ExceptionTest(IOException(SocketTimeoutException("read timeout")), expectedError = HttpErrorCode.SOCKET_TIMEOUT),
32+
ExceptionTest(IOException().apply { addSuppressed(SocketTimeoutException("read timeout")) }, expectedError = HttpErrorCode.SOCKET_TIMEOUT),
33+
ExceptionTest(SocketTimeoutException("read timeout"), expectedError = HttpErrorCode.SOCKET_TIMEOUT),
34+
ExceptionTest(IOException(SSLHandshakeException("negotiate error")), expectedError = HttpErrorCode.TLS_NEGOTIATION_ERROR),
35+
ExceptionTest(ConnectException("test connect error"), expectedError = HttpErrorCode.SDK_UNKNOWN, true),
36+
)
37+
38+
for (test in tests) {
39+
val ex = assertFailsWith<HttpException> {
40+
mapOkHttpExceptions {
41+
throw test.ex
42+
}
43+
}
44+
45+
assertEquals(test.expectedError, ex.errorCode, "ex=$ex; ${ex.suppressedExceptions}; cause=${ex.cause}; causeSuppressed=${ex.cause?.suppressedExceptions}")
46+
assertEquals(test.expectedRetryable, ex.sdkErrorMetadata.isRetryable)
47+
}
48+
}
49+
}

‎runtime/protocol/http/api/http.api‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,30 @@ public final class aws/smithy/kotlin/runtime/http/HttpBodyKt {
8989
public static final fun toHttpBody ([B)Laws/smithy/kotlin/runtime/http/HttpBody;
9090
}
9191

92+
public final class aws/smithy/kotlin/runtime/http/HttpErrorCode : java/lang/Enum {
93+
public static final field CONNECTION_ACQUIRE_TIMEOUT Laws/smithy/kotlin/runtime/http/HttpErrorCode;
94+
public static final field CONNECTION_CLOSED Laws/smithy/kotlin/runtime/http/HttpErrorCode;
95+
public static final field CONNECT_TIMEOUT Laws/smithy/kotlin/runtime/http/HttpErrorCode;
96+
public static final field PROTOCOL_NEGOTIATION_ERROR Laws/smithy/kotlin/runtime/http/HttpErrorCode;
97+
public static final field SDK_UNKNOWN Laws/smithy/kotlin/runtime/http/HttpErrorCode;
98+
public static final field SOCKET_TIMEOUT Laws/smithy/kotlin/runtime/http/HttpErrorCode;
99+
public static final field TLS_NEGOTIATION_ERROR Laws/smithy/kotlin/runtime/http/HttpErrorCode;
100+
public static final field TLS_NEGOTIATION_TIMEOUT Laws/smithy/kotlin/runtime/http/HttpErrorCode;
101+
public static fun valueOf (Ljava/lang/String;)Laws/smithy/kotlin/runtime/http/HttpErrorCode;
102+
public static fun values ()[Laws/smithy/kotlin/runtime/http/HttpErrorCode;
103+
}
104+
105+
public final class aws/smithy/kotlin/runtime/http/HttpException : aws/smithy/kotlin/runtime/SdkBaseException {
106+
public fun <init> (Ljava/lang/String;Laws/smithy/kotlin/runtime/http/HttpErrorCode;Z)V
107+
public synthetic fun <init> (Ljava/lang/String;Laws/smithy/kotlin/runtime/http/HttpErrorCode;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
108+
public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;Laws/smithy/kotlin/runtime/http/HttpErrorCode;Z)V
109+
public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Throwable;Laws/smithy/kotlin/runtime/http/HttpErrorCode;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
110+
public fun <init> (Ljava/lang/Throwable;Laws/smithy/kotlin/runtime/http/HttpErrorCode;Z)V
111+
public synthetic fun <init> (Ljava/lang/Throwable;Laws/smithy/kotlin/runtime/http/HttpErrorCode;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
112+
public final fun getErrorCode ()Laws/smithy/kotlin/runtime/http/HttpErrorCode;
113+
public fun toString ()Ljava/lang/String;
114+
}
115+
92116
public final class aws/smithy/kotlin/runtime/http/HttpMethod : java/lang/Enum {
93117
public static final field Companion Laws/smithy/kotlin/runtime/http/HttpMethod$Companion;
94118
public static final field DELETE Laws/smithy/kotlin/runtime/http/HttpMethod;

0 commit comments

Comments
 (0)