Custom S3 Error Metadata Support

[kggilmer]

Work necessary to successfully parse/deserialize error metadata from S3

1. Support errors in the body of a 200 response
2. Extract host id
3. Map error code to exception type
4. requestId2

estimate: 2 weeks (.5 completed)

[aajtodd]

Before I forget...

I'm not sure I understand (3) Map error code to exception type, can you expand on where this one is coming from and what needs done?

Relevant to (2)/(4) : https://aws.amazon.com/premiumsupport/knowledge-center/s3-request-id-values/

Here is what Go does for (1):

• https://github.com/aws/aws-sdk-go-v2/blob/main/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3ErrorWith200Status.java#L18
• https://github.com/aws/aws-sdk-go-v2/blob/d05dd9b08d180c3696c33130548881966b56ea29/service/s3/internal/customizations/handle_200_error.go#L33

I think we should separate some of these out into multiple issues, in particular the 200 response errors.

Unless I'm mistaken about (3) I also think all of these fall under supplemental middleware that extend/augment the existing response/error middleware.

The secondary request ID is a good candidate for implementing as S3 specific metadata. We have support for this in errors already. #263, if implemented, would provide the support for any response.

For the response/error metadata we could be generating service specific containers like S3ErrorMetadata. We don't currently do this but it was proposed. We should probably talk about the approach here before implementing.

public open class S3Exception : AwsServiceException {

    public constructor() : super()

    public constructor(message: String?) : super(message)

    public constructor(message: String?, cause: Throwable?) : super(message, cause)

    public constructor(cause: Throwable?) : super(cause)

    override val sdkErrorMetadata: S3ErrorMetadata = S3ErrorMetadata()
}



public open class S3ErrorMetadata : AwsErrorMetadata() {
    public companion object {
        public val RequestId2: AttributeKey<String> = AttributeKey("S3:RequestId2")
    }

    /**
     * The secondary request ID that was returned by the called service if it exists
     */
    public val requestId2: String?
        get() = attributes.getOrNull(RequestId2)
}

[kggilmer]

I'm not sure I understand (3) Map error code to exception type, can you expand on where this one is coming from and what needs done?

This may be a NOP. I see in the S3 model that there are no error codes defined (via trait) in the S3 smithy model. We default to 400 if client errors are unspecified. When looking at the codegen and seeing everything maps to 400 I was thinking there was a problem but now looks like it's simply not a mechanism (error code -> modeled exception type mapping) that S3 seems to support.

[kggilmer]

Thanks for all the details above @aajtodd , very helpful. As suggested I've broken this ticket into two. This will track handing the response fields particular to S3 and the new issue will deal w/ the 200 error handler. I have yet to hear anything definitive regarding modeling error codes for S3 in service, but at this time I would say it's invalid. Will update here if turns out to be otherwise.

[kggilmer]

status: got some first-pass work done of codegen of S3 error types. Using Aaron's sample code above.

[kggilmer]

I played around with trying to override the metadata property via the extension property mechanism. It compiles but does not allow for overridding a property of an existing type.

Sample:

open class AwsErrorMetadata() {

    private val _prop = "asdf"

    public val propA: String?
        get() = _prop
}

open class ServiceErrorMetadata(): AwsErrorMetadata() {
    private val _prop2 = "werwerw"
    
    public val propB: String? 
        get() = _prop2
}
open class AwsServiceException {
    open val sdkErrorMetadata: AwsErrorMetadata = AwsErrorMetadata()
}

open class SomeServiceException(): AwsServiceException() {
    override val sdkErrorMetadata: ServiceErrorMetadata
        get() = ServiceErrorMetadata()
}

// Doesn't fail to compile but does not change types
val AwsServiceException.sdkErrorMetadata: ServiceErrorMetadata
    get() = ServiceErrorMetadata()


fun main() {
    val a = SomeServiceException()

    println(a.sdkErrorMetadata.propB)
}

[kggilmer]

I've got a rough cut now working. Not my intention to put into a PR but what I got working in the way that was simplest and most obvious to me at the time:

smithy-kotlin

• In smithy-kotlin add a function to KotlinIntegration that allows integrations to add code into the base exception type of a service SDK.
• ExceptionBaseClassGenerator calls this new function when generating the base exception

aws-sdk-kotlin

• Add a S3-specific ModeledExceptionsMiddleware that will inject a custom error handler for S3 service
• Add a S3 integration that:
  ** Generates a new file S3ErrorMetadata which provides additional request2 property.
  ** Implements the new KotlinIntegration function to override the default sdkErrorMetadata property with S3ErrorMetadata
• in /services build.gradle.kt allow for another source tree common/src
• in /services/s3/common/src add a Feature to handle deserializing S3 errors, copied and adapted from RestXmlError

After codegening S3 SDK, run the following snippet:

fun main() = runBlocking {
    val c = S3Client { region = "us-east-2" }

    try {
        c.getObject(GetObjectRequest.invoke {
            key = "226721d"
            bucket = "32cdrwer"
        }) {
            println(it)
        }

    } catch (e: NoSuchBucketException) {
        println(e.sdkErrorMetadata.requestId)
        println(e.sdkErrorMetadata.requestId2)
    } finally {
        c.close()
    }
}

Observed the output:

NW7Q0AT3EPPD3WS4
ditIc2v0nDFjrGrn6x+Ffv9E0bzIrEj14l8wfZIbM5kiNPzM8fB6dIkjTARIvkHMK27mqqppCVI=

I also notice we're getting another field, BucketName. Asking in s3 interest to see if I can get more specifics.