Skip to content

Commit d12febb

Browse files
aajtoddaajtodd
authored
refactor!: replace default HTTP client engine (#554)
1 parent 14b2f3e commit d12febb

File tree

15 files changed

+149
-822
lines changed

15 files changed

+149
-822
lines changed

‎aws-runtime/aws-config/build.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ kotlin {
2424
implementation("aws.smithy.kotlin:logging:$smithyKotlinVersion")
2525
implementation("aws.smithy.kotlin:http:$smithyKotlinVersion")
2626
implementation("aws.smithy.kotlin:utils:$smithyKotlinVersion")
27-
implementation("aws.smithy.kotlin:http-client-engine-crt:$smithyKotlinVersion")
27+
implementation("aws.smithy.kotlin:http-client-engine-default:$smithyKotlinVersion")
2828
implementation(project(":aws-runtime:aws-http"))
2929

3030
// parsing common JSON credentials responses

‎aws-runtime/aws-config/common/src/aws/sdk/kotlin/runtime/auth/credentials/DefaultChainCredentialsProvider.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ package aws.sdk.kotlin.runtime.auth.credentials
77

88
import aws.sdk.kotlin.runtime.config.AwsSdkSetting
99
import aws.sdk.kotlin.runtime.config.imds.ImdsClient
10+
import aws.smithy.kotlin.runtime.http.engine.DefaultHttpEngine
1011
import aws.smithy.kotlin.runtime.http.engine.HttpClientEngine
11-
import aws.smithy.kotlin.runtime.http.engine.crt.CrtHttpEngine
1212
import aws.smithy.kotlin.runtime.io.Closeable
1313
import aws.smithy.kotlin.runtime.util.Platform
1414
import aws.smithy.kotlin.runtime.util.PlatformProvider
@@ -42,7 +42,7 @@ public class DefaultChainCredentialsProvider constructor(
4242
) : CredentialsProvider, Closeable {
4343

4444
private val manageEngine = httpClientEngine == null
45-
private val httpClientEngine = httpClientEngine ?: CrtHttpEngine()
45+
private val httpClientEngine = httpClientEngine ?: DefaultHttpEngine()
4646

4747
private val chain = CredentialsProviderChain(
4848
EnvironmentCredentialsProvider(platformProvider::getenv),

‎aws-runtime/aws-config/common/src/aws/sdk/kotlin/runtime/auth/credentials/EcsCredentialsProvider.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import aws.sdk.kotlin.runtime.config.resolve
1111
import aws.smithy.kotlin.runtime.ServiceException
1212
import aws.smithy.kotlin.runtime.client.ExecutionContext
1313
import aws.smithy.kotlin.runtime.http.*
14+
import aws.smithy.kotlin.runtime.http.engine.DefaultHttpEngine
1415
import aws.smithy.kotlin.runtime.http.engine.HttpClientEngine
15-
import aws.smithy.kotlin.runtime.http.engine.crt.CrtHttpEngine
1616
import aws.smithy.kotlin.runtime.http.middleware.ResolveEndpoint
1717
import aws.smithy.kotlin.runtime.http.middleware.Retry
1818
import aws.smithy.kotlin.runtime.http.operation.*
@@ -66,7 +66,7 @@ public class EcsCredentialsProvider internal constructor(
6666
public constructor() : this(Platform)
6767

6868
private val manageEngine = httpClientEngine == null
69-
private val httpClientEngine = httpClientEngine ?: CrtHttpEngine()
69+
private val httpClientEngine = httpClientEngine ?: DefaultHttpEngine()
7070

7171
private val retryMiddleware = run {
7272
val tokenBucket = StandardRetryTokenBucket(StandardRetryTokenBucketOptions.Default)

‎aws-runtime/aws-config/common/src/aws/sdk/kotlin/runtime/config/imds/ImdsClient.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import aws.smithy.kotlin.runtime.client.ExecutionContext
1313
import aws.smithy.kotlin.runtime.client.SdkClientOption
1414
import aws.smithy.kotlin.runtime.client.SdkLogMode
1515
import aws.smithy.kotlin.runtime.http.*
16+
import aws.smithy.kotlin.runtime.http.engine.DefaultHttpEngine
1617
import aws.smithy.kotlin.runtime.http.engine.HttpClientEngine
17-
import aws.smithy.kotlin.runtime.http.engine.crt.CrtHttpEngine
1818
import aws.smithy.kotlin.runtime.http.middleware.ResolveEndpoint
1919
import aws.smithy.kotlin.runtime.http.middleware.Retry
2020
import aws.smithy.kotlin.runtime.http.operation.*
@@ -73,7 +73,7 @@ public class ImdsClient private constructor(builder: Builder) : InstanceMetadata
7373

7474
init {
7575
require(maxRetries > 0) { "maxRetries must be greater than zero" }
76-
val engine = builder.engine ?: CrtHttpEngine {
76+
val engine = builder.engine ?: DefaultHttpEngine {
7777
connectTimeout = 1.seconds
7878
socketReadTimeout = 1.seconds
7979
}

‎aws-runtime/aws-config/common/test/aws/sdk/kotlin/runtime/config/imds/ImdsClientTest.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ class ImdsClientTest {
212212
connection.assertRequests()
213213
}
214214

215-
@IgnoreWindows("DNS fails faster on windows and results in a different error")
215+
@IgnoreWindows("DNS fails faster on windows and results in a different error: `socket connect failure, no route to host`")
216216
@Test
217217
fun testHttpConnectTimeouts() = runBlocking {
218218
// end-to-end real client times out after 1-second
@@ -227,8 +227,9 @@ class ImdsClientTest {
227227
client.get("/latest/metadata")
228228
}
229229
}
230-
// on windows DNS fails faster with message `socket connect failure, no route to host.
231-
assertTrue(ex.message!!.lowercase().contains("timed out"), "message `${ex.message}`")
230+
231+
// this message is asserted against whatever the default HTTP client engine throws for an exception...
232+
assertTrue(ex.message!!.lowercase().contains("timeout has expired"), "message `${ex.message}`")
232233
val elapsed = Instant.now().epochMilliseconds - start.epochMilliseconds
233234
assertTrue(elapsed >= 1000, "expected elapsed ms to be greater than 1000; actual = $elapsed")
234235
assertTrue(elapsed < 2000, "expected elapsed ms to be less than 2000; actual = $elapsed")

‎codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/core/AwsHttpProtocolClientGenerator.kt‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import aws.sdk.kotlin.codegen.protocols.middleware.AwsSignatureVersion4
1010
import aws.sdk.kotlin.codegen.sdkId
1111
import software.amazon.smithy.aws.traits.auth.UnsignedPayloadTrait
1212
import software.amazon.smithy.codegen.core.CodegenException
13-
import software.amazon.smithy.codegen.core.Symbol
1413
import software.amazon.smithy.kotlin.codegen.core.*
1514
import software.amazon.smithy.kotlin.codegen.model.buildSymbol
1615
import software.amazon.smithy.kotlin.codegen.model.hasIdempotentTokenMember
@@ -32,12 +31,6 @@ open class AwsHttpProtocolClientGenerator(
3231
httpBindingResolver: HttpBindingResolver
3332
) : HttpProtocolClientGenerator(ctx, middlewares, httpBindingResolver) {
3433

35-
override val defaultHttpClientEngineSymbol: Symbol
36-
get() = buildSymbol {
37-
name = "CrtHttpEngine"
38-
namespace(KotlinDependency.AWS_CRT_HTTP_ENGINE)
39-
}
40-
4134
override fun render(writer: KotlinWriter) {
4235
writer.write("\n\n")
4336
writer.write("const val ServiceId: String = #S", ctx.service.sdkId)

‎services/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ subprojects {
117117
implementation(kotlin("test"))
118118
implementation(kotlin("test-junit5"))
119119
implementation(project(":aws-runtime:testing"))
120+
implementation(project(":tests:e2e-test-util"))
120121
}
121122
}
122123
kotlinOptions {

‎services/polly/e2eTest/PollyPresignerTest.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package aws.sdk.kotlin.services.polly
22

3-
import aws.sdk.kotlin.runtime.http.engine.crt.CrtHttpEngine
43
import aws.sdk.kotlin.services.polly.model.OutputFormat
54
import aws.sdk.kotlin.services.polly.model.SynthesizeSpeechRequest
65
import aws.sdk.kotlin.services.polly.model.VoiceId
76
import aws.sdk.kotlin.services.polly.presigners.presign
7+
import aws.sdk.kotlin.testing.withAllEngines
88
import aws.smithy.kotlin.runtime.http.response.complete
99
import aws.smithy.kotlin.runtime.http.sdkHttpClient
1010
import kotlinx.coroutines.runBlocking
@@ -30,13 +30,13 @@ class PollyPresignerTest {
3030
val client = PollyClient { region = "us-east-1" }
3131
val presignedRequest = request.presign(client.config, 10.seconds)
3232

33-
CrtHttpEngine().use { engine ->
33+
withAllEngines { engine ->
3434
val httpClient = sdkHttpClient(engine)
3535

3636
val call = httpClient.call(presignedRequest)
3737
call.complete()
3838

39-
assertEquals(200, call.response.status.value)
39+
assertEquals(200, call.response.status.value, "presigned polly request failed for engine: $engine")
4040
}
4141
}
4242
}

‎services/s3/e2eTest/S3IntegrationTest.kt‎

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ package aws.sdk.kotlin.e2etest
66

77
import aws.sdk.kotlin.services.s3.S3Client
88
import aws.sdk.kotlin.services.s3.model.GetObjectRequest
9+
import aws.sdk.kotlin.testing.PRINTABLE_CHARS
10+
import aws.sdk.kotlin.testing.withAllEngines
911
import aws.smithy.kotlin.runtime.content.ByteStream
1012
import aws.smithy.kotlin.runtime.content.decodeToString
1113
import aws.smithy.kotlin.runtime.content.fromFile
@@ -100,15 +102,68 @@ class S3BucketOpsIntegrationTest {
100102
}
101103

102104
@Test
103-
fun testListObjectsWithDelimiter(): Unit = runBlocking {
105+
fun testQueryParameterEncoding(): Unit = runBlocking {
104106
// see: https://github.com/awslabs/aws-sdk-kotlin/issues/448
105107

106-
client.listObjects {
107-
bucket = testBucket
108-
delimiter = "/"
109-
prefix = null
108+
// this is mostly a stress test of signing w.r.t query parameter encoding (since
109+
// delimiter is bound via @httpQuery) and the ability of an HTTP engine to keep
110+
// the same encoding going out on the wire (e.g. not double percent encoding)
111+
112+
s3WithAllEngines { s3 ->
113+
s3.listObjects {
114+
bucket = testBucket
115+
delimiter = PRINTABLE_CHARS
116+
prefix = null
117+
}
118+
// only care that request is accepted, not the results
119+
}
120+
}
121+
122+
@Test
123+
fun testPathEncoding(): Unit = runBlocking {
124+
// this is mostly a stress test of signing w.r.t path encoding (since key is bound
125+
// via @httpLabel) and the ability of an HTTP engine to keep the same encoding going
126+
// out on the wire (e.g. not double percent encoding)
127+
128+
// NOTE: S3 provides guidance on choosing object key names: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html
129+
// This test includes all printable chars (including ones S3 recommends avoiding). Users should
130+
// strive to fall within the guidelines given by S3 though
131+
132+
s3WithAllEngines { s3 ->
133+
val objKey = "foo$PRINTABLE_CHARS"
134+
val content = "hello rfc3986"
135+
136+
s3.putObject {
137+
bucket = testBucket
138+
key = objKey
139+
body = ByteStream.fromString(content)
140+
}
141+
142+
val req = GetObjectRequest {
143+
bucket = testBucket
144+
key = objKey
145+
}
146+
147+
s3.getObject(req) { resp ->
148+
val actual = resp.body!!.decodeToString()
149+
assertEquals(content, actual)
150+
}
110151
}
152+
}
153+
}
111154

112-
// only care that request is accepted, not the results
155+
internal suspend fun s3WithAllEngines(block: suspend (S3Client) -> Unit) {
156+
withAllEngines { engine ->
157+
S3Client {
158+
region = S3BucketOpsIntegrationTest.DEFAULT_REGION
159+
httpClientEngine = engine
160+
}.use {
161+
try {
162+
block(it)
163+
} catch (ex: Exception) {
164+
println("test failed for engine $engine")
165+
throw ex
166+
}
167+
}
113168
}
114169
}

0 commit comments

Comments
 (0)