From 96a9f8441e1a6622c36d2b7a4d4155b303e0db59 Mon Sep 17 00:00:00 2001 From: Zelda Hessler Date: Tue, 10 Oct 2023 14:07:23 -0500 Subject: [PATCH] implement user-configurable retry classifiers (#2977) [Read the RFC here](https://github.com/awslabs/smithy-rs/pull/3018) ## Motivation and Context #2417 ## Description Exactly what it says on the tin. I have a related RFC to publish that goes into more depth. ## Testing I wrote an integration test that ensures a custom retry classifier can be set and is called. ## Checklist - [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._ --- CHANGELOG.next.toml | 20 ++ .../src/http_credential_provider.rs | 46 ++- .../aws-config/src/imds/client.rs | 52 ++- .../invalid_config/test-case.json | 2 +- .../retry_on_error/test-case.json | 2 +- aws/rust-runtime/aws-runtime/src/retries.rs | 2 +- .../retries/{classifier.rs => classifiers.rs} | 139 ++++---- .../src/http_request/canonical_request.rs | 3 +- .../smithy/rustsdk/InvocationIdDecorator.kt | 2 +- .../rustsdk/RecursionDetectionDecorator.kt | 2 +- .../rustsdk/RetryClassifierDecorator.kt | 40 +-- .../RetryInformationHeaderDecorator.kt | 4 +- .../smithy/rustsdk/UserAgentDecorator.kt | 2 +- .../apigateway/ApiGatewayDecorator.kt | 2 +- .../customize/glacier/GlacierDecorator.kt | 2 +- .../kms/tests/retryable_errors.rs | 19 +- aws/sdk/integration-tests/s3/Cargo.toml | 2 +- .../three-retries_and-then-success.json | 4 +- .../tests/retry-classifier-customization.rs | 135 ++++++++ .../ConnectionPoisoningConfigCustomization.kt | 2 +- .../RetryClassifierConfigCustomization.kt | 297 ++++++++++++++++++ .../customize/RequiredCustomizations.kt | 12 +- .../ClientRuntimeTypesReExportGenerator.kt | 6 +- .../generators/OperationCustomization.kt | 24 +- .../OperationRuntimePluginGenerator.kt | 19 +- .../ServiceRuntimePluginGenerator.kt | 31 +- .../client/FluentClientGenerator.kt | 12 +- ...onfigOverrideRuntimePluginGeneratorTest.kt | 4 +- .../config/ServiceConfigGeneratorTest.kt | 7 +- .../protocol/ProtocolTestGeneratorTest.kt | 2 +- .../src/client/retries.rs | 106 ++----- .../src/client/retries/classifiers.rs | 267 ++++++++++++++++ .../src/client/runtime_components.rs | 58 +++- .../src/client/runtime_plugin.rs | 4 +- .../src/client/http/connection_poisoning.rs | 16 +- .../src/client/orchestrator/operation.rs | 17 +- .../aws-smithy-runtime/src/client/retries.rs | 2 +- .../retries/{classifier.rs => classifiers.rs} | 172 ++++++---- .../src/client/retries/strategy.rs | 5 - .../client/retries/strategy/fixed_delay.rs | 105 ------- .../src/client/retries/strategy/standard.rs | 101 +++--- .../tests/reconnect_on_transient_error.rs | 45 +-- rust-runtime/aws-smithy-types/src/retry.rs | 11 + 43 files changed, 1190 insertions(+), 615 deletions(-) rename aws/rust-runtime/aws-runtime/src/retries/{classifier.rs => classifiers.rs} (62%) create mode 100644 aws/sdk/integration-tests/s3/tests/retry-classifier-customization.rs create mode 100644 codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/RetryClassifierConfigCustomization.kt create mode 100644 rust-runtime/aws-smithy-runtime-api/src/client/retries/classifiers.rs rename rust-runtime/aws-smithy-runtime/src/client/retries/{classifier.rs => classifiers.rs} (56%) delete mode 100644 rust-runtime/aws-smithy-runtime/src/client/retries/strategy/fixed_delay.rs diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 84bd2a26d5..8eb793b806 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -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" diff --git a/aws/rust-runtime/aws-config/src/http_credential_provider.rs b/aws/rust-runtime/aws-config/src/http_credential_provider.rs index 0a39adf7b9..c3f69208f2 100644 --- a/aws/rust-runtime/aws-config/src/http_credential_provider.rs +++ b/aws/rust-runtime/aws-config/src/http_credential_provider.rs @@ -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::::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::::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 { - 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")) diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index be93a1fe43..67054a67f4 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -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 { - 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 { + 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 diff --git a/aws/rust-runtime/aws-config/test-data/profile-provider/invalid_config/test-case.json b/aws/rust-runtime/aws-config/test-data/profile-provider/invalid_config/test-case.json index 7e15f5fdd4..21bce59c31 100644 --- a/aws/rust-runtime/aws-config/test-data/profile-provider/invalid_config/test-case.json +++ b/aws/rust-runtime/aws-config/test-data/profile-provider/invalid_config/test-case.json @@ -1,5 +1,5 @@ { - "name": "empty-config", + "name": "invalid-config", "docs": "config was invalid", "result": { "ErrorContains": "could not parse profile file" diff --git a/aws/rust-runtime/aws-config/test-data/profile-provider/retry_on_error/test-case.json b/aws/rust-runtime/aws-config/test-data/profile-provider/retry_on_error/test-case.json index 4119ccfb5e..c92373f783 100644 --- a/aws/rust-runtime/aws-config/test-data/profile-provider/retry_on_error/test-case.json +++ b/aws/rust-runtime/aws-config/test-data/profile-provider/retry_on_error/test-case.json @@ -1,5 +1,5 @@ { - "name": "e2e-assume-role", + "name": "retry-on-error", "docs": "end to end successful role assumption", "result": { "Ok": { diff --git a/aws/rust-runtime/aws-runtime/src/retries.rs b/aws/rust-runtime/aws-runtime/src/retries.rs index ed12aa9cde..acefc98028 100644 --- a/aws/rust-runtime/aws-runtime/src/retries.rs +++ b/aws/rust-runtime/aws-runtime/src/retries.rs @@ -4,4 +4,4 @@ */ /// Classifiers that can inspect a response and determine if it should be retried. -pub mod classifier; +pub mod classifiers; diff --git a/aws/rust-runtime/aws-runtime/src/retries/classifier.rs b/aws/rust-runtime/aws-runtime/src/retries/classifiers.rs similarity index 62% rename from aws/rust-runtime/aws-runtime/src/retries/classifier.rs rename to aws/rust-runtime/aws-runtime/src/retries/classifiers.rs index d73cdf4212..7dcb4f9a86 100644 --- a/aws/rust-runtime/aws-runtime/src/retries/classifier.rs +++ b/aws/rust-runtime/aws-runtime/src/retries/classifiers.rs @@ -6,7 +6,9 @@ use aws_smithy_http::http::HttpHeaders; use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; use aws_smithy_runtime_api::client::orchestrator::OrchestratorError; -use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryReason}; +use aws_smithy_runtime_api::client::retries::classifiers::{ + ClassifyRetry, RetryAction, RetryClassifierPriority, RetryReason, +}; use aws_smithy_types::error::metadata::ProvideErrorMetadata; use aws_smithy_types::retry::ErrorKind; use std::error::Error as StdError; @@ -52,88 +54,74 @@ impl ClassifyRetry for AwsErrorCodeClassifier where E: StdError + ProvideErrorMetadata + Send + Sync + 'static, { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { - let error = ctx - .output_or_error()? - .err() - .and_then(OrchestratorError::as_operation_error)? - .downcast_ref::()?; - - if let Some(error_code) = error.code() { + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + // Check for a result + let output_or_error = ctx.output_or_error(); + // Check for an error + let error = match output_or_error { + Some(Ok(_)) | None => return RetryAction::NoActionIndicated, + Some(Err(err)) => err, + }; + + let retry_after = ctx + .response() + .and_then(|res| res.http_headers().get("x-amz-retry-after")) + .and_then(|header| header.to_str().ok()) + .and_then(|header| header.parse::().ok()) + .map(std::time::Duration::from_millis); + + let error_code = OrchestratorError::as_operation_error(error) + .and_then(|err| err.downcast_ref::()) + .and_then(|err| err.code()); + + if let Some(error_code) = error_code { if THROTTLING_ERRORS.contains(&error_code) { - return Some(RetryReason::Error(ErrorKind::ThrottlingError)); - } else if TRANSIENT_ERRORS.contains(&error_code) { - return Some(RetryReason::Error(ErrorKind::TransientError)); + return RetryAction::RetryIndicated(RetryReason::RetryableError { + kind: ErrorKind::ThrottlingError, + retry_after, + }); + } + if TRANSIENT_ERRORS.contains(&error_code) { + return RetryAction::RetryIndicated(RetryReason::RetryableError { + kind: ErrorKind::TransientError, + retry_after, + }); } }; - None + debug_assert!( + retry_after.is_none(), + "retry_after should be None if the error wasn't an identifiable AWS error" + ); + + RetryAction::NoActionIndicated } fn name(&self) -> &'static str { "AWS Error Code" } -} - -/// A retry classifier that checks for `x-amz-retry-after` headers. If one is found, a -/// [`RetryReason::Explicit`] is returned containing the duration to wait before retrying. -#[derive(Debug, Default)] -pub struct AmzRetryAfterHeaderClassifier; -impl AmzRetryAfterHeaderClassifier { - /// Create a new `AmzRetryAfterHeaderClassifier`. - pub fn new() -> Self { - Self - } -} - -impl ClassifyRetry for AmzRetryAfterHeaderClassifier { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { - ctx.response() - .and_then(|res| res.http_headers().get("x-amz-retry-after")) - .and_then(|header| header.to_str().ok()) - .and_then(|header| header.parse::().ok()) - .map(|retry_after_delay| { - RetryReason::Explicit(std::time::Duration::from_millis(retry_after_delay)) - }) - } - - fn name(&self) -> &'static str { - "'Retry After' Header" + fn priority(&self) -> RetryClassifierPriority { + RetryClassifierPriority::with_lower_priority_than( + RetryClassifierPriority::modeled_as_retryable_classifier(), + ) } } #[cfg(test)] mod test { - use super::*; + use crate::retries::classifiers::AwsErrorCodeClassifier; use aws_smithy_http::body::SdkBody; + use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; use aws_smithy_runtime_api::client::interceptors::context::{Error, Input}; + use aws_smithy_runtime_api::client::orchestrator::OrchestratorError; + use aws_smithy_runtime_api::client::retries::classifiers::{ClassifyRetry, RetryAction}; + use aws_smithy_types::error::metadata::ProvideErrorMetadata; use aws_smithy_types::error::ErrorMetadata; - use aws_smithy_types::retry::{ErrorKind, ProvideErrorKind}; + use aws_smithy_types::retry::ErrorKind; use std::fmt; use std::time::Duration; - #[derive(Debug)] - struct UnmodeledError; - - impl fmt::Display for UnmodeledError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "UnmodeledError") - } - } - - impl std::error::Error for UnmodeledError {} - - impl ProvideErrorKind for UnmodeledError { - fn retryable_error_kind(&self) -> Option { - None - } - - fn code(&self) -> Option<&str> { - None - } - } - #[derive(Debug)] struct CodedError { metadata: ErrorMetadata, @@ -169,19 +157,13 @@ mod test { CodedError::new("Throttling"), )))); - assert_eq!( - policy.classify_retry(&ctx), - Some(RetryReason::Error(ErrorKind::ThrottlingError)) - ); + assert_eq!(policy.classify_retry(&ctx), RetryAction::throttling_error()); let mut ctx = InterceptorContext::new(Input::doesnt_matter()); ctx.set_output_or_error(Err(OrchestratorError::operation(Error::erase( CodedError::new("RequestTimeout"), )))); - assert_eq!( - policy.classify_retry(&ctx), - Some(RetryReason::Error(ErrorKind::TransientError)) - ) + assert_eq!(policy.classify_retry(&ctx), RetryAction::transient_error()) } #[test] @@ -194,15 +176,13 @@ mod test { ctx.set_response(test_response); ctx.set_output_or_error(Err(OrchestratorError::operation(Error::erase(err)))); - assert_eq!( - policy.classify_retry(&ctx), - Some(RetryReason::Error(ErrorKind::ThrottlingError)) - ); + assert_eq!(policy.classify_retry(&ctx), RetryAction::throttling_error()); } #[test] fn test_retry_after_header() { - let policy = AmzRetryAfterHeaderClassifier; + let policy = AwsErrorCodeClassifier::::new(); + let err = aws_smithy_types::Error::builder().code("SlowDown").build(); let res = http::Response::builder() .header("x-amz-retry-after", "5000") .body("retry later") @@ -210,13 +190,14 @@ mod test { .map(SdkBody::from); let mut ctx = InterceptorContext::new(Input::doesnt_matter()); ctx.set_response(res); - ctx.set_output_or_error(Err(OrchestratorError::operation(Error::erase( - UnmodeledError, - )))); + ctx.set_output_or_error(Err(OrchestratorError::operation(Error::erase(err)))); assert_eq!( policy.classify_retry(&ctx), - Some(RetryReason::Explicit(Duration::from_millis(5000))), + RetryAction::retryable_error_with_explicit_delay( + ErrorKind::ThrottlingError, + Duration::from_secs(5) + ) ); } } diff --git a/aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs b/aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs index eb9724666c..10d23338f8 100644 --- a/aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs +++ b/aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs @@ -926,6 +926,7 @@ mod tests { ); } + #[allow(clippy::ptr_arg)] // The proptest macro requires this arg to be a Vec instead of a slice. fn valid_input(input: &Vec) -> bool { [ "content-length".to_owned(), @@ -933,7 +934,7 @@ mod tests { "host".to_owned(), ] .iter() - .all(|element| !input.contains(&element)) + .all(|element| !input.contains(element)) } proptest! { diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt index 797203039f..def448bd55 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt @@ -45,7 +45,7 @@ private class InvocationIdRuntimePluginCustomization( override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.RegisterRuntimeComponents) { - section.registerInterceptor(codegenContext.runtimeConfig, this) { + section.registerInterceptor(this) { rustTemplate("#{InvocationIdInterceptor}::new()", *codegenScope) } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt index bb7d13625c..5809d8b4b3 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt @@ -29,7 +29,7 @@ private class RecursionDetectionRuntimePluginCustomization( ) : ServiceRuntimePluginCustomization() { override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.RegisterRuntimeComponents) { - section.registerInterceptor(codegenContext.runtimeConfig, this) { + section.registerInterceptor(this) { rust( "#T::new()", AwsRuntimeType.awsRuntime(codegenContext.runtimeConfig) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt index 12fec39968..6b3c0185e3 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt @@ -12,7 +12,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationCus import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationSection import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable -import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType class RetryClassifierDecorator : ClientCodegenDecorator { override val name: String = "RetryPolicy" @@ -28,39 +27,20 @@ class RetryClassifierDecorator : ClientCodegenDecorator { class OperationRetryClassifiersFeature( codegenContext: ClientCodegenContext, - operation: OperationShape, + val operation: OperationShape, ) : OperationCustomization() { private val runtimeConfig = codegenContext.runtimeConfig - private val awsRuntime = AwsRuntimeType.awsRuntime(runtimeConfig) - private val smithyRuntime = RuntimeType.smithyRuntime(runtimeConfig) - private val smithyRuntimeApi = RuntimeType.smithyRuntimeApi(runtimeConfig) - private val codegenScope = arrayOf( - // Classifiers - "SmithyErrorClassifier" to smithyRuntime.resolve("client::retries::classifier::SmithyErrorClassifier"), - "AmzRetryAfterHeaderClassifier" to awsRuntime.resolve("retries::classifier::AmzRetryAfterHeaderClassifier"), - "ModeledAsRetryableClassifier" to smithyRuntime.resolve("client::retries::classifier::ModeledAsRetryableClassifier"), - "AwsErrorCodeClassifier" to awsRuntime.resolve("retries::classifier::AwsErrorCodeClassifier"), - "HttpStatusCodeClassifier" to smithyRuntime.resolve("client::retries::classifier::HttpStatusCodeClassifier"), - // Other Types - "ClassifyRetry" to smithyRuntimeApi.resolve("client::retries::ClassifyRetry"), - "InterceptorContext" to RuntimeType.interceptorContext(runtimeConfig), - "OperationError" to codegenContext.symbolProvider.symbolForOperationError(operation), - "OrchestratorError" to smithyRuntimeApi.resolve("client::orchestrator::OrchestratorError"), - "RetryReason" to smithyRuntimeApi.resolve("client::retries::RetryReason"), - ) + private val symbolProvider = codegenContext.symbolProvider override fun section(section: OperationSection) = when (section) { - is OperationSection.RetryClassifier -> writable { - rustTemplate( - """ - .with_classifier(#{SmithyErrorClassifier}::<#{OperationError}>::new()) - .with_classifier(#{AmzRetryAfterHeaderClassifier}) - .with_classifier(#{ModeledAsRetryableClassifier}::<#{OperationError}>::new()) - .with_classifier(#{AwsErrorCodeClassifier}::<#{OperationError}>::new()) - .with_classifier(#{HttpStatusCodeClassifier}::default()) - """, - *codegenScope, - ) + is OperationSection.RetryClassifiers -> writable { + section.registerRetryClassifier(this) { + rustTemplate( + "#{AwsErrorCodeClassifier}::<#{OperationError}>::new()", + "AwsErrorCodeClassifier" to AwsRuntimeType.awsRuntime(runtimeConfig).resolve("retries::classifiers::AwsErrorCodeClassifier"), + "OperationError" to symbolProvider.symbolForOperationError(operation), + ) + } } else -> emptySection diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt index 1d9f949e45..2a354327f6 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt @@ -32,7 +32,7 @@ private class AddRetryInformationHeaderInterceptors(codegenContext: ClientCodege override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.RegisterRuntimeComponents) { // Track the latency between client and server. - section.registerInterceptor(runtimeConfig, this) { + section.registerInterceptor(this) { rust( "#T::new()", awsRuntime.resolve("service_clock_skew::ServiceClockSkewInterceptor"), @@ -40,7 +40,7 @@ private class AddRetryInformationHeaderInterceptors(codegenContext: ClientCodege } // Add request metadata to outgoing requests. Sets a header. - section.registerInterceptor(runtimeConfig, this) { + section.registerInterceptor(this) { rust("#T::new()", awsRuntime.resolve("request_info::RequestInfoInterceptor")) } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt index 2619b224fc..058acab9cc 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt @@ -91,7 +91,7 @@ class UserAgentDecorator : ClientCodegenDecorator { override fun section(section: ServiceRuntimePluginSection): Writable = writable { when (section) { is ServiceRuntimePluginSection.RegisterRuntimeComponents -> { - section.registerInterceptor(runtimeConfig, this) { + section.registerInterceptor(this) { rust("#T::new()", awsRuntime.resolve("user_agent::UserAgentInterceptor")) } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt index c58fb0c20a..9c64a26d8a 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt @@ -31,7 +31,7 @@ private class ApiGatewayAcceptHeaderInterceptorCustomization(private val codegen ServiceRuntimePluginCustomization() { override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.RegisterRuntimeComponents) { - section.registerInterceptor(codegenContext.runtimeConfig, this) { + section.registerInterceptor(this) { rustTemplate( "#{Interceptor}::default()", "Interceptor" to RuntimeType.forInlineDependency( diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/glacier/GlacierDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/glacier/GlacierDecorator.kt index 847eef9eac..713280ceaa 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/glacier/GlacierDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/glacier/GlacierDecorator.kt @@ -87,7 +87,7 @@ private class GlacierApiVersionCustomization(private val codegenContext: ClientC override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.RegisterRuntimeComponents) { val apiVersion = codegenContext.serviceShape.version - section.registerInterceptor(codegenContext.runtimeConfig, this) { + section.registerInterceptor(this) { rustTemplate( "#{Interceptor}::new(${apiVersion.dq()})", "Interceptor" to inlineModule(codegenContext.runtimeConfig).resolve("GlacierApiVersionInterceptor"), diff --git a/aws/sdk/integration-tests/kms/tests/retryable_errors.rs b/aws/sdk/integration-tests/kms/tests/retryable_errors.rs index 1b932e4be4..5e603d2827 100644 --- a/aws/sdk/integration-tests/kms/tests/retryable_errors.rs +++ b/aws/sdk/integration-tests/kms/tests/retryable_errors.rs @@ -4,14 +4,13 @@ */ use aws_credential_types::Credentials; -use aws_runtime::retries::classifier::AwsErrorCodeClassifier; +use aws_runtime::retries::classifiers::AwsErrorCodeClassifier; use aws_sdk_kms as kms; use aws_smithy_http::result::SdkError; use aws_smithy_runtime::client::http::test_util::infallible_client_fn; use aws_smithy_runtime_api::client::interceptors::context::{Error, Input, InterceptorContext}; use aws_smithy_runtime_api::client::orchestrator::{HttpResponse, OrchestratorError}; -use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryReason}; -use aws_smithy_types::retry::ErrorKind; +use aws_smithy_runtime_api::client::retries::classifiers::{ClassifyRetry, RetryAction}; use bytes::Bytes; use kms::operation::create_alias::CreateAliasError; @@ -50,11 +49,8 @@ async fn errors_are_retryable() { let mut ctx = InterceptorContext::new(Input::doesnt_matter()); let err = err.into_service_error(); ctx.set_output_or_error(Err(OrchestratorError::operation(Error::erase(err)))); - let retry_kind = classifier.classify_retry(&ctx); - assert_eq!( - Some(RetryReason::Error(ErrorKind::ThrottlingError)), - retry_kind - ); + let retry_action = classifier.classify_retry(&ctx); + assert_eq!(RetryAction::throttling_error(), retry_action); } #[tokio::test] @@ -72,9 +68,6 @@ async fn unmodeled_errors_are_retryable() { let mut ctx = InterceptorContext::new(Input::doesnt_matter()); let err = err.into_service_error(); ctx.set_output_or_error(Err(OrchestratorError::operation(Error::erase(err)))); - let retry_kind = classifier.classify_retry(&ctx); - assert_eq!( - Some(RetryReason::Error(ErrorKind::ThrottlingError)), - retry_kind - ); + let retry_action = classifier.classify_retry(&ctx); + assert_eq!(RetryAction::throttling_error(), retry_action); } diff --git a/aws/sdk/integration-tests/s3/Cargo.toml b/aws/sdk/integration-tests/s3/Cargo.toml index 1cf92d5d21..97c70e29de 100644 --- a/aws/sdk/integration-tests/s3/Cargo.toml +++ b/aws/sdk/integration-tests/s3/Cargo.toml @@ -31,7 +31,7 @@ aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types" } aws-types = { path = "../../build/aws-sdk/sdk/aws-types" } bytes = "1" bytes-utils = "0.1.2" -fastrand = "2.0.0" +fastrand = "2.0.1" futures-util = { version = "0.3.16", default-features = false } hdrhistogram = "7.5.2" http = "0.2.3" diff --git a/aws/sdk/integration-tests/s3/tests/data/request-information-headers/three-retries_and-then-success.json b/aws/sdk/integration-tests/s3/tests/data/request-information-headers/three-retries_and-then-success.json index 2903739925..141ded94ec 100644 --- a/aws/sdk/integration-tests/s3/tests/data/request-information-headers/three-retries_and-then-success.json +++ b/aws/sdk/integration-tests/s3/tests/data/request-information-headers/three-retries_and-then-success.json @@ -84,7 +84,7 @@ "action": { "Data": { "data": { - "Utf8": "\n \"\n Server\n InternalError\n We encountered an internal error. Please try again.\n foo-id\n\"" + "Utf8": "\n \n Server\n InternalError\n We encountered an internal error. Please try again.\n foo-id\n" }, "direction": "Response" } @@ -183,7 +183,7 @@ "action": { "Data": { "data": { - "Utf8": "\n \"\n Server\n InternalError\n We encountered an internal error. Please try again.\n foo-id\n\"" + "Utf8": "\n \n Server\n InternalError\n We encountered an internal error. Please try again.\n foo-id\n" }, "direction": "Response" } diff --git a/aws/sdk/integration-tests/s3/tests/retry-classifier-customization.rs b/aws/sdk/integration-tests/s3/tests/retry-classifier-customization.rs new file mode 100644 index 0000000000..15af110369 --- /dev/null +++ b/aws/sdk/integration-tests/s3/tests/retry-classifier-customization.rs @@ -0,0 +1,135 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use aws_sdk_s3::config::interceptors::InterceptorContext; +use aws_sdk_s3::config::retry::{ClassifyRetry, RetryAction, RetryConfig}; +use aws_sdk_s3::config::SharedAsyncSleep; +use aws_smithy_async::rt::sleep::TokioSleep; +use aws_smithy_http::body::SdkBody; +use aws_smithy_runtime::client::http::test_util::{ReplayEvent, StaticReplayClient}; +use std::sync::{Arc, Mutex}; + +#[derive(Debug, Clone)] +struct CustomizationTestClassifier { + counter: Arc>, +} + +impl CustomizationTestClassifier { + pub fn new() -> Self { + Self { + counter: Arc::new(Mutex::new(0u8)), + } + } + + pub fn counter(&self) -> u8 { + *self.counter.lock().unwrap() + } +} + +impl ClassifyRetry for CustomizationTestClassifier { + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + *self.counter.lock().unwrap() += 1; + + // Interceptors may call this classifier before a response is received. If a response was received, + // ensure that it has the expected status code. + if let Some(res) = ctx.response() { + assert_eq!( + res.status(), + 500, + "expected a 500 response from test connection" + ); + } + + RetryAction::RetryForbidden + } + + fn name(&self) -> &'static str { + "Custom Retry Classifier" + } +} + +fn req() -> http::Request { + http::Request::builder() + .body(SdkBody::from("request body")) + .unwrap() +} + +fn ok() -> http::Response { + http::Response::builder() + .status(200) + .body(SdkBody::from("Hello!")) + .unwrap() +} + +fn err() -> http::Response { + http::Response::builder() + .status(500) + .body(SdkBody::from("This was an error")) + .unwrap() +} + +#[tokio::test] +async fn test_retry_classifier_customization_for_service() { + let http_client = StaticReplayClient::new(vec![ + ReplayEvent::new(req(), err()), + ReplayEvent::new(req(), ok()), + ]); + + let customization_test_classifier = CustomizationTestClassifier::new(); + + let config = aws_sdk_s3::Config::builder() + .with_test_defaults() + .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) + .http_client(http_client) + .retry_config(RetryConfig::standard()) + .retry_classifier(customization_test_classifier.clone()) + .build(); + + let client = aws_sdk_s3::Client::from_conf(config); + let _ = client + .get_object() + .bucket("bucket") + .key("key") + .send() + .await + .expect_err("fails without attempting a retry"); + + // ensure our custom retry classifier was called at least once. + assert_ne!(customization_test_classifier.counter(), 0); +} + +#[tokio::test] +async fn test_retry_classifier_customization_for_operation() { + let http_client = StaticReplayClient::new(vec![ + ReplayEvent::new(req(), err()), + ReplayEvent::new(req(), ok()), + ]); + + let customization_test_classifier = CustomizationTestClassifier::new(); + + let config = aws_sdk_s3::Config::builder() + .with_test_defaults() + .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) + .http_client(http_client) + .retry_config(RetryConfig::standard()) + .build(); + + let client = aws_sdk_s3::Client::from_conf(config); + let _ = client + .get_object() + .bucket("bucket") + .key("key") + .customize() + .config_override( + aws_sdk_s3::config::Config::builder() + .retry_classifier(customization_test_classifier.clone()), + ) + .send() + .await + .expect_err("fails without attempting a retry"); + + // ensure our custom retry classifier was called at least once. + assert_ne!(customization_test_classifier.counter(), 0); +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt index d8a9b6818b..5f1688037e 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt @@ -23,7 +23,7 @@ class ConnectionPoisoningRuntimePluginCustomization( is ServiceRuntimePluginSection.RegisterRuntimeComponents -> { // This interceptor assumes that a compatible Connector is set. Otherwise, connection poisoning // won't work and an error message will be logged. - section.registerInterceptor(runtimeConfig, this) { + section.registerInterceptor(this) { rust( "#T::new()", smithyRuntime(runtimeConfig).resolve("client::http::connection_poisoning::ConnectionPoisoningInterceptor"), diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/RetryClassifierConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/RetryClassifierConfigCustomization.kt new file mode 100644 index 0000000000..8b23e4efba --- /dev/null +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/RetryClassifierConfigCustomization.kt @@ -0,0 +1,297 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.client.smithy.customizations + +import software.amazon.smithy.model.shapes.OperationShape +import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext +import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationCustomization +import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationSection +import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginCustomization +import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginSection +import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization +import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ServiceConfig +import software.amazon.smithy.rust.codegen.core.rustlang.Writable +import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.core.rustlang.writable +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType + +class RetryClassifierConfigCustomization(codegenContext: ClientCodegenContext) : ConfigCustomization() { + private val runtimeConfig = codegenContext.runtimeConfig + private val moduleUseName = codegenContext.moduleUseName() + + private val retries = RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::retries") + private val classifiers = retries.resolve("classifiers") + private val codegenScope = arrayOf( + "ClassifyRetry" to classifiers.resolve("ClassifyRetry"), + "RetryStrategy" to retries.resolve("RetryStrategy"), + "SharedRetryClassifier" to classifiers.resolve("SharedRetryClassifier"), + "RetryClassifierPriority" to classifiers.resolve("RetryClassifierPriority"), + ) + + override fun section(section: ServiceConfig) = + writable { + when (section) { + ServiceConfig.ConfigImpl -> rustTemplate( + """ + /// Returns retry classifiers currently registered by the user. + pub fn retry_classifiers(&self) -> impl Iterator + '_ { + self.runtime_components.retry_classifiers() + } + """, + *codegenScope, + ) + + ServiceConfig.BuilderImpl -> + rustTemplate( + """ + /// Add type implementing [`ClassifyRetry`](#{ClassifyRetry}) that will be used by the + /// [`RetryStrategy`](#{RetryStrategy}) to determine what responses should be retried. + /// + /// A retry classifier configured by this method will run according to its [priority](#{RetryClassifierPriority}). + /// + /// ## Examples + /// ```no_run + /// ## ##[cfg(test)] + /// ## mod tests { + /// ## ##[test] + /// ## fn example() { + /// use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; + /// use aws_smithy_runtime_api::client::orchestrator::OrchestratorError; + /// use aws_smithy_runtime_api::client::retries::classifiers::{ + /// ClassifyRetry, RetryAction, RetryClassifierPriority, + /// }; + /// use aws_smithy_types::error::metadata::ProvideErrorMetadata; + /// use aws_smithy_types::retry::ErrorKind; + /// use std::error::Error as StdError; + /// use std::marker::PhantomData; + /// use $moduleUseName::config::Config; + /// ## struct SomeOperationError {} + /// + /// const RETRYABLE_ERROR_CODES: &[&str] = [ + /// // List error codes to be retried here... + /// ]; + /// + /// // When classifying at an operation's error type, classifiers require a generic parameter. + /// // When classifying the HTTP response alone, no generic is needed. + /// ##[derive(Debug, Default)] + /// pub struct ErrorCodeClassifier { + /// _inner: PhantomData, + /// } + /// + /// impl ExampleErrorCodeClassifier { + /// pub fn new() -> Self { + /// Self { + /// _inner: PhantomData, + /// } + /// } + /// } + /// + /// impl ClassifyRetry for ExampleErrorCodeClassifier + /// where + /// // Adding a trait bound for ProvideErrorMetadata allows us to inspect the error code. + /// E: StdError + ProvideErrorMetadata + Send + Sync + 'static, + /// { + /// fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + /// // Check for a result + /// let output_or_error = ctx.output_or_error(); + /// // Check for an error + /// let error = match output_or_error { + /// Some(Ok(_)) | None => return RetryAction::NoActionIndicated, + /// Some(Err(err)) => err, + /// }; + /// + /// // Downcast the generic error and extract the code + /// let error_code = OrchestratorError::as_operation_error(error) + /// .and_then(|err| err.downcast_ref::()) + /// .and_then(|err| err.code()); + /// + /// // If this error's code is in our list, return an action that tells the RetryStrategy to retry this request. + /// if let Some(error_code) = error_code { + /// if RETRYABLE_ERROR_CODES.contains(&error_code) { + /// return RetryAction::transient_error(); + /// } + /// } + /// + /// // Otherwise, return that no action is indicated i.e. that this classifier doesn't require a retry. + /// // Another classifier may still classify this response as retryable. + /// RetryAction::NoActionIndicated + /// } + /// + /// fn name(&self) -> &'static str { "Example Error Code Classifier" } + /// } + /// + /// let config = Config::builder() + /// .retry_classifier(ExampleErrorCodeClassifier::::new()) + /// .build(); + /// ## } + /// ## } + /// ``` + pub fn retry_classifier(mut self, retry_classifier: impl #{ClassifyRetry} + 'static) -> Self { + self.push_retry_classifier(#{SharedRetryClassifier}::new(retry_classifier)); + self + } + + /// Add a [`SharedRetryClassifier`](#{SharedRetryClassifier}) that will be used by the + /// [`RetryStrategy`](#{RetryStrategy}) to determine what responses should be retried. + /// + /// A retry classifier configured by this method will run according to its priority. + /// + /// ## Examples + /// ```no_run + /// ## ##[cfg(test)] + /// ## mod tests { + /// ## ##[test] + /// ## fn example() { + /// use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; + /// use aws_smithy_runtime_api::client::orchestrator::OrchestratorError; + /// use aws_smithy_runtime_api::client::retries::classifiers::{ + /// ClassifyRetry, RetryAction, RetryClassifierPriority, + /// }; + /// use aws_smithy_types::error::metadata::ProvideErrorMetadata; + /// use aws_smithy_types::retry::ErrorKind; + /// use std::error::Error as StdError; + /// use std::marker::PhantomData; + /// use $moduleUseName::config::{Builder, Config}; + /// ## struct SomeOperationError {} + /// + /// const RETRYABLE_ERROR_CODES: &[&str] = [ + /// // List error codes to be retried here... + /// ]; + /// fn set_example_error_code_classifier(builder: &mut Builder) { + /// // When classifying at an operation's error type, classifiers require a generic parameter. + /// // When classifying the HTTP response alone, no generic is needed. + /// ##[derive(Debug, Default)] + /// pub struct ExampleErrorCodeClassifier { + /// _inner: PhantomData, + /// } + /// + /// impl ExampleErrorCodeClassifier { + /// pub fn new() -> Self { + /// Self { + /// _inner: PhantomData, + /// } + /// } + /// } + /// + /// impl ClassifyRetry for ExampleErrorCodeClassifier + /// where + /// // Adding a trait bound for ProvideErrorMetadata allows us to inspect the error code. + /// E: StdError + ProvideErrorMetadata + Send + Sync + 'static, + /// { + /// fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + /// // Check for a result + /// let output_or_error = ctx.output_or_error(); + /// // Check for an error + /// let error = match output_or_error { + /// Some(Ok(_)) | None => return RetryAction::NoActionIndicated, + /// Some(Err(err)) => err, + /// }; + /// + /// // Downcast the generic error and extract the code + /// let error_code = OrchestratorError::as_operation_error(error) + /// .and_then(|err| err.downcast_ref::()) + /// .and_then(|err| err.code()); + /// + /// // If this error's code is in our list, return an action that tells the RetryStrategy to retry this request. + /// if let Some(error_code) = error_code { + /// if RETRYABLE_ERROR_CODES.contains(&error_code) { + /// return RetryAction::transient_error(); + /// } + /// } + /// + /// // Otherwise, return that no action is indicated i.e. that this classifier doesn't require a retry. + /// // Another classifier may still classify this response as retryable. + /// RetryAction::NoActionIndicated + /// } + /// + /// fn name(&self) -> &'static str { "Example Error Code Classifier" } + /// } + /// + /// builder.push_retry_classifier(ExampleErrorCodeClassifier::::new()) + /// } + /// + /// let mut builder = Config::builder(); + /// set_example_error_code_classifier(&mut builder); + /// let config = builder.build(); + /// ## } + /// ## } + /// ``` + pub fn push_retry_classifier(&mut self, retry_classifier: #{SharedRetryClassifier}) -> &mut Self { + self.runtime_components.push_retry_classifier(retry_classifier); + self + } + + /// Set [`SharedRetryClassifier`](#{SharedRetryClassifier})s for the builder, replacing any that + /// were previously set. + pub fn set_retry_classifiers(&mut self, retry_classifiers: impl IntoIterator) -> &mut Self { + self.runtime_components.set_retry_classifiers(retry_classifiers.into_iter()); + self + } + """, + *codegenScope, + ) + + else -> emptySection + } + } +} + +class RetryClassifierServiceRuntimePluginCustomization(codegenContext: ClientCodegenContext) : ServiceRuntimePluginCustomization() { + private val runtimeConfig = codegenContext.runtimeConfig + private val retries = RuntimeType.smithyRuntime(runtimeConfig).resolve("client::retries") + + override fun section(section: ServiceRuntimePluginSection): Writable = writable { + when (section) { + is ServiceRuntimePluginSection.RegisterRuntimeComponents -> { + section.registerRetryClassifier(this) { + rustTemplate( + "#{HttpStatusCodeClassifier}::default()", + "HttpStatusCodeClassifier" to retries.resolve("classifiers::HttpStatusCodeClassifier"), + ) + } + } + + else -> emptySection + } + } +} + +class RetryClassifierOperationCustomization( + codegenContext: ClientCodegenContext, + val operation: OperationShape, +) : OperationCustomization() { + private val runtimeConfig = codegenContext.runtimeConfig + private val symbolProvider = codegenContext.symbolProvider + + override fun section(section: OperationSection): Writable = writable { + val classifiers = RuntimeType.smithyRuntime(runtimeConfig).resolve("client::retries::classifiers") + + val codegenScope = arrayOf( + *RuntimeType.preludeScope, + "TransientErrorClassifier" to classifiers.resolve("TransientErrorClassifier"), + "ModeledAsRetryableClassifier" to classifiers.resolve("ModeledAsRetryableClassifier"), + "OperationError" to symbolProvider.symbolForOperationError(operation), + ) + + when (section) { + is OperationSection.RetryClassifiers -> { + section.registerRetryClassifier(this) { + rustTemplate( + "#{TransientErrorClassifier}::<#{OperationError}>::new()", + *codegenScope, + ) + } + section.registerRetryClassifier(this) { + rustTemplate( + "#{ModeledAsRetryableClassifier}::<#{OperationError}>::new()", + *codegenScope, + ) + } + } + else -> emptySection + } + } +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt index e2f780ddd8..a1f11cf4a2 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt @@ -16,6 +16,9 @@ import software.amazon.smithy.rust.codegen.client.smithy.customizations.Metadata import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyConfigCustomization import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyReExportCustomization import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyServiceRuntimePluginCustomization +import software.amazon.smithy.rust.codegen.client.smithy.customizations.RetryClassifierConfigCustomization +import software.amazon.smithy.rust.codegen.client.smithy.customizations.RetryClassifierOperationCustomization +import software.amazon.smithy.rust.codegen.client.smithy.customizations.RetryClassifierServiceRuntimePluginCustomization import software.amazon.smithy.rust.codegen.client.smithy.customizations.TimeSourceCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginCustomization @@ -48,7 +51,8 @@ class RequiredCustomizations : ClientCodegenDecorator { baseCustomizations + MetadataCustomization(codegenContext, operation) + IdempotencyTokenGenerator(codegenContext, operation) + - HttpChecksumRequiredGenerator(codegenContext, operation) + HttpChecksumRequiredGenerator(codegenContext, operation) + + RetryClassifierOperationCustomization(codegenContext, operation) override fun configCustomizations( codegenContext: ClientCodegenContext, @@ -56,7 +60,8 @@ class RequiredCustomizations : ClientCodegenDecorator { ): List = baseCustomizations + ResiliencyConfigCustomization(codegenContext) + InterceptorConfigCustomization(codegenContext) + - TimeSourceCustomization(codegenContext) + TimeSourceCustomization(codegenContext) + + RetryClassifierConfigCustomization(codegenContext) override fun libRsCustomizations( codegenContext: ClientCodegenContext, @@ -111,5 +116,6 @@ class RequiredCustomizations : ClientCodegenDecorator { baseCustomizations: List, ): List = baseCustomizations + ResiliencyServiceRuntimePluginCustomization(codegenContext) + - ConnectionPoisoningRuntimePluginCustomization(codegenContext) + ConnectionPoisoningRuntimePluginCustomization(codegenContext) + + RetryClassifierServiceRuntimePluginCustomization(codegenContext) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt index b7151f6727..8177430bde 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt @@ -60,11 +60,11 @@ class ClientRuntimeTypesReExportGenerator( rustTemplate( """ pub use #{ClassifyRetry}; - pub use #{RetryReason}; + pub use #{RetryAction}; pub use #{ShouldAttempt}; """, - "ClassifyRetry" to smithyRuntimeApi.resolve("client::retries::ClassifyRetry"), - "RetryReason" to smithyRuntimeApi.resolve("client::retries::RetryReason"), + "ClassifyRetry" to smithyRuntimeApi.resolve("client::retries::classifiers::ClassifyRetry"), + "RetryAction" to smithyRuntimeApi.resolve("client::retries::classifiers::RetryAction"), "ShouldAttempt" to smithyRuntimeApi.resolve("client::retries::ShouldAttempt"), ) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt index 48bfc75895..db534910ef 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt @@ -78,21 +78,6 @@ sealed class OperationSection(name: String) : Section(name) { } } - /** - * Hook for adding retry classifiers to an operation's `RetryClassifiers` bundle. - * - * Should emit 1+ lines of code that look like the following: - * ```rust - * .with_classifier(AwsErrorCodeClassifier::new()) - * .with_classifier(HttpStatusCodeClassifier::new()) - * ``` - */ - data class RetryClassifier( - override val customizations: List, - val configBagName: String, - val operationShape: OperationShape, - ) : OperationSection("RetryClassifier") - /** * Hook for adding supporting types for operation-specific runtime plugins. * Examples include various operation-specific types (retry classifiers, config bag types, etc.) @@ -118,6 +103,15 @@ sealed class OperationSection(name: String) : Section(name) { writer.rustTemplate(".with_operation_plugin(#{plugin})", "plugin" to plugin) } } + + data class RetryClassifiers( + override val customizations: List, + val operationShape: OperationShape, + ) : OperationSection("RetryClassifiers") { + fun registerRetryClassifier(writer: RustWriter, classifier: Writable) { + writer.rustTemplate(".with_retry_classifier(#{classifier})", "classifier" to classifier) + } + } } abstract class OperationCustomization : NamedCustomization() diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt index fc011307b2..6782d2d8dc 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt @@ -40,6 +40,7 @@ class OperationRuntimePluginGenerator( "ConfigBag" to RuntimeType.configBag(codegenContext.runtimeConfig), "Cow" to RuntimeType.Cow, "FrozenLayer" to smithyTypes.resolve("config_bag::FrozenLayer"), + "IntoShared" to runtimeApi.resolve("shared::IntoShared"), "Layer" to smithyTypes.resolve("config_bag::Layer"), "RetryClassifiers" to runtimeApi.resolve("client::retries::RetryClassifiers"), "RuntimeComponentsBuilder" to RuntimeType.runtimeComponentsBuilder(codegenContext.runtimeConfig), @@ -77,15 +78,11 @@ class OperationRuntimePluginGenerator( } fn runtime_components(&self, _: &#{RuntimeComponentsBuilder}) -> #{Cow}<'_, #{RuntimeComponentsBuilder}> { - // Retry classifiers are operation-specific because they need to downcast operation-specific error types. - let retry_classifiers = #{RetryClassifiers}::new() - #{retry_classifier_customizations}; - #{Cow}::Owned( #{RuntimeComponentsBuilder}::new(${operationShape.id.name.dq()}) - .with_retry_classifiers(#{Some}(retry_classifiers)) #{auth_options} #{interceptors} + #{retry_classifiers} ) } } @@ -105,12 +102,6 @@ class OperationRuntimePluginGenerator( ), ) }, - "retry_classifier_customizations" to writable { - writeCustomizations( - customizations, - OperationSection.RetryClassifier(customizations, "cfg", operationShape), - ) - }, "runtime_plugin_supporting_types" to writable { writeCustomizations( customizations, @@ -123,6 +114,12 @@ class OperationRuntimePluginGenerator( OperationSection.AdditionalInterceptors(customizations, operationShape), ) }, + "retry_classifiers" to writable { + writeCustomizations( + customizations, + OperationSection.RetryClassifiers(customizations, operationShape), + ) + }, ) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt index 921d16a3e5..dccbd924af 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt @@ -12,7 +12,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.isNotEmpty import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable -import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope import software.amazon.smithy.rust.codegen.core.smithy.customize.NamedCustomization @@ -40,31 +39,20 @@ sealed class ServiceRuntimePluginSection(name: String) : Section(name) { data class RegisterRuntimeComponents(val serviceConfigName: String) : ServiceRuntimePluginSection("RegisterRuntimeComponents") { /** Generates the code to register an interceptor */ - fun registerInterceptor(runtimeConfig: RuntimeConfig, writer: RustWriter, interceptor: Writable) { - writer.rustTemplate( - """ - runtime_components.push_interceptor(#{interceptor}); - """, - "interceptor" to interceptor, - ) + fun registerInterceptor(writer: RustWriter, interceptor: Writable) { + writer.rust("runtime_components.push_interceptor(#T);", interceptor) } fun registerAuthScheme(writer: RustWriter, authScheme: Writable) { - writer.rustTemplate( - """ - runtime_components.push_auth_scheme(#{auth_scheme}); - """, - "auth_scheme" to authScheme, - ) + writer.rust("runtime_components.push_auth_scheme(#T);", authScheme) } fun registerIdentityResolver(writer: RustWriter, identityResolver: Writable) { - writer.rustTemplate( - """ - runtime_components.push_identity_resolver(#{identity_resolver}); - """, - "identity_resolver" to identityResolver, - ) + writer.rust("runtime_components.push_identity_resolver(#T);", identityResolver) + } + + fun registerRetryClassifier(writer: RustWriter, classifier: Writable) { + writer.rust("runtime_components.push_retry_classifier(#T);", classifier) } } } @@ -84,8 +72,9 @@ class ServiceRuntimePluginGenerator( "Arc" to RuntimeType.Arc, "BoxError" to RuntimeType.boxError(codegenContext.runtimeConfig), "Cow" to RuntimeType.Cow, - "Layer" to smithyTypes.resolve("config_bag::Layer"), "FrozenLayer" to smithyTypes.resolve("config_bag::FrozenLayer"), + "IntoShared" to runtimeApi.resolve("shared::IntoShared"), + "Layer" to smithyTypes.resolve("config_bag::Layer"), "RuntimeComponentsBuilder" to RuntimeType.runtimeComponentsBuilder(rc), "RuntimePlugin" to RuntimeType.runtimePlugin(rc), ) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index c1d9dbae02..9166ccd598 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt @@ -134,12 +134,14 @@ class FluentClientGenerator( /// If you experience this panic, it can be fixed by setting the `sleep_impl`, or by disabling /// retries and timeouts. pub fn from_conf(conf: crate::Config) -> Self { - let retry_config = conf.retry_config().cloned().unwrap_or_else(#{RetryConfig}::disabled); - let timeout_config = conf.timeout_config().cloned().unwrap_or_else(#{TimeoutConfig}::disabled); + let has_retry_config = conf.retry_config().map(#{RetryConfig}::has_retry).unwrap_or_default(); + let has_timeout_config = conf.timeout_config().map(#{TimeoutConfig}::has_timeouts).unwrap_or_default(); let sleep_impl = conf.sleep_impl(); - if (retry_config.has_retry() || timeout_config.has_timeouts()) && sleep_impl.is_none() { - panic!("An async sleep implementation is required for retries or timeouts to work. \ - Set the `sleep_impl` on the Config passed into this function to fix this panic."); + if (has_retry_config || has_timeout_config) && sleep_impl.is_none() { + panic!( + "An async sleep implementation is required for retries or timeouts to work. \ + Set the `sleep_impl` on the Config passed into this function to fix this panic." + ); } Self { diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt index 6772f463af..f167d35767 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt @@ -201,9 +201,7 @@ internal class ConfigOverrideRuntimePluginGeneratorTest { cfg.push_shared_layer(client_config_layer.clone()); let retry_classifiers_component = #{RuntimeComponentsBuilder}::new("retry_classifier") - .with_retry_classifiers(#{Some}( - #{RetryClassifiers}::new().with_classifier(#{AlwaysRetry}(#{ErrorKind}::TransientError)), - )); + .with_retry_classifier(#{AlwaysRetry}(#{ErrorKind}::TransientError)); // Emulate the merging of runtime components from runtime plugins that the orchestrator does let runtime_components = #{RuntimeComponentsBuilder}::for_tests() diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGeneratorTest.kt index ae8adb702b..c92c44db7c 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGeneratorTest.kt @@ -7,8 +7,6 @@ package software.amazon.smithy.rust.codegen.client.smithy.generators.config import io.kotest.matchers.shouldBe import org.junit.jupiter.api.Test -import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.ValueSource import software.amazon.smithy.model.shapes.ServiceShape import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule @@ -82,9 +80,8 @@ internal class ServiceConfigGeneratorTest { model.lookup("com.example#ResourceService").needsIdempotencyToken(model) shouldBe true } - @ParameterizedTest - @ValueSource(strings = ["middleware", "orchestrator"]) - fun `generate customizations as specified`(smithyRuntimeModeStr: String) { + @Test + fun `generate customizations as specified`() { class ServiceCustomizer(private val codegenContext: ClientCodegenContext) : NamedCustomization() { diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ProtocolTestGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ProtocolTestGeneratorTest.kt index 066c25b8fd..af1dbd11c5 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ProtocolTestGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ProtocolTestGeneratorTest.kt @@ -34,7 +34,7 @@ private class TestServiceRuntimePluginCustomization( override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.RegisterRuntimeComponents) { val rc = context.runtimeConfig - section.registerInterceptor(rc, this) { + section.registerInterceptor(this) { rustTemplate( """ { diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs b/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs index 0fd61b5771..e3616e42b3 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs @@ -8,13 +8,20 @@ //! This code defines when and how failed requests should be retried. It also defines the behavior //! used to limit the rate that requests are sent. +pub mod classifiers; + +use crate::box_error::BoxError; use crate::client::interceptors::context::InterceptorContext; +use crate::client::runtime_components::RuntimeComponents; use aws_smithy_types::config_bag::{ConfigBag, Storable, StoreReplace}; -use std::fmt::Debug; +use std::fmt; +use std::sync::Arc; use std::time::Duration; -use tracing::trace; +use crate::impl_shared_conversions; pub use aws_smithy_types::retry::ErrorKind; +#[cfg(feature = "test-util")] +pub use test_util::AlwaysRetry; #[derive(Debug, Clone, PartialEq, Eq)] /// An answer to the question "should I make a request attempt?" @@ -38,13 +45,15 @@ impl ShouldAttempt { } } +impl_shared_conversions!(convert SharedRetryStrategy from RetryStrategy using SharedRetryStrategy::new); + /// Decider for whether or not to attempt a request, and when. /// /// The orchestrator consults the retry strategy every time before making a request. /// This includes the initial request, and any retry attempts thereafter. The /// orchestrator will retry indefinitely (until success) if the retry strategy /// always returns `ShouldAttempt::Yes` from `should_attempt_retry`. -pub trait RetryStrategy: Send + Sync + Debug { +pub trait RetryStrategy: Send + Sync + fmt::Debug { /// Decides if the initial attempt should be made. fn should_attempt_initial_request( &self, @@ -98,78 +107,6 @@ impl RetryStrategy for SharedRetryStrategy { } } -impl_shared_conversions!(convert SharedRetryStrategy from RetryStrategy using SharedRetryStrategy::new); - -/// Classification result from [`ClassifyRetry`]. -#[non_exhaustive] -#[derive(Clone, Eq, PartialEq, Debug)] -pub enum RetryReason { - /// There was an unexpected error, and this is the kind of error so that it can be properly retried. - Error(ErrorKind), - /// The server explicitly told us to back off by this amount of time. - Explicit(Duration), -} - -/// Classifies what kind of retry is needed for a given an [`InterceptorContext`]. -pub trait ClassifyRetry: Send + Sync + Debug { - /// Run this classifier against an error to determine if it should be retried. Returns - /// `Some(RetryKind)` if the error should be retried; Otherwise returns `None`. - fn classify_retry(&self, ctx: &InterceptorContext) -> Option; - - /// The name that this classifier should report for debugging purposes. - fn name(&self) -> &'static str; -} - -/// Classifies an error into a [`RetryReason`]. -#[derive(Clone, Debug)] -pub struct RetryClassifiers { - inner: Vec>, -} - -impl RetryClassifiers { - /// Creates a new [`RetryClassifiers`]. - pub fn new() -> Self { - Self { - // It's always expected that at least one classifier will be defined, - // so we eagerly allocate for it. - inner: Vec::with_capacity(1), - } - } - - /// Adds a classifier to this collection. - pub fn with_classifier(mut self, retry_classifier: impl ClassifyRetry + 'static) -> Self { - self.inner.push(Arc::new(retry_classifier)); - self - } - - // TODO(https://github.com/awslabs/smithy-rs/issues/2632) make a map function so users can front-run or second-guess the classifier's decision - // pub fn map_classifiers(mut self, fun: Fn() -> RetryClassifiers) -} - -impl ClassifyRetry for RetryClassifiers { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { - // return the first non-None result - self.inner.iter().find_map(|cr| { - let maybe_reason = cr.classify_retry(ctx); - - match maybe_reason.as_ref() { - Some(reason) => trace!( - "\"{}\" classifier classified error as {:?}", - cr.name(), - reason - ), - None => trace!("\"{}\" classifier ignored the error", cr.name()), - }; - - maybe_reason - }) - } - - fn name(&self) -> &'static str { - "Collection of Classifiers" - } -} - /// A type to track the number of requests sent by the orchestrator for a given operation. /// /// `RequestAttempts` is added to the `ConfigBag` by the orchestrator, @@ -209,20 +146,20 @@ impl Storable for RequestAttempts { #[cfg(feature = "test-util")] mod test_util { - use super::{ClassifyRetry, ErrorKind, RetryReason}; + use super::ErrorKind; use crate::client::interceptors::context::InterceptorContext; - use tracing::trace; + use crate::client::retries::classifiers::{ClassifyRetry, RetryAction}; /// A retry classifier for testing purposes. This classifier always returns - /// `Some(RetryReason::Error(ErrorKind))` where `ErrorKind` is the value provided when creating + /// `Some(RetryAction::Error(ErrorKind))` where `ErrorKind` is the value provided when creating /// this classifier. #[derive(Debug)] pub struct AlwaysRetry(pub ErrorKind); impl ClassifyRetry for AlwaysRetry { - fn classify_retry(&self, error: &InterceptorContext) -> Option { - trace!("Retrying error {:?} as an {:?}", error, self.0); - Some(RetryReason::Error(self.0)) + fn classify_retry(&self, error: &InterceptorContext) -> RetryAction { + tracing::debug!("Retrying error {:?} as an {:?}", error, self.0); + RetryAction::retryable_error(self.0) } fn name(&self) -> &'static str { @@ -230,10 +167,3 @@ mod test_util { } } } - -use crate::box_error::BoxError; -use crate::client::runtime_components::RuntimeComponents; -use crate::impl_shared_conversions; -use std::sync::Arc; -#[cfg(feature = "test-util")] -pub use test_util::AlwaysRetry; diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/retries/classifiers.rs b/rust-runtime/aws-smithy-runtime-api/src/client/retries/classifiers.rs new file mode 100644 index 0000000000..549b855956 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/src/client/retries/classifiers.rs @@ -0,0 +1,267 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Classifier for determining if a retry is necessary and related code. + +use crate::client::interceptors::context::InterceptorContext; +use crate::impl_shared_conversions; +use aws_smithy_types::retry::ErrorKind; +use std::fmt; +use std::sync::Arc; +use std::time::Duration; + +/// The result of running a [`ClassifyRetry`] on a [`InterceptorContext`]. +#[non_exhaustive] +#[derive(Clone, Eq, PartialEq, Debug, Default)] +pub enum RetryAction { + /// When a classifier can't run or has no opinion, this action is returned. + /// + /// For example, if a classifier requires a parsed response and response parsing failed, + /// this action is returned. If all classifiers return this action, no retry should be + /// attempted. + #[default] + NoActionIndicated, + /// When a classifier runs and thinks a response should be retried, this action is returned. + RetryIndicated(RetryReason), + /// When a classifier runs and decides a response must not be retried, this action is returned. + /// + /// This action stops retry classification immediately, skipping any following classifiers. + RetryForbidden, +} + +impl fmt::Display for RetryAction { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::NoActionIndicated => write!(f, "no action indicated"), + Self::RetryForbidden => write!(f, "retry forbidden"), + Self::RetryIndicated(reason) => write!(f, "retry {reason}"), + } + } +} + +impl RetryAction { + /// Create a new `RetryAction` indicating that a retry is necessary. + pub fn retryable_error(kind: ErrorKind) -> Self { + Self::RetryIndicated(RetryReason::RetryableError { + kind, + retry_after: None, + }) + } + + /// Create a new `RetryAction` indicating that a retry is necessary after an explicit delay. + pub fn retryable_error_with_explicit_delay(kind: ErrorKind, retry_after: Duration) -> Self { + Self::RetryIndicated(RetryReason::RetryableError { + kind, + retry_after: Some(retry_after), + }) + } + + /// Create a new `RetryAction` indicating that a retry is necessary because of a transient error. + pub fn transient_error() -> Self { + Self::retryable_error(ErrorKind::TransientError) + } + + /// Create a new `RetryAction` indicating that a retry is necessary because of a throttling error. + pub fn throttling_error() -> Self { + Self::retryable_error(ErrorKind::ThrottlingError) + } + + /// Create a new `RetryAction` indicating that a retry is necessary because of a server error. + pub fn server_error() -> Self { + Self::retryable_error(ErrorKind::ServerError) + } + + /// Create a new `RetryAction` indicating that a retry is necessary because of a client error. + pub fn client_error() -> Self { + Self::retryable_error(ErrorKind::ClientError) + } +} + +/// The reason for a retry. +#[non_exhaustive] +#[derive(Clone, Eq, PartialEq, Debug)] +pub enum RetryReason { + /// When an error is received that should be retried, this reason is returned. + RetryableError { + /// The kind of error. + kind: ErrorKind, + /// A server may tells us to retry only after a specific time has elapsed. + retry_after: Option, + }, +} + +impl fmt::Display for RetryReason { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::RetryableError { kind, retry_after } => { + let after = retry_after + .map(|d| format!(" after {d:?}")) + .unwrap_or_default(); + write!(f, "{kind} error{after}") + } + } + } +} + +/// The priority of a retry classifier. Classifiers with a higher priority will run before +/// classifiers with a lower priority. Classifiers with equal priorities make no guarantees +/// about which will run first. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct RetryClassifierPriority { + inner: Inner, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Inner { + // The default priority for the `HttpStatusCodeClassifier`. + HttpStatusCodeClassifier, + // The default priority for the `ModeledAsRetryableClassifier`. + ModeledAsRetryableClassifier, + // The default priority for the `TransientErrorClassifier`. + TransientErrorClassifier, + // The priority of some other classifier. + Other(i8), +} + +impl PartialOrd for RetryClassifierPriority { + fn partial_cmp(&self, other: &Self) -> Option { + Some(other.as_i8().cmp(&self.as_i8())) + } +} + +impl Ord for RetryClassifierPriority { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + other.as_i8().cmp(&self.as_i8()) + } +} + +impl RetryClassifierPriority { + /// Create a new `RetryClassifierPriority` with the default priority for the `HttpStatusCodeClassifier`. + pub fn http_status_code_classifier() -> Self { + Self { + inner: Inner::HttpStatusCodeClassifier, + } + } + + /// Create a new `RetryClassifierPriority` with the default priority for the `ModeledAsRetryableClassifier`. + pub fn modeled_as_retryable_classifier() -> Self { + Self { + inner: Inner::ModeledAsRetryableClassifier, + } + } + + /// Create a new `RetryClassifierPriority` with the default priority for the `TransientErrorClassifier`. + pub fn transient_error_classifier() -> Self { + Self { + inner: Inner::TransientErrorClassifier, + } + } + + /// Create a new `RetryClassifierPriority` with lower priority than the given priority. + pub fn with_lower_priority_than(other: Self) -> Self { + Self { + inner: Inner::Other(other.as_i8() + 1), + } + } + + /// Create a new `RetryClassifierPriority` with higher priority than the given priority. + pub fn with_higher_priority_than(other: Self) -> Self { + Self { + inner: Inner::Other(other.as_i8() - 1), + } + } + + fn as_i8(&self) -> i8 { + match self.inner { + Inner::HttpStatusCodeClassifier => 0, + Inner::ModeledAsRetryableClassifier => 10, + Inner::TransientErrorClassifier => 20, + Inner::Other(i) => i, + } + } +} + +impl Default for RetryClassifierPriority { + fn default() -> Self { + Self { + inner: Inner::Other(0), + } + } +} + +/// Classifies what kind of retry is needed for a given [`InterceptorContext`]. +pub trait ClassifyRetry: Send + Sync + fmt::Debug { + /// Run this classifier on the [`InterceptorContext`] to determine if the previous request + /// should be retried. Returns a [`RetryAction`]. + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction; + + /// The name of this retry classifier. + /// + /// Used for debugging purposes. + fn name(&self) -> &'static str; + + /// The priority of this retry classifier. Classifiers with a higher priority will override the + /// results of classifiers with a lower priority. Classifiers with equal priorities make no + /// guarantees about which will override the other. + fn priority(&self) -> RetryClassifierPriority { + RetryClassifierPriority::default() + } +} + +impl_shared_conversions!(convert SharedRetryClassifier from ClassifyRetry using SharedRetryClassifier::new); + +#[derive(Debug, Clone)] +/// Retry classifier used by the retry strategy to classify responses as retryable or not. +pub struct SharedRetryClassifier(Arc); + +impl SharedRetryClassifier { + /// Given a [`ClassifyRetry`] trait object, create a new `SharedRetryClassifier`. + pub fn new(retry_classifier: impl ClassifyRetry + 'static) -> Self { + Self(Arc::new(retry_classifier)) + } +} + +impl ClassifyRetry for SharedRetryClassifier { + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + self.0.classify_retry(ctx) + } + + fn name(&self) -> &'static str { + self.0.name() + } + + fn priority(&self) -> RetryClassifierPriority { + self.0.priority() + } +} + +#[cfg(test)] +mod tests { + use super::RetryClassifierPriority; + + #[test] + fn test_classifier_lower_priority_than() { + let classifier_a = RetryClassifierPriority::default(); + let classifier_b = RetryClassifierPriority::with_lower_priority_than(classifier_a); + let classifier_c = RetryClassifierPriority::with_lower_priority_than(classifier_b); + + let mut list = vec![classifier_b, classifier_a, classifier_c]; + list.sort(); + + assert_eq!(vec![classifier_c, classifier_b, classifier_a], list); + } + + #[test] + fn test_classifier_higher_priority_than() { + let classifier_c = RetryClassifierPriority::default(); + let classifier_b = RetryClassifierPriority::with_higher_priority_than(classifier_c); + let classifier_a = RetryClassifierPriority::with_higher_priority_than(classifier_b); + + let mut list = vec![classifier_b, classifier_c, classifier_a]; + list.sort(); + + assert_eq!(vec![classifier_c, classifier_b, classifier_a], list); + } +} diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs index 0f12d407a5..9d262738d1 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs @@ -21,7 +21,8 @@ use crate::client::identity::{ ConfiguredIdentityResolver, IdentityResolver, SharedIdentityResolver, }; use crate::client::interceptors::{Interceptor, SharedInterceptor}; -use crate::client::retries::{RetryClassifiers, RetryStrategy, SharedRetryStrategy}; +use crate::client::retries::classifiers::{ClassifyRetry, SharedRetryClassifier}; +use crate::client::retries::{RetryStrategy, SharedRetryStrategy}; use crate::shared::IntoShared; use aws_smithy_async::rt::sleep::{AsyncSleep, SharedAsyncSleep}; use aws_smithy_async::time::{SharedTimeSource, TimeSource}; @@ -200,7 +201,7 @@ declare_runtime_components! { interceptors: Vec, - retry_classifiers: Option, + retry_classifiers: Vec, #[required] retry_strategy: Option, @@ -245,9 +246,9 @@ impl RuntimeComponents { self.interceptors.iter().map(|s| s.value.clone()) } - /// Returns the retry classifiers. - pub fn retry_classifiers(&self) -> Option<&RetryClassifiers> { - self.retry_classifiers.as_ref().map(|s| &s.value) + /// Returns an iterator over the retry classifiers. + pub fn retry_classifiers(&self) -> impl Iterator + '_ { + self.retry_classifiers.iter().map(|s| s.value.clone()) } /// Returns the retry strategy. @@ -424,22 +425,46 @@ impl RuntimeComponentsBuilder { } /// Returns the retry classifiers. - pub fn retry_classifiers(&self) -> Option<&RetryClassifiers> { - self.retry_classifiers.as_ref().map(|s| &s.value) + pub fn retry_classifiers(&self) -> impl Iterator + '_ { + self.retry_classifiers.iter().map(|s| s.value.clone()) } - /// Sets the retry classifiers. - pub fn set_retry_classifiers( + /// Adds all the given retry_classifiers. + pub fn extend_retry_classifiers( &mut self, - retry_classifiers: Option, + retry_classifiers: impl Iterator, ) -> &mut Self { - self.retry_classifiers = retry_classifiers.map(|s| Tracked::new(self.builder_name, s)); + self.retry_classifiers + .extend(retry_classifiers.map(|s| Tracked::new(self.builder_name, s))); self } - /// Sets the retry classifiers. - pub fn with_retry_classifiers(mut self, retry_classifiers: Option) -> Self { - self.retry_classifiers = retry_classifiers.map(|s| Tracked::new(self.builder_name, s)); + /// Adds an retry_classifier. + pub fn push_retry_classifier( + &mut self, + retry_classifier: impl ClassifyRetry + 'static, + ) -> &mut Self { + self.retry_classifiers.push(Tracked::new( + self.builder_name, + retry_classifier.into_shared(), + )); + self + } + + /// Adds an retry_classifier. + pub fn with_retry_classifier(mut self, retry_classifier: impl ClassifyRetry + 'static) -> Self { + self.push_retry_classifier(retry_classifier); + self + } + + /// Directly sets the retry_classifiers and clears out any that were previously pushed. + pub fn set_retry_classifiers( + &mut self, + retry_classifiers: impl Iterator, + ) -> &mut Self { + self.retry_classifiers.clear(); + self.retry_classifiers + .extend(retry_classifiers.map(|s| Tracked::new(self.builder_name, s))); self } @@ -633,7 +658,6 @@ impl RuntimeComponentsBuilder { .with_endpoint_resolver(Some(FakeEndpointResolver)) .with_http_client(Some(FakeClient)) .with_identity_resolver(AuthSchemeId::new("fake"), FakeIdentityResolver) - .with_retry_classifiers(Some(RetryClassifiers::new())) .with_retry_strategy(Some(FakeRetryStrategy)) .with_sleep_impl(Some(SharedAsyncSleep::new(FakeSleep))) .with_time_source(Some(SharedTimeSource::new(FakeTimeSource))) @@ -654,7 +678,7 @@ impl fmt::Display for BuildError { /// A trait for retrieving a shared identity resolver. /// -/// This trait exists so that [`AuthScheme::identity_resolver`](crate::client::auth::AuthScheme::identity_resolver) +/// This trait exists so that [`AuthScheme::identity_resolver`] /// can have access to configured identity resolvers without having access to all the runtime components. pub trait GetIdentityResolver: Send + Sync { /// Returns the requested identity resolver if it is set. @@ -672,7 +696,7 @@ impl GetIdentityResolver for RuntimeComponents { #[cfg(all(test, feature = "test-util"))] mod tests { - use super::*; + use super::{BuildError, RuntimeComponentsBuilder, Tracked}; #[test] #[allow(unreachable_pub)] diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs index 56596a6087..4260b6af81 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs @@ -9,7 +9,7 @@ //! This can include: //! - Registering interceptors //! - Registering auth schemes -//! - Adding entries to the [`ConfigBag`](aws_smithy_types::config_bag::ConfigBag) for orchestration +//! - Adding entries to the [`ConfigBag`] for orchestration //! - Setting runtime components //! //! Runtime plugins are divided into service/operation "levels", with service runtime plugins @@ -77,7 +77,7 @@ pub trait RuntimePlugin: Debug + Send + Sync { DEFAULT_ORDER } - /// Optionally returns additional config that should be added to the [`ConfigBag`](aws_smithy_types::config_bag::ConfigBag). + /// Optionally returns additional config that should be added to the [`ConfigBag`]. /// /// As a best practice, a frozen layer should be stored on the runtime plugin instance as /// a member, and then cloned upon return since that clone is cheap. Constructing a new diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/connection_poisoning.rs b/rust-runtime/aws-smithy-runtime/src/client/http/connection_poisoning.rs index 438a94752d..99f8e67591 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/connection_poisoning.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/connection_poisoning.rs @@ -3,16 +3,17 @@ * SPDX-License-Identifier: Apache-2.0 */ +use crate::client::retries::classifiers::run_classifiers_on_ctx; use aws_smithy_http::connection::ConnectionMetadata; use aws_smithy_runtime_api::box_error::BoxError; use aws_smithy_runtime_api::client::interceptors::context::{ AfterDeserializationInterceptorContextRef, BeforeTransmitInterceptorContextMut, }; use aws_smithy_runtime_api::client::interceptors::Interceptor; -use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryReason}; +use aws_smithy_runtime_api::client::retries::classifiers::RetryAction; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; use aws_smithy_types::config_bag::{ConfigBag, Storable, StoreReplace}; -use aws_smithy_types::retry::{ErrorKind, ReconnectMode, RetryConfig}; +use aws_smithy_types::retry::{ReconnectMode, RetryConfig}; use std::fmt; use std::sync::{Arc, Mutex}; use tracing::{debug, error}; @@ -74,14 +75,9 @@ impl Interceptor for ConnectionPoisoningInterceptor { .map(RetryConfig::reconnect_mode) .unwrap_or(ReconnectMode::ReconnectOnTransientError); let captured_connection = cfg.load::().cloned(); - let retry_classifiers = runtime_components - .retry_classifiers() - .ok_or("retry classifiers are required for connection poisoning to work")?; - - let error_is_transient = retry_classifiers - .classify_retry(context.inner()) - .map(|reason| reason == RetryReason::Error(ErrorKind::TransientError)) - .unwrap_or_default(); + let retry_classifier_result = + run_classifiers_on_ctx(runtime_components.retry_classifiers(), context.inner()); + let error_is_transient = retry_classifier_result == RetryAction::transient_error(); let connection_poisoning_is_enabled = reconnect_mode == ReconnectMode::ReconnectOnTransientError; diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs index 9c4e71237c..cbc560402f 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/operation.rs @@ -24,7 +24,8 @@ use aws_smithy_runtime_api::client::interceptors::context::{Error, Input, Output use aws_smithy_runtime_api::client::interceptors::Interceptor; use aws_smithy_runtime_api::client::orchestrator::HttpResponse; use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, OrchestratorError}; -use aws_smithy_runtime_api::client::retries::{RetryClassifiers, SharedRetryStrategy}; +use aws_smithy_runtime_api::client::retries::classifiers::ClassifyRetry; +use aws_smithy_runtime_api::client::retries::SharedRetryStrategy; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use aws_smithy_runtime_api::client::runtime_plugin::{ RuntimePlugin, RuntimePlugins, SharedRuntimePlugin, StaticRuntimePlugin, @@ -206,15 +207,15 @@ impl OperationBuilder { self } - pub fn no_retry(mut self) -> Self { + pub fn retry_classifier(mut self, retry_classifier: impl ClassifyRetry + 'static) -> Self { self.runtime_components - .set_retry_strategy(Some(SharedRetryStrategy::new(NeverRetryStrategy::new()))); + .push_retry_classifier(retry_classifier); self } - pub fn retry_classifiers(mut self, retry_classifiers: RetryClassifiers) -> Self { + pub fn no_retry(mut self) -> Self { self.runtime_components - .set_retry_classifiers(Some(retry_classifiers)); + .set_retry_strategy(Some(SharedRetryStrategy::new(NeverRetryStrategy::new()))); self } @@ -383,7 +384,7 @@ impl OperationBuilder { mod tests { use super::*; use crate::client::http::test_util::{capture_request, ReplayEvent, StaticReplayClient}; - use crate::client::retries::classifier::HttpStatusCodeClassifier; + use crate::client::retries::classifiers::HttpStatusCodeClassifier; use aws_smithy_async::rt::sleep::{SharedAsyncSleep, TokioSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; @@ -459,10 +460,8 @@ mod tests { .http_client(connector.clone()) .endpoint_url("http://localhost:1234") .no_auth() - .retry_classifiers( - RetryClassifiers::new().with_classifier(HttpStatusCodeClassifier::default()), - ) .standard_retry(&RetryConfig::standard()) + .retry_classifier(HttpStatusCodeClassifier::default()) .timeout_config(TimeoutConfig::disabled()) .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) .serializer(|input: String| { diff --git a/rust-runtime/aws-smithy-runtime/src/client/retries.rs b/rust-runtime/aws-smithy-runtime/src/client/retries.rs index 8ea71ebb5e..46bc4332c6 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/retries.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/retries.rs @@ -4,7 +4,7 @@ */ /// Smithy retry classifiers. -pub mod classifier; +pub mod classifiers; /// Smithy retry strategies. pub mod strategy; diff --git a/rust-runtime/aws-smithy-runtime/src/client/retries/classifier.rs b/rust-runtime/aws-smithy-runtime/src/client/retries/classifiers.rs similarity index 56% rename from rust-runtime/aws-smithy-runtime/src/client/retries/classifier.rs rename to rust-runtime/aws-smithy-runtime/src/client/retries/classifiers.rs index 254cb636d5..a1abd092c9 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/retries/classifier.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/retries/classifiers.rs @@ -4,8 +4,10 @@ */ use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; -use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryReason}; -use aws_smithy_types::retry::{ErrorKind, ProvideErrorKind}; +use aws_smithy_runtime_api::client::retries::classifiers::{ + ClassifyRetry, RetryAction, RetryClassifierPriority, SharedRetryClassifier, +}; +use aws_smithy_types::retry::ProvideErrorKind; use std::borrow::Cow; use std::error::Error as StdError; use std::marker::PhantomData; @@ -23,76 +25,100 @@ impl ModeledAsRetryableClassifier { _inner: PhantomData, } } + + /// Return the priority of this retry classifier. + pub fn priority() -> RetryClassifierPriority { + RetryClassifierPriority::modeled_as_retryable_classifier() + } } impl ClassifyRetry for ModeledAsRetryableClassifier where E: StdError + ProvideErrorKind + Send + Sync + 'static, { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { // Check for a result - let output_or_error = ctx.output_or_error()?; + let output_or_error = ctx.output_or_error(); // Check for an error let error = match output_or_error { - Ok(_) => return None, - Err(err) => err, + Some(Ok(_)) | None => return RetryAction::NoActionIndicated, + Some(Err(err)) => err, }; // Check that the error is an operation error - let error = error.as_operation_error()?; - // Downcast the error - let error = error.downcast_ref::()?; - // Check if the error is retryable - error.retryable_error_kind().map(RetryReason::Error) + error + .as_operation_error() + // Downcast the error + .and_then(|err| err.downcast_ref::()) + // Check if the error is retryable + .and_then(|err| err.retryable_error_kind().map(RetryAction::retryable_error)) + .unwrap_or_default() } fn name(&self) -> &'static str { "Errors Modeled As Retryable" } + + fn priority(&self) -> RetryClassifierPriority { + Self::priority() + } } /// Classifies response, timeout, and connector errors as retryable or not. #[derive(Debug, Default)] -pub struct SmithyErrorClassifier { +pub struct TransientErrorClassifier { _inner: PhantomData, } -impl SmithyErrorClassifier { - /// Create a new `SmithyErrorClassifier` +impl TransientErrorClassifier { + /// Create a new `TransientErrorClassifier` pub fn new() -> Self { Self { _inner: PhantomData, } } + + /// Return the priority of this retry classifier. + pub fn priority() -> RetryClassifierPriority { + RetryClassifierPriority::transient_error_classifier() + } } -impl ClassifyRetry for SmithyErrorClassifier +impl ClassifyRetry for TransientErrorClassifier where E: StdError + Send + Sync + 'static, { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { - let output_or_error = ctx.output_or_error()?; + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + // Check for a result + let output_or_error = ctx.output_or_error(); // Check for an error let error = match output_or_error { - Ok(_) => return None, - Err(err) => err, + Some(Ok(_)) | None => return RetryAction::NoActionIndicated, + Some(Err(err)) => err, }; if error.is_response_error() || error.is_timeout_error() { - Some(RetryReason::Error(ErrorKind::TransientError)) + RetryAction::transient_error() } else if let Some(error) = error.as_connector_error() { if error.is_timeout() || error.is_io() { - Some(RetryReason::Error(ErrorKind::TransientError)) + RetryAction::transient_error() } else { - error.as_other().map(RetryReason::Error) + error + .as_other() + .map(RetryAction::retryable_error) + .unwrap_or_default() } } else { - None + RetryAction::NoActionIndicated } } fn name(&self) -> &'static str { "Retryable Smithy Errors" } + + fn priority(&self) -> RetryClassifierPriority { + Self::priority() + } } const TRANSIENT_ERROR_STATUS_CODES: &[u16] = &[500, 502, 503, 504]; @@ -119,45 +145,87 @@ impl HttpStatusCodeClassifier { retryable_status_codes: retryable_status_codes.into(), } } + + /// Return the priority of this retry classifier. + pub fn priority() -> RetryClassifierPriority { + RetryClassifierPriority::http_status_code_classifier() + } } impl ClassifyRetry for HttpStatusCodeClassifier { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { - ctx.response() + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + let is_retryable = ctx + .response() .map(|res| res.status().as_u16()) .map(|status| self.retryable_status_codes.contains(&status)) - .unwrap_or_default() - .then_some(RetryReason::Error(ErrorKind::TransientError)) + .unwrap_or_default(); + + if is_retryable { + RetryAction::transient_error() + } else { + RetryAction::NoActionIndicated + } } fn name(&self) -> &'static str { "HTTP Status Code" } + + fn priority(&self) -> RetryClassifierPriority { + Self::priority() + } } -// Generic smithy clients would have something like this: -// pub fn default_retry_classifiers() -> RetryClassifiers { -// RetryClassifiers::new() -// .with_classifier(SmithyErrorClassifier::new()) -// .with_classifier(ModeledAsRetryableClassifier::new()) -// .with_classifier(HttpStatusCodeClassifier::new()) -// } -// This ordering is different than the default AWS ordering because the old generic client classifier -// was the same. +/// Given an iterator of retry classifiers and an interceptor context, run retry classifiers on the +/// context. Each classifier is passed the classification result from the previous classifier (the +/// 'root' classifier is passed `None`.) +pub fn run_classifiers_on_ctx( + classifiers: impl Iterator, + ctx: &InterceptorContext, +) -> RetryAction { + // By default, don't retry + let mut result = RetryAction::NoActionIndicated; + + for classifier in classifiers { + let new_result = classifier.classify_retry(ctx); + + // If the result is `NoActionIndicated`, continue to the next classifier + // without overriding any previously-set result. + if new_result == RetryAction::NoActionIndicated { + continue; + } + + // Otherwise, set the result to the new result. + tracing::trace!( + "Classifier '{}' set the result of classification to '{}'", + classifier.name(), + new_result + ); + result = new_result; + + // If the result is `RetryForbidden`, stop running classifiers. + if result == RetryAction::RetryForbidden { + tracing::trace!("retry classification ending early because a `RetryAction::RetryForbidden` was emitted",); + break; + } + } + + result +} #[cfg(test)] mod test { - use crate::client::retries::classifier::{ + use crate::client::retries::classifiers::{ HttpStatusCodeClassifier, ModeledAsRetryableClassifier, }; use aws_smithy_http::body::SdkBody; use aws_smithy_runtime_api::client::interceptors::context::{Error, Input, InterceptorContext}; use aws_smithy_runtime_api::client::orchestrator::OrchestratorError; - use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryReason}; + use aws_smithy_runtime_api::client::retries::classifiers::{ClassifyRetry, RetryAction}; use aws_smithy_types::retry::{ErrorKind, ProvideErrorKind}; use std::fmt; - use super::SmithyErrorClassifier; + use super::TransientErrorClassifier; #[derive(Debug, PartialEq, Eq, Clone)] struct UnmodeledError; @@ -180,10 +248,7 @@ mod test { .map(SdkBody::from); let mut ctx = InterceptorContext::new(Input::doesnt_matter()); ctx.set_response(res); - assert_eq!( - policy.classify_retry(&ctx), - Some(RetryReason::Error(ErrorKind::TransientError)) - ); + assert_eq!(policy.classify_retry(&ctx), RetryAction::transient_error()); } #[test] @@ -196,7 +261,7 @@ mod test { .map(SdkBody::from); let mut ctx = InterceptorContext::new(Input::doesnt_matter()); ctx.set_response(res); - assert_eq!(policy.classify_retry(&ctx), None); + assert_eq!(policy.classify_retry(&ctx), RetryAction::NoActionIndicated); } #[test] @@ -229,35 +294,26 @@ mod test { RetryableError, )))); - assert_eq!( - policy.classify_retry(&ctx), - Some(RetryReason::Error(ErrorKind::ClientError)), - ); + assert_eq!(policy.classify_retry(&ctx), RetryAction::client_error(),); } #[test] fn classify_response_error() { - let policy = SmithyErrorClassifier::::new(); + let policy = TransientErrorClassifier::::new(); let mut ctx = InterceptorContext::new(Input::doesnt_matter()); ctx.set_output_or_error(Err(OrchestratorError::response( "I am a response error".into(), ))); - assert_eq!( - policy.classify_retry(&ctx), - Some(RetryReason::Error(ErrorKind::TransientError)), - ); + assert_eq!(policy.classify_retry(&ctx), RetryAction::transient_error(),); } #[test] fn test_timeout_error() { - let policy = SmithyErrorClassifier::::new(); + let policy = TransientErrorClassifier::::new(); let mut ctx = InterceptorContext::new(Input::doesnt_matter()); ctx.set_output_or_error(Err(OrchestratorError::timeout( "I am a timeout error".into(), ))); - assert_eq!( - policy.classify_retry(&ctx), - Some(RetryReason::Error(ErrorKind::TransientError)), - ); + assert_eq!(policy.classify_retry(&ctx), RetryAction::transient_error(),); } } diff --git a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy.rs b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy.rs index c441aa4816..05c039f63d 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy.rs @@ -8,8 +8,3 @@ pub(crate) mod standard; pub use never::NeverRetryStrategy; pub use standard::StandardRetryStrategy; - -#[cfg(feature = "test-util")] -mod fixed_delay; -#[cfg(feature = "test-util")] -pub use fixed_delay::FixedDelayRetryStrategy; diff --git a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/fixed_delay.rs b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/fixed_delay.rs deleted file mode 100644 index 886b7a61b2..0000000000 --- a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/fixed_delay.rs +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use aws_smithy_runtime_api::box_error::BoxError; -use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; -use aws_smithy_runtime_api::client::retries::{ - ClassifyRetry, RequestAttempts, RetryReason, RetryStrategy, ShouldAttempt, -}; -use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; -use aws_smithy_types::config_bag::ConfigBag; -use std::time::Duration; - -/// A retry policy used in tests. This relies on an error classifier already present in the config bag. -/// If a server response is retryable, it will be retried after a fixed delay. -#[derive(Debug, Clone)] -pub struct FixedDelayRetryStrategy { - fixed_delay: Duration, - max_attempts: u32, -} - -impl FixedDelayRetryStrategy { - /// Create a new retry policy with a fixed delay. - pub fn new(fixed_delay: Duration) -> Self { - Self { - fixed_delay, - max_attempts: 4, - } - } - - /// Create a new retry policy with a one second delay. - pub fn one_second_delay() -> Self { - Self::new(Duration::from_secs(1)) - } -} - -impl RetryStrategy for FixedDelayRetryStrategy { - fn should_attempt_initial_request( - &self, - _runtime_components: &RuntimeComponents, - _cfg: &ConfigBag, - ) -> Result { - Ok(ShouldAttempt::Yes) - } - - fn should_attempt_retry( - &self, - ctx: &InterceptorContext, - runtime_components: &RuntimeComponents, - cfg: &ConfigBag, - ) -> Result { - // Look a the result. If it's OK then we're done; No retry required. Otherwise, we need to inspect it - let output_or_error = ctx.output_or_error().expect( - "This must never be called without reaching the point where the result exists.", - ); - if output_or_error.is_ok() { - tracing::trace!("request succeeded, no retry necessary"); - return Ok(ShouldAttempt::No); - } - - // Check if we're out of attempts - let request_attempts = cfg - .load::() - .expect("at least one request attempt is made before any retry is attempted"); - if request_attempts.attempts() >= self.max_attempts { - tracing::trace!( - attempts = request_attempts.attempts(), - max_attempts = self.max_attempts, - "not retrying because we are out of attempts" - ); - return Ok(ShouldAttempt::No); - } - - let retry_classifiers = runtime_components - .retry_classifiers() - .expect("a retry classifier is set"); - let retry_reason = retry_classifiers.classify_retry(ctx); - - let backoff = match retry_reason { - Some(RetryReason::Explicit(_)) => self.fixed_delay, - Some(RetryReason::Error(_)) => self.fixed_delay, - Some(_) => { - unreachable!("RetryReason is non-exhaustive. Therefore, we need to cover this unreachable case.") - } - None => { - tracing::trace!( - attempts = request_attempts.attempts(), - max_attempts = self.max_attempts, - "encountered unretryable error" - ); - return Ok(ShouldAttempt::No); - } - }; - - tracing::debug!( - "attempt {} failed with {:?}; retrying after {:?}", - request_attempts.attempts(), - retry_reason, - backoff - ); - - Ok(ShouldAttempt::YesAfterDelay(backoff)) - } -} diff --git a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs index ee32a6a63d..70efea493a 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +use crate::client::retries::classifiers::run_classifiers_on_ctx; use crate::client::retries::client_rate_limiter::{ClientRateLimiter, RequestReason}; use crate::client::retries::strategy::standard::ReleaseResult::{ APermitWasReleased, NoPermitWasReleased, @@ -10,9 +11,8 @@ use crate::client::retries::strategy::standard::ReleaseResult::{ use crate::client::retries::token_bucket::TokenBucket; use aws_smithy_runtime_api::box_error::BoxError; use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; -use aws_smithy_runtime_api::client::retries::{ - ClassifyRetry, RequestAttempts, RetryReason, RetryStrategy, ShouldAttempt, -}; +use aws_smithy_runtime_api::client::retries::classifiers::{RetryAction, RetryReason}; +use aws_smithy_runtime_api::client::retries::{RequestAttempts, RetryStrategy, ShouldAttempt}; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; use aws_smithy_types::config_bag::{ConfigBag, Storable, StoreReplace}; use aws_smithy_types::retry::{ErrorKind, RetryConfig}; @@ -102,7 +102,7 @@ impl StandardRetryStrategy { &self, runtime_components: &RuntimeComponents, cfg: &ConfigBag, - retry_reason: Option<&RetryReason>, + retry_reason: &RetryAction, ) -> Result { let request_attempts = cfg .load::() @@ -111,14 +111,20 @@ impl StandardRetryStrategy { let token_bucket = cfg.load::(); match retry_reason { - Some(RetryReason::Explicit(backoff)) => Ok(*backoff), - Some(RetryReason::Error(kind)) => { + RetryAction::RetryIndicated(RetryReason::RetryableError { kind, retry_after }) => { update_rate_limiter_if_exists( runtime_components, cfg, *kind == ErrorKind::ThrottlingError, ); - if let Some(delay) = check_rate_limiter_for_delay(runtime_components, cfg, *kind) { + + if let Some(delay) = *retry_after { + let delay = delay.min(self.max_backoff); + debug!("explicit request from server to delay {delay:?} before retrying"); + Ok(delay) + } else if let Some(delay) = + check_rate_limiter_for_delay(runtime_components, cfg, *kind) + { let delay = delay.min(self.max_backoff); debug!("rate limiter has requested a {delay:?} delay before retrying"); Ok(delay) @@ -145,8 +151,7 @@ impl StandardRetryStrategy { Ok(Duration::from_secs_f64(backoff).min(self.max_backoff)) } } - Some(_) => unreachable!("RetryReason is non-exhaustive"), - None => { + RetryAction::RetryForbidden | RetryAction::NoActionIndicated => { update_rate_limiter_if_exists(runtime_components, cfg, false); debug!( attempts = request_attempts, @@ -155,6 +160,7 @@ impl StandardRetryStrategy { ); Err(ShouldAttempt::No) } + _ => unreachable!("RetryAction is non-exhaustive"), } } } @@ -242,22 +248,19 @@ impl RetryStrategy for StandardRetryStrategy { return Ok(ShouldAttempt::No); } - // Run the classifiers against the context to determine if we should retry - let retry_classifiers = runtime_components - .retry_classifiers() - .ok_or("retry classifiers are required by the retry configuration")?; - let retry_reason = retry_classifiers.classify_retry(ctx); + // Run the classifier against the context to determine if we should retry + let retry_classifiers = runtime_components.retry_classifiers(); + let classifier_result = run_classifiers_on_ctx(retry_classifiers, ctx); // Calculate the appropriate backoff time. - let backoff = match self.calculate_backoff(runtime_components, cfg, retry_reason.as_ref()) { + let backoff = match self.calculate_backoff(runtime_components, cfg, &classifier_result) { Ok(value) => value, // In some cases, backoff calculation will decide that we shouldn't retry at all. Err(value) => return Ok(value), }; debug!( "attempt #{request_attempts} failed with {:?}; retrying after {:?}", - retry_reason.expect("the match statement above ensures this is not None"), - backoff, + classifier_result, backoff, ); Ok(ShouldAttempt::YesAfterDelay(backoff)) @@ -316,9 +319,10 @@ fn get_seconds_since_unix_epoch(runtime_components: &RuntimeComponents) -> f64 { mod tests { use super::*; use aws_smithy_runtime_api::client::orchestrator::OrchestratorError; - use aws_smithy_runtime_api::client::retries::{ - AlwaysRetry, ClassifyRetry, RetryClassifiers, RetryReason, RetryStrategy, + use aws_smithy_runtime_api::client::retries::classifiers::{ + ClassifyRetry, RetryAction, SharedRetryClassifier, }; + use aws_smithy_runtime_api::client::retries::{AlwaysRetry, RetryStrategy}; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use aws_smithy_types::config_bag::Layer; use aws_smithy_types::retry::{ErrorKind, ProvideErrorKind}; @@ -350,9 +354,7 @@ mod tests { let mut ctx = InterceptorContext::new(Input::doesnt_matter()); ctx.set_output_or_error(Err(OrchestratorError::other("doesn't matter"))); let rc = RuntimeComponentsBuilder::for_tests() - .with_retry_classifiers(Some( - RetryClassifiers::new().with_classifier(AlwaysRetry(error_kind)), - )) + .with_retry_classifier(SharedRetryClassifier::new(AlwaysRetry(error_kind))) .build() .unwrap(); let mut layer = Layer::new("test"); @@ -429,31 +431,35 @@ mod tests { #[derive(Debug)] struct PresetReasonRetryClassifier { - retry_reasons: Mutex>, + retry_actions: Mutex>, } #[cfg(feature = "test-util")] impl PresetReasonRetryClassifier { - fn new(mut retry_reasons: Vec) -> Self { + fn new(mut retry_reasons: Vec) -> Self { // We'll pop the retry_reasons in reverse order so we reverse the list to fix that. retry_reasons.reverse(); Self { - retry_reasons: Mutex::new(retry_reasons), + retry_actions: Mutex::new(retry_reasons), } } } impl ClassifyRetry for PresetReasonRetryClassifier { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { - if ctx.output_or_error().map(|it| it.is_ok()).unwrap_or(false) { - return None; - } - - let mut retry_reasons = self.retry_reasons.lock().unwrap(); - if retry_reasons.len() == 1 { - Some(retry_reasons.first().unwrap().clone()) + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { + // Check for a result + let output_or_error = ctx.output_or_error(); + // Check for an error + match output_or_error { + Some(Ok(_)) | None => return RetryAction::NoActionIndicated, + _ => (), + }; + + let mut retry_actions = self.retry_actions.lock().unwrap(); + if retry_actions.len() == 1 { + retry_actions.first().unwrap().clone() } else { - retry_reasons.pop() + retry_actions.pop().unwrap() } } @@ -464,12 +470,11 @@ mod tests { #[cfg(feature = "test-util")] fn setup_test( - retry_reasons: Vec, + retry_reasons: Vec, ) -> (ConfigBag, RuntimeComponents, InterceptorContext) { let rc = RuntimeComponentsBuilder::for_tests() - .with_retry_classifiers(Some( - RetryClassifiers::new() - .with_classifier(PresetReasonRetryClassifier::new(retry_reasons)), + .with_retry_classifier(SharedRetryClassifier::new( + PresetReasonRetryClassifier::new(retry_reasons), )) .build() .unwrap(); @@ -484,7 +489,7 @@ mod tests { #[cfg(feature = "test-util")] #[test] fn eventual_success() { - let (mut cfg, rc, mut ctx) = setup_test(vec![RetryReason::Error(ErrorKind::ServerError)]); + let (mut cfg, rc, mut ctx) = setup_test(vec![RetryAction::server_error()]); let strategy = StandardRetryStrategy::default() .with_base(|| 1.0) .with_max_attempts(5); @@ -514,7 +519,7 @@ mod tests { #[cfg(feature = "test-util")] #[test] fn no_more_attempts() { - let (mut cfg, rc, ctx) = setup_test(vec![RetryReason::Error(ErrorKind::ServerError)]); + let (mut cfg, rc, ctx) = setup_test(vec![RetryAction::server_error()]); let strategy = StandardRetryStrategy::default() .with_base(|| 1.0) .with_max_attempts(3); @@ -542,7 +547,7 @@ mod tests { #[cfg(feature = "test-util")] #[test] fn no_quota() { - let (mut cfg, rc, ctx) = setup_test(vec![RetryReason::Error(ErrorKind::ServerError)]); + let (mut cfg, rc, ctx) = setup_test(vec![RetryAction::server_error()]); let strategy = StandardRetryStrategy::default() .with_base(|| 1.0) .with_max_attempts(5); @@ -565,8 +570,11 @@ mod tests { #[test] fn quota_replenishes_on_success() { let (mut cfg, rc, mut ctx) = setup_test(vec![ - RetryReason::Error(ErrorKind::TransientError), - RetryReason::Explicit(Duration::from_secs(1)), + RetryAction::transient_error(), + RetryAction::retryable_error_with_explicit_delay( + ErrorKind::TransientError, + Duration::from_secs(1), + ), ]); let strategy = StandardRetryStrategy::default() .with_base(|| 1.0) @@ -599,8 +607,7 @@ mod tests { #[test] fn quota_replenishes_on_first_try_success() { const PERMIT_COUNT: usize = 20; - let (mut cfg, rc, mut ctx) = - setup_test(vec![RetryReason::Error(ErrorKind::TransientError)]); + let (mut cfg, rc, mut ctx) = setup_test(vec![RetryAction::transient_error()]); let strategy = StandardRetryStrategy::default() .with_base(|| 1.0) .with_max_attempts(u32::MAX); @@ -650,7 +657,7 @@ mod tests { #[cfg(feature = "test-util")] #[test] fn backoff_timing() { - let (mut cfg, rc, ctx) = setup_test(vec![RetryReason::Error(ErrorKind::ServerError)]); + let (mut cfg, rc, ctx) = setup_test(vec![RetryAction::server_error()]); let strategy = StandardRetryStrategy::default() .with_base(|| 1.0) .with_max_attempts(5); @@ -690,7 +697,7 @@ mod tests { #[cfg(feature = "test-util")] #[test] fn max_backoff_time() { - let (mut cfg, rc, ctx) = setup_test(vec![RetryReason::Error(ErrorKind::ServerError)]); + let (mut cfg, rc, ctx) = setup_test(vec![RetryAction::server_error()]); let strategy = StandardRetryStrategy::default() .with_base(|| 1.0) .with_max_attempts(5) diff --git a/rust-runtime/aws-smithy-runtime/tests/reconnect_on_transient_error.rs b/rust-runtime/aws-smithy-runtime/tests/reconnect_on_transient_error.rs index bf1d8220d9..2057690cc1 100644 --- a/rust-runtime/aws-smithy-runtime/tests/reconnect_on_transient_error.rs +++ b/rust-runtime/aws-smithy-runtime/tests/reconnect_on_transient_error.rs @@ -9,10 +9,9 @@ feature = "connector-hyper-0-14-x", ))] -use ::aws_smithy_runtime::client::retries::classifier::{ - HttpStatusCodeClassifier, SmithyErrorClassifier, +use ::aws_smithy_runtime::client::retries::classifiers::{ + HttpStatusCodeClassifier, TransientErrorClassifier, }; -use ::aws_smithy_runtime_api::client::retries::RetryClassifiers; use aws_smithy_async::rt::sleep::TokioSleep; use aws_smithy_http::body::{BoxBody, SdkBody}; use aws_smithy_runtime::client::http::hyper_014::HyperClientBuilder; @@ -24,7 +23,7 @@ use aws_smithy_runtime::test_util::capture_test_logs::capture_test_logs; use aws_smithy_runtime::{ev, match_events}; use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; use aws_smithy_runtime_api::client::orchestrator::OrchestratorError; -use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryReason}; +use aws_smithy_runtime_api::client::retries::classifiers::{ClassifyRetry, RetryAction}; use aws_smithy_types::retry::{ErrorKind, ProvideErrorKind, ReconnectMode, RetryConfig}; use aws_smithy_types::timeout::TimeoutConfig; use hyper::client::Builder as HyperBuilder; @@ -58,22 +57,32 @@ impl std::error::Error for OperationError {} struct TestRetryClassifier; impl ClassifyRetry for TestRetryClassifier { - fn classify_retry(&self, ctx: &InterceptorContext) -> Option { + fn classify_retry(&self, ctx: &InterceptorContext) -> RetryAction { tracing::info!("classifying retry for {ctx:?}"); - let classification = ctx.output_or_error().unwrap().err().and_then(|err| { - if let Some(err) = err.as_operation_error() { - tracing::info!("its an operation error: {err:?}"); - let err = err.downcast_ref::().unwrap(); - Some(RetryReason::Error(err.0)) + // Check for a result + let output_or_error = ctx.output_or_error(); + // Check for an error + let error = match output_or_error { + Some(Ok(_)) | None => return RetryAction::NoActionIndicated, + Some(Err(err)) => err, + }; + + let action = if let Some(err) = error.as_operation_error() { + tracing::info!("its an operation error: {err:?}"); + let err = err.downcast_ref::().unwrap(); + RetryAction::retryable_error(err.0) + } else { + tracing::info!("its something else... using other classifiers"); + let action = TransientErrorClassifier::::new().classify_retry(ctx); + if action == RetryAction::NoActionIndicated { + HttpStatusCodeClassifier::default().classify_retry(ctx) } else { - tracing::info!("its something else... using other classifiers"); - SmithyErrorClassifier::::new() - .classify_retry(ctx) - .or_else(|| HttpStatusCodeClassifier::default().classify_retry(ctx)) + action } - }); - tracing::info!("classified as {classification:?}"); - classification + }; + + tracing::info!("classified as {action:?}"); + action } fn name(&self) -> &'static str { @@ -132,7 +141,7 @@ async fn wire_level_test( .build(), ) .standard_retry(&RetryConfig::standard().with_reconnect_mode(reconnect_mode)) - .retry_classifiers(RetryClassifiers::new().with_classifier(TestRetryClassifier)) + .retry_classifier(TestRetryClassifier) .sleep_impl(TokioSleep::new()) .with_connection_poisoning() .serializer({ diff --git a/rust-runtime/aws-smithy-types/src/retry.rs b/rust-runtime/aws-smithy-types/src/retry.rs index a2866b84c1..911a37ca3a 100644 --- a/rust-runtime/aws-smithy-types/src/retry.rs +++ b/rust-runtime/aws-smithy-types/src/retry.rs @@ -41,6 +41,17 @@ pub enum ErrorKind { ClientError, } +impl fmt::Display for ErrorKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::TransientError => write!(f, "transient error"), + Self::ThrottlingError => write!(f, "throttling error"), + Self::ServerError => write!(f, "server error"), + Self::ClientError => write!(f, "client error"), + } + } +} + /// Trait that provides an `ErrorKind` and an error code. pub trait ProvideErrorKind { /// Returns the `ErrorKind` when the error is modeled as retryable