Skip to content

Commit 7b3d0cb

Browse files
authored
feat: handle S3 error responses with HTTP 200 status code (#1117)
1 parent cfff2e4 commit 7b3d0cb

File tree

7 files changed

+221
-2
lines changed

7 files changed

+221
-2
lines changed
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
{
22
"id": "4cbc20ed-789f-4040-b81c-328ecd3d0700",
33
"type": "feature",
4-
"description": "Make the AWS retry policy inheritable",
4+
"description": "**BREAKING**: Make the AWS retry policy inheritable",
5+
"requiresMinorVersionBump": true,
56
"issues": [
67
"awslabs/aws-sdk-kotlin#1055"
78
]
8-
}
9+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"id": "745b8c2f-49d6-42dd-a1a1-872e13bb97f4",
3+
"type": "feature",
4+
"description": "Handle S3 errors returned with an HTTP 200 response",
5+
"issues": [
6+
"awslabs/aws-sdk-kotlin#199"
7+
]
8+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package aws.sdk.kotlin.codegen.customization.s3
6+
7+
import software.amazon.smithy.kotlin.codegen.KotlinSettings
8+
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
9+
import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration
10+
import software.amazon.smithy.kotlin.codegen.model.buildSymbol
11+
import software.amazon.smithy.kotlin.codegen.model.expectShape
12+
import software.amazon.smithy.kotlin.codegen.model.isStreaming
13+
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator
14+
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolMiddleware
15+
import software.amazon.smithy.model.Model
16+
import software.amazon.smithy.model.shapes.OperationShape
17+
import software.amazon.smithy.model.shapes.ServiceShape
18+
19+
/**
20+
* Register interceptor to handle S3 error responses returned with an HTTP 200 status code.
21+
* see [aws-sdk-kotlin#199](https://github.com/awslabs/aws-sdk-kotlin/issues/199)
22+
*/
23+
class S3ErrorWith200StatusIntegration : KotlinIntegration {
24+
override fun enabledForService(model: Model, settings: KotlinSettings): Boolean =
25+
model.expectShape<ServiceShape>(settings.service).isS3
26+
27+
override fun customizeMiddleware(
28+
ctx: ProtocolGenerator.GenerationContext,
29+
resolved: List<ProtocolMiddleware>,
30+
): List<ProtocolMiddleware> = resolved + listOf(S3HandleError200ResponseMiddleware)
31+
}
32+
33+
private object S3HandleError200ResponseMiddleware : ProtocolMiddleware {
34+
override fun isEnabledFor(ctx: ProtocolGenerator.GenerationContext, op: OperationShape): Boolean {
35+
// we don't know for sure what operations S3 does this on. Go customized this for only a select few
36+
// like CopyObject/UploadPartCopy/CompleteMultipartUpload but Rust hit it on additional operations
37+
// (DeleteObjects).
38+
// Instead of playing whack-a-mole broadly apply this interceptor to everything but streaming responses
39+
// which adds a small amount of overhead to response processing.
40+
val output = ctx.model.expectShape(op.output.get())
41+
return output.members().none { it.isStreaming || ctx.model.expectShape(it.target).isStreaming }
42+
}
43+
44+
override val name: String = "Handle200ErrorsInterceptor"
45+
override fun render(ctx: ProtocolGenerator.GenerationContext, op: OperationShape, writer: KotlinWriter) {
46+
val symbol = buildSymbol {
47+
name = this@S3HandleError200ResponseMiddleware.name
48+
namespace = ctx.settings.pkg.subpackage("internal")
49+
}
50+
51+
writer.write("op.interceptors.add(#T)", symbol)
52+
}
53+
}

codegen/smithy-aws-kotlin-codegen/src/main/resources/META-INF/services/software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ aws.sdk.kotlin.codegen.customization.s3.HostPrefixRequestRouteFilter
2323
aws.sdk.kotlin.codegen.customization.s3.UnwrappedXmlOutputIntegration
2424
aws.sdk.kotlin.codegen.customization.s3control.HostPrefixFilter
2525
aws.sdk.kotlin.codegen.customization.s3control.ClientConfigIntegration
26+
aws.sdk.kotlin.codegen.customization.s3.S3ErrorWith200StatusIntegration
2627
aws.sdk.kotlin.codegen.customization.apigateway.ApiGatewayAddAcceptHeader
2728
aws.sdk.kotlin.codegen.customization.glacier.GlacierAddVersionHeader
2829
aws.sdk.kotlin.codegen.customization.glacier.GlacierAccountIdDefault

services/build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ subprojects {
5353

5454
dependencies {
5555
implementation(libraries.kotlinx.coroutines.test)
56+
implementation(libraries.smithy.kotlin.http.test)
5657
}
5758
}
5859
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package aws.sdk.kotlin.services.s3.internal
6+
7+
import aws.smithy.kotlin.runtime.client.ProtocolResponseInterceptorContext
8+
import aws.smithy.kotlin.runtime.http.HttpBody
9+
import aws.smithy.kotlin.runtime.http.HttpStatusCode
10+
import aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor
11+
import aws.smithy.kotlin.runtime.http.isSuccess
12+
import aws.smithy.kotlin.runtime.http.readAll
13+
import aws.smithy.kotlin.runtime.http.request.HttpRequest
14+
import aws.smithy.kotlin.runtime.http.response.HttpResponse
15+
import aws.smithy.kotlin.runtime.http.response.copy
16+
import aws.smithy.kotlin.runtime.serde.xml.*
17+
18+
/**
19+
* Interceptor to handle S3 responses with HTTP 200 status but have an error in the payload.
20+
*
21+
* See [S3 200 error](https://aws.amazon.com/premiumsupport/knowledge-center/s3-resolve-200-internalerror/)
22+
* and [aws-sdk-kotlin#199](https://github.com/awslabs/aws-sdk-kotlin/issues/199)
23+
*/
24+
internal object Handle200ErrorsInterceptor : HttpInterceptor {
25+
override suspend fun modifyBeforeDeserialization(context: ProtocolResponseInterceptorContext<Any, HttpRequest, HttpResponse>): HttpResponse {
26+
val response = context.protocolResponse
27+
if (!response.status.isSuccess()) return response
28+
val payload = response.body.readAll() ?: return response
29+
val reader = xmlStreamReader(payload)
30+
val token = runCatching { reader.seek<XmlToken.BeginElement>() }.getOrNull()
31+
32+
// according to the knowledge center article above we should treat these as 5xx,
33+
// our retry policy will handle standard error codes like `SlowDown`
34+
val statusCode = HttpStatusCode.InternalServerError
35+
.takeIf { token?.name?.local == "Error" }
36+
?: response.status
37+
return response.copy(status = statusCode, body = HttpBody.fromBytes(payload))
38+
}
39+
}
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package aws.sdk.kotlin.services.s3.internal
6+
7+
import aws.sdk.kotlin.services.s3.S3Client
8+
import aws.sdk.kotlin.services.s3.deleteObject
9+
import aws.sdk.kotlin.services.s3.deleteObjects
10+
import aws.sdk.kotlin.services.s3.model.S3Exception
11+
import aws.smithy.kotlin.runtime.auth.awscredentials.Credentials
12+
import aws.smithy.kotlin.runtime.auth.awscredentials.CredentialsProvider
13+
import aws.smithy.kotlin.runtime.http.HttpBody
14+
import aws.smithy.kotlin.runtime.http.HttpStatusCode
15+
import aws.smithy.kotlin.runtime.http.response.HttpResponse
16+
import aws.smithy.kotlin.runtime.httptest.buildTestConnection
17+
import aws.smithy.kotlin.runtime.util.Attributes
18+
import kotlinx.coroutines.test.runTest
19+
import kotlin.test.Test
20+
import kotlin.test.assertEquals
21+
import kotlin.test.assertFailsWith
22+
23+
class Handle200ErrorsInterceptorTest {
24+
25+
object TestCredentialsProvider : CredentialsProvider {
26+
override suspend fun resolve(attributes: Attributes): Credentials = Credentials("AKID", "SECRET")
27+
}
28+
private val errorResponsePayload = """
29+
<Error>
30+
<Code>SlowDown</Code>
31+
<Message>Please reduce your request rate.</Message>
32+
<RequestId>K2H6N7ZGQT6WHCEG</RequestId>
33+
<HostId>WWoZlnK4pTjKCYn6eNV7GgOurabfqLkjbSyqTvDMGBaI9uwzyNhSaDhOCPs8paFGye7S6b/AB3A=</HostId>
34+
</Error>
35+
""".trimIndent().encodeToByteArray()
36+
37+
private fun newTestClient(
38+
status: HttpStatusCode = HttpStatusCode.OK,
39+
payload: ByteArray = errorResponsePayload,
40+
): S3Client =
41+
S3Client {
42+
region = "us-east-1"
43+
credentialsProvider = TestCredentialsProvider
44+
retryStrategy {
45+
maxAttempts = 1
46+
}
47+
httpClient = buildTestConnection {
48+
expect(HttpResponse(status, body = HttpBody.fromBytes(payload)))
49+
}
50+
}
51+
52+
fun assertException(ex: S3Exception) {
53+
val expectedMessage = "Please reduce your request rate."
54+
assertEquals("SlowDown", ex.sdkErrorMetadata.errorCode)
55+
assertEquals(expectedMessage, ex.sdkErrorMetadata.errorMessage)
56+
assertEquals(expectedMessage, ex.message)
57+
assertEquals("K2H6N7ZGQT6WHCEG", ex.sdkErrorMetadata.requestId)
58+
assertEquals("WWoZlnK4pTjKCYn6eNV7GgOurabfqLkjbSyqTvDMGBaI9uwzyNhSaDhOCPs8paFGye7S6b/AB3A=", ex.sdkErrorMetadata.requestId2)
59+
}
60+
61+
@Test
62+
fun testHandle200ErrorsWithNoExpectedBody() = runTest {
63+
val s3 = newTestClient()
64+
val ex = assertFailsWith<S3Exception> {
65+
s3.deleteObject { bucket = "test"; key = "key" }
66+
}
67+
assertException(ex)
68+
}
69+
70+
@Test
71+
fun testHandle200ErrorsWithExpectedBody() = runTest {
72+
val s3 = newTestClient()
73+
val ex = assertFailsWith<S3Exception> {
74+
s3.deleteObjects { bucket = "test" }
75+
}
76+
assertException(ex)
77+
}
78+
79+
@Test
80+
fun testNonErrorPayload() = runTest {
81+
val payload = """
82+
<?xml version="1.0" encoding="UTF-8"?>
83+
<DeleteResult>
84+
<Deleted>
85+
<Key>my-key</Key>
86+
</Deleted>
87+
</DeleteResult>
88+
""".trimIndent().encodeToByteArray()
89+
val s3 = newTestClient(payload = payload)
90+
val response = s3.deleteObjects { bucket = "test" }
91+
assertEquals("my-key", response.deleted?.first()?.key)
92+
}
93+
94+
@Test
95+
fun testErrorPayloadUnmodified() = runTest {
96+
val payload = """
97+
<?xml version="1.0" encoding="UTF-8"?>
98+
<Error>
99+
<Code>FooError</Code>
100+
<Message>Please use less foos.</Message>
101+
<RequestId>rid</RequestId>
102+
<HostId>rid2</HostId>
103+
</Error>
104+
""".trimIndent().encodeToByteArray()
105+
val s3 = newTestClient(HttpStatusCode.BadRequest, payload)
106+
val ex = assertFailsWith<S3Exception> {
107+
s3.deleteObjects { bucket = "test" }
108+
}
109+
val expectedMessage = "Please use less foos."
110+
assertEquals(expectedMessage, ex.message)
111+
assertEquals(expectedMessage, ex.sdkErrorMetadata.errorMessage)
112+
assertEquals("FooError", ex.sdkErrorMetadata.errorCode)
113+
assertEquals("rid", ex.sdkErrorMetadata.requestId)
114+
assertEquals("rid2", ex.sdkErrorMetadata.requestId2)
115+
}
116+
}

0 commit comments

Comments
 (0)