Unverified Commit 57ed310e authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Allow S3 to retry on HTTP response 200 with InternalError (#3699) 

## Motivation and Context
https://github.com/awslabs/aws-sdk-rust/issues/1163

## Description
When the S3 SDK processes a response with the 200 status code but with
`InternalError`, the SDK today does not trigger a retry through any
classifier in the chain: `AwsErrorCodeClassifier`,
`ModeledAsRetryableClassifier`, `HttpStatusCodeClassifier`, and
`TransientErrorClassifier`. To address it, this PR updates
`AwsErrorCodeClassifier` only for S3 so that it classifies
`InternalError` as retryable.

## Testing
- [x] CI
- [x] Added traced test to the existing test file `status-200-errors.rs`

## Checklist
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent 9009d1b6

CHANGELOG.next.toml

+6 −0
Original line number Diff line number Diff line
@@ -37,3 +37,9 @@ Then, call `PresignedRequest::make_http_1x_request` or `PresignedRequest::into_h 
references = ["smithy-rs#3696"]

meta = { "breaking" = false, "tada" = true, "bug" = false }

author = "Velfi"



[[aws-sdk-rust]]

message = "`AwsErrorCodeClassifier` for S3 now treats `InternalError` as a transient error to trigger retries."

references = ["aws-sdk-rust#1163"]

meta = { "breaking" = false, "tada" = false, "bug" = true }

author = "ysaito1001"

aws/rust-runtime/aws-runtime/Cargo.toml

+1 −1
Original line number Diff line number Diff line 
[package]

name = "aws-runtime"

version = "1.2.3"

version = "1.3.0"

authors = ["AWS Rust SDK Team <aws-sdk-rust@amazon.com>"]

description = "Runtime support code for the AWS SDK. This crate isn't intended to be used directly."

edition = "2021"

aws/rust-runtime/aws-runtime/src/retries/classifiers.rs

+53 −5
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ use aws_smithy_runtime_api::client::retries::classifiers::{ 
};

use aws_smithy_types::error::metadata::ProvideErrorMetadata;

use aws_smithy_types::retry::ErrorKind;

use std::borrow::Cow;

use std::error::Error as StdError;

use std::marker::PhantomData;
@@ -35,15 +36,62 @@ pub const THROTTLING_ERRORS: &[&str] = &[ 
pub const TRANSIENT_ERRORS: &[&str] = &["RequestTimeout", "RequestTimeoutException"];



/// A retry classifier for determining if the response sent by an AWS service requires a retry.

#[derive(Debug, Default)]

#[derive(Debug)]

pub struct AwsErrorCodeClassifier<E> {

    throttling_errors: Cow<'static, [&'static str]>,

    transient_errors: Cow<'static, [&'static str]>,

    _inner: PhantomData<E>,

}



impl<E> Default for AwsErrorCodeClassifier<E> {

    fn default() -> Self {

        Self {

            throttling_errors: THROTTLING_ERRORS.into(),

            transient_errors: TRANSIENT_ERRORS.into(),

            _inner: PhantomData,

        }

    }

}



/// Builder for [`AwsErrorCodeClassifier`]

#[derive(Debug)]

pub struct AwsErrorCodeClassifierBuilder<E> {

    throttling_errors: Option<Cow<'static, [&'static str]>>,

    transient_errors: Option<Cow<'static, [&'static str]>>,

    _inner: PhantomData<E>,

}



impl<E> AwsErrorCodeClassifierBuilder<E> {

    /// Set `transient_errors` for the builder

    pub fn transient_errors(

        mut self,

        transient_errors: impl Into<Cow<'static, [&'static str]>>,

    ) -> Self {

        self.transient_errors = Some(transient_errors.into());

        self

    }



    /// Build a new [`AwsErrorCodeClassifier`]

    pub fn build(self) -> AwsErrorCodeClassifier<E> {

        AwsErrorCodeClassifier {

            throttling_errors: self.throttling_errors.unwrap_or(THROTTLING_ERRORS.into()),

            transient_errors: self.transient_errors.unwrap_or(TRANSIENT_ERRORS.into()),

            _inner: self._inner,

        }

    }

}



impl<E> AwsErrorCodeClassifier<E> {

    /// Create a new AwsErrorCodeClassifier

    /// Create a new [`AwsErrorCodeClassifier`]

    pub fn new() -> Self {

        Self {

        Self::default()

    }



    /// Return a builder that can create a new [`AwsErrorCodeClassifier`]

    pub fn builder() -> AwsErrorCodeClassifierBuilder<E> {

        AwsErrorCodeClassifierBuilder {

            throttling_errors: None,

            transient_errors: None,

            _inner: PhantomData,

        }

    }
@@ -73,13 +121,13 @@ where 
            .and_then(|err| err.code());



        if let Some(error_code) = error_code {

            if THROTTLING_ERRORS.contains(&error_code) {

            if self.throttling_errors.contains(&error_code) {

                return RetryAction::RetryIndicated(RetryReason::RetryableError {

                    kind: ErrorKind::ThrottlingError,

                    retry_after,

                });

            }

            if TRANSIENT_ERRORS.contains(&error_code) {

            if self.transient_errors.contains(&error_code) {

                return RetryAction::RetryIndicated(RetryReason::RetryableError {

                    kind: ErrorKind::TransientError,

                    retry_after,

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt

+6 −1
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ import software.amazon.smithy.rustsdk.customize.IsTruncatedPaginatorDecorator 
import software.amazon.smithy.rustsdk.customize.RemoveDefaultsDecorator

import software.amazon.smithy.rustsdk.customize.apigateway.ApiGatewayDecorator

import software.amazon.smithy.rustsdk.customize.applyDecorators

import software.amazon.smithy.rustsdk.customize.applyExceptFor

import software.amazon.smithy.rustsdk.customize.ec2.Ec2Decorator

import software.amazon.smithy.rustsdk.customize.glacier.GlacierDecorator

import software.amazon.smithy.rustsdk.customize.lambda.LambdaDecorator
@@ -41,7 +42,6 @@ val DECORATORS: List<ClientCodegenDecorator> = 
            SigV4AuthDecorator(),

            HttpRequestChecksumDecorator(),

            HttpResponseChecksumDecorator(),

            RetryClassifierDecorator(),

            IntegrationTestDecorator(),

            AwsFluentClientDecorator(),

            CrateLicenseDecorator(),
@@ -63,6 +63,11 @@ val DECORATORS: List<ClientCodegenDecorator> = 
            ServiceEnvConfigDecorator(),

            HttpRequestCompressionDecorator(),

        ),

        // S3 needs `AwsErrorCodeClassifier` to handle an `InternalError` as a transient error. We need to customize

        // that behavior for S3 in a way that does not conflict with the globally applied `RetryClassifierDecorator`.

        // Therefore, that decorator is applied to all but S3, and S3 customizes the creation of `AwsErrorCodeClassifier`

        // accordingly (see https://github.com/smithy-lang/smithy-rs/pull/3699).

        RetryClassifierDecorator().applyExceptFor("com.amazonaws.s3#AmazonS3"),

        // Service specific decorators

        ApiGatewayDecorator().onlyApplyTo("com.amazonaws.apigateway#BackplaneControlService"),

        Ec2Decorator().onlyApplyTo("com.amazonaws.ec2#AmazonEC2"),

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/ServiceSpecificDecorator.kt

+8 −0
Original line number Diff line number Diff line
@@ -17,6 +17,14 @@ fun ClientCodegenDecorator.onlyApplyTo(serviceId: String): List<ClientCodegenDec 
        },

    )



/** Apply this decorator to all services but the one identified by ID */

fun ClientCodegenDecorator.applyExceptFor(serviceId: String): List<ClientCodegenDecorator> =

    listOf(

        ConditionalDecorator(this) { _, serviceShapeId ->

            serviceShapeId != ShapeId.from(serviceId)

        },

    )



/** Apply the given decorators only to this service ID */

fun String.applyDecorators(vararg decorators: ClientCodegenDecorator): List<ClientCodegenDecorator> =

    decorators.map { it.onlyApplyTo(this) }.flatten()