Unverified Commit 96a9f844 authored by Zelda Hessler's avatar Zelda Hessler Committed by GitHub
Browse files

implement user-configurable retry classifiers (#2977) 

[Read the RFC here](https://github.com/awslabs/smithy-rs/pull/3018)

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
#2417

## Description
<!--- Describe your changes in detail -->
Exactly what it says on the tin. I have a related RFC to publish that
goes into more depth.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
I wrote an integration test that ensures a custom retry classifier can
be set and is called.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [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 d1ad9373

CHANGELOG.next.toml

+20 −0
Original line number Diff line number Diff line
@@ -303,3 +303,23 @@ message = "Our algorithm for converting identifiers to `snake_case` has been upd 
references = ["smithy-rs#3037", "aws-sdk-rust#756"]

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

author = "rcoh"



[[smithy-rs]]

message = """

Retry classifiers are now configurable at the service and operation levels. Users may also define their own custom retry classifiers.



For more information, see the [guide](https://github.com/awslabs/smithy-rs/discussions/3050).

"""

references = ["smithy-rs#2417", "smithy-rs#3018"]

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

author = "Velfi"



[[aws-sdk-rust]]

message = """

Retry classifiers are now configurable at the service and operation levels. Users may also define their own custom retry classifiers.



For more information, see the [guide](https://github.com/awslabs/smithy-rs/discussions/3050).

"""

references = ["smithy-rs#2417", "smithy-rs#3018"]

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

author = "Velfi"

aws/rust-runtime/aws-config/src/http_credential_provider.rs

+22 −24
Original line number Diff line number Diff line
@@ -15,18 +15,19 @@ use aws_credential_types::Credentials; 
use aws_smithy_http::body::SdkBody;

use aws_smithy_http::result::SdkError;

use aws_smithy_runtime::client::orchestrator::operation::Operation;

use aws_smithy_runtime::client::retries::classifier::{

    HttpStatusCodeClassifier, SmithyErrorClassifier,

use aws_smithy_runtime::client::retries::classifiers::{

    HttpStatusCodeClassifier, TransientErrorClassifier,

};

use aws_smithy_runtime_api::client::http::HttpConnectorSettings;

use aws_smithy_runtime_api::client::interceptors::context::{Error, InterceptorContext};

use aws_smithy_runtime_api::client::orchestrator::{

    HttpResponse, OrchestratorError, SensitiveOutput,

};

use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryClassifiers, RetryReason};

use aws_smithy_runtime_api::client::retries::classifiers::ClassifyRetry;

use aws_smithy_runtime_api::client::retries::classifiers::RetryAction;

use aws_smithy_runtime_api::client::runtime_plugin::StaticRuntimePlugin;

use aws_smithy_types::config_bag::Layer;

use aws_smithy_types::retry::{ErrorKind, RetryConfig};

use aws_smithy_types::retry::RetryConfig;

use aws_smithy_types::timeout::TimeoutConfig;

use http::header::{ACCEPT, AUTHORIZATION};

use http::{HeaderValue, Response};
@@ -88,18 +89,6 @@ impl Builder { 
    ) -> HttpCredentialProvider {

        let provider_config = self.provider_config.unwrap_or_default();



        // The following errors are retryable:

        //   - Socket errors

        //   - Networking timeouts

        //   - 5xx errors

        //   - Non-parseable 200 responses.

        let retry_classifiers = RetryClassifiers::new()

            .with_classifier(HttpCredentialRetryClassifier)

            // Socket errors and network timeouts

            .with_classifier(SmithyErrorClassifier::<Error>::new())

            // 5xx errors

            .with_classifier(HttpStatusCodeClassifier::default());



        let mut builder = Operation::builder()

            .service_name("HttpCredentialProvider")

            .operation_name("LoadCredentials")
@@ -123,7 +112,16 @@ impl Builder { 
        if let Some(sleep_impl) = provider_config.sleep_impl() {

            builder = builder

                .standard_retry(&RetryConfig::standard())

                .retry_classifiers(retry_classifiers)

                // The following errors are retryable:

                //   - Socket errors

                //   - Networking timeouts

                //   - 5xx errors

                //   - Non-parseable 200 responses.

                .retry_classifier(HttpCredentialRetryClassifier)

                // Socket errors and network timeouts

                .retry_classifier(TransientErrorClassifier::<Error>::new())

                // 5xx errors

                .retry_classifier(HttpStatusCodeClassifier::default())

                .sleep_impl(sleep_impl);

        } else {

            builder = builder.no_retry();
@@ -192,11 +190,11 @@ impl ClassifyRetry for HttpCredentialRetryClassifier { 
        "HttpCredentialRetryClassifier"

    }



    fn classify_retry(&self, ctx: &InterceptorContext) -> Option<RetryReason> {

        let output_or_error = ctx.output_or_error()?;

    fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction {

        let output_or_error = ctx.output_or_error();

        let error = match output_or_error {

            Ok(_) => return None,

            Err(err) => err,

            Some(Ok(_)) | None => return RetryAction::NoActionIndicated,

            Some(Err(err)) => err,

        };



        // Retry non-parseable 200 responses
@@ -206,11 +204,11 @@ impl ClassifyRetry for HttpCredentialRetryClassifier { 
            .zip(ctx.response().map(HttpResponse::status))

        {

            if matches!(err, CredentialsError::Unhandled { .. }) && status.is_success() {

                return Some(RetryReason::Error(ErrorKind::ServerError));

                return RetryAction::server_error();

            }

        }



        None

        RetryAction::NoActionIndicated

    }

}
@@ -308,7 +306,7 @@ mod test { 
    }



    #[tokio::test]

    async fn explicit_error_not_retriable() {

    async fn explicit_error_not_retryable() {

        let http_client = StaticReplayClient::new(vec![ReplayEvent::new(

            Request::builder()

                .uri(Uri::from_static("http://localhost:1234/some-creds"))

aws/rust-runtime/aws-config/src/imds/client.rs

+25 −27
Original line number Diff line number Diff line
@@ -21,15 +21,15 @@ use aws_smithy_runtime::client::retries::strategy::StandardRetryStrategy; 
use aws_smithy_runtime_api::client::auth::AuthSchemeOptionResolverParams;

use aws_smithy_runtime_api::client::endpoint::{EndpointResolver, EndpointResolverParams};

use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext;

use aws_smithy_runtime_api::client::orchestrator::{

    Future, HttpResponse, OrchestratorError, SensitiveOutput,

use aws_smithy_runtime_api::client::orchestrator::{Future, OrchestratorError, SensitiveOutput};

use aws_smithy_runtime_api::client::retries::classifiers::{

    ClassifyRetry, RetryAction, SharedRetryClassifier,

};

use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryClassifiers, RetryReason};

use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder;

use aws_smithy_runtime_api::client::runtime_plugin::{RuntimePlugin, SharedRuntimePlugin};

use aws_smithy_types::config_bag::{FrozenLayer, Layer};

use aws_smithy_types::endpoint::Endpoint;

use aws_smithy_types::retry::{ErrorKind, RetryConfig};

use aws_smithy_types::retry::RetryConfig;

use aws_smithy_types::timeout::TimeoutConfig;

use aws_types::os_shim_internal::Env;

use http::Uri;
@@ -250,9 +250,7 @@ impl ImdsCommonRuntimePlugin { 
                .with_http_client(config.http_client())

                .with_endpoint_resolver(Some(endpoint_resolver))

                .with_interceptor(UserAgentInterceptor::new())

                .with_retry_classifiers(Some(

                    RetryClassifiers::new().with_classifier(ImdsResponseRetryClassifier),

                ))

                .with_retry_classifier(SharedRetryClassifier::new(ImdsResponseRetryClassifier))

                .with_retry_strategy(Some(StandardRetryStrategy::new(retry_config)))

                .with_time_source(Some(config.time_source()))

                .with_sleep_impl(config.sleep_impl()),
@@ -548,32 +546,26 @@ impl EndpointResolver for ImdsEndpointResolver { 
#[derive(Clone, Debug)]

struct ImdsResponseRetryClassifier;



impl ImdsResponseRetryClassifier {

    fn classify(response: &HttpResponse) -> Option<RetryReason> {

        let status = response.status();

        match status {

            _ if status.is_server_error() => Some(RetryReason::Error(ErrorKind::ServerError)),

            // 401 indicates that the token has expired, this is retryable

            _ if status.as_u16() == 401 => Some(RetryReason::Error(ErrorKind::ServerError)),

            // This catch-all includes successful responses that fail to parse. These should not be retried.

            _ => None,

        }

    }

}



impl ClassifyRetry for ImdsResponseRetryClassifier {

    fn name(&self) -> &'static str {

        "ImdsResponseRetryClassifier"

    }



    fn classify_retry(&self, ctx: &InterceptorContext) -> Option<RetryReason> {

    fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction {

        if let Some(response) = ctx.response() {

            Self::classify(response)

            let status = response.status();

            match status {

                _ if status.is_server_error() => RetryAction::server_error(),

                // 401 indicates that the token has expired, this is retryable

                _ if status.as_u16() == 401 => RetryAction::server_error(),

                // This catch-all includes successful responses that fail to parse. These should not be retried.

                _ => RetryAction::NoActionIndicated,

            }

        } else {

            // Don't retry timeouts for IMDS, or else it will take ~30 seconds for the default

            // credentials provider chain to fail to provide credentials.

            // Also don't retry non-responses.

            None

            RetryAction::NoActionIndicated

        }

    }

}
@@ -596,7 +588,7 @@ pub(crate) mod test { 
    use aws_smithy_runtime_api::client::orchestrator::{

        HttpRequest, HttpResponse, OrchestratorError,

    };

    use aws_smithy_runtime_api::client::retries::ClassifyRetry;

    use aws_smithy_runtime_api::client::retries::classifiers::{ClassifyRetry, RetryAction};

    use aws_smithy_types::error::display::DisplayErrorContext;

    use aws_types::os_shim_internal::{Env, Fs};

    use http::header::USER_AGENT;
@@ -915,21 +907,27 @@ pub(crate) mod test { 
        http_client.assert_requests_match(&[]);

    }



    /// Successful responses should classify as `RetryKind::Unnecessary`

    /// The classifier should return `None` when classifying a successful response.

    #[test]

    fn successful_response_properly_classified() {

        let mut ctx = InterceptorContext::new(Input::doesnt_matter());

        ctx.set_output_or_error(Ok(Output::doesnt_matter()));

        ctx.set_response(imds_response("").map(|_| SdkBody::empty()));

        let classifier = ImdsResponseRetryClassifier;

        assert_eq!(None, classifier.classify_retry(&ctx));

        assert_eq!(

            RetryAction::NoActionIndicated,

            classifier.classify_retry(&ctx)

        );



        // Emulate a failure to parse the response body (using an io error since it's easy to construct in a test)

        let mut ctx = InterceptorContext::new(Input::doesnt_matter());

        ctx.set_output_or_error(Err(OrchestratorError::connector(ConnectorError::io(

            io::Error::new(io::ErrorKind::BrokenPipe, "fail to parse").into(),

        ))));

        assert_eq!(None, classifier.classify_retry(&ctx));

        assert_eq!(

            RetryAction::NoActionIndicated,

            classifier.classify_retry(&ctx)

        );

    }



    // since tokens are sent as headers, the tokens need to be valid header values

aws/rust-runtime/aws-config/test-data/profile-provider/invalid_config/test-case.json

+1 −1
Original line number Diff line number Diff line 
{

  "name": "empty-config",

  "name": "invalid-config",

  "docs": "config was invalid",

  "result": {

    "ErrorContains": "could not parse profile file"

aws/rust-runtime/aws-config/test-data/profile-provider/retry_on_error/test-case.json

+1 −1
Original line number Diff line number Diff line 
{

  "name": "e2e-assume-role",

  "name": "retry-on-error",

  "docs": "end to end successful role assumption",

  "result": {

    "Ok": {