From 7d619b152ee3f05e8553fc90d23afea48d619fdb Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 13 Sep 2022 16:03:49 -0700 Subject: [PATCH 01/15] Overhaul timeout configuration --- aws/rust-runtime/aws-config/src/connector.rs | 5 +- .../src/default_provider/timeout_config.rs | 16 +- aws/rust-runtime/aws-config/src/ecs.rs | 12 +- .../src/environment/timeout_config.rs | 95 +++--- .../src/http_credential_provider.rs | 39 +-- .../aws-config/src/imds/client.rs | 26 +- .../aws-config/src/imds/client/token.rs | 15 +- aws/rust-runtime/aws-config/src/lib.rs | 39 ++- .../aws-config/src/profile/timeout_config.rs | 76 ++--- .../aws-config/src/provider_config.rs | 4 +- aws/rust-runtime/aws-config/src/sso.rs | 7 +- aws/rust-runtime/aws-config/src/sts.rs | 19 +- .../aws-config/src/sts/assume_role.rs | 8 +- aws/rust-runtime/aws-types/src/sdk_config.rs | 44 +-- .../rustsdk/AwsFluentClientDecorator.kt | 30 +- .../dynamodb/tests/timeouts.rs | 37 +-- .../s3/tests/alternative-async-runtime.rs | 20 +- .../s3/tests/sleep_impl_use_cases.rs | 88 ++--- .../integration-tests/s3/tests/timeouts.rs | 11 +- .../ResiliencyConfigCustomization.kt | 20 +- rust-runtime/aws-smithy-client/src/builder.rs | 44 ++- .../aws-smithy-client/src/http_connector.rs | 102 +++++- .../aws-smithy-client/src/hyper_ext.rs | 163 ++++++---- rust-runtime/aws-smithy-client/src/lib.rs | 29 +- rust-runtime/aws-smithy-client/src/timeout.rs | 97 +++--- rust-runtime/aws-smithy-types/src/lib.rs | 1 - .../aws-smithy-types/src/timeout/api.rs | 75 ----- .../aws-smithy-types/src/timeout/config.rs | 301 +++++++++++++----- .../aws-smithy-types/src/timeout/http.rs | 82 ----- .../aws-smithy-types/src/timeout/mod.rs | 8 +- .../aws-smithy-types/src/timeout/tcp.rs | 48 --- rust-runtime/aws-smithy-types/src/tristate.rs | 101 ------ 32 files changed, 802 insertions(+), 860 deletions(-) delete mode 100644 rust-runtime/aws-smithy-types/src/timeout/api.rs delete mode 100644 rust-runtime/aws-smithy-types/src/timeout/http.rs delete mode 100644 rust-runtime/aws-smithy-types/src/timeout/tcp.rs delete mode 100644 rust-runtime/aws-smithy-types/src/tristate.rs diff --git a/aws/rust-runtime/aws-config/src/connector.rs b/aws/rust-runtime/aws-config/src/connector.rs index 35670e594a..ae826a7b71 100644 --- a/aws/rust-runtime/aws-config/src/connector.rs +++ b/aws/rust-runtime/aws-config/src/connector.rs @@ -5,11 +5,10 @@ //! Functionality related to creating new HTTP Connectors -use std::sync::Arc; - use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_client::erase::DynConnector; use aws_smithy_client::http_connector::HttpSettings; +use std::sync::Arc; // unused when all crate features are disabled /// Unwrap an [`Option`](aws_smithy_client::erase::DynConnector), and panic with a helpful error message if it's `None` @@ -23,7 +22,7 @@ fn base( sleep: Option>, ) -> aws_smithy_client::hyper_ext::Builder { let mut hyper = - aws_smithy_client::hyper_ext::Adapter::builder().timeout(&settings.http_timeout_config); + aws_smithy_client::hyper_ext::Adapter::builder().http_settings(settings.clone()); if let Some(sleep) = sleep { hyper = hyper.sleep_impl(sleep); } diff --git a/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs b/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs index 868d686606..42a4368fb4 100644 --- a/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs +++ b/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs @@ -3,11 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_types::timeout; - use crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider; use crate::profile; use crate::provider_config::ProviderConfig; +use aws_sdk_sso::config::TimeoutConfig; +use std::time::Duration; + +const SDK_DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_millis(3100); /// Default [`timeout::Config`] Provider chain /// @@ -79,7 +81,7 @@ impl Builder { /// This will panic if: /// - a timeout is set to `NaN`, a negative number, or infinity /// - a timeout can't be parsed as a floating point number - pub async fn timeout_config(self) -> timeout::Config { + pub async fn timeout_config(self) -> TimeoutConfig { // Both of these can return errors due to invalid config settings and we want to surface those as early as possible // hence, we'll panic if any config values are invalid (missing values are OK though) // We match this instead of unwrapping so we can print the error with the `Display` impl instead of the `Debug` impl that unwrap uses @@ -92,6 +94,12 @@ impl Builder { Err(err) => panic!("{}", err), }; - builder_from_env.take_unset_from(builder_from_profile) + // TODO(https://github.com/awslabs/smithy-rs/issues/1732): Implement complete timeout defaults specification + let defaults = TimeoutConfig::builder().connect_timeout(SDK_DEFAULT_CONNECT_TIMEOUT); + + builder_from_env + .take_unset_from(builder_from_profile) + .take_unset_from(defaults) + .build() } } diff --git a/aws/rust-runtime/aws-config/src/ecs.rs b/aws/rust-runtime/aws-config/src/ecs.rs index 8b642b5540..920772d8eb 100644 --- a/aws/rust-runtime/aws-config/src/ecs.rs +++ b/aws/rust-runtime/aws-config/src/ecs.rs @@ -61,11 +61,15 @@ use tower::{Service, ServiceExt}; use crate::http_credential_provider::HttpCredentialProvider; use crate::provider_config::ProviderConfig; +use aws_smithy_client::http_connector::HttpSettings; use aws_types::os_shim_internal::Env; use http::header::InvalidHeaderValue; use std::time::Duration; use tokio::sync::OnceCell; +const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(5); +const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(2); + // URL from https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-metadata-endpoint-v2.html const BASE_HOST: &str = "http://169.254.170.2"; const ENV_RELATIVE_URI: &str = "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI"; @@ -164,8 +168,12 @@ impl Provider { }; let http_provider = HttpCredentialProvider::builder() .configure(&provider_config) - .connect_timeout(builder.connect_timeout) - .read_timeout(builder.read_timeout) + .http_settings( + HttpSettings::builder() + .connect_timeout(DEFAULT_CONNECT_TIMEOUT) + .read_timeout(DEFAULT_READ_TIMEOUT) + .build(), + ) .build("EcsContainer", uri); Provider::Configured(http_provider) } diff --git a/aws/rust-runtime/aws-config/src/environment/timeout_config.rs b/aws/rust-runtime/aws-config/src/environment/timeout_config.rs index ef46c1c316..b58ad7d6af 100644 --- a/aws/rust-runtime/aws-config/src/environment/timeout_config.rs +++ b/aws/rust-runtime/aws-config/src/environment/timeout_config.rs @@ -6,29 +6,28 @@ //! Load timeout configuration properties from environment variables use crate::parsing::parse_str_as_timeout; - +use aws_sdk_sso::config::{TimeoutConfig, TimeoutConfigBuilder}; use aws_smithy_types::timeout; -use aws_smithy_types::tristate::TriState; use aws_types::os_shim_internal::Env; - use std::time::Duration; // Currently unsupported timeouts -const ENV_VAR_CONNECT_TIMEOUT: &str = "AWS_CONNECT_TIMEOUT"; const ENV_VAR_TLS_NEGOTIATION_TIMEOUT: &str = "AWS_TLS_NEGOTIATION_TIMEOUT"; -const ENV_VAR_READ_TIMEOUT: &str = "AWS_READ_TIMEOUT"; // Supported timeouts -const ENV_VAR_API_CALL_ATTEMPT_TIMEOUT: &str = "AWS_API_CALL_ATTEMPT_TIMEOUT"; -const ENV_VAR_API_CALL_TIMEOUT: &str = "AWS_API_CALL_TIMEOUT"; +const ENV_VAR_CONNECT_TIMEOUT: &str = "AWS_CONNECT_TIMEOUT"; +const ENV_VAR_READ_TIMEOUT: &str = "AWS_READ_TIMEOUT"; +const ENV_VAR_OPERATION_ATTEMPT_TIMEOUT: &str = "AWS_OPERATION_ATTEMPT_TIMEOUT"; +const ENV_VAR_OPERATION_TIMEOUT: &str = "AWS_OPERATION_TIMEOUT"; -/// Load a timeout_config from environment variables +/// Load timeout config from environment variables /// -/// This provider will check the values of the following variables in order to build a -/// [`timeout::Config`](aws_smithy_types::timeout::Config) +/// This provider will check the values of the following variables in order to build a [`TimeoutConfig`]: /// -/// - `AWS_API_CALL_ATTEMPT_TIMEOUT` -/// - `AWS_API_CALL_TIMEOUT` +/// - `AWS_CONNECT_TIMEOUT` +/// - `AWS_READ_TIMEOUT` +/// - `AWS_OPERATION_ATTEMPT_TIMEOUT` +/// - `AWS_OPERATION_TIMEOUT` /// /// Timeout values represent the number of seconds before timing out and must be non-negative floats /// or integers. NaN and infinity are also invalid. @@ -52,37 +51,34 @@ impl EnvironmentVariableTimeoutConfigProvider { } /// Attempt to create a new [`timeout::Config`](aws_smithy_types::timeout::Config) from environment variables - pub fn timeout_config(&self) -> Result { + pub fn timeout_config(&self) -> Result { // Warn users that set unsupported timeouts in their profile - for timeout in [ - ENV_VAR_CONNECT_TIMEOUT, - ENV_VAR_TLS_NEGOTIATION_TIMEOUT, - ENV_VAR_READ_TIMEOUT, - ] { + for timeout in [ENV_VAR_TLS_NEGOTIATION_TIMEOUT] { warn_if_unsupported_timeout_is_set(&self.env, timeout); } - let api_call_attempt_timeout = - construct_timeout_from_env_var(&self.env, ENV_VAR_API_CALL_ATTEMPT_TIMEOUT)?; - let api_call_timeout = construct_timeout_from_env_var(&self.env, ENV_VAR_API_CALL_TIMEOUT)?; - - let api_timeouts = timeout::Api::new() - .with_call_timeout(api_call_timeout) - .with_call_attempt_timeout(api_call_attempt_timeout); + let mut builder = TimeoutConfig::builder(); + builder.set_connect_timeout(timeout_from_env_var(&self.env, ENV_VAR_CONNECT_TIMEOUT)?); + builder.set_read_timeout(timeout_from_env_var(&self.env, ENV_VAR_READ_TIMEOUT)?); + builder.set_operation_attempt_timeout(timeout_from_env_var( + &self.env, + ENV_VAR_OPERATION_ATTEMPT_TIMEOUT, + )?); + builder.set_operation_timeout(timeout_from_env_var(&self.env, ENV_VAR_OPERATION_TIMEOUT)?); - // Only API-related timeouts are currently supported - Ok(timeout::Config::new().with_api_timeouts(api_timeouts)) + Ok(builder) } } -fn construct_timeout_from_env_var( +fn timeout_from_env_var( env: &Env, var: &'static str, -) -> Result, timeout::ConfigError> { +) -> Result, timeout::ConfigError> { match env.get(var).ok() { - Some(timeout) => parse_str_as_timeout(&timeout, var.into(), "environment variable".into()) - .map(TriState::Set), - None => Ok(TriState::Unset), + Some(timeout) => { + parse_str_as_timeout(&timeout, var.into(), "environment variable".into()).map(Some) + } + None => Ok(None), } } @@ -99,11 +95,10 @@ fn warn_if_unsupported_timeout_is_set(env: &Env, var: &'static str) { #[cfg(test)] mod test { use super::{ - EnvironmentVariableTimeoutConfigProvider, ENV_VAR_API_CALL_ATTEMPT_TIMEOUT, - ENV_VAR_API_CALL_TIMEOUT, + EnvironmentVariableTimeoutConfigProvider, ENV_VAR_CONNECT_TIMEOUT, + ENV_VAR_OPERATION_ATTEMPT_TIMEOUT, ENV_VAR_OPERATION_TIMEOUT, ENV_VAR_READ_TIMEOUT, }; - use aws_smithy_types::timeout; - use aws_smithy_types::tristate::TriState; + use aws_sdk_sso::config::TimeoutConfig; use aws_types::os_shim_internal::Env; use std::time::Duration; @@ -113,28 +108,34 @@ mod test { #[test] fn no_defaults() { - let built = test_provider(&[]).timeout_config().unwrap(); - - assert_eq!(built.api.call_timeout(), TriState::Unset); - assert_eq!(built.api.call_attempt_timeout(), TriState::Unset); + let built = test_provider(&[]).timeout_config().unwrap().build(); + assert_eq!(built.connect_timeout(), None); + assert_eq!(built.read_timeout(), None); + assert_eq!(built.operation_timeout(), None); + assert_eq!(built.operation_attempt_timeout(), None); } #[test] fn all_fields_can_be_set_at_once() { - let expected_api_timeouts = timeout::Api::new() - .with_call_attempt_timeout(TriState::Set(Duration::from_secs_f32(4.0))) + let expected_timeouts = TimeoutConfig::builder() + .connect_timeout(Duration::from_secs_f32(3.1)) + .read_timeout(Duration::from_secs_f32(500.0)) + .operation_attempt_timeout(Duration::from_secs_f32(4.0)) // Some floats can't be represented as f32 so this duration will end up equalling the // duration from the env. - .with_call_timeout(TriState::Set(Duration::from_secs_f32(900012350.0))); - let expected_timeouts = timeout::Config::new().with_api_timeouts(expected_api_timeouts); + .operation_timeout(Duration::from_secs_f32(900012350.0)) + .build(); assert_eq!( test_provider(&[ - (ENV_VAR_API_CALL_ATTEMPT_TIMEOUT, "04.000"), - (ENV_VAR_API_CALL_TIMEOUT, "900012345.0") + (ENV_VAR_CONNECT_TIMEOUT, "3.1"), + (ENV_VAR_READ_TIMEOUT, "500"), + (ENV_VAR_OPERATION_ATTEMPT_TIMEOUT, "04.000"), + (ENV_VAR_OPERATION_TIMEOUT, "900012345.0") ]) .timeout_config() - .unwrap(), + .unwrap() + .build(), expected_timeouts ); } 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 2e6d6e137f..b219332cae 100644 --- a/aws/rust-runtime/aws-config/src/http_credential_provider.rs +++ b/aws/rust-runtime/aws-config/src/http_credential_provider.rs @@ -16,8 +16,6 @@ use aws_smithy_http::response::ParseStrictResponse; use aws_smithy_http::result::{SdkError, SdkSuccess}; use aws_smithy_http::retry::ClassifyRetry; use aws_smithy_types::retry::{ErrorKind, RetryKind}; -use aws_smithy_types::timeout; -use aws_smithy_types::tristate::TriState; use aws_types::credentials::CredentialsError; use aws_types::{credentials, Credentials}; @@ -80,7 +78,7 @@ impl HttpCredentialProvider { #[derive(Default)] pub(crate) struct Builder { provider_config: Option, - http_timeout_config: timeout::Http, + http_settings: Option, } impl Builder { @@ -89,36 +87,25 @@ impl Builder { self } - // read_timeout and connect_timeout accept options to enable easy pass through from - // other builders - pub(crate) fn read_timeout(mut self, read_timeout: Option) -> Self { - self.http_timeout_config = self - .http_timeout_config - .with_read_timeout(read_timeout.into()); - self - } - - pub(crate) fn connect_timeout(mut self, connect_timeout: Option) -> Self { - self.http_timeout_config = self - .http_timeout_config - .with_connect_timeout(connect_timeout.into()); + pub(crate) fn http_settings(mut self, http_settings: HttpSettings) -> Self { + self.http_settings = Some(http_settings); self } pub(crate) fn build(self, provider_name: &'static str, uri: Uri) -> HttpCredentialProvider { let provider_config = self.provider_config.unwrap_or_default(); - let default_timeout_config = timeout::Http::new() - .with_connect_timeout(TriState::Set(DEFAULT_CONNECT_TIMEOUT)) - .with_read_timeout(TriState::Set(DEFAULT_READ_TIMEOUT)); - let http_timeout_config = self - .http_timeout_config - .take_unset_from(default_timeout_config); - let http_settings = HttpSettings::default().with_http_timeout_config(http_timeout_config); + let http_settings = self.http_settings.unwrap_or_else(|| { + HttpSettings::builder() + .connect_timeout(DEFAULT_CONNECT_TIMEOUT) + .read_timeout(DEFAULT_READ_TIMEOUT) + .build() + }); let connector = expect_connector(provider_config.connector(&http_settings)); - let client = aws_smithy_client::Builder::new() + let mut client_builder = aws_smithy_client::Client::builder() .connector(connector) - .sleep_impl(provider_config.sleep()) - .build(); + .middleware(Identity::new()); + client_builder.set_sleep_impl(provider_config.sleep()); + let client = client_builder.build(); HttpCredentialProvider { uri, client, diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 9e1fd43c7d..4aae4679c6 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -28,7 +28,6 @@ use aws_smithy_http_tower::map_request::{ AsyncMapRequestLayer, AsyncMapRequestService, MapRequestLayer, MapRequestService, }; use aws_smithy_types::retry::{ErrorKind, RetryKind}; -use aws_smithy_types::timeout; use aws_types::os_shim_internal::{Env, Fs}; use bytes::Bytes; @@ -41,6 +40,7 @@ use crate::imds::client::token::TokenMiddleware; use crate::profile::ProfileParseError; use crate::provider_config::ProviderConfig; use crate::{profile, PKG_VERSION}; +use aws_sdk_sso::config::TimeoutConfig; use aws_smithy_client::http_connector::HttpSettings; mod token; @@ -48,8 +48,8 @@ mod token; // 6 hours const DEFAULT_TOKEN_TTL: Duration = Duration::from_secs(21_600); const DEFAULT_ATTEMPTS: u32 = 4; -const DEFAULT_CONNECT_TIMEOUT: Option = Some(Duration::from_secs(1)); -const DEFAULT_READ_TIMEOUT: Option = Some(Duration::from_secs(1)); +const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(1); +const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(1); fn user_agent() -> AwsUserAgent { AwsUserAgent::new_from_environment(Env::real(), ApiMetadata::new("imds", PKG_VERSION)) @@ -555,10 +555,11 @@ impl Builder { /// Build an IMDSv2 Client pub async fn build(self) -> Result { let config = self.config.unwrap_or_default(); - let http_timeout_config = timeout::Http::new() - .with_connect_timeout(self.connect_timeout.or(DEFAULT_CONNECT_TIMEOUT).into()) - .with_read_timeout(self.read_timeout.or(DEFAULT_READ_TIMEOUT).into()); - let http_settings = HttpSettings::default().with_http_timeout_config(http_timeout_config); + let timeout_config = TimeoutConfig::builder() + .connect_timeout(DEFAULT_CONNECT_TIMEOUT) + .read_timeout(DEFAULT_READ_TIMEOUT) + .build(); + let http_settings = HttpSettings::from_timeout_config(&timeout_config); let connector = expect_connector(config.connector(&http_settings)); let endpoint_source = self .endpoint @@ -567,7 +568,6 @@ impl Builder { let endpoint = Endpoint::immutable(endpoint); let retry_config = retry::Config::default() .with_max_attempts(self.max_attempts.unwrap_or(DEFAULT_ATTEMPTS)); - let timeout_config = timeout::Config::default(); let token_loader = token::TokenMiddleware::new( connector.clone(), config.time_source(), @@ -578,13 +578,13 @@ impl Builder { config.sleep(), ); let middleware = ImdsMiddleware { token_loader }; - let smithy_client = aws_smithy_client::Builder::new() + let mut smithy_builder = aws_smithy_client::Client::builder() .connector(connector.clone()) .middleware(middleware) - .sleep_impl(config.sleep()) - .build() - .with_retry_config(retry_config) - .with_timeout_config(timeout_config); + .retry_config(retry_config) + .timeout_config(timeout_config); + smithy_builder.set_sleep_impl(config.sleep()); + let smithy_client = smithy_builder.build(); let client = Client { inner: Arc::new(ClientInner { diff --git a/aws/rust-runtime/aws-config/src/imds/client/token.rs b/aws/rust-runtime/aws-config/src/imds/client/token.rs index ba14c092b8..846e4e118f 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/token.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/token.rs @@ -32,13 +32,13 @@ use aws_smithy_http::operation::Operation; use aws_smithy_http::operation::{Metadata, Request}; use aws_smithy_http::response::ParseStrictResponse; use aws_smithy_http_tower::map_request::MapRequestLayer; -use aws_smithy_types::timeout; use aws_types::os_shim_internal::TimeSource; use http::{HeaderValue, Uri}; use crate::cache::ExpiringCache; use crate::imds::client::{ImdsError, ImdsResponseRetryClassifier, TokenError}; +use aws_sdk_sso::config::TimeoutConfig; /// Token Refresh Buffer /// @@ -85,15 +85,16 @@ impl TokenMiddleware { endpoint: Endpoint, token_ttl: Duration, retry_config: retry::Config, - timeout_config: timeout::Config, + timeout_config: TimeoutConfig, sleep_impl: Option>, ) -> Self { - let inner_client = aws_smithy_client::Builder::new() + let mut inner_builder = aws_smithy_client::Client::builder() .connector(connector) - .sleep_impl(sleep_impl) - .build() - .with_retry_config(retry_config) - .with_timeout_config(timeout_config); + .middleware(MapRequestLayer::::default()) + .retry_config(retry_config) + .timeout_config(timeout_config); + inner_builder.set_sleep_impl(sleep_impl); + let inner_client = inner_builder.build(); let client = Arc::new(inner_client); Self { client, diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index 0f8fa39257..77890053b5 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -135,7 +135,7 @@ pub mod credential_process; pub(crate) mod parsing; // Re-export types from smithy-types -pub use aws_smithy_types::retry::RetryConfig; +pub use aws_smithy_types::retry; pub use aws_smithy_types::timeout; // Re-export types from aws-types @@ -174,7 +174,7 @@ mod loader { use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_client::http_connector::{HttpConnector, HttpSettings}; use aws_smithy_types::retry::RetryConfig; - use aws_smithy_types::timeout; + use aws_smithy_types::timeout::TimeoutConfig; use aws_types::app_name::AppName; use aws_types::credentials::{ProvideCredentials, SharedCredentialsProvider}; use aws_types::endpoint::ResolveAwsEndpoint; @@ -198,7 +198,7 @@ mod loader { region: Option>, retry_config: Option, sleep: Option>, - timeout_config: Option, + timeout_config: Option, provider_config: Option, http_connector: Option, } @@ -244,18 +244,19 @@ mod loader { /// ```no_run /// # use std::time::Duration; /// # async fn create_config() { - /// use aws_smithy_types::{timeout, tristate::TriState}; + /// use aws_smithy_types::timeout::TimeoutConfig; /// - /// let api_timeout_config = timeout::Api::new() - /// .with_call_timeout(TriState::Set(Duration::from_secs(1))); - /// let timeout_config = timeout::Config::new().with_api_timeouts(api_timeout_config); /// let config = aws_config::from_env() - /// .timeout_config(timeout_config) + /// .timeout_config( + /// TimeoutConfig::builder() + /// .operation_timeout(Duration::from_secs(5)) + /// .build() + /// ) /// .load() /// .await; /// # } /// ``` - pub fn timeout_config(mut self, timeout_config: timeout::Config) -> Self { + pub fn timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { self.timeout_config = Some(timeout_config); self } @@ -401,17 +402,6 @@ mod loader { self.sleep }; - let http_connector = if let Some(http_connector) = self.http_connector { - http_connector - } else { - let timeouts = self.timeout_config.clone().unwrap_or_default(); - let settings = HttpSettings::default() - .with_http_timeout_config(timeouts.http_timeouts()) - .with_tcp_timeout_config(timeouts.tcp_timeouts()); - let sleep_impl = sleep_impl.clone(); - HttpConnector::Prebuilt(default_connector(&settings, sleep_impl)) - }; - let timeout_config = if let Some(timeout_config) = self.timeout_config { timeout_config } else { @@ -421,6 +411,15 @@ mod loader { .await }; + let http_connector = if let Some(http_connector) = self.http_connector { + http_connector + } else { + HttpConnector::Prebuilt(default_connector( + &HttpSettings::from_timeout_config(&timeout_config), + sleep_impl.clone(), + )) + }; + let credentials_provider = if let Some(provider) = self.credentials_provider { provider } else { diff --git a/aws/rust-runtime/aws-config/src/profile/timeout_config.rs b/aws/rust-runtime/aws-config/src/profile/timeout_config.rs index f04d58b4f0..3e48a653dd 100644 --- a/aws/rust-runtime/aws-config/src/profile/timeout_config.rs +++ b/aws/rust-runtime/aws-config/src/profile/timeout_config.rs @@ -10,19 +10,15 @@ use crate::profile::Profile; use crate::provider_config::ProviderConfig; use aws_smithy_types::timeout; -use aws_smithy_types::tristate::TriState; use aws_types::os_shim_internal::{Env, Fs}; +use aws_sdk_sso::config::{TimeoutConfig, TimeoutConfigBuilder}; use std::time::Duration; -// Currently unsupported timeouts const PROFILE_VAR_CONNECT_TIMEOUT: &str = "connect_timeout"; -const PROFILE_VAR_TLS_NEGOTIATION_TIMEOUT: &str = "tls_negotiation_timeout"; const PROFILE_VAR_READ_TIMEOUT: &str = "read_timeout"; - -// Supported timeouts -const PROFILE_VAR_API_CALL_ATTEMPT_TIMEOUT: &str = "api_call_attempt_timeout"; -const PROFILE_VAR_API_CALL_TIMEOUT: &str = "api_call_timeout"; +const PROFILE_VAR_OPERATION_ATTEMPT_TIMEOUT: &str = "operation_attempt_timeout"; +const PROFILE_VAR_OPERATION_TIMEOUT: &str = "operation_timeout"; /// Load timeout configuration properties from a profile file /// @@ -36,15 +32,15 @@ const PROFILE_VAR_API_CALL_TIMEOUT: &str = "api_call_timeout"; /// **Sets timeouts for the `default` profile** /// ```ini /// [default] -/// api_call_attempt_timeout = 2 -/// api_call_timeout = 3 +/// operation_attempt_timeout = 2 +/// operation_timeout = 3 /// ``` /// -/// **Sets the `api_call_attempt_timeout` to 0.5 seconds _if and only if_ the `other` profile is selected.** +/// **Sets the `operation_attempt_timeout` to 0.5 seconds _if and only if_ the `other` profile is selected.** /// /// ```ini /// [profile other] -/// api_call_attempt_timeout = 0.5 +/// operation_attempt_timeout = 0.5 /// ``` /// /// This provider is part of the [default timeout config provider chain](crate::default_provider::timeout_config). @@ -104,7 +100,7 @@ impl ProfileFileTimeoutConfigProvider { } /// Attempt to create a new [`timeout::Config`](aws_smithy_types::timeout::Config) from a profile file. - pub async fn timeout_config(&self) -> Result { + pub async fn timeout_config(&self) -> Result { let profile = match super::parser::load(&self.fs, &self.env).await { Ok(profile) => profile, Err(err) => { @@ -129,55 +125,39 @@ impl ProfileFileTimeoutConfigProvider { ); } // return an empty config - return Ok(timeout::Config::new()); + return Ok(TimeoutConfigBuilder::new()); } }; - // Warn users that set unsupported timeouts in their profile - for timeout in [ + let mut builder = TimeoutConfig::builder(); + builder.set_connect_timeout(timeout_from_profile_var( + selected_profile, PROFILE_VAR_CONNECT_TIMEOUT, - PROFILE_VAR_TLS_NEGOTIATION_TIMEOUT, + )?); + builder.set_read_timeout(timeout_from_profile_var( + selected_profile, PROFILE_VAR_READ_TIMEOUT, - ] { - warn_if_unsupported_timeout_is_set(selected_profile, timeout); - } - - let api_call_attempt_timeout = construct_timeout_from_profile_var( + )?); + builder.set_operation_attempt_timeout(timeout_from_profile_var( selected_profile, - PROFILE_VAR_API_CALL_ATTEMPT_TIMEOUT, - )?; - let api_call_timeout = - construct_timeout_from_profile_var(selected_profile, PROFILE_VAR_API_CALL_TIMEOUT)?; - - let api_timeouts = timeout::Api::new() - .with_call_timeout(api_call_timeout) - .with_call_attempt_timeout(api_call_attempt_timeout); + PROFILE_VAR_OPERATION_ATTEMPT_TIMEOUT, + )?); + builder.set_operation_timeout(timeout_from_profile_var( + selected_profile, + PROFILE_VAR_OPERATION_TIMEOUT, + )?); - // Only API-related timeouts are currently supported - Ok(timeout::Config::new().with_api_timeouts(api_timeouts)) + Ok(builder) } } -fn construct_timeout_from_profile_var( +fn timeout_from_profile_var( profile: &Profile, var: &'static str, -) -> Result, timeout::ConfigError> { +) -> Result, timeout::ConfigError> { let profile_name = format!("aws profile [{}]", profile.name()); match profile.get(var) { - Some(timeout) => { - parse_str_as_timeout(timeout, var.into(), profile_name.into()).map(TriState::Set) - } - None => Ok(TriState::Unset), - } -} - -fn warn_if_unsupported_timeout_is_set(profile: &Profile, var: &'static str) { - if profile.get(var).is_some() { - tracing::warn!( - "Profile '{}' set {} timeout but that feature is currently unimplemented so the setting will be ignored. \ - To help us prioritize support for this feature, please upvote aws-sdk-rust#151 (https://github.com/awslabs/aws-sdk-rust/issues/151)", - profile.name(), - var - ) + Some(timeout) => parse_str_as_timeout(timeout, var.into(), profile_name.into()).map(Some), + None => Ok(None), } } diff --git a/aws/rust-runtime/aws-config/src/provider_config.rs b/aws/rust-runtime/aws-config/src/provider_config.rs index 3acb94cf4e..e32cc74f49 100644 --- a/aws/rust-runtime/aws-config/src/provider_config.rs +++ b/aws/rust-runtime/aws-config/src/provider_config.rs @@ -250,8 +250,8 @@ impl ProviderConfig { C::Error: Into>, { let connector_fn = move |settings: &HttpSettings, sleep: Option>| { - let mut builder = aws_smithy_client::hyper_ext::Adapter::builder() - .timeout(&settings.http_timeout_config); + let mut builder = + aws_smithy_client::hyper_ext::Adapter::builder().http_settings(settings.clone()); if let Some(sleep) = sleep { builder = builder.sleep_impl(sleep); }; diff --git a/aws/rust-runtime/aws-config/src/sso.rs b/aws/rust-runtime/aws-config/src/sso.rs index eb04f98df1..8c3b4ea948 100644 --- a/aws/rust-runtime/aws-config/src/sso.rs +++ b/aws/rust-runtime/aws-config/src/sso.rs @@ -41,10 +41,11 @@ impl crate::provider_config::ProviderConfig { use crate::connector::expect_connector; use aws_smithy_client::http_connector::HttpSettings; - aws_smithy_client::Builder::<(), SsoMiddleware>::new() + let mut client_builder = aws_smithy_client::Client::builder() .connector(expect_connector(self.connector(&HttpSettings::default()))) - .sleep_impl(self.sleep()) - .build() + .middleware(SsoMiddleware::default()); + client_builder.set_sleep_impl(self.sleep()); + client_builder.build() } } diff --git a/aws/rust-runtime/aws-config/src/sts.rs b/aws/rust-runtime/aws-config/src/sts.rs index 5270b8f8b7..802850c008 100644 --- a/aws/rust-runtime/aws-config/src/sts.rs +++ b/aws/rust-runtime/aws-config/src/sts.rs @@ -5,23 +5,24 @@ //! Credential provider augmentation through the AWS Security Token Service (STS). -mod assume_role; +use crate::connector::expect_connector; +use aws_sdk_sts::middleware::DefaultMiddleware; +use aws_smithy_client::erase::DynConnector; +use aws_smithy_client::http_connector::HttpSettings; +use aws_smithy_client::Client; pub(crate) mod util; -use crate::connector::expect_connector; pub use assume_role::{AssumeRoleProvider, AssumeRoleProviderBuilder}; -use aws_sdk_sts::middleware::DefaultMiddleware; -use aws_smithy_client::erase::DynConnector; -use aws_smithy_client::http_connector::HttpSettings; -use aws_smithy_client::{Builder, Client}; +mod assume_role; impl crate::provider_config::ProviderConfig { pub(crate) fn sts_client(&self) -> Client { - Builder::<(), DefaultMiddleware>::new() + let mut builder = Client::builder() .connector(expect_connector(self.connector(&HttpSettings::default()))) - .sleep_impl(self.sleep()) - .build() + .middleware(DefaultMiddleware::default()); + builder.set_sleep_impl(self.sleep()); + builder.build() } } diff --git a/aws/rust-runtime/aws-config/src/sts/assume_role.rs b/aws/rust-runtime/aws-config/src/sts/assume_role.rs index f9c98103b3..c33f3a11d7 100644 --- a/aws/rust-runtime/aws-config/src/sts/assume_role.rs +++ b/aws/rust-runtime/aws-config/src/sts/assume_role.rs @@ -175,11 +175,11 @@ impl AssumeRoleProviderBuilder { let conn = conf .connector(&HttpSettings::default()) .expect("A connector must be provided"); - let client = aws_smithy_client::Builder::new() + let mut client_builder = aws_smithy_client::Client::builder() .connector(conn) - .middleware(DefaultMiddleware::new()) - .sleep_impl(conf.sleep()) - .build(); + .middleware(DefaultMiddleware::new()); + client_builder.set_sleep_impl(conf.sleep()); + let client = client_builder.build(); let session_name = self .session_name diff --git a/aws/rust-runtime/aws-types/src/sdk_config.rs b/aws/rust-runtime/aws-types/src/sdk_config.rs index 98ce157cb2..08849fe6ee 100644 --- a/aws/rust-runtime/aws-types/src/sdk_config.rs +++ b/aws/rust-runtime/aws-types/src/sdk_config.rs @@ -14,7 +14,7 @@ use std::sync::Arc; use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_client::http_connector::HttpConnector; use aws_smithy_types::retry::RetryConfig; -use aws_smithy_types::timeout; +use aws_smithy_types::timeout::TimeoutConfig; use crate::app_name::AppName; use crate::credentials::SharedCredentialsProvider; @@ -30,7 +30,7 @@ pub struct SdkConfig { endpoint_resolver: Option>, retry_config: Option, sleep_impl: Option>, - timeout_config: Option, + timeout_config: Option, http_connector: Option, } @@ -47,7 +47,7 @@ pub struct Builder { endpoint_resolver: Option>, retry_config: Option, sleep_impl: Option>, - timeout_config: Option, + timeout_config: Option, http_connector: Option, } @@ -171,7 +171,7 @@ impl Builder { self } - /// Set the [`timeout::Config`](aws_smithy_types::timeout::Config) for the builder + /// Set the [`TimeoutConfig`] for the builder /// /// _Note:_ Timeouts require a sleep implementation in order to work. /// When enabling timeouts, be sure to set one with [Self::sleep_impl] or @@ -182,21 +182,22 @@ impl Builder { /// ```rust /// # use std::time::Duration; /// use aws_types::SdkConfig; - /// use aws_smithy_types::{timeout, tristate::TriState}; - /// - /// let api_timeout_config = timeout::Api::new() - /// .with_call_attempt_timeout(TriState::Set(Duration::from_secs(2))) - /// .with_call_timeout(TriState::Set(Duration::from_secs(5))); - /// let timeout_config = timeout::Config::new() - /// .with_api_timeouts(api_timeout_config); - /// let config = SdkConfig::builder().timeout_config(timeout_config).build(); + /// use aws_smithy_types::timeout::TimeoutConfig; + /// + /// let timeout_config = TimeoutConfig::builder() + /// .operation_attempt_timeout(Duration::from_secs(2)) + /// .operation_timeout(Duration::from_secs(5)) + /// .build(); + /// let config = SdkConfig::builder() + /// .timeout_config(timeout_config) + /// .build(); /// ``` - pub fn timeout_config(mut self, timeout_config: timeout::Config) -> Self { + pub fn timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { self.set_timeout_config(Some(timeout_config)); self } - /// Set the [`timeout::Config`](aws_smithy_types::timeout::Config) for the builder + /// Set the [`TimeoutConfig`] for the builder /// /// _Note:_ Timeouts require a sleep implementation in order to work. /// When enabling timeouts, be sure to set one with [Self::sleep_impl] or @@ -206,14 +207,13 @@ impl Builder { /// ```rust /// # use std::time::Duration; /// use aws_types::sdk_config::{SdkConfig, Builder}; - /// use aws_smithy_types::{timeout, tristate::TriState}; + /// use aws_smithy_types::timeout::TimeoutConfig; /// /// fn set_preferred_timeouts(builder: &mut Builder) { - /// let api_timeout_config = timeout::Api::new() - /// .with_call_attempt_timeout(TriState::Set(Duration::from_secs(2))) - /// .with_call_timeout(TriState::Set(Duration::from_secs(5))); - /// let timeout_config = timeout::Config::new() - /// .with_api_timeouts(api_timeout_config); + /// let timeout_config = TimeoutConfig::builder() + /// .operation_attempt_timeout(Duration::from_secs(2)) + /// .operation_timeout(Duration::from_secs(5)) + /// .build(); /// builder.set_timeout_config(Some(timeout_config)); /// } /// @@ -221,7 +221,7 @@ impl Builder { /// set_preferred_timeouts(&mut builder); /// let config = builder.build(); /// ``` - pub fn set_timeout_config(&mut self, timeout_config: Option) -> &mut Self { + pub fn set_timeout_config(&mut self, timeout_config: Option) -> &mut Self { self.timeout_config = timeout_config; self } @@ -403,7 +403,7 @@ impl SdkConfig { } /// Configured timeout config - pub fn timeout_config(&self) -> Option<&timeout::Config> { + pub fn timeout_config(&self) -> Option<&TimeoutConfig> { self.timeout_config.as_ref() } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt index 8fca907308..536226dd5c 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt @@ -50,6 +50,7 @@ private class Types(runtimeConfig: RuntimeConfig) { val dynMiddleware = RuntimeType("DynMiddleware", smithyClientDep, "aws_smithy_client::erase") val smithyConnector = RuntimeType("SmithyConnector", smithyClientDep, "aws_smithy_client::bounds") val retryConfig = RuntimeType("RetryConfig", smithyTypesDep, "aws_smithy_types::retry") + val timeoutConfig = RuntimeType("TimeoutConfig", smithyTypesDep, "aws_smithy_types::timeout") val connectorError = RuntimeType("ConnectorError", smithyHttpDep, "aws_smithy_http::result") } @@ -137,6 +138,7 @@ private class AwsFluentClientExtensions(types: Types) { "DynMiddleware" to types.dynMiddleware, "Middleware" to types.defaultMiddleware, "RetryConfig" to types.retryConfig, + "TimeoutConfig" to types.timeoutConfig, "SmithyConnector" to types.smithyConnector, "aws_smithy_client" to types.awsSmithyClient, "aws_types" to types.awsTypes, @@ -154,15 +156,13 @@ private class AwsFluentClientExtensions(types: Types) { E: Into<#{ConnectorError}>, { let retry_config = conf.retry_config().cloned().unwrap_or_else(#{RetryConfig}::disabled); - let timeout_config = conf.timeout_config().cloned().unwrap_or_default(); + let timeout_config = conf.timeout_config().cloned().unwrap_or_else(#{TimeoutConfig}::disabled); let mut builder = #{aws_smithy_client}::Builder::new() .connector(#{DynConnector}::new(conn)) - .middleware(#{DynMiddleware}::new(#{Middleware}::new())); - builder.set_retry_config(retry_config.into()); - builder.set_timeout_config(timeout_config); - if let Some(sleep_impl) = conf.sleep_impl() { - builder.set_sleep_impl(Some(sleep_impl)); - } + .middleware(#{DynMiddleware}::new(#{Middleware}::new())) + .retry_config(retry_config.into()) + .timeout_config(timeout_config); + builder.set_sleep_impl(conf.sleep_impl()); let client = builder.build(); Self { handle: std::sync::Arc::new(Handle { client, conf }) } } @@ -177,21 +177,17 @@ private class AwsFluentClientExtensions(types: Types) { ##[cfg(any(feature = "rustls", feature = "native-tls"))] 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_default(); + let timeout_config = conf.timeout_config().cloned().unwrap_or_else(#{TimeoutConfig}::disabled); 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."); } let mut builder = #{aws_smithy_client}::Builder::dyn_https() - .middleware(#{DynMiddleware}::new(#{Middleware}::new())); - builder.set_retry_config(retry_config.into()); - builder.set_timeout_config(timeout_config); - // the builder maintains a try-state. To avoid suppressing the warning when sleep is unset, - // only set it if we actually have a sleep impl. - if let Some(sleep_impl) = sleep_impl { - builder.set_sleep_impl(Some(sleep_impl)); - } + .middleware(#{DynMiddleware}::new(#{Middleware}::new())) + .retry_config(retry_config.into()) + .timeout_config(timeout_config); + builder.set_sleep_impl(sleep_impl); let client = builder.build(); Self { handle: std::sync::Arc::new(Handle { client, conf }) } @@ -249,7 +245,7 @@ private class AwsFluentClientDocs(private val coreCodegenContext: CoreCodegenCon /// ``` /// **Constructing a client with custom configuration** /// ```rust,no_run - /// use #{aws_config}::RetryConfig; + /// use #{aws_config}::retry::RetryConfig; /// ## async fn docs() { /// let shared_config = #{aws_config}::load_from_env().await; /// let config = $crateName::config::Builder::from(&shared_config) diff --git a/aws/sdk/integration-tests/dynamodb/tests/timeouts.rs b/aws/sdk/integration-tests/dynamodb/tests/timeouts.rs index 1b2740d759..170822acc8 100644 --- a/aws/sdk/integration-tests/dynamodb/tests/timeouts.rs +++ b/aws/sdk/integration-tests/dynamodb/tests/timeouts.rs @@ -7,9 +7,7 @@ use aws_sdk_dynamodb::types::SdkError; use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; use aws_smithy_client::never::NeverConnector; use aws_smithy_types::retry::RetryConfig; -use aws_smithy_types::timeout; -use aws_smithy_types::timeout::Api; -use aws_smithy_types::tristate::TriState; +use aws_smithy_types::timeout::TimeoutConfig; use aws_types::credentials::SharedCredentialsProvider; use aws_types::region::Region; use aws_types::{Credentials, SdkConfig}; @@ -32,9 +30,11 @@ async fn api_call_timeout_retries() { .credentials_provider(SharedCredentialsProvider::new(Credentials::new( "stub", "stub", None, None, "test", ))) - .timeout_config(timeout::Config::new().with_api_timeouts( - Api::new().with_call_attempt_timeout(TriState::Set(Duration::new(123, 0))), - )) + .timeout_config( + TimeoutConfig::builder() + .operation_attempt_timeout(Duration::new(123, 0)) + .build(), + ) .retry_config(RetryConfig::standard()) .sleep_impl(Arc::new(InstantSleep)) .build(); @@ -62,18 +62,19 @@ async fn api_call_timeout_retries() { #[tokio::test] async fn no_retries_on_operation_timeout() { let conn = NeverConnector::new(); - let conf = - SdkConfig::builder() - .region(Region::new("us-east-2")) - .credentials_provider(SharedCredentialsProvider::new(Credentials::new( - "stub", "stub", None, None, "test", - ))) - .timeout_config(timeout::Config::new().with_api_timeouts( - Api::new().with_call_timeout(TriState::Set(Duration::new(123, 0))), - )) - .retry_config(RetryConfig::standard()) - .sleep_impl(Arc::new(InstantSleep)) - .build(); + let conf = SdkConfig::builder() + .region(Region::new("us-east-2")) + .credentials_provider(SharedCredentialsProvider::new(Credentials::new( + "stub", "stub", None, None, "test", + ))) + .timeout_config( + TimeoutConfig::builder() + .operation_timeout(Duration::new(123, 0)) + .build(), + ) + .retry_config(RetryConfig::standard()) + .sleep_impl(Arc::new(InstantSleep)) + .build(); let client = aws_sdk_dynamodb::Client::from_conf_conn( aws_sdk_dynamodb::Config::new(&conf), conn.clone(), diff --git a/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs b/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs index 0ff185734f..1526a6d32a 100644 --- a/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs +++ b/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_config::RetryConfig; +use aws_config::retry::RetryConfig; use aws_sdk_s3::model::{ CompressionType, CsvInput, CsvOutput, ExpressionType, FileHeaderInfo, InputSerialization, OutputSerialization, @@ -13,8 +13,7 @@ use aws_smithy_async::assert_elapsed; use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; use aws_smithy_client::never::NeverConnector; use aws_smithy_http::result::SdkError; -use aws_smithy_types::timeout; -use aws_smithy_types::tristate::TriState; +use aws_smithy_types::timeout::TimeoutConfig; use std::fmt::Debug; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -78,9 +77,9 @@ async fn timeout_test(sleep_impl: Arc) -> Result<(), Box) -> Result<(), Box) -> Result<(), Box = Arc::new(TokioSleep::new()); let config = Config::builder() .region(region) @@ -64,6 +63,6 @@ async fn test_timeout_service_ends_request_that_never_completes() { .await .unwrap_err(); - assert_eq!(format!("{:?}", err), "TimeoutError(RequestTimeoutError { kind: \"API call (all attempts including retries)\", duration: 500ms })"); + assert_eq!(format!("{:?}", err), "TimeoutError(RequestTimeoutError { kind: \"operation timeout (all attempts including retries)\", duration: 500ms })"); assert_elapsed!(now, std::time::Duration::from_secs_f32(0.5)); } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt index 0f45842289..2df6468810 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt @@ -25,7 +25,7 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co "AsyncSleep" to sleepModule.member("AsyncSleep"), "RetryConfig" to retryConfig.member("RetryConfig"), "Sleep" to sleepModule.member("Sleep"), - "TimeoutConfig" to timeoutModule.member("Config"), + "TimeoutConfig" to timeoutModule.member("TimeoutConfig"), ) override fun section(section: ServiceConfig) = writable { @@ -172,12 +172,11 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// ```no_run /// ## use std::time::Duration; /// use $moduleUseName::config::Config; - /// use aws_smithy_types::{timeout, tristate::TriState}; + /// use aws_smithy_types::timeout::TimeoutConfig; /// - /// let api_timeouts = timeout::Api::new() - /// .with_call_attempt_timeout(TriState::Set(Duration::from_secs(1))); - /// let timeout_config = timeout::Config::new() - /// .with_api_timeouts(api_timeouts); + /// let timeout_config = TimeoutConfig::builder() + /// .operation_attempt_timeout(Duration::from_secs(1)) + /// .build(); /// let config = Config::builder().timeout_config(timeout_config).build(); /// ``` pub fn timeout_config(mut self, timeout_config: #{TimeoutConfig}) -> Self { @@ -195,10 +194,9 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// use aws_smithy_types::{timeout, tristate::TriState}; /// /// fn set_request_timeout(builder: &mut Builder) { - /// let api_timeouts = timeout::Api::new() - /// .with_call_attempt_timeout(TriState::Set(Duration::from_secs(1))); - /// let timeout_config = timeout::Config::new() - /// .with_api_timeouts(api_timeouts); + /// let timeout_config = TimeoutConfig::builder() + /// .operation_attempt_timeout(Duration::from_secs(1)) + /// .build(); /// builder.set_timeout_config(Some(timeout_config)); /// } /// @@ -234,7 +232,7 @@ class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) """ pub use #{retry}::RetryConfig; pub use #{sleep}::AsyncSleep; - pub use #{timeout}::Config as TimeoutConfig; + pub use #{timeout}::TimeoutConfig; """, "retry" to smithyTypesRetry(runtimeConfig), "sleep" to smithyAsyncRtSleep(runtimeConfig), diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index a9f5586fc1..b4b5707e68 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -3,13 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -use std::sync::Arc; - use crate::{bounds, erase, retry, Client}; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; -use aws_smithy_types::timeout; +use aws_smithy_types::timeout::TimeoutConfig; +use std::sync::Arc; /// A builder that provides more customization options when constructing a [`Client`]. /// @@ -25,7 +24,7 @@ pub struct Builder { // the client. standard_retry_config: Option, retry_policy: R, - timeout_config: timeout::Config, + timeout_config: TimeoutConfig, sleep_impl: Option>, } @@ -40,7 +39,7 @@ where middleware: Default::default(), standard_retry_config: Some(retry::Config::default()), retry_policy: Default::default(), - timeout_config: Default::default(), + timeout_config: TimeoutConfig::disabled(), sleep_impl: default_async_sleep(), } } @@ -200,19 +199,34 @@ impl Builder { impl Builder { /// Set the standard retry policy's configuration. - pub fn set_retry_config(&mut self, config: retry::Config) { + pub fn set_retry_config(&mut self, config: retry::Config) -> &mut Self { self.standard_retry_config = Some(config.clone()); self.retry_policy.with_config(config); + self + } + + /// Set the standard retry policy's configuration. + pub fn retry_config(mut self, config: retry::Config) -> Self { + self.set_retry_config(config); + self } /// Set a timeout config for the builder - pub fn set_timeout_config(&mut self, timeout_config: timeout::Config) { + pub fn set_timeout_config(&mut self, timeout_config: TimeoutConfig) -> &mut Self { self.timeout_config = timeout_config; + self + } + + /// Set a timeout config for the builder + pub fn timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { + self.timeout_config = timeout_config; + self } /// Set the [`AsyncSleep`] function that the [`Client`] will use to create things like timeout futures. - pub fn set_sleep_impl(&mut self, async_sleep: Option>) { + pub fn set_sleep_impl(&mut self, async_sleep: Option>) -> &mut Self { self.sleep_impl = async_sleep; + self } /// Set the [`AsyncSleep`] function that the [`Client`] will use to create things like timeout futures. @@ -319,8 +333,6 @@ mod tests { use super::*; use crate::never::NeverConnector; use aws_smithy_async::rt::sleep::Sleep; - use aws_smithy_types::timeout; - use aws_smithy_types::tristate::TriState; use std::panic::{self, AssertUnwindSafe}; use std::time::Duration; @@ -361,9 +373,9 @@ mod tests { .middleware(tower::layer::util::Identity::new()); builder.set_sleep_impl(None); - let timeout_config = timeout::Config::new().with_http_timeouts( - timeout::Http::new().with_connect_timeout(TriState::Set(Duration::from_secs(1))), - ); + let timeout_config = TimeoutConfig::builder() + .connect_timeout(Duration::from_secs(1)) + .build(); assert!(timeout_config.has_timeouts()); builder.set_timeout_config(timeout_config); @@ -408,9 +420,9 @@ mod tests { .connector(NeverConnector::new()) .middleware(tower::layer::util::Identity::new()); - let timeout_config = timeout::Config::new().with_http_timeouts( - timeout::Http::new().with_connect_timeout(TriState::Set(Duration::from_secs(1))), - ); + let timeout_config = TimeoutConfig::builder() + .connect_timeout(Duration::from_secs(1)) + .build(); assert!(timeout_config.has_timeouts()); builder.set_timeout_config(timeout_config); diff --git a/rust-runtime/aws-smithy-client/src/http_connector.rs b/rust-runtime/aws-smithy-client/src/http_connector.rs index 4cf30f52c5..8112f8b96d 100644 --- a/rust-runtime/aws-smithy-client/src/http_connector.rs +++ b/rust-runtime/aws-smithy-client/src/http_connector.rs @@ -8,7 +8,8 @@ use crate::erase::DynConnector; use aws_smithy_async::rt::sleep::AsyncSleep; -use aws_smithy_types::timeout; +use aws_smithy_types::timeout::TimeoutConfig; +use std::time::Duration; use std::{fmt::Debug, sync::Arc}; /// Type alias for a Connector factory function. @@ -55,26 +56,99 @@ impl HttpConnector { } } -/// HttpSettings for HTTP Connectors +/// Builder for [`HttpSettings`]. #[non_exhaustive] #[derive(Default, Debug)] -pub struct HttpSettings { - /// Timeout configuration used when making HTTP connections - pub http_timeout_config: timeout::Http, - /// Timeout configuration used when creating TCP connections - pub tcp_timeout_config: timeout::Tcp, +pub struct HttpSettingsBuilder { + connect_timeout: Option, + read_timeout: Option, } -impl HttpSettings { - /// Set the HTTP timeouts to be used when making HTTP connections - pub fn with_http_timeout_config(mut self, http_timeout_config: timeout::Http) -> Self { - self.http_timeout_config = http_timeout_config; +impl HttpSettingsBuilder { + /// Creates a new builder. + pub fn new() -> Self { + Default::default() + } + + /// Sets the connect timeout that should be used. + /// + /// The connect timeout is a limit on the amount of time it takes to initiate a socket connection. + pub fn connect_timeout(mut self, connect_timeout: Duration) -> Self { + self.connect_timeout = Some(connect_timeout); + self + } + + /// Sets the connect timeout that should be used. + /// + /// The connect timeout is a limit on the amount of time it takes to initiate a socket connection. + pub fn set_connect_timeout(&mut self, connect_timeout: Option) -> &mut Self { + self.connect_timeout = connect_timeout; self } - /// Set the TCP timeouts to be used when creating TCP connections - pub fn with_tcp_timeout_config(mut self, tcp_timeout_config: timeout::Tcp) -> Self { - self.tcp_timeout_config = tcp_timeout_config; + /// Sets the read timeout that should be used. + /// + /// The read timeout is the limit on the amount of time it takes to read the first byte of a response + /// from the time the request is initiated. + pub fn read_timeout(mut self, read_timeout: Duration) -> Self { + self.read_timeout = Some(read_timeout); self } + + /// Sets the read timeout that should be used. + /// + /// The read timeout is the limit on the amount of time it takes to read the first byte of a response + /// from the time the request is initiated. + pub fn set_read_timeout(&mut self, read_timeout: Option) -> &mut Self { + self.read_timeout = read_timeout; + self + } + + /// Builds the [`HttpSettings`]. + pub fn build(self) -> HttpSettings { + HttpSettings { + connect_timeout: self.connect_timeout, + read_timeout: self.read_timeout, + } + } +} + +/// Settings for HTTP Connectors +#[non_exhaustive] +#[derive(Clone, Default, Debug)] +pub struct HttpSettings { + connect_timeout: Option, + read_timeout: Option, +} + +impl HttpSettings { + /// Returns a builder for `HttpSettings`. + pub fn builder() -> HttpSettingsBuilder { + Default::default() + } + + /// Returns the connect timeout that should be used. + /// + /// The connect timeout is a limit on the amount of time it takes to initiate a socket connection. + pub fn connect_timeout(&self) -> Option { + self.connect_timeout + } + + /// Returns the read timeout that should be used. + /// + /// The read timeout is the limit on the amount of time it takes to read the first byte of a response + /// from the time the request is initiated. + pub fn read_timeout(&self) -> Option { + self.read_timeout + } + + // This function may be removed/refactored in the future if other non-timeout + // properties are added to the `HttpSettings` struct. + #[doc(hidden)] + pub fn from_timeout_config(timeout_config: &TimeoutConfig) -> Self { + Self { + connect_timeout: timeout_config.connect_timeout(), + read_timeout: timeout_config.read_timeout(), + } + } } diff --git a/rust-runtime/aws-smithy-client/src/hyper_ext.rs b/rust-runtime/aws-smithy-client/src/hyper_ext.rs index 0da4238a22..d2ccac54e0 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -9,10 +9,13 @@ //! [`Client`](crate::Client). //! //! # Examples +//! //! ### Construct a Smithy Client with Hyper and Rustls +//! //! In the basic case, customers should not need to use this module. A default implementation of Hyper //! with `rustls` will be constructed during client creation. However, if you are creating a Smithy //! [`Client`](crate::Client), directly, use the `https()` method to match the default behavior: +//! //! ```no_run //! use aws_smithy_client::Builder; //! use aws_smithy_client::erase::DynConnector; @@ -23,16 +26,23 @@ //! ``` //! //! ### Create a Hyper client with a custom timeout -//! One common use case for constructing a connector directly is setting `CONNECT` timeouts. Since the -//! internal connector is cheap to clone, you can also use this to share a connector between multiple services. +//! +//! One common use case for constructing a connector directly is setting socket connect or HTTP read timeouts. +//! Since the internal connector is cheap to clone, you can also use this to share a connector between multiple services. +//! //! ```no_run //! use std::time::Duration; //! use aws_smithy_client::{Client, conns, hyper_ext}; //! use aws_smithy_client::erase::DynConnector; -//! use aws_smithy_types::timeout; +//! use aws_smithy_client::http_connector::HttpSettings; //! -//! let timeout = timeout::Http::new().with_connect_timeout(Some(Duration::from_secs(1)).into()); -//! let connector = hyper_ext::Adapter::builder().timeout(&timeout).build(conns::https()); +//! let connector = hyper_ext::Adapter::builder() +//! .http_settings( +//! HttpSettings::builder() +//! .connect_timeout(Duration::from_secs(5)) +//! .build() +//! ) +//! .build(conns::https()); //! // Replace this with your middleware //! type MyMiddleware = tower::layer::util::Identity; //! // once you have a connector, use it to construct a Smithy client: @@ -52,15 +62,13 @@ use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; use aws_smithy_types::retry::ErrorKind; -use aws_smithy_types::timeout; -use aws_smithy_types::tristate::TriState; +use self::timeout_middleware::{ConnectTimeout, HttpReadTimeout, HttpTimeoutError}; use crate::erase::DynConnector; +use crate::http_connector::HttpSettings; use crate::never::stream::EmptyStream; use crate::Builder as ClientBuilder; -use self::timeout_middleware::{ConnectTimeout, HttpReadTimeout, HttpTimeoutError}; - /// Adapter from a [`hyper::Client`](hyper::Client) to a connector usable by a Smithy [`Client`](crate::Client). /// /// This adapter also enables TCP `CONNECT` and HTTP `READ` timeouts via [`Adapter::builder`]. For examples @@ -160,7 +168,6 @@ fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option None } -#[derive(Default, Debug)] /// Builder for [`hyper_ext::Adapter`](Adapter) /// /// Unlike a Smithy client, the [`tower::Service`] inside a [`hyper_ext::Adapter`](Adapter) is actually a service that @@ -181,10 +188,11 @@ fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option /// // Replace `Identity` with your middleware implementation: /// let client = aws_smithy_client::Client::::new(DynConnector::new(hyper_connector)); /// ``` +#[derive(Default, Debug)] pub struct Builder { - http_timeout_config: timeout::Http, - sleep: Option>, - client_builder: hyper::client::Builder, + http_settings: Option, + sleep_impl: Option>, + client_builder: Option, } impl Builder { @@ -197,63 +205,88 @@ impl Builder { C::Future: Unpin + Send + 'static, C::Error: Into, { + let client_builder = self.client_builder.unwrap_or_default(); + let sleep_impl = self.sleep_impl.or_else(default_async_sleep); + let (connect_timeout, read_timeout) = self + .http_settings + .map(|c| (c.connect_timeout(), c.read_timeout())) + .unwrap_or((None, None)); + // if we are using Hyper, Tokio must already be enabled so we can fallback to Tokio. - let sleep = self.sleep.or_else(default_async_sleep); - let connector = match self.http_timeout_config.connect_timeout() { - TriState::Set(duration) => ConnectTimeout::new( + let connector = match connect_timeout { + Some(duration) => ConnectTimeout::new( connector, - sleep + sleep_impl .clone() - .expect("a sleep impl must be provided to use timeouts"), + .expect("a sleep impl must be provided in order to have a connect timeout"), duration, ), - // Some day, we could provide a default timeout if none is set. Today is not that day. - TriState::Unset | TriState::Disabled => ConnectTimeout::no_timeout(connector), + None => ConnectTimeout::no_timeout(connector), }; - let base = self.client_builder.build(connector); - let http_timeout = match self.http_timeout_config.read_timeout() { - TriState::Set(duration) => HttpReadTimeout::new( + let base = client_builder.build(connector); + let read_timeout = match read_timeout { + Some(duration) => HttpReadTimeout::new( base, - sleep + sleep_impl .clone() - .expect("a sleep impl must be provided to use timeouts"), + .expect("a sleep impl must be provided in order to have a read timeout"), duration, ), - // Some day, we could provide a default timeout if none is set. Today is not that day. - TriState::Unset | TriState::Disabled => HttpReadTimeout::no_timeout(base), + None => HttpReadTimeout::no_timeout(base), }; - Adapter(http_timeout) + Adapter(read_timeout) } /// Set the async sleep implementation used for timeouts /// /// Calling this is only necessary for testing or to use something other than /// [`aws_smithy_async::rt::sleep::default_async_sleep`]. - pub fn sleep_impl(self, sleep_impl: impl AsyncSleep + 'static) -> Self { - Self { - sleep: Some(Arc::new(sleep_impl)), - ..self - } + pub fn sleep_impl(mut self, sleep_impl: Arc) -> Self { + self.sleep_impl = Some(sleep_impl); + self } - /// Configure the timeout for the HyperAdapter + /// Set the async sleep implementation used for timeouts /// - /// When unset, the underlying adaptor will not use any timeouts. - pub fn timeout(self, http_timeout_config: &timeout::Http) -> Self { - Self { - http_timeout_config: http_timeout_config.clone(), - ..self - } + /// Calling this is only necessary for testing or to use something other than + /// [`aws_smithy_async::rt::sleep::default_async_sleep`]. + pub fn set_sleep_impl( + &mut self, + sleep_impl: Option>, + ) -> &mut Self { + self.sleep_impl = sleep_impl; + self + } + + /// Configure the HTTP settings for the `HyperAdapter` + pub fn http_settings(mut self, http_settings: HttpSettings) -> Self { + self.http_settings = Some(http_settings); + self + } + + /// Configure the HTTP settings for the `HyperAdapter` + pub fn set_http_settings(&mut self, http_settings: Option) -> &mut Self { + self.http_settings = http_settings; + self } /// Override the Hyper client [`Builder`](hyper::client::Builder) used to construct this client. /// /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. - pub fn hyper_builder(self, hyper_builder: hyper::client::Builder) -> Self { - Self { - client_builder: hyper_builder, - ..self - } + pub fn hyper_builder(mut self, hyper_builder: hyper::client::Builder) -> Self { + self.client_builder = Some(hyper_builder); + self + } + + /// Override the Hyper client [`Builder`](hyper::client::Builder) used to construct this client. + /// + /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. + pub fn set_hyper_builder( + &mut self, + hyper_builder: Option, + ) -> &mut Self { + self.client_builder = hyper_builder; + self } } @@ -527,18 +560,16 @@ mod timeout_middleware { #[cfg(test)] mod test { - use std::time::Duration; - - use tower::Service; - + use crate::http_connector::HttpSettings; + use crate::hyper_ext::Adapter; + use crate::never::{NeverConnected, NeverReplies}; use aws_smithy_async::assert_elapsed; use aws_smithy_async::rt::sleep::TokioSleep; use aws_smithy_http::body::SdkBody; - use aws_smithy_types::timeout; - use aws_smithy_types::tristate::TriState; - - use crate::hyper_ext::Adapter; - use crate::never::{NeverConnected, NeverReplies}; + use aws_smithy_types::timeout::TimeoutConfig; + use std::sync::Arc; + use std::time::Duration; + use tower::Service; #[allow(unused)] fn connect_timeout_is_correct() { @@ -551,11 +582,14 @@ mod timeout_middleware { #[tokio::test] async fn http_connect_timeout_works() { let inner = NeverConnected::new(); - let timeout = - timeout::Http::new().with_connect_timeout(TriState::Set(Duration::from_secs(1))); + let http_settings = HttpSettings::from_timeout_config( + &TimeoutConfig::builder() + .connect_timeout(Duration::from_secs(1)) + .build(), + ); let mut hyper = Adapter::builder() - .timeout(&timeout) - .sleep_impl(TokioSleep::new()) + .http_settings(http_settings) + .sleep_impl(Arc::new(TokioSleep::new())) .build(inner); let now = tokio::time::Instant::now(); tokio::time::pause(); @@ -583,12 +617,15 @@ mod timeout_middleware { #[tokio::test] async fn http_read_timeout_works() { let inner = NeverReplies::new(); - let timeout = timeout::Http::new() - .with_connect_timeout(TriState::Set(Duration::from_secs(1))) - .with_read_timeout(TriState::Set(Duration::from_secs(2))); + let http_settings = HttpSettings::from_timeout_config( + &TimeoutConfig::builder() + .connect_timeout(Duration::from_secs(1)) + .read_timeout(Duration::from_secs(2)) + .build(), + ); let mut hyper = Adapter::builder() - .timeout(&timeout) - .sleep_impl(TokioSleep::new()) + .http_settings(http_settings) + .sleep_impl(Arc::new(TokioSleep::new())) .build(inner); let now = tokio::time::Instant::now(); tokio::time::pause(); diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index 1ab3fb4f86..da8a2ef335 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -84,11 +84,6 @@ pub mod conns { crate::hyper_ext::Adapter>; } -use std::error::Error; -use std::sync::Arc; -use tower::{Layer, Service, ServiceBuilder, ServiceExt}; - -use crate::timeout::generate_timeout_service_params_from_timeout_config; use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_http::body::SdkBody; use aws_smithy_http::operation::Operation; @@ -98,6 +93,11 @@ use aws_smithy_http::retry::ClassifyRetry; use aws_smithy_http_tower::dispatch::DispatchLayer; use aws_smithy_http_tower::parse_response::ParseResponseLayer; use aws_smithy_types::retry::ProvideErrorKind; +use aws_smithy_types::timeout::TimeoutConfig; +use std::error::Error; +use std::sync::Arc; +use timeout::ClientTimeoutParams; +use tower::{Layer, Service, ServiceBuilder, ServiceExt}; /// Smithy service client. /// @@ -127,7 +127,7 @@ pub struct Client< connector: Connector, middleware: Middleware, retry_policy: RetryPolicy, - timeout_config: aws_smithy_types::timeout::Config, + timeout_config: TimeoutConfig, sleep_impl: Option>, } @@ -162,15 +162,12 @@ impl Client { impl Client { /// Set the client's timeout configuration. - pub fn set_timeout_config(&mut self, timeout_config: aws_smithy_types::timeout::Config) { + pub fn set_timeout_config(&mut self, timeout_config: TimeoutConfig) { self.timeout_config = timeout_config; } /// Set the client's timeout configuration. - pub fn with_timeout_config( - mut self, - timeout_config: aws_smithy_types::timeout::Config, - ) -> Self { + pub fn with_timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { self.set_timeout_config(timeout_config); self } @@ -237,18 +234,16 @@ where { let connector = self.connector.clone(); - let timeout_service_params = generate_timeout_service_params_from_timeout_config( - &self.timeout_config.api, - self.sleep_impl.clone(), - ); + let timeout_params = + ClientTimeoutParams::new(&self.timeout_config, self.sleep_impl.clone()); let svc = ServiceBuilder::new() - .layer(TimeoutLayer::new(timeout_service_params.api_call)) + .layer(TimeoutLayer::new(timeout_params.operation_timeout)) .retry( self.retry_policy .new_request_policy(self.sleep_impl.clone()), ) - .layer(TimeoutLayer::new(timeout_service_params.api_call_attempt)) + .layer(TimeoutLayer::new(timeout_params.operation_attempt_timeout)) .layer(ParseResponseLayer::::new()) // These layers can be considered as occurring in order. That is, first invoke the // customer-provided middleware, then dispatch dispatch over the wire. diff --git a/rust-runtime/aws-smithy-client/src/timeout.rs b/rust-runtime/aws-smithy-client/src/timeout.rs index c7a83fc0ad..489160a4b8 100644 --- a/rust-runtime/aws-smithy-client/src/timeout.rs +++ b/rust-runtime/aws-smithy-client/src/timeout.rs @@ -9,17 +9,18 @@ //! //! As timeout and HTTP configuration stabilizes, this will move to aws-types and become a part of //! HttpSettings. -use std::future::Future; -use std::pin::Pin; -use std::sync::Arc; -use std::task::{Context, Poll}; -use std::time::Duration; use crate::SdkError; use aws_smithy_async::future::timeout::Timeout; use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; use aws_smithy_http::operation::Operation; +use aws_smithy_types::timeout::TimeoutConfig; use pin_project_lite::pin_project; +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context, Poll}; +use std::time::Duration; use tower::Layer; #[derive(Debug)] @@ -36,11 +37,7 @@ impl RequestTimeoutError { impl std::fmt::Display for RequestTimeoutError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{} timeout occurred after {:?}", - self.kind, self.duration - ) + write!(f, "{} occurred after {:?}", self.kind, self.duration) } } @@ -59,40 +56,37 @@ pub struct TimeoutServiceParams { #[derive(Clone, Debug, Default)] /// A struct of structs containing everything needed to create new [`TimeoutService`]s -pub struct ClientTimeoutParams { +pub(crate) struct ClientTimeoutParams { /// Params used to create a new API call [`TimeoutService`] - pub(crate) api_call: Option, + pub(crate) operation_timeout: Option, /// Params used to create a new API call attempt [`TimeoutService`] - pub(crate) api_call_attempt: Option, + pub(crate) operation_attempt_timeout: Option, } -/// Convert a [`timeout::Api`](aws_smithy_types::timeout::Api) into an [`ClientTimeoutParams`] in order to create -/// the set of [`TimeoutService`]s needed by a [`crate::Client`] -pub fn generate_timeout_service_params_from_timeout_config( - api_timeout_config: &aws_smithy_types::timeout::Api, - async_sleep: Option>, -) -> ClientTimeoutParams { - if let Some(async_sleep) = async_sleep { - ClientTimeoutParams { - api_call: api_timeout_config - .call_timeout() - .map(|duration| TimeoutServiceParams { - duration, - kind: "API call (all attempts including retries)", - async_sleep: async_sleep.clone(), - }) - .into(), - api_call_attempt: api_timeout_config - .call_attempt_timeout() - .map(|duration| TimeoutServiceParams { - duration, - kind: "API call (single attempt)", - async_sleep: async_sleep.clone(), - }) - .into(), +impl ClientTimeoutParams { + pub fn new(timeout_config: &TimeoutConfig, async_sleep: Option>) -> Self { + if let Some(async_sleep) = async_sleep { + Self { + operation_timeout: timeout_config + .operation_timeout() + .map(|duration| TimeoutServiceParams { + duration, + kind: "operation timeout (all attempts including retries)", + async_sleep: async_sleep.clone(), + }) + .into(), + operation_attempt_timeout: timeout_config + .operation_attempt_timeout() + .map(|duration| TimeoutServiceParams { + duration, + kind: "operation attempt timeout (single attempt)", + async_sleep: async_sleep.clone(), + }) + .into(), + } + } else { + Default::default() } - } else { - Default::default() } } @@ -238,19 +232,16 @@ where #[cfg(test)] mod test { - use std::sync::Arc; - use std::time::Duration; - + use super::*; use crate::never::NeverService; - use crate::timeout::generate_timeout_service_params_from_timeout_config; use crate::{SdkError, TimeoutLayer}; - use aws_smithy_async::assert_elapsed; use aws_smithy_async::rt::sleep::{AsyncSleep, TokioSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::operation::{Operation, Request}; - use aws_smithy_types::tristate::TriState; - + use aws_smithy_types::timeout::TimeoutConfig; + use std::sync::Arc; + use std::time::Duration; use tower::{Service, ServiceBuilder, ServiceExt}; #[tokio::test] @@ -258,13 +249,13 @@ mod test { let req = Request::new(http::Request::new(SdkBody::empty())); let op = Operation::new(req, ()); let never_service: NeverService<_, (), _> = NeverService::new(); - let timeout_config = aws_smithy_types::timeout::Api::new() - .with_call_timeout(TriState::Set(Duration::from_secs_f32(0.25))); - let sleep_impl: Option> = Some(Arc::new(TokioSleep::new())); - let timeout_service_params = - generate_timeout_service_params_from_timeout_config(&timeout_config, sleep_impl); + let timeout_config = TimeoutConfig::builder() + .operation_timeout(Duration::from_secs_f32(0.25)) + .build(); + let sleep_impl: Arc = Arc::new(TokioSleep::new()); + let timeout_service_params = ClientTimeoutParams::new(&timeout_config, Some(sleep_impl)); let mut svc = ServiceBuilder::new() - .layer(TimeoutLayer::new(timeout_service_params.api_call)) + .layer(TimeoutLayer::new(timeout_service_params.operation_timeout)) .service(never_service); let now = tokio::time::Instant::now(); @@ -273,7 +264,7 @@ mod test { let err: SdkError> = svc.ready().await.unwrap().call(op).await.unwrap_err(); - assert_eq!(format!("{:?}", err), "TimeoutError(RequestTimeoutError { kind: \"API call (all attempts including retries)\", duration: 250ms })"); + assert_eq!(format!("{:?}", err), "TimeoutError(RequestTimeoutError { kind: \"operation timeout (all attempts including retries)\", duration: 250ms })"); assert_elapsed!(now, Duration::from_secs_f32(0.25)); } } diff --git a/rust-runtime/aws-smithy-types/src/lib.rs b/rust-runtime/aws-smithy-types/src/lib.rs index 14705db77e..3e7b599e15 100644 --- a/rust-runtime/aws-smithy-types/src/lib.rs +++ b/rust-runtime/aws-smithy-types/src/lib.rs @@ -21,7 +21,6 @@ pub mod endpoint; pub mod primitive; pub mod retry; pub mod timeout; -pub mod tristate; pub use crate::date_time::DateTime; diff --git a/rust-runtime/aws-smithy-types/src/timeout/api.rs b/rust-runtime/aws-smithy-types/src/timeout/api.rs deleted file mode 100644 index cd0134aa0a..0000000000 --- a/rust-runtime/aws-smithy-types/src/timeout/api.rs +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use crate::tristate::TriState; -use std::time::Duration; - -/// API timeouts used by Smithy `Client`s -#[non_exhaustive] -#[derive(Clone, PartialEq, Default, Debug)] -pub struct Api { - /// A limit on the amount of time it takes for the first byte to be sent over an established, - /// open connection and when the last byte is received from the service for a single attempt. - /// If you want to set a timeout for an entire request including retry attempts, - /// use [`Api::call_attempt`] instead. - call: TriState, - /// A limit on the amount of time it takes for request to complete. A single request may be - /// comprised of several attempts depending on an app's [`RetryConfig`](crate::retry::RetryConfig). If you want - /// to control timeouts for a single attempt, use [`Api::call`]. - call_attempt: TriState, -} - -impl Api { - /// Create a new API timeout config with no timeouts set - pub fn new() -> Self { - Default::default() - } - - /// Return this config's `call` timeout - pub fn call_timeout(&self) -> TriState { - self.call.clone() - } - - /// Mutate this `timeout::Api` config, setting the API call timeout - pub fn with_call_timeout(mut self, timeout: TriState) -> Self { - self.call = timeout; - self - } - - /// Return this config's `call_attempt` timeout - pub fn call_attempt_timeout(&self) -> TriState { - self.call_attempt.clone() - } - - /// Mutate this `timeout::Api` config, setting the API call single attempt timeout - pub fn with_call_attempt_timeout(mut self, timeout: TriState) -> Self { - self.call_attempt = timeout; - self - } - - /// Return true if any timeouts are intentionally set or disabled - pub fn has_timeouts(&self) -> bool { - !self.is_unset() - } - - /// Return true if all timeouts are unset - fn is_unset(&self) -> bool { - self.call.is_unset() && self.call_attempt.is_unset() - } - - /// Merges two API timeout configs together. - pub fn take_unset_from(self, other: Self) -> Self { - Self { - call: self.call.or(other.call), - call_attempt: self.call_attempt.or(other.call_attempt), - } - } -} - -impl From for Api { - fn from(config: super::Config) -> Self { - config.api - } -} diff --git a/rust-runtime/aws-smithy-types/src/timeout/config.rs b/rust-runtime/aws-smithy-types/src/timeout/config.rs index 5ced915b6e..9cbc6b4e0b 100644 --- a/rust-runtime/aws-smithy-types/src/timeout/config.rs +++ b/rust-runtime/aws-smithy-types/src/timeout/config.rs @@ -3,83 +3,114 @@ * SPDX-License-Identifier: Apache-2.0 */ -/// Top-level configuration for timeouts -/// -/// # Example -/// -/// ```rust -/// # use std::time::Duration; -/// -/// # fn main() { -/// use aws_smithy_types::{timeout, tristate::TriState}; -/// -/// let api_timeouts = timeout::Api::new() -/// .with_call_timeout(TriState::Set(Duration::from_secs(2))) -/// .with_call_attempt_timeout(TriState::Set(Duration::from_secs_f32(0.5))); -/// let timeout_config = timeout::Config::new() -/// .with_api_timeouts(api_timeouts); -/// -/// assert_eq!( -/// timeout_config.api.call_timeout(), -/// TriState::Set(Duration::from_secs(2)) -/// ); -/// -/// assert_eq!( -/// timeout_config.api.call_attempt_timeout(), -/// TriState::Set(Duration::from_secs_f32(0.5)) -/// ); -/// # } -/// ``` -#[derive(Clone, PartialEq, Default, Debug)] -pub struct Config { - /// API timeouts used by Smithy `Client`s - pub api: super::Api, - /// HTTP timeouts used by `DynConnector`s - pub http: super::Http, - /// TCP timeouts used by lower-level `DynConnector`s - pub tcp: super::Tcp, +use std::time::Duration; + +/// Builder for [`TimeoutConfig`]. +#[non_exhaustive] +#[derive(Clone, Debug, Default)] +pub struct TimeoutConfigBuilder { + connect_timeout: Option, + read_timeout: Option, + operation_timeout: Option, + operation_attempt_timeout: Option, } -impl Config { - /// Create a new `Config` with no timeouts set +impl TimeoutConfigBuilder { + /// Creates a new builder with no timeouts set. pub fn new() -> Self { Default::default() } - /// Return the API-related timeouts from this config - pub fn api_timeouts(&self) -> super::Api { - self.api.clone() + /// Sets the connect timeout. + /// + /// The connect timeout is a limit on the amount of time it takes to initiate a socket connection. + pub fn connect_timeout(mut self, connect_timeout: Duration) -> Self { + self.connect_timeout = Some(connect_timeout); + self } - /// Return the API-related timeouts from this config - pub fn http_timeouts(&self) -> super::Http { - self.http.clone() + /// Sets the connect timeout. + /// + /// The connect timeout is a limit on the amount of time it takes to initiate a socket connection. + pub fn set_connect_timeout(&mut self, connect_timeout: Option) -> &mut Self { + self.connect_timeout = connect_timeout; + self } - /// Return the API-related timeouts from this config - pub fn tcp_timeouts(&self) -> super::Tcp { - self.tcp.clone() + /// Sets the read timeout. + /// + /// The read timeout is the limit on the amount of time it takes to read the first byte of a response + /// from the time the request is initiated. + pub fn read_timeout(mut self, read_timeout: Duration) -> Self { + self.read_timeout = Some(read_timeout); + self + } + + /// Sets the read timeout. + /// + /// The read timeout is the limit on the amount of time it takes to read the first byte of a response + /// from the time the request is initiated. + pub fn set_read_timeout(&mut self, read_timeout: Option) -> &mut Self { + self.read_timeout = read_timeout; + self } - /// Consume an `Config` to create a new one, setting the API-related timeouts - pub fn with_api_timeouts(mut self, timeouts: super::Api) -> Self { - self.api = timeouts; + /// Sets the operation timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// The operation timeout is a limit on the total amount of time it takes for an operation to be + /// fully serviced, including the time for all retries that may have been attempted for it. + /// + /// If you want to set a timeout on individual retry attempts, then see [`Self::operation_attempt_timeout`] + /// or [`Self::set_operation_attempt_timeout`]. + pub fn operation_timeout(mut self, operation_timeout: Duration) -> Self { + self.operation_timeout = Some(operation_timeout); + self + } + + /// Sets the operation timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// The operation timeout is a limit on the total amount of time it takes for an operation to be + /// fully serviced, including the time for all retries that may have been attempted for it. + /// + /// If you want to set a timeout on individual retry attempts, then see [`Self::operation_attempt_timeout`] + /// or [`Self::set_operation_attempt_timeout`]. + pub fn set_operation_timeout(&mut self, operation_timeout: Option) -> &mut Self { + self.operation_timeout = operation_timeout; self } - /// Consume a `Config` to create a new one, setting HTTP-related timeouts - pub fn with_http_timeouts(mut self, timeouts: super::Http) -> Self { - self.http = timeouts; + /// Sets the operation attempt timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// When retries are enabled, then this setting makes it possible to set a timeout for individual + /// retry attempts (including the initial attempt) for an operation. + /// + /// If you want to set a timeout on the total time for an entire request including all of its retries, + /// then see [`Self::operation_timeout`] /// or [`Self::set_operation_timeout`]. + pub fn operation_attempt_timeout(mut self, operation_attempt_timeout: Duration) -> Self { + self.operation_attempt_timeout = Some(operation_attempt_timeout); self } - /// Consume a `Config` to create a new one, setting TCP-related timeouts - pub fn with_tcp_timeouts(mut self, timeouts: super::Tcp) -> Self { - self.tcp = timeouts; + /// Sets the operation attempt timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// When retries are enabled, then this setting makes it possible to set a timeout for individual + /// retry attempts (including the initial attempt) for an operation. + /// + /// If you want to set a timeout on individual retry attempts, then see [`Self::operation_attempt_timeout`] + /// or [`Self::set_operation_attempt_timeout`]. + pub fn set_operation_attempt_timeout( + &mut self, + operation_attempt_timeout: Option, + ) -> &mut Self { + self.operation_attempt_timeout = operation_attempt_timeout; self } - /// Merges two timeout configs together. + /// Merges two timeout config builders together. /// /// Values from `other` will only be used as a fallback for values /// from `self`. Useful for merging configs from different sources together when you want to @@ -89,30 +120,154 @@ impl Config { /// /// ```rust /// # use std::time::Duration; - /// # use aws_smithy_types::timeout; - /// # use aws_smithy_types::tristate::TriState; - /// let a = timeout::Config::new().with_api_timeouts( - /// timeout::Api::new().with_call_timeout(TriState::Set(Duration::from_secs(2))) - /// ); - /// let b = timeout::Config::new().with_api_timeouts( - /// timeout::Api::new().with_call_attempt_timeout(TriState::Set(Duration::from_secs(3))) - /// ); - /// let timeout_config = a.take_unset_from(b); + /// # use aws_smithy_types::timeout::TimeoutConfig; + /// let a = TimeoutConfig::builder() + /// .connect_timeout(Duration::from_secs(3)); + /// let b = TimeoutConfig::builder() + /// .connect_timeout(Duration::from_secs(5)) + /// .operation_timeout(Duration::from_secs(3)); + /// let timeout_config = a.take_unset_from(b).build(); + /// /// // A's value take precedence over B's value - /// assert_eq!(timeout_config.api.call_timeout(), TriState::Set(Duration::from_secs(2))); - /// // A never set a connect timeout so B's value was used - /// assert_eq!(timeout_config.api.call_attempt_timeout(), TriState::Set(Duration::from_secs(3))); + /// assert_eq!(timeout_config.connect_timeout(), Some(Duration::from_secs(3))); + /// // A never set an operation timeout so B's value is used + /// assert_eq!(timeout_config.operation_timeout(), Some(Duration::from_secs(3))); /// ``` pub fn take_unset_from(self, other: Self) -> Self { Self { - api: self.api.take_unset_from(other.api), - http: self.http.take_unset_from(other.http), - tcp: self.tcp.take_unset_from(other.tcp), + connect_timeout: self.connect_timeout.or(other.connect_timeout), + read_timeout: self.read_timeout.or(other.read_timeout), + operation_timeout: self.operation_timeout.or(other.operation_timeout), + operation_attempt_timeout: self + .operation_attempt_timeout + .or(other.operation_attempt_timeout), } } - /// Returns true if any of the possible timeouts are set + /// Builds a `TimeoutConfig`. + pub fn build(self) -> TimeoutConfig { + TimeoutConfig { + connect_timeout: self.connect_timeout, + read_timeout: self.read_timeout, + operation_timeout: self.operation_timeout, + operation_attempt_timeout: self.operation_attempt_timeout, + } + } +} + +impl From for TimeoutConfigBuilder { + fn from(timeout_config: TimeoutConfig) -> Self { + TimeoutConfigBuilder { + connect_timeout: timeout_config.connect_timeout, + read_timeout: timeout_config.read_timeout, + operation_timeout: timeout_config.operation_timeout, + operation_attempt_timeout: timeout_config.operation_attempt_timeout, + } + } +} + +/// Top-level configuration for timeouts +/// +/// # Example +/// +/// ```rust +/// # use std::time::Duration; +/// +/// # fn main() { +/// use aws_smithy_types::timeout::TimeoutConfig; +/// +/// let timeout_config = TimeoutConfig::builder() +/// .operation_timeout(Duration::from_secs(30)) +/// .operation_attempt_timeout(Duration::from_secs(10)) +/// .connect_timeout(Duration::from_secs(3)) +/// .build(); +/// +/// assert_eq!( +/// timeout_config.operation_timeout(), +/// Some(Duration::from_secs(30)) +/// ); +/// assert_eq!( +/// timeout_config.operation_attempt_timeout(), +/// Some(Duration::from_secs(10)) +/// ); +/// assert_eq!( +/// timeout_config.connect_timeout(), +/// Some(Duration::from_secs(3)) +/// ); +/// # } +/// ``` +#[non_exhaustive] +#[derive(Clone, PartialEq, Debug)] +pub struct TimeoutConfig { + connect_timeout: Option, + read_timeout: Option, + operation_timeout: Option, + operation_attempt_timeout: Option, +} + +impl TimeoutConfig { + /// Returns a builder to create a `TimeoutConfig`. + pub fn builder() -> TimeoutConfigBuilder { + TimeoutConfigBuilder::new() + } + + /// Returns a builder equivalent of this `TimeoutConfig`. + pub fn to_builder(&self) -> TimeoutConfigBuilder { + TimeoutConfigBuilder::from(self.clone()) + } + + /// Converts this `TimeoutConfig` into a builder. + pub fn into_builder(self) -> TimeoutConfigBuilder { + TimeoutConfigBuilder::from(self) + } + + /// Returns a timeout config with all timeouts disabled. + pub fn disabled() -> TimeoutConfig { + TimeoutConfig { + connect_timeout: None, + read_timeout: None, + operation_timeout: None, + operation_attempt_timeout: None, + } + } + + /// Returns this config's connect timeout. + /// + /// The connect timeout is a limit on the amount of time it takes to initiate a socket connection. + pub fn connect_timeout(&self) -> Option { + self.connect_timeout + } + + /// Returns this config's read timeout. + /// + /// The read timeout is the limit on the amount of time it takes to read the first byte of a response + /// from the time the request is initiated. + pub fn read_timeout(&self) -> Option { + self.read_timeout + } + + /// Returns this config's operation timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// The operation timeout is a limit on the total amount of time it takes for an operation to be + /// fully serviced, including the time for all retries that may have been attempted for it. + pub fn operation_timeout(&self) -> Option { + self.operation_timeout + } + + /// Returns this config's operation attempt timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// When retries are enabled, then this setting makes it possible to set a timeout for individual + /// retry attempts (including the initial attempt) for an operation. + pub fn operation_attempt_timeout(&self) -> Option { + self.operation_attempt_timeout + } + + /// Returns true if any of the possible timeouts are set. pub fn has_timeouts(&self) -> bool { - self.api.has_timeouts() || self.http.has_timeouts() || self.tcp.has_timeouts() + self.connect_timeout.is_some() + || self.operation_timeout.is_some() + || self.operation_attempt_timeout.is_some() } } diff --git a/rust-runtime/aws-smithy-types/src/timeout/http.rs b/rust-runtime/aws-smithy-types/src/timeout/http.rs deleted file mode 100644 index a9a84010fe..0000000000 --- a/rust-runtime/aws-smithy-types/src/timeout/http.rs +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use crate::tristate::TriState; -use std::time::Duration; - -/// HTTP timeouts used by `DynConnector`s -#[non_exhaustive] -#[derive(Clone, PartialEq, Default, Debug)] -pub struct Http { - connect: TriState, - write: TriState, - read: TriState, - tls_negotiation: TriState, -} - -impl Http { - /// Create a new HTTP timeout config with no timeouts set - pub fn new() -> Self { - Default::default() - } - - /// Return this config's read timeout. - /// - /// A limit on the amount of time an application takes to attempt to read the first byte over an - /// established, open connection after write request. This is also known as the - /// "time to first byte" timeout. - pub fn read_timeout(&self) -> TriState { - self.read.clone() - } - - /// Mutate this `timeout::Http` config, setting the HTTP read timeout - pub fn with_read_timeout(mut self, timeout: TriState) -> Self { - self.read = timeout; - self - } - - /// Return this config's read timeout - /// - /// A limit on the amount of time after making an initial connect attempt on a socket to - /// complete the connect-handshake. - pub fn connect_timeout(&self) -> TriState { - self.connect.clone() - } - - /// Mutate this `timeout::Http` config, setting the HTTP connect timeout - pub fn with_connect_timeout(mut self, timeout: TriState) -> Self { - self.connect = timeout; - self - } - - /// Return true if any timeouts are intentionally set or disabled - pub fn has_timeouts(&self) -> bool { - !self.is_unset() - } - - /// Return true if all timeouts are unset - fn is_unset(&self) -> bool { - self.connect.is_unset() - && self.write.is_unset() - && self.read.is_unset() - && self.tls_negotiation.is_unset() - } - - /// Merges two HTTP timeout configs together. - pub fn take_unset_from(self, other: Self) -> Self { - Self { - connect: self.connect.or(other.connect), - write: self.write.or(other.write), - read: self.read.or(other.read), - tls_negotiation: self.tls_negotiation.or(other.tls_negotiation), - } - } -} - -impl From for Http { - fn from(config: super::Config) -> Self { - config.http - } -} diff --git a/rust-runtime/aws-smithy-types/src/timeout/mod.rs b/rust-runtime/aws-smithy-types/src/timeout/mod.rs index aed9cc3fe9..e0a7c34a92 100644 --- a/rust-runtime/aws-smithy-types/src/timeout/mod.rs +++ b/rust-runtime/aws-smithy-types/src/timeout/mod.rs @@ -6,14 +6,8 @@ //! This module defines types that describe timeouts that can be applied to various stages of the //! Smithy networking stack. -mod api; mod config; mod error; -mod http; -mod tcp; -pub use api::Api; -pub use config::Config; +pub use config::{TimeoutConfig, TimeoutConfigBuilder}; pub use error::ConfigError; -pub use http::Http; -pub use tcp::Tcp; diff --git a/rust-runtime/aws-smithy-types/src/timeout/tcp.rs b/rust-runtime/aws-smithy-types/src/timeout/tcp.rs deleted file mode 100644 index c224f47f4e..0000000000 --- a/rust-runtime/aws-smithy-types/src/timeout/tcp.rs +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use crate::tristate::TriState; -use std::time::Duration; - -/// TCP timeouts used by lower-level `DynConnector`s -#[non_exhaustive] -#[derive(Clone, PartialEq, Default, Debug)] -pub struct Tcp { - connect: TriState, - write: TriState, - read: TriState, -} - -impl Tcp { - /// Create a new TCP timeout config with no timeouts set - pub fn new() -> Self { - Default::default() - } - - /// Return true if any timeouts are intentionally set or disabled - pub fn has_timeouts(&self) -> bool { - !self.is_unset() - } - - /// Return true if all timeouts are unset - fn is_unset(&self) -> bool { - self.connect.is_unset() && self.write.is_unset() && self.read.is_unset() - } - - /// Merges two TCP timeout configs together. - pub fn take_unset_from(self, other: Self) -> Self { - Self { - connect: self.connect.or(other.connect), - write: self.write.or(other.write), - read: self.read.or(other.read), - } - } -} - -impl From for Tcp { - fn from(config: super::Config) -> Self { - config.tcp - } -} diff --git a/rust-runtime/aws-smithy-types/src/tristate.rs b/rust-runtime/aws-smithy-types/src/tristate.rs deleted file mode 100644 index 0162ab58f5..0000000000 --- a/rust-runtime/aws-smithy-types/src/tristate.rs +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -//! Contains [`TriState`](TriState) definition and impls - -/// Utility for tracking set vs. unset vs explicitly disabled -/// -/// If someone explicitly disables something, we don't need to warn them that it may be missing. This -/// enum impls `From`/`Into` `Option` for ease of use. -#[derive(Debug, Clone, Eq, PartialEq)] -pub enum TriState { - /// This variant represents something that was unset by default - Unset, - /// This variant represents something that was intentionally unset - Disabled, - /// This variant represents something that was intentionally set - Set(T), -} - -impl TriState { - /// Create a TriState, returning `Unset` when `None` is passed - pub fn or_unset(t: Option) -> Self { - match t { - Some(t) => Self::Set(t), - None => Self::Unset, - } - } - - /// Return `true` if this `TriState` is `Unset` - pub fn is_unset(&self) -> bool { - matches!(self, TriState::Unset) - } - - /// Returns the tristate if it contains a set value or is disabled, otherwise returns `other` - /// - /// # Examples - /// - /// ``` - /// # use std::time::Duration; - /// # use aws_smithy_types::tristate::TriState; - /// let disabled_timeout: TriState = TriState::Disabled; - /// let timeout: TriState = TriState::Set(Duration::from_secs(1)); - /// assert_eq!(timeout.or(disabled_timeout), TriState::Set(Duration::from_secs(1))); - /// - /// let disabled_timeout: TriState = TriState::Disabled; - /// let timeout: TriState = TriState::Set(Duration::from_secs(2)); - /// assert_eq!(disabled_timeout.or(timeout), TriState::Disabled); - /// - /// let unset_timeout: TriState = TriState::Unset; - /// let timeout: TriState = TriState::Set(Duration::from_secs(3)); - /// assert_eq!(unset_timeout.or(timeout), TriState::Set(Duration::from_secs(3))); - /// ``` - pub fn or(self, other: TriState) -> TriState { - use TriState::*; - - match self { - Set(_) | Disabled => self, - Unset => other, - } - } - - /// Maps a `TriState` to `TriState` by applying a function to a contained value. - pub fn map(self, f: F) -> TriState - where - F: FnOnce(T) -> U, - { - use TriState::*; - - match self { - Set(x) => Set(f(x)), - Unset => Unset, - Disabled => Disabled, - } - } -} - -impl Default for TriState { - fn default() -> Self { - Self::Unset - } -} - -impl From> for TriState { - fn from(t: Option) -> Self { - match t { - Some(t) => TriState::Set(t), - None => TriState::Disabled, - } - } -} - -impl From> for Option { - fn from(t: TriState) -> Self { - match t { - TriState::Disabled | TriState::Unset => None, - TriState::Set(t) => Some(t), - } - } -} From 587ed16ee3a094ff655b6b42bb7f013d5d2ed532 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 14 Sep 2022 10:50:18 -0700 Subject: [PATCH 02/15] Improve resiliency re-exports --- .../ResiliencyConfigCustomization.kt | 59 ++++++++----------- .../customize/RequiredCustomizations.kt | 6 +- 2 files changed, 30 insertions(+), 35 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt index 2df6468810..5735439a43 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt @@ -5,14 +5,13 @@ package software.amazon.smithy.rust.codegen.client.smithy.customizations -import software.amazon.smithy.rust.codegen.client.rustlang.Writable +import software.amazon.smithy.rust.codegen.client.rustlang.RustModule import software.amazon.smithy.rust.codegen.client.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.client.rustlang.writable import software.amazon.smithy.rust.codegen.client.smithy.CoreCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.client.smithy.RuntimeType -import software.amazon.smithy.rust.codegen.client.smithy.generators.LibRsCustomization -import software.amazon.smithy.rust.codegen.client.smithy.generators.LibRsSection +import software.amazon.smithy.rust.codegen.client.smithy.RustCrate import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ServiceConfig @@ -75,8 +74,7 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ## Examples /// ```no_run - /// use $moduleUseName::config::Config; - /// use #{RetryConfig}; + /// use $moduleUseName::config::{Config, RetryConfig}; /// /// let retry_config = RetryConfig::standard().with_max_attempts(5); /// let config = Config::builder().retry_config(retry_config).build(); @@ -90,8 +88,7 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ## Examples /// ```no_run - /// use $moduleUseName::config::{Builder, Config}; - /// use #{RetryConfig}; + /// use $moduleUseName::config::{Builder, Config, RetryConfig}; /// /// fn disable_retries(builder: &mut Builder) { /// let retry_config = RetryConfig::standard().with_max_attempts(1); @@ -112,9 +109,7 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// ## Examples /// /// ```no_run - /// use $moduleUseName::config::Config; - /// use #{AsyncSleep}; - /// use #{Sleep}; + /// use $moduleUseName::config::{AsyncSleep, Sleep, Config}; /// /// ##[derive(Debug)] /// pub struct ForeverSleep; @@ -138,9 +133,7 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// ## Examples /// /// ```no_run - /// use $moduleUseName::config::{Builder, Config}; - /// use #{AsyncSleep}; - /// use #{Sleep}; + /// use $moduleUseName::config::{AsyncSleep, Sleep, Builder, Config}; /// /// ##[derive(Debug)] /// pub struct ForeverSleep; @@ -171,8 +164,7 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ```no_run /// ## use std::time::Duration; - /// use $moduleUseName::config::Config; - /// use aws_smithy_types::timeout::TimeoutConfig; + /// use $moduleUseName::config::{Config, TimeoutConfig}; /// /// let timeout_config = TimeoutConfig::builder() /// .operation_attempt_timeout(Duration::from_secs(1)) @@ -190,8 +182,7 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ```no_run /// ## use std::time::Duration; - /// use $moduleUseName::config::{Builder, Config}; - /// use aws_smithy_types::{timeout, tristate::TriState}; + /// use $moduleUseName::config::{Builder, Config, TimeoutConfig}; /// /// fn set_request_timeout(builder: &mut Builder) { /// let timeout_config = TimeoutConfig::builder() @@ -224,22 +215,24 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co } } -class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) : LibRsCustomization() { - override fun section(section: LibRsSection): Writable { - return when (section) { - is LibRsSection.Body -> writable { - rustTemplate( - """ - pub use #{retry}::RetryConfig; - pub use #{sleep}::AsyncSleep; - pub use #{timeout}::TimeoutConfig; - """, - "retry" to smithyTypesRetry(runtimeConfig), - "sleep" to smithyAsyncRtSleep(runtimeConfig), - "timeout" to smithyTypesTimeout(runtimeConfig), - ) - } - else -> emptySection +class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) { + fun extras(rustCrate: RustCrate) { + rustCrate.withModule(RustModule.Config) { writer -> + writer.rustTemplate( + """ + pub use #{sleep}::{AsyncSleep, Sleep}; + pub use #{types_retry}::{RetryConfig, RetryConfigBuilder}; + /// Config structs required by [`RetryConfig`] + pub mod retry { + pub use #{types_retry}::RetryMode; + pub use #{types_retry}::RetryKind; + } + pub use #{timeout}::{TimeoutConfig, TimeoutConfigBuilder}; + """, + "types_retry" to smithyTypesRetry(runtimeConfig), + "sleep" to smithyAsyncRtSleep(runtimeConfig), + "timeout" to smithyTypesTimeout(runtimeConfig), + ) } } } 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 d70ab9a6d3..d792847b9a 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 @@ -54,12 +54,14 @@ class RequiredCustomizations : RustCodegenDecorator { ): List = baseCustomizations + CrateVersionGenerator() + SmithyTypesPubUseGenerator(codegenContext.runtimeConfig) + - AllowLintsGenerator() + - ResiliencyReExportCustomization(codegenContext.runtimeConfig) + AllowLintsGenerator() override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) { // Add rt-tokio feature for `ByteStream::from_path` rustCrate.mergeFeature(Feature("rt-tokio", true, listOf("aws-smithy-http/rt-tokio"))) + + // Re-export resiliency types + ResiliencyReExportCustomization(codegenContext.runtimeConfig).extras(rustCrate) } override fun supportsCodegenContext(clazz: Class): Boolean = From f003d8c184f14a32b8010bc9af79ae54cbcfb2f7 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 14 Sep 2022 16:56:31 -0700 Subject: [PATCH 03/15] Refactor Smithy client retry config handling in builder --- rust-runtime/aws-smithy-client/src/builder.rs | 90 ++++++++++--------- rust-runtime/aws-smithy-client/src/lib.rs | 46 ++-------- .../aws-smithy-client/tests/e2e_test.rs | 9 +- 3 files changed, 61 insertions(+), 84 deletions(-) diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index b4b5707e68..a639e164d3 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -10,6 +10,14 @@ use aws_smithy_http::result::ConnectorError; use aws_smithy_types::timeout::TimeoutConfig; use std::sync::Arc; +#[derive(Clone, Debug)] +enum BuilderRetryConfig { + // Since generic specialization isn't a thing yet, we have to keep a copy of the retry + // config around when using standard retry so that it can be properly validated. + Standard { has_retry: bool, policy: R }, + Overridden(R), +} + /// A builder that provides more customization options when constructing a [`Client`]. /// /// To start, call [`Builder::new`]. Then, chain the method calls to configure the `Builder`. @@ -19,12 +27,8 @@ use std::sync::Arc; pub struct Builder { connector: C, middleware: M, - // Keep a copy of standard retry config when the standard policy is used - // so that we can do additional validation against the `sleep_impl` when - // the client. - standard_retry_config: Option, - retry_policy: R, - timeout_config: TimeoutConfig, + retry_config: BuilderRetryConfig, + timeout_config: Option, sleep_impl: Option>, } @@ -34,12 +38,15 @@ where M: Default, { fn default() -> Self { + let default_retry_config = retry::Config::default(); Self { connector: Default::default(), middleware: Default::default(), - standard_retry_config: Some(retry::Config::default()), - retry_policy: Default::default(), - timeout_config: TimeoutConfig::disabled(), + retry_config: BuilderRetryConfig::Standard { + has_retry: default_retry_config.has_retry(), + policy: retry::Standard::new(default_retry_config), + }, + timeout_config: None, sleep_impl: default_async_sleep(), } } @@ -78,9 +85,8 @@ impl Builder<(), M, R> { pub fn connector(self, connector: C) -> Builder { Builder { connector, - standard_retry_config: self.standard_retry_config, - retry_policy: self.retry_policy, middleware: self.middleware, + retry_config: self.retry_config, timeout_config: self.timeout_config, sleep_impl: self.sleep_impl, } @@ -135,8 +141,7 @@ impl Builder { pub fn middleware(self, middleware: M) -> Builder { Builder { connector: self.connector, - standard_retry_config: self.standard_retry_config, - retry_policy: self.retry_policy, + retry_config: self.retry_config, timeout_config: self.timeout_config, middleware, sleep_impl: self.sleep_impl, @@ -187,9 +192,7 @@ impl Builder { pub fn retry_policy(self, retry_policy: R) -> Builder { Builder { connector: self.connector, - // Intentionally clear out the standard retry config when the retry policy is overridden. - standard_retry_config: None, - retry_policy, + retry_config: BuilderRetryConfig::Overridden(retry_policy), timeout_config: self.timeout_config, middleware: self.middleware, sleep_impl: self.sleep_impl, @@ -199,27 +202,30 @@ impl Builder { impl Builder { /// Set the standard retry policy's configuration. - pub fn set_retry_config(&mut self, config: retry::Config) -> &mut Self { - self.standard_retry_config = Some(config.clone()); - self.retry_policy.with_config(config); + pub fn set_retry_config(&mut self, config: Option) -> &mut Self { + let config = config.unwrap_or_default(); + self.retry_config = BuilderRetryConfig::Standard { + has_retry: config.has_retry(), + policy: retry::Standard::new(config), + }; self } /// Set the standard retry policy's configuration. pub fn retry_config(mut self, config: retry::Config) -> Self { - self.set_retry_config(config); + self.set_retry_config(Some(config)); self } /// Set a timeout config for the builder - pub fn set_timeout_config(&mut self, timeout_config: TimeoutConfig) -> &mut Self { + pub fn set_timeout_config(&mut self, timeout_config: Option) -> &mut Self { self.timeout_config = timeout_config; self } /// Set a timeout config for the builder pub fn timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { - self.timeout_config = timeout_config; + self.timeout_config = Some(timeout_config); self } @@ -230,8 +236,8 @@ impl Builder { } /// Set the [`AsyncSleep`] function that the [`Client`] will use to create things like timeout futures. - pub fn sleep_impl(mut self, async_sleep: Option>) -> Self { - self.set_sleep_impl(async_sleep); + pub fn sleep_impl(mut self, async_sleep: Arc) -> Self { + self.set_sleep_impl(Some(async_sleep)); self } } @@ -245,8 +251,7 @@ impl Builder { Builder { connector: map(self.connector), middleware: self.middleware, - standard_retry_config: self.standard_retry_config, - retry_policy: self.retry_policy, + retry_config: self.retry_config, timeout_config: self.timeout_config, sleep_impl: self.sleep_impl, } @@ -260,8 +265,7 @@ impl Builder { Builder { connector: self.connector, middleware: map(self.middleware), - standard_retry_config: self.standard_retry_config, - retry_policy: self.retry_policy, + retry_config: self.retry_config, timeout_config: self.timeout_config, sleep_impl: self.sleep_impl, } @@ -269,6 +273,7 @@ impl Builder { /// Build a Smithy service [`Client`]. pub fn build(self) -> Client { + let timeout_config = self.timeout_config.unwrap_or_else(TimeoutConfig::disabled); if self.sleep_impl.is_none() { const ADDITIONAL_HELP: &str = "Either disable retry by setting max attempts to one, or pass in a `sleep_impl`. \ @@ -276,22 +281,23 @@ impl Builder { the `aws-smithy-async` crate is required for your async runtime. If you are using \ Tokio, then make sure the `rt-tokio` feature is enabled to have its sleep \ implementation set automatically."; - if self - .standard_retry_config - .map(|src| src.has_retry()) - .unwrap_or(false) - { - panic!("Retries require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); - } else if self.timeout_config.has_timeouts() { + if let BuilderRetryConfig::Standard { has_retry, .. } = self.retry_config { + if has_retry { + panic!("Retries require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); + } + } + if timeout_config.has_timeouts() { panic!("Timeouts require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); } } - Client { connector: self.connector, - retry_policy: self.retry_policy, + retry_policy: match self.retry_config { + BuilderRetryConfig::Standard { policy, .. } => policy, + BuilderRetryConfig::Overridden(policy) => policy, + }, middleware: self.middleware, - timeout_config: self.timeout_config, + timeout_config, sleep_impl: self.sleep_impl, } } @@ -377,7 +383,7 @@ mod tests { .connect_timeout(Duration::from_secs(1)) .build(); assert!(timeout_config.has_timeouts()); - builder.set_timeout_config(timeout_config); + builder.set_timeout_config(Some(timeout_config)); let result = panic::catch_unwind(AssertUnwindSafe(move || { let _ = builder.build(); @@ -394,7 +400,7 @@ mod tests { let retry_config = retry::Config::default(); assert!(retry_config.has_retry()); - builder.set_retry_config(retry_config); + builder.set_retry_config(Some(retry_config)); let result = panic::catch_unwind(AssertUnwindSafe(move || { let _ = builder.build(); @@ -424,11 +430,11 @@ mod tests { .connect_timeout(Duration::from_secs(1)) .build(); assert!(timeout_config.has_timeouts()); - builder.set_timeout_config(timeout_config); + builder.set_timeout_config(Some(timeout_config)); let retry_config = retry::Config::default(); assert!(retry_config.has_retry()); - builder.set_retry_config(retry_config); + builder.set_retry_config(Some(retry_config)); let _ = builder.build(); } diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index da8a2ef335..a5b44ae484 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -131,6 +131,13 @@ pub struct Client< sleep_impl: Option>, } +impl Client<(), (), ()> { + /// Returns a client builder + pub fn builder() -> Builder { + Builder::new() + } +} + // Quick-create for people who just want "the default". impl Client where @@ -147,45 +154,6 @@ where } } -impl Client { - /// Set the standard retry policy's configuration. - pub fn set_retry_config(&mut self, config: retry::Config) { - self.retry_policy.with_config(config); - } - - /// Adjust a standard retry client with the given policy configuration. - pub fn with_retry_config(mut self, config: retry::Config) -> Self { - self.set_retry_config(config); - self - } -} - -impl Client { - /// Set the client's timeout configuration. - pub fn set_timeout_config(&mut self, timeout_config: TimeoutConfig) { - self.timeout_config = timeout_config; - } - - /// Set the client's timeout configuration. - pub fn with_timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { - self.set_timeout_config(timeout_config); - self - } - - /// Set the [`AsyncSleep`] function that the client will use to create things like timeout futures. - /// - /// *Note: If `None` is passed, this will prevent the client from using retries or timeouts.* - pub fn set_sleep_impl(&mut self, sleep_impl: Option>) { - self.sleep_impl = sleep_impl; - } - - /// Set the [`AsyncSleep`] function that the client will use to create things like timeout futures. - pub fn with_sleep_impl(mut self, sleep_impl: Arc) -> Self { - self.set_sleep_impl(Some(sleep_impl)); - self - } -} - fn check_send_sync(t: T) -> T { t } diff --git a/rust-runtime/aws-smithy-client/tests/e2e_test.rs b/rust-runtime/aws-smithy-client/tests/e2e_test.rs index d2e393f8a8..5d5e0aa9c7 100644 --- a/rust-runtime/aws-smithy-client/tests/e2e_test.rs +++ b/rust-runtime/aws-smithy-client/tests/e2e_test.rs @@ -139,9 +139,12 @@ async fn end_to_end_retry_test() { // This is the default, just setting it to be explicit .with_initial_backoff(Duration::from_secs(1)) .with_base(|| 1_f64); - let client = Client::, Identity>::new(conn.clone()) - .with_retry_config(retry_config) - .with_sleep_impl(Arc::new(TokioSleep::new())); + let client = Client::builder() + .connector(conn.clone()) + .middleware(Identity::new()) + .retry_config(retry_config) + .sleep_impl(Arc::new(TokioSleep::new())) + .build(); tokio::time::pause(); let initial = tokio::time::Instant::now(); let resp = client From 8148af9380f20af69f3147df6b5618fc183c83c8 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 15 Sep 2022 14:15:42 -0700 Subject: [PATCH 04/15] Remove non-standard timeout env vars and profile keys --- .../src/default_provider/timeout_config.rs | 82 +-------- .../aws-config/src/environment/mod.rs | 4 - .../src/environment/timeout_config.rs | 142 --------------- aws/rust-runtime/aws-config/src/lib.rs | 2 - aws/rust-runtime/aws-config/src/parsing.rs | 102 ----------- .../aws-config/src/profile/mod.rs | 1 - .../aws-config/src/profile/timeout_config.rs | 163 ------------------ 7 files changed, 9 insertions(+), 487 deletions(-) delete mode 100644 aws/rust-runtime/aws-config/src/environment/timeout_config.rs delete mode 100644 aws/rust-runtime/aws-config/src/parsing.rs delete mode 100644 aws/rust-runtime/aws-config/src/profile/timeout_config.rs diff --git a/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs b/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs index 42a4368fb4..21b9cbbf8f 100644 --- a/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs +++ b/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs @@ -3,103 +3,39 @@ * SPDX-License-Identifier: Apache-2.0 */ -use crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider; -use crate::profile; use crate::provider_config::ProviderConfig; use aws_sdk_sso::config::TimeoutConfig; use std::time::Duration; const SDK_DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_millis(3100); -/// Default [`timeout::Config`] Provider chain +/// Default [`TimeoutConfig`] provider chain /// -/// Unlike other credentials and region, [`timeout::Config`] has no related `TimeoutConfigProvider` trait. Instead, +/// Unlike other credentials and region, [`TimeoutConfig`] has no related `TimeoutConfigProvider` trait. Instead, /// a builder struct is returned which has a similar API. /// -/// This provider will check the following sources in order: -/// 1. [Environment variables](EnvironmentVariableTimeoutConfigProvider) -/// 2. [Profile file](crate::profile::timeout_config::ProfileFileTimeoutConfigProvider) (`~/.aws/config`) -/// -/// # Example -/// -/// ```no_run -/// # use std::error::Error; -/// # #[tokio::main] -/// # async fn main() { -/// use aws_config::default_provider::timeout_config; -/// -/// // Load a timeout config from a specific profile -/// let timeout_config = timeout_config::default_provider() -/// .profile_name("other_profile") -/// .timeout_config() -/// .await; -/// let config = aws_config::from_env() -/// // Override the timeout config set by the default profile -/// .timeout_config(timeout_config) -/// .load() -/// .await; -/// // instantiate a service client: -/// // ::Client::new(&config); -/// # } -/// ``` pub fn default_provider() -> Builder { Builder::default() } -/// Builder for [`timeout::Config`](aws_smithy_types::timeout::Config) that checks the environment variables and AWS profile files for configuration +/// Builder for [`TimeoutConfig`] that resolves the default timeout configuration +#[non_exhaustive] #[derive(Debug, Default)] -pub struct Builder { - env_provider: EnvironmentVariableTimeoutConfigProvider, - profile_file: profile::timeout_config::Builder, -} +pub struct Builder; impl Builder { /// Configure the default chain /// /// Exposed for overriding the environment when unit-testing providers - pub fn configure(mut self, configuration: &ProviderConfig) -> Self { - self.env_provider = - EnvironmentVariableTimeoutConfigProvider::new_with_env(configuration.env()); - self.profile_file = self.profile_file.configure(configuration); - self - } - - /// Override the profile name used by this provider - pub fn profile_name(mut self, name: &str) -> Self { - self.profile_file = self.profile_file.profile_name(name); + pub fn configure(self, _configuration: &ProviderConfig) -> Self { self } - /// Attempt to create a [`timeout::Config`](aws_smithy_types::timeout::Config) from following sources in order: - /// 1. [Environment variables](crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider) - /// 2. [Profile file](crate::profile::timeout_config::ProfileFileTimeoutConfigProvider) - /// - /// Precedence is considered on a per-field basis. If no timeout is specified, requests will never time out. - /// - /// # Panics - /// - /// This will panic if: - /// - a timeout is set to `NaN`, a negative number, or infinity - /// - a timeout can't be parsed as a floating point number + /// Resolve default timeout configuration pub async fn timeout_config(self) -> TimeoutConfig { - // Both of these can return errors due to invalid config settings and we want to surface those as early as possible - // hence, we'll panic if any config values are invalid (missing values are OK though) - // We match this instead of unwrapping so we can print the error with the `Display` impl instead of the `Debug` impl that unwrap uses - let builder_from_env = match self.env_provider.timeout_config() { - Ok(timeout_config_builder) => timeout_config_builder, - Err(err) => panic!("{}", err), - }; - let builder_from_profile = match self.profile_file.build().timeout_config().await { - Ok(timeout_config_builder) => timeout_config_builder, - Err(err) => panic!("{}", err), - }; - // TODO(https://github.com/awslabs/smithy-rs/issues/1732): Implement complete timeout defaults specification - let defaults = TimeoutConfig::builder().connect_timeout(SDK_DEFAULT_CONNECT_TIMEOUT); - - builder_from_env - .take_unset_from(builder_from_profile) - .take_unset_from(defaults) + TimeoutConfig::builder() + .connect_timeout(SDK_DEFAULT_CONNECT_TIMEOUT) .build() } } diff --git a/aws/rust-runtime/aws-config/src/environment/mod.rs b/aws/rust-runtime/aws-config/src/environment/mod.rs index 29e717ca9c..26cbda6b09 100644 --- a/aws/rust-runtime/aws-config/src/environment/mod.rs +++ b/aws/rust-runtime/aws-config/src/environment/mod.rs @@ -18,7 +18,3 @@ pub use region::EnvironmentVariableRegionProvider; /// Load retry behavior configuration from the environment pub mod retry_config; pub use retry_config::EnvironmentVariableRetryConfigProvider; - -/// Load timeout configuration from the environment -pub mod timeout_config; -pub use timeout_config::EnvironmentVariableTimeoutConfigProvider; diff --git a/aws/rust-runtime/aws-config/src/environment/timeout_config.rs b/aws/rust-runtime/aws-config/src/environment/timeout_config.rs deleted file mode 100644 index b58ad7d6af..0000000000 --- a/aws/rust-runtime/aws-config/src/environment/timeout_config.rs +++ /dev/null @@ -1,142 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -//! Load timeout configuration properties from environment variables - -use crate::parsing::parse_str_as_timeout; -use aws_sdk_sso::config::{TimeoutConfig, TimeoutConfigBuilder}; -use aws_smithy_types::timeout; -use aws_types::os_shim_internal::Env; -use std::time::Duration; - -// Currently unsupported timeouts -const ENV_VAR_TLS_NEGOTIATION_TIMEOUT: &str = "AWS_TLS_NEGOTIATION_TIMEOUT"; - -// Supported timeouts -const ENV_VAR_CONNECT_TIMEOUT: &str = "AWS_CONNECT_TIMEOUT"; -const ENV_VAR_READ_TIMEOUT: &str = "AWS_READ_TIMEOUT"; -const ENV_VAR_OPERATION_ATTEMPT_TIMEOUT: &str = "AWS_OPERATION_ATTEMPT_TIMEOUT"; -const ENV_VAR_OPERATION_TIMEOUT: &str = "AWS_OPERATION_TIMEOUT"; - -/// Load timeout config from environment variables -/// -/// This provider will check the values of the following variables in order to build a [`TimeoutConfig`]: -/// -/// - `AWS_CONNECT_TIMEOUT` -/// - `AWS_READ_TIMEOUT` -/// - `AWS_OPERATION_ATTEMPT_TIMEOUT` -/// - `AWS_OPERATION_TIMEOUT` -/// -/// Timeout values represent the number of seconds before timing out and must be non-negative floats -/// or integers. NaN and infinity are also invalid. -#[derive(Debug, Default)] -pub struct EnvironmentVariableTimeoutConfigProvider { - env: Env, -} - -impl EnvironmentVariableTimeoutConfigProvider { - /// Create a new [`EnvironmentVariableTimeoutConfigProvider`] - pub fn new() -> Self { - EnvironmentVariableTimeoutConfigProvider { env: Env::real() } - } - - #[doc(hidden)] - /// Create a timeout config provider from a given [`Env`] - /// - /// This method is used for tests that need to override environment variables. - pub fn new_with_env(env: Env) -> Self { - EnvironmentVariableTimeoutConfigProvider { env } - } - - /// Attempt to create a new [`timeout::Config`](aws_smithy_types::timeout::Config) from environment variables - pub fn timeout_config(&self) -> Result { - // Warn users that set unsupported timeouts in their profile - for timeout in [ENV_VAR_TLS_NEGOTIATION_TIMEOUT] { - warn_if_unsupported_timeout_is_set(&self.env, timeout); - } - - let mut builder = TimeoutConfig::builder(); - builder.set_connect_timeout(timeout_from_env_var(&self.env, ENV_VAR_CONNECT_TIMEOUT)?); - builder.set_read_timeout(timeout_from_env_var(&self.env, ENV_VAR_READ_TIMEOUT)?); - builder.set_operation_attempt_timeout(timeout_from_env_var( - &self.env, - ENV_VAR_OPERATION_ATTEMPT_TIMEOUT, - )?); - builder.set_operation_timeout(timeout_from_env_var(&self.env, ENV_VAR_OPERATION_TIMEOUT)?); - - Ok(builder) - } -} - -fn timeout_from_env_var( - env: &Env, - var: &'static str, -) -> Result, timeout::ConfigError> { - match env.get(var).ok() { - Some(timeout) => { - parse_str_as_timeout(&timeout, var.into(), "environment variable".into()).map(Some) - } - None => Ok(None), - } -} - -fn warn_if_unsupported_timeout_is_set(env: &Env, var: &'static str) { - if env.get(var).is_ok() { - tracing::warn!( - "Environment variable for '{}' timeout was set but that feature is currently unimplemented so the setting will be ignored. \ - To help us prioritize support for this feature, please upvote aws-sdk-rust#151 (https://github.com/awslabs/aws-sdk-rust/issues/151)", - var - ) - } -} - -#[cfg(test)] -mod test { - use super::{ - EnvironmentVariableTimeoutConfigProvider, ENV_VAR_CONNECT_TIMEOUT, - ENV_VAR_OPERATION_ATTEMPT_TIMEOUT, ENV_VAR_OPERATION_TIMEOUT, ENV_VAR_READ_TIMEOUT, - }; - use aws_sdk_sso::config::TimeoutConfig; - use aws_types::os_shim_internal::Env; - use std::time::Duration; - - fn test_provider(vars: &[(&str, &str)]) -> EnvironmentVariableTimeoutConfigProvider { - EnvironmentVariableTimeoutConfigProvider::new_with_env(Env::from_slice(vars)) - } - - #[test] - fn no_defaults() { - let built = test_provider(&[]).timeout_config().unwrap().build(); - assert_eq!(built.connect_timeout(), None); - assert_eq!(built.read_timeout(), None); - assert_eq!(built.operation_timeout(), None); - assert_eq!(built.operation_attempt_timeout(), None); - } - - #[test] - fn all_fields_can_be_set_at_once() { - let expected_timeouts = TimeoutConfig::builder() - .connect_timeout(Duration::from_secs_f32(3.1)) - .read_timeout(Duration::from_secs_f32(500.0)) - .operation_attempt_timeout(Duration::from_secs_f32(4.0)) - // Some floats can't be represented as f32 so this duration will end up equalling the - // duration from the env. - .operation_timeout(Duration::from_secs_f32(900012350.0)) - .build(); - - assert_eq!( - test_provider(&[ - (ENV_VAR_CONNECT_TIMEOUT, "3.1"), - (ENV_VAR_READ_TIMEOUT, "500"), - (ENV_VAR_OPERATION_ATTEMPT_TIMEOUT, "04.000"), - (ENV_VAR_OPERATION_TIMEOUT, "900012345.0") - ]) - .timeout_config() - .unwrap() - .build(), - expected_timeouts - ); - } -} diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index 77890053b5..fe795005fd 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -132,8 +132,6 @@ pub mod connector; pub mod credential_process; -pub(crate) mod parsing; - // Re-export types from smithy-types pub use aws_smithy_types::retry; pub use aws_smithy_types::timeout; diff --git a/aws/rust-runtime/aws-config/src/parsing.rs b/aws/rust-runtime/aws-config/src/parsing.rs deleted file mode 100644 index 4d079ac3e6..0000000000 --- a/aws/rust-runtime/aws-config/src/parsing.rs +++ /dev/null @@ -1,102 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use std::borrow::Cow; -use std::time::Duration; - -use aws_smithy_types::timeout; - -/// Parse a given string as a [`Duration`] that will be used to set a timeout. This will return an -/// error result if the given string is negative, infinite, equal to zero, NaN, or if the string -/// can't be parsed as an `f32`. The `name` and `set_by` fields are used to provide context when an -/// error occurs -/// -/// # Example -/// -/// ```dont_run -/// # use std::time::Duration; -/// use aws_config::parsing::parse_str_as_timeout; -/// let duration = parse_str_as_timeout("8", "timeout".into(), "test_success".into()).unwrap(); -/// assert_eq!(duration, Duration::from_secs_f32(8.0)); -/// -/// // This line will panic with `InvalidTimeout { name: "timeout", reason: "timeout must not be less than or equal to zero", set_by: "test_error" }` -/// let _ = parse_str_as_timeout("-1.0", "timeout".into(), "test_error".into()).unwrap(); -/// ``` -pub(crate) fn parse_str_as_timeout( - timeout: &str, - name: Cow<'static, str>, - set_by: Cow<'static, str>, -) -> Result { - match timeout.parse::() { - Ok(timeout) if timeout <= 0.0 => Err(timeout::ConfigError::InvalidTimeout { - set_by, - name, - reason: "timeout must not be less than or equal to zero".into(), - }), - Ok(timeout) if timeout.is_nan() => Err(timeout::ConfigError::InvalidTimeout { - set_by, - name, - reason: "timeout must not be NaN".into(), - }), - Ok(timeout) if timeout.is_infinite() => Err(timeout::ConfigError::InvalidTimeout { - set_by, - name, - reason: "timeout must not be infinite".into(), - }), - Ok(timeout) => Ok(Duration::from_secs_f32(timeout)), - Err(err) => Err(timeout::ConfigError::ParseError { - set_by, - name, - source: Box::new(err), - }), - } -} - -#[cfg(test)] -mod tests { - use super::parse_str_as_timeout; - use std::time::Duration; - - #[test] - fn test_integer_timeouts_are_parseable() { - let duration = parse_str_as_timeout("8", "timeout".into(), "test".into()).unwrap(); - assert_eq!(duration, Duration::from_secs_f32(8.0)); - } - - #[test] - #[should_panic = r#"ParseError { name: "timeout", set_by: "test", source: ParseFloatError { kind: Invalid } }"#] - fn test_unparseable_timeouts_produce_an_error() { - let _ = parse_str_as_timeout( - "this sentence cant be parsed as a number", - "timeout".into(), - "test".into(), - ) - .unwrap(); - } - - #[test] - #[should_panic = r#"InvalidTimeout { name: "timeout", reason: "timeout must not be less than or equal to zero", set_by: "test" }"#] - fn test_negative_timeouts_are_invalid() { - let _ = parse_str_as_timeout("-1.0", "timeout".into(), "test".into()).unwrap(); - } - - #[test] - #[should_panic = r#"InvalidTimeout { name: "timeout", reason: "timeout must not be less than or equal to zero", set_by: "test" }"#] - fn test_setting_timeout_to_zero_is_invalid() { - let _ = parse_str_as_timeout("0", "timeout".into(), "test".into()).unwrap(); - } - - #[test] - #[should_panic = r#"InvalidTimeout { name: "timeout", reason: "timeout must not be NaN", set_by: "test" }"#] - fn test_nan_timeouts_are_invalid() { - let _ = parse_str_as_timeout("NaN", "timeout".into(), "test".into()).unwrap(); - } - - #[test] - #[should_panic = r#"InvalidTimeout { name: "timeout", reason: "timeout must not be infinite", set_by: "test" }"#] - fn test_infinite_timeouts_are_invalid() { - let _ = parse_str_as_timeout("inf", "timeout".into(), "test".into()).unwrap(); - } -} diff --git a/aws/rust-runtime/aws-config/src/profile/mod.rs b/aws/rust-runtime/aws-config/src/profile/mod.rs index 1bb0df6c94..62071013ba 100644 --- a/aws/rust-runtime/aws-config/src/profile/mod.rs +++ b/aws/rust-runtime/aws-config/src/profile/mod.rs @@ -22,7 +22,6 @@ pub mod app_name; pub mod credentials; pub mod region; pub mod retry_config; -pub mod timeout_config; #[doc(inline)] pub use credentials::ProfileFileCredentialsProvider; diff --git a/aws/rust-runtime/aws-config/src/profile/timeout_config.rs b/aws/rust-runtime/aws-config/src/profile/timeout_config.rs deleted file mode 100644 index 3e48a653dd..0000000000 --- a/aws/rust-runtime/aws-config/src/profile/timeout_config.rs +++ /dev/null @@ -1,163 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -//! Load timeout configuration properties from an AWS profile - -use crate::parsing::parse_str_as_timeout; -use crate::profile::Profile; -use crate::provider_config::ProviderConfig; - -use aws_smithy_types::timeout; -use aws_types::os_shim_internal::{Env, Fs}; - -use aws_sdk_sso::config::{TimeoutConfig, TimeoutConfigBuilder}; -use std::time::Duration; - -const PROFILE_VAR_CONNECT_TIMEOUT: &str = "connect_timeout"; -const PROFILE_VAR_READ_TIMEOUT: &str = "read_timeout"; -const PROFILE_VAR_OPERATION_ATTEMPT_TIMEOUT: &str = "operation_attempt_timeout"; -const PROFILE_VAR_OPERATION_TIMEOUT: &str = "operation_timeout"; - -/// Load timeout configuration properties from a profile file -/// -/// This provider will attempt to load AWS shared configuration, then read timeout configuration -/// properties from the active profile. Timeout values represent the number of seconds before timing -/// out and must be non-negative floats or integers. NaN and infinity are also invalid. If at least -/// one of these values is valid, construction will succeed. -/// -/// # Examples -/// -/// **Sets timeouts for the `default` profile** -/// ```ini -/// [default] -/// operation_attempt_timeout = 2 -/// operation_timeout = 3 -/// ``` -/// -/// **Sets the `operation_attempt_timeout` to 0.5 seconds _if and only if_ the `other` profile is selected.** -/// -/// ```ini -/// [profile other] -/// operation_attempt_timeout = 0.5 -/// ``` -/// -/// This provider is part of the [default timeout config provider chain](crate::default_provider::timeout_config). -#[derive(Debug, Default)] -pub struct ProfileFileTimeoutConfigProvider { - fs: Fs, - env: Env, - profile_override: Option, -} - -/// Builder for [`ProfileFileTimeoutConfigProvider`] -#[derive(Debug, Default)] -pub struct Builder { - config: Option, - profile_override: Option, -} - -impl Builder { - /// Override the configuration for this provider - pub fn configure(mut self, config: &ProviderConfig) -> Self { - self.config = Some(config.clone()); - self - } - - /// Override the profile name used by the [`ProfileFileTimeoutConfigProvider`] - pub fn profile_name(mut self, profile_name: impl Into) -> Self { - self.profile_override = Some(profile_name.into()); - self - } - - /// Build a [`ProfileFileTimeoutConfigProvider`] from this builder - pub fn build(self) -> ProfileFileTimeoutConfigProvider { - let conf = self.config.unwrap_or_default(); - ProfileFileTimeoutConfigProvider { - env: conf.env(), - fs: conf.fs(), - profile_override: self.profile_override, - } - } -} - -impl ProfileFileTimeoutConfigProvider { - /// Create a new [`ProfileFileTimeoutConfigProvider`] - /// - /// To override the selected profile, set the `AWS_PROFILE` environment variable or use the [`Builder`]. - pub fn new() -> Self { - Self { - fs: Fs::real(), - env: Env::real(), - profile_override: None, - } - } - - /// [`Builder`] to construct a [`ProfileFileTimeoutConfigProvider`] - pub fn builder() -> Builder { - Builder::default() - } - - /// Attempt to create a new [`timeout::Config`](aws_smithy_types::timeout::Config) from a profile file. - pub async fn timeout_config(&self) -> Result { - let profile = match super::parser::load(&self.fs, &self.env).await { - Ok(profile) => profile, - Err(err) => { - tracing::warn!(err = %err, "failed to parse profile, skipping it"); - // return an empty builder - return Ok(Default::default()); - } - }; - - let selected_profile = self - .profile_override - .as_deref() - .unwrap_or_else(|| profile.selected_profile()); - let selected_profile = match profile.get_profile(selected_profile) { - Some(profile) => profile, - None => { - // Only warn if the user specified a profile name to use. - if self.profile_override.is_some() { - tracing::warn!( - "failed to get selected '{}' profile, skipping it", - selected_profile - ); - } - // return an empty config - return Ok(TimeoutConfigBuilder::new()); - } - }; - - let mut builder = TimeoutConfig::builder(); - builder.set_connect_timeout(timeout_from_profile_var( - selected_profile, - PROFILE_VAR_CONNECT_TIMEOUT, - )?); - builder.set_read_timeout(timeout_from_profile_var( - selected_profile, - PROFILE_VAR_READ_TIMEOUT, - )?); - builder.set_operation_attempt_timeout(timeout_from_profile_var( - selected_profile, - PROFILE_VAR_OPERATION_ATTEMPT_TIMEOUT, - )?); - builder.set_operation_timeout(timeout_from_profile_var( - selected_profile, - PROFILE_VAR_OPERATION_TIMEOUT, - )?); - - Ok(builder) - } -} - -fn timeout_from_profile_var( - profile: &Profile, - var: &'static str, -) -> Result, timeout::ConfigError> { - let profile_name = format!("aws profile [{}]", profile.name()); - match profile.get(var) { - Some(timeout) => parse_str_as_timeout(timeout, var.into(), profile_name.into()).map(Some), - None => Ok(None), - } -} From 815f02218c8749477bb67c52dd9ed4f5d8ee9766 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 15 Sep 2022 16:48:02 -0700 Subject: [PATCH 05/15] Break operation customization codegen into its own file --- .../client/CustomizableOperationGenerator.kt | 160 ++++++++++++++++++ .../client/FluentClientGenerator.kt | 148 +--------------- 2 files changed, 165 insertions(+), 143 deletions(-) create mode 100644 codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt new file mode 100644 index 0000000000..bed6e2a404 --- /dev/null +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt @@ -0,0 +1,160 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.client.smithy.generators.client + +import software.amazon.smithy.rust.codegen.client.rustlang.CargoDependency +import software.amazon.smithy.rust.codegen.client.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.client.rustlang.asType +import software.amazon.smithy.rust.codegen.client.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.client.smithy.RuntimeConfig +import software.amazon.smithy.rust.codegen.client.smithy.RustCrate +import software.amazon.smithy.rust.codegen.client.smithy.generators.GenericTypeArg +import software.amazon.smithy.rust.codegen.client.smithy.generators.GenericsGenerator + +/** + * Generates the code required to add the `.customize()` function to the + * fluent client builders. + */ +class CustomizableOperationGenerator( + private val runtimeConfig: RuntimeConfig, + private val generics: FluentClientGenerics, + private val includeFluentClient: Boolean, +) { + fun render(crate: RustCrate) { + crate.withModule(FluentClientGenerator.customizableOperationModule) { writer -> + renderCustomizableOperationModule(writer) + + if (includeFluentClient) { + renderCustomizableOperationSend(writer) + } + } + } + + private fun renderCustomizableOperationModule(writer: RustWriter) { + val smithyHttp = CargoDependency.SmithyHttp(runtimeConfig).asType() + + val operationGenerics = GenericsGenerator(GenericTypeArg("O"), GenericTypeArg("Retry")) + val handleGenerics = generics.toGenericsGenerator() + val combinedGenerics = operationGenerics + handleGenerics + + val codegenScope = arrayOf( + // SDK Types + "http_result" to smithyHttp.member("result"), + "http_body" to smithyHttp.member("body"), + "http_operation" to smithyHttp.member("operation"), + "HttpRequest" to CargoDependency.Http.asType().member("Request"), + "handle_generics_decl" to handleGenerics.declaration(), + "handle_generics_bounds" to handleGenerics.bounds(), + "operation_generics_decl" to operationGenerics.declaration(), + "combined_generics_decl" to combinedGenerics.declaration(), + ) + + writer.rustTemplate( + """ + use crate::client::Handle; + + use #{http_body}::SdkBody; + use #{http_operation}::Operation; + use #{http_result}::SdkError; + + use std::convert::Infallible; + use std::sync::Arc; + + /// A wrapper type for [`Operation`](aws_smithy_http::operation::Operation)s that allows for + /// customization of the operation before it is sent. A `CustomizableOperation` may be sent + /// by calling its [`.send()`][crate::customizable_operation::CustomizableOperation::send] method. + ##[derive(Debug)] + pub struct CustomizableOperation#{combined_generics_decl:W} { + pub(crate) handle: Arc, + pub(crate) operation: Operation#{operation_generics_decl:W}, + } + + impl#{combined_generics_decl:W} CustomizableOperation#{combined_generics_decl:W} + where + #{handle_generics_bounds:W} + { + /// Allows for customizing the operation's request + pub fn map_request( + mut self, + f: impl FnOnce(#{HttpRequest}) -> Result<#{HttpRequest}, E>, + ) -> Result { + let (request, response) = self.operation.into_request_response(); + let request = request.augment(|req, _props| f(req))?; + self.operation = Operation::from_parts(request, response); + Ok(self) + } + + /// Convenience for `map_request` where infallible direct mutation of request is acceptable + pub fn mutate_request(self, f: impl FnOnce(&mut #{HttpRequest})) -> Self { + self.map_request(|mut req| { + f(&mut req); + Result::<_, Infallible>::Ok(req) + }) + .expect("infallible") + } + + /// Allows for customizing the entire operation + pub fn map_operation( + mut self, + f: impl FnOnce(Operation#{operation_generics_decl:W}) -> Result, + ) -> Result { + self.operation = f(self.operation)?; + Ok(self) + } + + /// Direct access to read the HTTP request + pub fn request(&self) -> &#{HttpRequest} { + self.operation.request() + } + + /// Direct access to mutate the HTTP request + pub fn request_mut(&mut self) -> &mut #{HttpRequest} { + self.operation.request_mut() + } + } + """, + *codegenScope, + ) + } + + private fun renderCustomizableOperationSend(writer: RustWriter) { + val smithyHttp = CargoDependency.SmithyHttp(runtimeConfig).asType() + val smithyClient = CargoDependency.SmithyClient(runtimeConfig).asType() + + val operationGenerics = GenericsGenerator(GenericTypeArg("O"), GenericTypeArg("Retry")) + val handleGenerics = generics.toGenericsGenerator() + val combinedGenerics = operationGenerics + handleGenerics + + val codegenScope = arrayOf( + "combined_generics_decl" to combinedGenerics.declaration(), + "handle_generics_bounds" to handleGenerics.bounds(), + "ParseHttpResponse" to smithyHttp.member("response::ParseHttpResponse"), + "NewRequestPolicy" to smithyClient.member("retry::NewRequestPolicy"), + "SmithyRetryPolicy" to smithyClient.member("bounds::SmithyRetryPolicy"), + ) + + writer.rustTemplate( + """ + impl#{combined_generics_decl:W} CustomizableOperation#{combined_generics_decl:W} + where + #{handle_generics_bounds:W} + { + /// Sends this operation's request + pub async fn send(self) -> Result> + where + E: std::error::Error, + O: #{ParseHttpResponse}> + Send + Sync + Clone + 'static, + Retry: Send + Sync + Clone, + ::Policy: #{SmithyRetryPolicy} + Clone, + { + self.handle.client.call(self.operation).await + } + } + """, + *codegenScope, + ) + } +} 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 1555af2c17..41fed6e9d6 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 @@ -35,15 +35,12 @@ import software.amazon.smithy.rust.codegen.client.rustlang.rustTypeParameters import software.amazon.smithy.rust.codegen.client.rustlang.stripOuter import software.amazon.smithy.rust.codegen.client.rustlang.writable import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext -import software.amazon.smithy.rust.codegen.client.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.client.smithy.RuntimeType import software.amazon.smithy.rust.codegen.client.smithy.RustCrate import software.amazon.smithy.rust.codegen.client.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.client.smithy.customize.writeCustomizations import software.amazon.smithy.rust.codegen.client.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.client.smithy.generators.CodegenTarget -import software.amazon.smithy.rust.codegen.client.smithy.generators.GenericTypeArg -import software.amazon.smithy.rust.codegen.client.smithy.generators.GenericsGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.PaginatorGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.builderSymbol import software.amazon.smithy.rust.codegen.client.smithy.generators.error.errorSymbol @@ -98,13 +95,11 @@ class FluentClientGenerator( renderFluentClient(writer) } - crate.withModule(customizableOperationModule) { writer -> - renderCustomizableOperationModule(runtimeConfig, generics, writer) - - if (codegenContext.settings.codegenConfig.includeFluentClient) { - renderCustomizableOperationSend(runtimeConfig, generics, writer) - } - } + CustomizableOperationGenerator( + runtimeConfig, + generics, + codegenContext.settings.codegenConfig.includeFluentClient, + ).render(crate) } private fun renderFluentClient(writer: RustWriter) { @@ -370,139 +365,6 @@ class FluentClientGenerator( } } -private fun renderCustomizableOperationModule( - runtimeConfig: RuntimeConfig, - generics: FluentClientGenerics, - writer: RustWriter, -) { - val smithyHttp = CargoDependency.SmithyHttp(runtimeConfig).asType() - - val operationGenerics = GenericsGenerator(GenericTypeArg("O"), GenericTypeArg("Retry")) - val handleGenerics = generics.toGenericsGenerator() - val combinedGenerics = operationGenerics + handleGenerics - - val codegenScope = arrayOf( - // SDK Types - "http_result" to smithyHttp.member("result"), - "http_body" to smithyHttp.member("body"), - "http_operation" to smithyHttp.member("operation"), - "HttpRequest" to CargoDependency.Http.asType().member("Request"), - "handle_generics_decl" to handleGenerics.declaration(), - "handle_generics_bounds" to handleGenerics.bounds(), - "operation_generics_decl" to operationGenerics.declaration(), - "combined_generics_decl" to combinedGenerics.declaration(), - ) - - writer.rustTemplate( - """ - use crate::client::Handle; - - use #{http_body}::SdkBody; - use #{http_operation}::Operation; - use #{http_result}::SdkError; - - use std::convert::Infallible; - use std::sync::Arc; - - /// A wrapper type for [`Operation`](aws_smithy_http::operation::Operation)s that allows for - /// customization of the operation before it is sent. A `CustomizableOperation` may be sent - /// by calling its [`.send()`][crate::customizable_operation::CustomizableOperation::send] method. - ##[derive(Debug)] - pub struct CustomizableOperation#{combined_generics_decl:W} { - pub(crate) handle: Arc, - pub(crate) operation: Operation#{operation_generics_decl:W}, - } - - impl#{combined_generics_decl:W} CustomizableOperation#{combined_generics_decl:W} - where - #{handle_generics_bounds:W} - { - /// Allows for customizing the operation's request - pub fn map_request( - mut self, - f: impl FnOnce(#{HttpRequest}) -> Result<#{HttpRequest}, E>, - ) -> Result { - let (request, response) = self.operation.into_request_response(); - let request = request.augment(|req, _props| f(req))?; - self.operation = Operation::from_parts(request, response); - Ok(self) - } - - /// Convenience for `map_request` where infallible direct mutation of request is acceptable - pub fn mutate_request(self, f: impl FnOnce(&mut #{HttpRequest})) -> Self { - self.map_request(|mut req| { - f(&mut req); - Result::<_, Infallible>::Ok(req) - }) - .expect("infallible") - } - - /// Allows for customizing the entire operation - pub fn map_operation( - mut self, - f: impl FnOnce(Operation#{operation_generics_decl:W}) -> Result, - ) -> Result { - self.operation = f(self.operation)?; - Ok(self) - } - - /// Direct access to read the HTTP request - pub fn request(&self) -> &#{HttpRequest} { - self.operation.request() - } - - /// Direct access to mutate the HTTP request - pub fn request_mut(&mut self) -> &mut #{HttpRequest} { - self.operation.request_mut() - } - } - """, - *codegenScope, - ) -} - -private fun renderCustomizableOperationSend( - runtimeConfig: RuntimeConfig, - generics: FluentClientGenerics, - writer: RustWriter, -) { - val smithyHttp = CargoDependency.SmithyHttp(runtimeConfig).asType() - val smithyClient = CargoDependency.SmithyClient(runtimeConfig).asType() - - val operationGenerics = GenericsGenerator(GenericTypeArg("O"), GenericTypeArg("Retry")) - val handleGenerics = generics.toGenericsGenerator() - val combinedGenerics = operationGenerics + handleGenerics - - val codegenScope = arrayOf( - "combined_generics_decl" to combinedGenerics.declaration(), - "handle_generics_bounds" to handleGenerics.bounds(), - "ParseHttpResponse" to smithyHttp.member("response::ParseHttpResponse"), - "NewRequestPolicy" to smithyClient.member("retry::NewRequestPolicy"), - "SmithyRetryPolicy" to smithyClient.member("bounds::SmithyRetryPolicy"), - ) - - writer.rustTemplate( - """ - impl#{combined_generics_decl:W} CustomizableOperation#{combined_generics_decl:W} - where - #{handle_generics_bounds:W} - { - /// Sends this operation's request - pub async fn send(self) -> Result> - where - E: std::error::Error, - O: #{ParseHttpResponse}> + Send + Sync + Clone + 'static, - Retry: Send + Sync + Clone, - ::Policy: #{SmithyRetryPolicy} + Clone, - { - self.handle.client.call(self.operation).await - } - } - """, - *codegenScope, - ) -} - /** * For a given `operation` shape, return a list of strings where each string describes the name and input type of one of * the operation's corresponding fluent builder methods as well as that method's documentation from the smithy model From bed694fad1ac51344a6c43feb25916dfe9a10b47 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 15 Sep 2022 16:52:43 -0700 Subject: [PATCH 06/15] Move operation customization into a submodule --- .../rustsdk/AwsFluentClientDecorator.kt | 3 ++- .../codegen/client/rustlang/RustModule.kt | 3 ++- .../codegen/client/smithy/CodegenDelegator.kt | 25 +++++++++++++++---- .../codegen/client/smithy/SymbolVisitor.kt | 1 - .../client/CustomizableOperationGenerator.kt | 16 ++++++++++-- .../client/FluentClientGenerator.kt | 9 ++----- .../generators/ServerServiceGenerator.kt | 3 +-- 7 files changed, 41 insertions(+), 19 deletions(-) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt index 536226dd5c..578b3cc86d 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt @@ -29,6 +29,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.GenericTypeA import software.amazon.smithy.rust.codegen.client.smithy.generators.GenericsGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.LibRsCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.LibRsSection +import software.amazon.smithy.rust.codegen.client.smithy.generators.client.CustomizableOperationGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.client.FluentClientCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.client.FluentClientGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.client.FluentClientGenerics @@ -101,7 +102,7 @@ class AwsFluentClientDecorator : RustCodegenDecorator { ), retryClassifier = runtimeConfig.awsHttp().asType().member("retry::AwsResponseRetryClassifier"), ).render(rustCrate) - rustCrate.withModule(FluentClientGenerator.customizableOperationModule) { writer -> + rustCrate.withNonRootModule(CustomizableOperationGenerator.CUSTOMIZE_MODULE) { writer -> renderCustomizableOperationSendMethod(runtimeConfig, generics, writer) } rustCrate.withModule(FluentClientGenerator.clientModule) { writer -> diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/rustlang/RustModule.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/rustlang/RustModule.kt index 066bbadd5e..382bd1811f 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/rustlang/RustModule.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/rustlang/RustModule.kt @@ -24,6 +24,7 @@ data class RustModule(val name: String, val rustMetadata: RustMetadata, val docu default(name, visibility = Visibility.PRIVATE, documentation = documentation) val Config = public("config", documentation = "Configuration for the service.") - val Error = public("error", documentation = "Errors that can occur when calling the service.") + val Error = public("error", documentation = "All error types that operations can return.") + val Operation = public("operation", documentation = "All operations that this crate can perform.") } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt index a2c01b4b35..7032d17cfc 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt @@ -16,7 +16,6 @@ import software.amazon.smithy.rust.codegen.client.rustlang.InlineDependency import software.amazon.smithy.rust.codegen.client.rustlang.RustDependency import software.amazon.smithy.rust.codegen.client.rustlang.RustModule import software.amazon.smithy.rust.codegen.client.rustlang.RustWriter -import software.amazon.smithy.rust.codegen.client.rustlang.Visibility import software.amazon.smithy.rust.codegen.client.smithy.generators.CargoTomlGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.LibRsCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.LibRsGenerator @@ -95,7 +94,7 @@ open class RustCrate( ) { injectInlineDependencies() val modules = inner.writers.values.mapNotNull { it.module() }.filter { it != "lib" } - .map { modules[it] ?: RustModule.default(it, visibility = Visibility.PRIVATE) } + .mapNotNull { modules[it] } inner.finalize( settings, model, @@ -139,6 +138,22 @@ open class RustCrate( return this } + /** + * Create a new non-root module directly. + */ + fun withNonRootModule( + namespace: String, + moduleWriter: (RustWriter) -> Unit, + ): RustCrate { + val parts = namespace.split("::") + require(parts.size > 2) { "Cannot create root modules using withNonRootModule" } + require(parts[0] == "crate") { "Namespace must start with crate::" } + + val fileName = "src/" + parts.filterIndexed { index, _ -> index > 0 }.joinToString("/") + ".rs" + inner.useFileWriter(fileName, namespace, moduleWriter) + return this + } + /** * Create a new file directly */ @@ -153,12 +168,12 @@ open class RustCrate( * Allowlist of modules that will be exposed publicly in generated crates */ val DefaultPublicModules = setOf( - RustModule.public("error", documentation = "All error types that operations can return."), - RustModule.public("operation", documentation = "All operations that this crate can perform."), + RustModule.Error, + RustModule.Operation, RustModule.public("model", documentation = "Data structures used by operation inputs/outputs."), RustModule.public("input", documentation = "Input structures for operations."), RustModule.public("output", documentation = "Output structures for operations."), - RustModule.public("config", documentation = "Client configuration."), + RustModule.Config, ).associateBy { it.name } /** diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/SymbolVisitor.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/SymbolVisitor.kt index e3087f1e21..0d2e1c6231 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/SymbolVisitor.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/SymbolVisitor.kt @@ -80,7 +80,6 @@ data class SymbolLocation(val namespace: String) { val Models = SymbolLocation("model") val Errors = SymbolLocation("error") val Operations = SymbolLocation("operation") -val Serializers = SymbolLocation("serializer") val Inputs = SymbolLocation("input") val Outputs = SymbolLocation("output") diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt index bed6e2a404..0851a1f1f1 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt @@ -6,8 +6,11 @@ package software.amazon.smithy.rust.codegen.client.smithy.generators.client import software.amazon.smithy.rust.codegen.client.rustlang.CargoDependency +import software.amazon.smithy.rust.codegen.client.rustlang.RustModule import software.amazon.smithy.rust.codegen.client.rustlang.RustWriter import software.amazon.smithy.rust.codegen.client.rustlang.asType +import software.amazon.smithy.rust.codegen.client.rustlang.docs +import software.amazon.smithy.rust.codegen.client.rustlang.rust import software.amazon.smithy.rust.codegen.client.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.client.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.client.smithy.RustCrate @@ -23,8 +26,17 @@ class CustomizableOperationGenerator( private val generics: FluentClientGenerics, private val includeFluentClient: Boolean, ) { + companion object { + const val CUSTOMIZE_MODULE = "crate::operation::customize" + } + fun render(crate: RustCrate) { - crate.withModule(FluentClientGenerator.customizableOperationModule) { writer -> + crate.withModule(RustModule.Operation) { writer -> + writer.docs("Operation customization and supporting types") + writer.rust("pub mod customize;") + } + + crate.withNonRootModule(CUSTOMIZE_MODULE) { writer -> renderCustomizableOperationModule(writer) if (includeFluentClient) { @@ -65,7 +77,7 @@ class CustomizableOperationGenerator( /// A wrapper type for [`Operation`](aws_smithy_http::operation::Operation)s that allows for /// customization of the operation before it is sent. A `CustomizableOperation` may be sent - /// by calling its [`.send()`][crate::customizable_operation::CustomizableOperation::send] method. + /// by calling its [`.send()`][crate::operation::customize::CustomizableOperation::send] method. ##[derive(Debug)] pub struct CustomizableOperation#{combined_generics_decl:W} { pub(crate) handle: Arc, 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 41fed6e9d6..ae83e920a6 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 @@ -74,11 +74,6 @@ class FluentClientGenerator( "client", "Client and fluent builders for calling the service.", ) - - val customizableOperationModule = RustModule.public( - "customizable_operation", - "Wrap operations in a special type allowing for the modification of operations and the requests inside before sending them", - ) } private val serviceShape = codegenContext.serviceShape @@ -284,7 +279,7 @@ class FluentClientGenerator( /// Consume this builder, creating a customizable operation that can be modified before being /// sent. The operation's inner [http::Request] can be modified as well. pub async fn customize(self) -> std::result::Result< - crate::customizable_operation::CustomizableOperation#{customizable_op_type_params:W}, + crate::operation::customize::CustomizableOperation#{customizable_op_type_params:W}, #{SdkError}<#{OperationError}> > #{send_bounds:W} { let handle = self.handle.clone(); @@ -292,7 +287,7 @@ class FluentClientGenerator( .make_operation(&handle.conf) .await .map_err(|err|#{SdkError}::ConstructionFailure(err.into()))?; - Ok(crate::customizable_operation::CustomizableOperation { handle, operation }) + Ok(crate::operation::customize::CustomizableOperation { handle, operation }) } /// Sends the request and returns the response. diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt index 99c59ff51b..3afd9cb8de 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt @@ -13,7 +13,6 @@ import software.amazon.smithy.rust.codegen.client.rustlang.RustModule import software.amazon.smithy.rust.codegen.client.rustlang.RustWriter import software.amazon.smithy.rust.codegen.client.rustlang.Visibility import software.amazon.smithy.rust.codegen.client.smithy.CoreCodegenContext -import software.amazon.smithy.rust.codegen.client.smithy.DefaultPublicModules import software.amazon.smithy.rust.codegen.client.smithy.RustCrate import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.ProtocolGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.ProtocolSupport @@ -41,7 +40,7 @@ open class ServerServiceGenerator( * which assigns a symbol location to each shape. */ fun render() { - rustCrate.withModule(DefaultPublicModules["operation"]!!) { writer -> + rustCrate.withModule(RustModule.Operation) { writer -> ServerProtocolTestGenerator(coreCodegenContext, protocolSupport, protocolGenerator).render(writer) } From 9c4255210cbff2a4e006954ffedbf36044142ea2 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 15 Sep 2022 17:36:25 -0700 Subject: [PATCH 07/15] Improve operation customization re-exports --- .../src/default_provider/timeout_config.rs | 2 +- .../aws-config/src/imds/client.rs | 2 +- .../aws-config/src/imds/client/token.rs | 2 +- .../ResiliencyConfigCustomization.kt | 23 +++++++++++-------- .../client/CustomizableOperationGenerator.kt | 17 ++++++++++---- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs b/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs index 21b9cbbf8f..4ac803e597 100644 --- a/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs +++ b/aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs @@ -4,7 +4,7 @@ */ use crate::provider_config::ProviderConfig; -use aws_sdk_sso::config::TimeoutConfig; +use aws_sdk_sso::config::timeout::TimeoutConfig; use std::time::Duration; const SDK_DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_millis(3100); diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 4aae4679c6..54d431850a 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -40,7 +40,7 @@ use crate::imds::client::token::TokenMiddleware; use crate::profile::ProfileParseError; use crate::provider_config::ProviderConfig; use crate::{profile, PKG_VERSION}; -use aws_sdk_sso::config::TimeoutConfig; +use aws_sdk_sso::config::timeout::TimeoutConfig; use aws_smithy_client::http_connector::HttpSettings; mod token; diff --git a/aws/rust-runtime/aws-config/src/imds/client/token.rs b/aws/rust-runtime/aws-config/src/imds/client/token.rs index 846e4e118f..f39291d41a 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/token.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/token.rs @@ -38,7 +38,7 @@ use http::{HeaderValue, Uri}; use crate::cache::ExpiringCache; use crate::imds::client::{ImdsError, ImdsResponseRetryClassifier, TokenError}; -use aws_sdk_sso::config::TimeoutConfig; +use aws_sdk_sso::config::timeout::TimeoutConfig; /// Token Refresh Buffer /// diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt index 5735439a43..75fcdcf1ee 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt @@ -74,7 +74,8 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ## Examples /// ```no_run - /// use $moduleUseName::config::{Config, RetryConfig}; + /// use $moduleUseName::config::Config; + /// use $moduleUseName::config::retry::RetryConfig; /// /// let retry_config = RetryConfig::standard().with_max_attempts(5); /// let config = Config::builder().retry_config(retry_config).build(); @@ -88,7 +89,8 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ## Examples /// ```no_run - /// use $moduleUseName::config::{Builder, Config, RetryConfig}; + /// use $moduleUseName::config::{Builder, Config}; + /// use $moduleUseName::config::retry::RetryConfig; /// /// fn disable_retries(builder: &mut Builder) { /// let retry_config = RetryConfig::standard().with_max_attempts(1); @@ -164,7 +166,8 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ```no_run /// ## use std::time::Duration; - /// use $moduleUseName::config::{Config, TimeoutConfig}; + /// use $moduleUseName::config::Config; + /// use $moduleUseName::config::timeout::TimeoutConfig; /// /// let timeout_config = TimeoutConfig::builder() /// .operation_attempt_timeout(Duration::from_secs(1)) @@ -182,7 +185,8 @@ class ResiliencyConfigCustomization(coreCodegenContext: CoreCodegenContext) : Co /// /// ```no_run /// ## use std::time::Duration; - /// use $moduleUseName::config::{Builder, Config, TimeoutConfig}; + /// use $moduleUseName::config::{Builder, Config}; + /// use $moduleUseName::config::timeout::TimeoutConfig; /// /// fn set_request_timeout(builder: &mut Builder) { /// let timeout_config = TimeoutConfig::builder() @@ -221,13 +225,14 @@ class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) writer.rustTemplate( """ pub use #{sleep}::{AsyncSleep, Sleep}; - pub use #{types_retry}::{RetryConfig, RetryConfigBuilder}; - /// Config structs required by [`RetryConfig`] + /// Retry configuration pub mod retry { - pub use #{types_retry}::RetryMode; - pub use #{types_retry}::RetryKind; + pub use #{types_retry}::{RetryConfig, RetryConfigBuilder, RetryMode}; + } + /// Timeout configuration + pub mod timeout { + pub use #{timeout}::{TimeoutConfig, TimeoutConfigBuilder}; } - pub use #{timeout}::{TimeoutConfig, TimeoutConfigBuilder}; """, "types_retry" to smithyTypesRetry(runtimeConfig), "sleep" to smithyAsyncRtSleep(runtimeConfig), diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt index 0851a1f1f1..7e795be721 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt @@ -30,6 +30,9 @@ class CustomizableOperationGenerator( const val CUSTOMIZE_MODULE = "crate::operation::customize" } + private val smithyHttp = CargoDependency.SmithyHttp(runtimeConfig).asType() + private val smithyTypes = CargoDependency.SmithyTypes(runtimeConfig).asType() + fun render(crate: RustCrate) { crate.withModule(RustModule.Operation) { writer -> writer.docs("Operation customization and supporting types") @@ -37,6 +40,16 @@ class CustomizableOperationGenerator( } crate.withNonRootModule(CUSTOMIZE_MODULE) { writer -> + writer.rustTemplate( + """ + pub use #{Operation}; + pub use #{ClassifyRetry}; + pub use #{RetryKind}; + """, + "Operation" to smithyHttp.member("operation::Operation"), + "ClassifyRetry" to smithyHttp.member("retry::ClassifyRetry"), + "RetryKind" to smithyTypes.member("retry::RetryKind"), + ) renderCustomizableOperationModule(writer) if (includeFluentClient) { @@ -46,8 +59,6 @@ class CustomizableOperationGenerator( } private fun renderCustomizableOperationModule(writer: RustWriter) { - val smithyHttp = CargoDependency.SmithyHttp(runtimeConfig).asType() - val operationGenerics = GenericsGenerator(GenericTypeArg("O"), GenericTypeArg("Retry")) val handleGenerics = generics.toGenericsGenerator() val combinedGenerics = operationGenerics + handleGenerics @@ -56,7 +67,6 @@ class CustomizableOperationGenerator( // SDK Types "http_result" to smithyHttp.member("result"), "http_body" to smithyHttp.member("body"), - "http_operation" to smithyHttp.member("operation"), "HttpRequest" to CargoDependency.Http.asType().member("Request"), "handle_generics_decl" to handleGenerics.declaration(), "handle_generics_bounds" to handleGenerics.bounds(), @@ -69,7 +79,6 @@ class CustomizableOperationGenerator( use crate::client::Handle; use #{http_body}::SdkBody; - use #{http_operation}::Operation; use #{http_result}::SdkError; use std::convert::Infallible; From e61b9eb57c9b5a55e161b8c09d15606e116c7704 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 16 Sep 2022 16:41:53 -0700 Subject: [PATCH 08/15] Add integration tests for read and connect timeouts --- .../integration-tests/s3/tests/timeouts.rs | 105 +++++++++++++++++- 1 file changed, 102 insertions(+), 3 deletions(-) diff --git a/aws/sdk/integration-tests/s3/tests/timeouts.rs b/aws/sdk/integration-tests/s3/tests/timeouts.rs index 346b8d9aff..f444c63c22 100644 --- a/aws/sdk/integration-tests/s3/tests/timeouts.rs +++ b/aws/sdk/integration-tests/s3/tests/timeouts.rs @@ -3,20 +3,23 @@ * SPDX-License-Identifier: Apache-2.0 */ +use aws_config::SdkConfig; use aws_sdk_s3::model::{ CompressionType, CsvInput, CsvOutput, ExpressionType, FileHeaderInfo, InputSerialization, OutputSerialization, }; -use aws_sdk_s3::{Client, Config, Credentials, Region}; +use aws_sdk_s3::{Client, Config, Credentials, Endpoint, Region}; use aws_smithy_async::assert_elapsed; -use aws_smithy_async::rt::sleep::{AsyncSleep, TokioSleep}; +use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, TokioSleep}; use aws_smithy_client::never::NeverService; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; use aws_smithy_types::timeout::TimeoutConfig; - +use aws_types::credentials::SharedCredentialsProvider; use std::sync::Arc; use std::time::Duration; +use tokio::net::TcpListener; +use tokio::time::timeout; #[tokio::test(start_paused = true)] async fn test_timeout_service_ends_request_that_never_completes() { @@ -66,3 +69,99 @@ async fn test_timeout_service_ends_request_that_never_completes() { assert_eq!(format!("{:?}", err), "TimeoutError(RequestTimeoutError { kind: \"operation timeout (all attempts including retries)\", duration: 500ms })"); assert_elapsed!(now, std::time::Duration::from_secs_f32(0.5)); } + +#[tokio::test] +async fn test_read_timeout() { + async fn run_server(mut shutdown_receiver: tokio::sync::oneshot::Receiver<()>) { + let listener = TcpListener::bind("127.0.0.1:18103").await.unwrap(); + while shutdown_receiver.try_recv().is_err() { + if let Ok(result) = timeout(Duration::from_millis(100), listener.accept()).await { + if let Ok((_socket, _)) = result { + tokio::time::sleep(Duration::from_millis(1000)).await; + } + } + } + } + let (server_shutdown, server_shutdown_receiver) = tokio::sync::oneshot::channel(); + let server_handle = tokio::spawn(run_server(server_shutdown_receiver)); + tokio::time::sleep(Duration::from_millis(100)).await; + + let config = SdkConfig::builder() + .sleep_impl(default_async_sleep().unwrap()) + .timeout_config( + TimeoutConfig::builder() + .read_timeout(Duration::from_millis(300)) + .build(), + ) + .endpoint_resolver(Endpoint::immutable( + "http://127.0.0.1:18103".parse().unwrap(), + )) + .region(Some(Region::from_static("us-east-1"))) + .credentials_provider(SharedCredentialsProvider::new(Credentials::new( + "test", "test", None, None, "test", + ))) + .build(); + let client = Client::new(&config); + + if let Ok(result) = timeout( + Duration::from_millis(1000), + client.get_object().bucket("test").key("test").send(), + ) + .await + { + match result { + Ok(_) => panic!("should not have succeeded"), + Err(err) => { + assert_eq!( + "timeout: HTTP read timeout occurred after 300ms", + format!("{}", dbg!(err)) + ); + } + } + } else { + panic!("the client didn't timeout"); + } + + server_shutdown.send(()).unwrap(); + server_handle.await.unwrap(); +} + +#[tokio::test] +async fn test_connect_timeout() { + let config = SdkConfig::builder() + .sleep_impl(default_async_sleep().unwrap()) + .timeout_config( + TimeoutConfig::builder() + .connect_timeout(Duration::from_millis(300)) + .build(), + ) + .endpoint_resolver(Endpoint::immutable( + // Emulate a connect timeout error by hitting an unroutable IP + "http://172.255.255.0:18104".parse().unwrap(), + )) + .region(Some(Region::from_static("us-east-1"))) + .credentials_provider(SharedCredentialsProvider::new(Credentials::new( + "test", "test", None, None, "test", + ))) + .build(); + let client = Client::new(&config); + + if let Ok(result) = timeout( + Duration::from_millis(1000), + client.get_object().bucket("test").key("test").send(), + ) + .await + { + match result { + Ok(_) => panic!("should not have succeeded"), + Err(err) => { + assert_eq!( + "timeout: error trying to connect: HTTP connect timeout occurred after 300ms", + format!("{}", dbg!(err)) + ); + } + } + } else { + panic!("the client didn't timeout"); + } +} From 97c1a1b03d212832a464551ec2ef9d91f11fa1e0 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 16 Sep 2022 19:26:16 -0700 Subject: [PATCH 09/15] Fix connector timeout config in the Smithy `Client` builder --- .../aws-config/src/imds/client.rs | 2 +- .../aws-config/src/imds/client/token.rs | 2 +- .../rustsdk/AwsFluentClientDecorator.kt | 13 +- rust-runtime/aws-smithy-client/Cargo.toml | 3 +- rust-runtime/aws-smithy-client/src/builder.rs | 176 ++++++++++++------ rust-runtime/aws-smithy-client/src/erase.rs | 4 +- .../aws-smithy-client/src/hyper_ext.rs | 154 +++++---------- rust-runtime/aws-smithy-client/src/lib.rs | 13 +- rust-runtime/aws-smithy-client/src/timeout.rs | 33 ++-- .../aws-smithy-types/src/timeout/config.rs | 48 +++++ .../aws-smithy-types/src/timeout/mod.rs | 2 +- 11 files changed, 257 insertions(+), 193 deletions(-) diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 54d431850a..75d9f507b4 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -582,7 +582,7 @@ impl Builder { .connector(connector.clone()) .middleware(middleware) .retry_config(retry_config) - .timeout_config(timeout_config); + .operation_timeout_config(timeout_config.into()); smithy_builder.set_sleep_impl(config.sleep()); let smithy_client = smithy_builder.build(); diff --git a/aws/rust-runtime/aws-config/src/imds/client/token.rs b/aws/rust-runtime/aws-config/src/imds/client/token.rs index f39291d41a..4cf6f299fa 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/token.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/token.rs @@ -92,7 +92,7 @@ impl TokenMiddleware { .connector(connector) .middleware(MapRequestLayer::::default()) .retry_config(retry_config) - .timeout_config(timeout_config); + .operation_timeout_config(timeout_config.into()); inner_builder.set_sleep_impl(sleep_impl); let inner_client = inner_builder.build(); let client = Arc::new(inner_client); diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt index 578b3cc86d..d15bed7ce6 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt @@ -49,8 +49,9 @@ private class Types(runtimeConfig: RuntimeConfig) { val defaultMiddleware = runtimeConfig.defaultMiddleware() val dynConnector = RuntimeType("DynConnector", smithyClientDep, "aws_smithy_client::erase") val dynMiddleware = RuntimeType("DynMiddleware", smithyClientDep, "aws_smithy_client::erase") - val smithyConnector = RuntimeType("SmithyConnector", smithyClientDep, "aws_smithy_client::bounds") + val httpSettings = RuntimeType("HttpSettings", smithyClientDep, "aws_smithy_client::http_connector") val retryConfig = RuntimeType("RetryConfig", smithyTypesDep, "aws_smithy_types::retry") + val smithyConnector = RuntimeType("SmithyConnector", smithyClientDep, "aws_smithy_client::bounds") val timeoutConfig = RuntimeType("TimeoutConfig", smithyTypesDep, "aws_smithy_types::timeout") val connectorError = RuntimeType("ConnectorError", smithyHttpDep, "aws_smithy_http::result") @@ -137,10 +138,11 @@ private class AwsFluentClientExtensions(types: Types) { "ConnectorError" to types.connectorError, "DynConnector" to types.dynConnector, "DynMiddleware" to types.dynMiddleware, + "HttpSettings" to types.httpSettings, "Middleware" to types.defaultMiddleware, "RetryConfig" to types.retryConfig, - "TimeoutConfig" to types.timeoutConfig, "SmithyConnector" to types.smithyConnector, + "TimeoutConfig" to types.timeoutConfig, "aws_smithy_client" to types.awsSmithyClient, "aws_types" to types.awsTypes, "retry" to types.smithyClientRetry, @@ -162,7 +164,7 @@ private class AwsFluentClientExtensions(types: Types) { .connector(#{DynConnector}::new(conn)) .middleware(#{DynMiddleware}::new(#{Middleware}::new())) .retry_config(retry_config.into()) - .timeout_config(timeout_config); + .operation_timeout_config(timeout_config.into()); builder.set_sleep_impl(conf.sleep_impl()); let client = builder.build(); Self { handle: std::sync::Arc::new(Handle { client, conf }) } @@ -184,10 +186,11 @@ private class AwsFluentClientExtensions(types: Types) { 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."); } - let mut builder = #{aws_smithy_client}::Builder::dyn_https() + let mut builder = #{aws_smithy_client}::Builder::new() + .dyn_https_connector(#{HttpSettings}::from_timeout_config(&timeout_config)) .middleware(#{DynMiddleware}::new(#{Middleware}::new())) .retry_config(retry_config.into()) - .timeout_config(timeout_config); + .operation_timeout_config(timeout_config.into()); builder.set_sleep_impl(sleep_impl); let client = builder.build(); diff --git a/rust-runtime/aws-smithy-client/Cargo.toml b/rust-runtime/aws-smithy-client/Cargo.toml index 30337e9db5..598207acf0 100644 --- a/rust-runtime/aws-smithy-client/Cargo.toml +++ b/rust-runtime/aws-smithy-client/Cargo.toml @@ -25,7 +25,7 @@ fastrand = "1.4.0" http = "0.2.3" http-body = "0.4.4" hyper = { version = "0.14.12", features = ["client", "http2", "http1"], optional = true } -hyper-rustls = { version = "0.22.1", optional = true, features = ["rustls-native-certs"] } +hyper-rustls = { version = "0.23.0", optional = true, features = ["rustls-native-certs", "http2"] } hyper-tls = { version = "0.5.0", optional = true } lazy_static = { version = "1", optional = true } pin-project-lite = "0.2.7" @@ -36,6 +36,7 @@ tracing = "0.1" [dev-dependencies] aws-smithy-async = { path = "../aws-smithy-async", features = ["rt-tokio"] } +hyper-rustls = { version = "0.23.0", features = ["webpki-roots"] } serde = { version = "1", features = ["derive"] } serde_json = "1" tokio = { version = "1.8.4", features = ["full", "test-util"] } diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index a639e164d3..e19985d783 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -3,19 +3,29 @@ * SPDX-License-Identifier: Apache-2.0 */ +use crate::erase::DynConnector; +use crate::http_connector::HttpSettings; +use crate::hyper_ext::Adapter as HyperAdapter; use crate::{bounds, erase, retry, Client}; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; -use aws_smithy_types::timeout::TimeoutConfig; +use aws_smithy_types::timeout::{OperationTimeoutConfig, TimeoutConfig}; use std::sync::Arc; #[derive(Clone, Debug)] -enum BuilderRetryConfig { - // Since generic specialization isn't a thing yet, we have to keep a copy of the retry - // config around when using standard retry so that it can be properly validated. - Standard { has_retry: bool, policy: R }, - Overridden(R), +struct MaybeRequiresSleep { + requires_sleep: bool, + implementation: I, +} + +impl MaybeRequiresSleep { + fn new(requires_sleep: bool, implementation: I) -> Self { + Self { + requires_sleep, + implementation, + } + } } /// A builder that provides more customization options when constructing a [`Client`]. @@ -25,10 +35,10 @@ enum BuilderRetryConfig { /// documentation. #[derive(Clone, Debug)] pub struct Builder { - connector: C, + connector: MaybeRequiresSleep, middleware: M, - retry_config: BuilderRetryConfig, - timeout_config: Option, + retry_policy: MaybeRequiresSleep, + operation_timeout_config: Option, sleep_impl: Option>, } @@ -40,13 +50,13 @@ where fn default() -> Self { let default_retry_config = retry::Config::default(); Self { - connector: Default::default(), + connector: MaybeRequiresSleep::new(false, Default::default()), middleware: Default::default(), - retry_config: BuilderRetryConfig::Standard { - has_retry: default_retry_config.has_retry(), - policy: retry::Standard::new(default_retry_config), - }, - timeout_config: None, + retry_policy: MaybeRequiresSleep::new( + default_retry_config.has_retry(), + retry::Standard::new(default_retry_config), + ), + operation_timeout_config: None, sleep_impl: default_async_sleep(), } } @@ -73,6 +83,51 @@ where } } +#[cfg(feature = "rustls")] +impl Builder<(), M, R> { + /// Connect to the service over HTTPS using Rustls using dynamic dispatch. + pub fn rustls_connector(self, http_settings: HttpSettings) -> Builder { + self.connector(DynConnector::new( + HyperAdapter::builder() + .http_settings(http_settings) + .build(crate::conns::https()), + )) + } +} + +#[cfg(feature = "native-tls")] +impl Builder<(), M, R> { + /// Connect to the service over HTTPS using the native TLS library on your + /// platform using dynamic dispatch. + pub fn native_tls_connector(self, http_settings: HttpSettings) -> Builder { + self.connector(DynConnector::new( + HyperAdapter::builder() + .http_settings(http_settings) + .build(crate::conns::native_tls()), + )) + } +} + +#[cfg(any(feature = "rustls", feature = "native-tls"))] +impl Builder<(), M, R> { + /// Create a Smithy client builder with an HTTPS connector and the [standard retry + /// policy](crate::retry::Standard) over the default middleware implementation. + /// + /// For convenience, this constructor type-erases the concrete TLS connector backend used using + /// dynamic dispatch. This comes at a slight runtime performance cost. See + /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use + /// [`Builder::rustls_connector`] or [`Builder::native_tls_connector`] instead. + pub fn dyn_https_connector(self, http_settings: HttpSettings) -> Builder { + #[cfg(feature = "rustls")] + let with_https = |b: Builder<_, M, R>| b.rustls_connector(http_settings); + // If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled + #[cfg(not(feature = "rustls"))] + let with_https = |b: Builder<_, M, R>| b.native_tls_connector(); + + with_https(self) + } +} + impl Builder<(), M, R> { /// Specify the connector for the eventual client to use. /// @@ -84,10 +139,10 @@ impl Builder<(), M, R> { /// [`Builder::connector_fn`]. pub fn connector(self, connector: C) -> Builder { Builder { - connector, + connector: MaybeRequiresSleep::new(false, connector), middleware: self.middleware, - retry_config: self.retry_config, - timeout_config: self.timeout_config, + retry_policy: self.retry_policy, + operation_timeout_config: self.operation_timeout_config, sleep_impl: self.sleep_impl, } } @@ -141,8 +196,8 @@ impl Builder { pub fn middleware(self, middleware: M) -> Builder { Builder { connector: self.connector, - retry_config: self.retry_config, - timeout_config: self.timeout_config, + retry_policy: self.retry_policy, + operation_timeout_config: self.operation_timeout_config, middleware, sleep_impl: self.sleep_impl, } @@ -192,8 +247,8 @@ impl Builder { pub fn retry_policy(self, retry_policy: R) -> Builder { Builder { connector: self.connector, - retry_config: BuilderRetryConfig::Overridden(retry_policy), - timeout_config: self.timeout_config, + retry_policy: MaybeRequiresSleep::new(false, retry_policy), + operation_timeout_config: self.operation_timeout_config, middleware: self.middleware, sleep_impl: self.sleep_impl, } @@ -204,10 +259,8 @@ impl Builder { /// Set the standard retry policy's configuration. pub fn set_retry_config(&mut self, config: Option) -> &mut Self { let config = config.unwrap_or_default(); - self.retry_config = BuilderRetryConfig::Standard { - has_retry: config.has_retry(), - policy: retry::Standard::new(config), - }; + self.retry_policy = + MaybeRequiresSleep::new(config.has_retry(), retry::Standard::new(config)); self } @@ -217,15 +270,21 @@ impl Builder { self } - /// Set a timeout config for the builder - pub fn set_timeout_config(&mut self, timeout_config: Option) -> &mut Self { - self.timeout_config = timeout_config; + /// Set operation timeout config for the client. + pub fn set_operation_timeout_config( + &mut self, + operation_timeout_config: Option, + ) -> &mut Self { + self.operation_timeout_config = operation_timeout_config; self } - /// Set a timeout config for the builder - pub fn timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { - self.timeout_config = Some(timeout_config); + /// Set operation timeout config for the client. + pub fn operation_timeout_config( + mut self, + operation_timeout_config: OperationTimeoutConfig, + ) -> Self { + self.operation_timeout_config = Some(operation_timeout_config); self } @@ -249,10 +308,13 @@ impl Builder { F: FnOnce(C) -> C2, { Builder { - connector: map(self.connector), + connector: MaybeRequiresSleep::new( + self.connector.requires_sleep, + map(self.connector.implementation), + ), middleware: self.middleware, - retry_config: self.retry_config, - timeout_config: self.timeout_config, + retry_policy: self.retry_policy, + operation_timeout_config: self.operation_timeout_config, sleep_impl: self.sleep_impl, } } @@ -265,15 +327,17 @@ impl Builder { Builder { connector: self.connector, middleware: map(self.middleware), - retry_config: self.retry_config, - timeout_config: self.timeout_config, + retry_policy: self.retry_policy, + operation_timeout_config: self.operation_timeout_config, sleep_impl: self.sleep_impl, } } /// Build a Smithy service [`Client`]. pub fn build(self) -> Client { - let timeout_config = self.timeout_config.unwrap_or_else(TimeoutConfig::disabled); + let operation_timeout_config = self + .operation_timeout_config + .unwrap_or_else(|| TimeoutConfig::disabled().into()); if self.sleep_impl.is_none() { const ADDITIONAL_HELP: &str = "Either disable retry by setting max attempts to one, or pass in a `sleep_impl`. \ @@ -281,23 +345,21 @@ impl Builder { the `aws-smithy-async` crate is required for your async runtime. If you are using \ Tokio, then make sure the `rt-tokio` feature is enabled to have its sleep \ implementation set automatically."; - if let BuilderRetryConfig::Standard { has_retry, .. } = self.retry_config { - if has_retry { - panic!("Retries require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); - } + if self.connector.requires_sleep { + panic!("Socket-level retries for the default connector require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); } - if timeout_config.has_timeouts() { - panic!("Timeouts require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); + if self.retry_policy.requires_sleep { + panic!("Retries require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); + } + if operation_timeout_config.has_timeouts() { + panic!("Operation timeouts require a `sleep_impl`, but none was passed into the builder. {ADDITIONAL_HELP}"); } } Client { - connector: self.connector, - retry_policy: match self.retry_config { - BuilderRetryConfig::Standard { policy, .. } => policy, - BuilderRetryConfig::Overridden(policy) => policy, - }, + connector: self.connector.implementation, + retry_policy: self.retry_policy.implementation, middleware: self.middleware, - timeout_config, + operation_timeout_config, sleep_impl: self.sleep_impl, } } @@ -383,7 +445,7 @@ mod tests { .connect_timeout(Duration::from_secs(1)) .build(); assert!(timeout_config.has_timeouts()); - builder.set_timeout_config(Some(timeout_config)); + builder.set_operation_timeout_config(Some(timeout_config.into())); let result = panic::catch_unwind(AssertUnwindSafe(move || { let _ = builder.build(); @@ -430,7 +492,7 @@ mod tests { .connect_timeout(Duration::from_secs(1)) .build(); assert!(timeout_config.has_timeouts()); - builder.set_timeout_config(Some(timeout_config)); + builder.set_operation_timeout_config(Some(timeout_config.into())); let retry_config = retry::Config::default(); assert!(retry_config.has_retry()); @@ -438,4 +500,14 @@ mod tests { let _ = builder.build(); } + + #[test] + fn builder_connection_helpers_are_dyn() { + #[cfg(feature = "rustls")] + let _builder: Builder = + Builder::new().rustls_connector(HttpSettings::default()); + #[cfg(feature = "native-tls")] + let _builder: Builder = + Builder::new().native_tls_connector(HttpSettings::default()); + } } diff --git a/rust-runtime/aws-smithy-client/src/erase.rs b/rust-runtime/aws-smithy-client/src/erase.rs index d633d995ca..a097b75d67 100644 --- a/rust-runtime/aws-smithy-client/src/erase.rs +++ b/rust-runtime/aws-smithy-client/src/erase.rs @@ -56,7 +56,7 @@ where connector: self.connector, middleware: DynMiddleware::new(self.middleware), retry_policy: self.retry_policy, - timeout_config: self.timeout_config, + operation_timeout_config: self.operation_timeout_config, sleep_impl: self.sleep_impl, } } @@ -96,7 +96,7 @@ where connector: DynConnector::new(self.connector), middleware: self.middleware, retry_policy: self.retry_policy, - timeout_config: self.timeout_config, + operation_timeout_config: self.operation_timeout_config, sleep_impl: self.sleep_impl, } } diff --git a/rust-runtime/aws-smithy-client/src/hyper_ext.rs b/rust-runtime/aws-smithy-client/src/hyper_ext.rs index d2ccac54e0..20e9af5a0c 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -6,7 +6,8 @@ //! Implementation of [`SmithyConnector`](crate::bounds::SmithyConnector) for Hyper //! //! The module provides [`Adapter`] which enables using a [`hyper::Client`] as the connector for a Smithy -//! [`Client`](crate::Client). +//! [`Client`](crate::Client). For most use cases, this shouldn't need to be used directly, but it is +//! available as an option. //! //! # Examples //! @@ -14,21 +15,25 @@ //! //! In the basic case, customers should not need to use this module. A default implementation of Hyper //! with `rustls` will be constructed during client creation. However, if you are creating a Smithy -//! [`Client`](crate::Client), directly, use the `https()` method to match the default behavior: +//! [`Client`](crate::Client), directly, use the `dyn_https_https()` method to match that default behavior: //! //! ```no_run -//! use aws_smithy_client::Builder; -//! use aws_smithy_client::erase::DynConnector; +//! use aws_smithy_client::Client; +//! use aws_smithy_client::http_connector::HttpSettings; //! -//! // Replace this with your middleware type -//! type MyMiddleware = tower::layer::util::Identity; -//! let client = Builder::::dyn_https().build(); +//! let client = Client::builder() +//! .dyn_https_connector(HttpSettings::default()) +//! .middleware( +//! // Replace this with your middleware type +//! tower::layer::util::Identity::default() +//! ) +//! .build(); //! ``` //! -//! ### Create a Hyper client with a custom timeout +//! ### Use a Hyper client that uses WebPKI roots //! -//! One common use case for constructing a connector directly is setting socket connect or HTTP read timeouts. -//! Since the internal connector is cheap to clone, you can also use this to share a connector between multiple services. +//! A use case for where you may want to use the [`Adapter`] is when settings Hyper client settings +//! that aren't otherwise exposed by the `Client` builder interface. //! //! ```no_run //! use std::time::Duration; @@ -36,38 +41,42 @@ //! use aws_smithy_client::erase::DynConnector; //! use aws_smithy_client::http_connector::HttpSettings; //! -//! let connector = hyper_ext::Adapter::builder() +//! let https_connector = hyper_rustls::HttpsConnectorBuilder::new() +//! .with_webpki_roots() +//! .https_only() +//! .enable_http1() +//! .enable_http2() +//! .build(); +//! let smithy_connector = hyper_ext::Adapter::builder() +//! // Optionally set things like timeouts as well //! .http_settings( //! HttpSettings::builder() //! .connect_timeout(Duration::from_secs(5)) //! .build() //! ) -//! .build(conns::https()); -//! // Replace this with your middleware -//! type MyMiddleware = tower::layer::util::Identity; -//! // once you have a connector, use it to construct a Smithy client: -//! let client = Client::::new(DynConnector::new(connector)); +//! .build(https_connector); +//! +//! // Once you have a Smithy connector, use it to construct a Smithy client: +//! let client = Client::builder() +//! .connector(smithy_connector) +//! .middleware(tower::layer::util::Identity::default()) +//! .build(); //! ``` -use std::error::Error; -use std::sync::Arc; - -use http::Uri; -use hyper::client::connect::{Connected, Connection}; -use tokio::io::{AsyncRead, AsyncWrite}; -use tower::{BoxError, Service}; - +use crate::http_connector::HttpSettings; +use crate::hyper_ext::timeout_middleware::{ConnectTimeout, HttpReadTimeout, HttpTimeoutError}; +use crate::never::stream::EmptyStream; use aws_smithy_async::future::timeout::TimedOutError; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; use aws_smithy_types::retry::ErrorKind; - -use self::timeout_middleware::{ConnectTimeout, HttpReadTimeout, HttpTimeoutError}; -use crate::erase::DynConnector; -use crate::http_connector::HttpSettings; -use crate::never::stream::EmptyStream; -use crate::Builder as ClientBuilder; +use http::Uri; +use hyper::client::connect::{Connected, Connection}; +use std::error::Error; +use std::sync::Arc; +use tokio::io::{AsyncRead, AsyncWrite}; +use tower::{BoxError, Service}; /// Adapter from a [`hyper::Client`](hyper::Client) to a connector usable by a Smithy [`Client`](crate::Client). /// @@ -290,70 +299,6 @@ impl Builder { } } -#[cfg(any(feature = "rustls", feature = "native-tls"))] -impl crate::Builder -where - M: Default, -{ - /// Create a Smithy client builder with an HTTPS connector and the [standard retry - /// policy](crate::retry::Standard) over the default middleware implementation. - /// - /// For convenience, this constructor type-erases the concrete TLS connector backend used using - /// dynamic dispatch. This comes at a slight runtime performance cost. See - /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use - /// [`Builder::rustls`](ClientBuilder::rustls) or `Builder::native_tls` instead. - pub fn dyn_https() -> Self { - #[cfg(feature = "rustls")] - let with_https = |b: ClientBuilder<_>| b.rustls(); - // If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled - #[cfg(not(feature = "rustls"))] - let with_https = |b: ClientBuilder<_>| b.native_tls(); - - with_https(ClientBuilder::new()) - .middleware(M::default()) - .map_connector(DynConnector::new) - } -} - -#[cfg(any(feature = "rustls", feature = "native_tls"))] -impl crate::Client -where - M: Default, - M: crate::bounds::SmithyMiddleware + Send + Sync + 'static, -{ - /// Create a Smithy client builder with an HTTPS connector and the [standard retry - /// policy](crate::retry::Standard) over the default middleware implementation. - /// - /// For convenience, this constructor type-erases the concrete TLS connector backend used using - /// dynamic dispatch. This comes at a slight runtime performance cost. See - /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use - /// [`Builder::rustls`](ClientBuilder::rustls) or `Builder::native_tls` instead. - pub fn dyn_https() -> Self { - ClientBuilder::::dyn_https().build() - } -} - -#[cfg(feature = "rustls")] -impl ClientBuilder<(), M, R> { - /// Connect to the service over HTTPS using Rustls using dynamic dispatch. - pub fn rustls(self) -> ClientBuilder { - self.connector(DynConnector::new( - Adapter::builder().build(crate::conns::https()), - )) - } -} - -#[cfg(feature = "native-tls")] -impl ClientBuilder<(), M, R> { - /// Connect to the service over HTTPS using the native TLS library on your - /// platform using dynamic dispatch. - pub fn native_tls(self) -> ClientBuilder { - self.connector(DynConnector::new( - Adapter::builder().build(crate::conns::native_tls()), - )) - } -} - mod timeout_middleware { use std::error::Error; use std::fmt::Formatter; @@ -661,29 +606,16 @@ impl Connection for EmptyStream { #[cfg(test)] mod test { + use crate::hyper_ext::Adapter; + use aws_smithy_http::body::SdkBody; + use http::Uri; + use hyper::client::connect::{Connected, Connection}; use std::io::{Error, ErrorKind}; use std::pin::Pin; use std::task::{Context, Poll}; - - use http::Uri; - use hyper::client::connect::{Connected, Connection}; use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; use tower::BoxError; - use aws_smithy_http::body::SdkBody; - - use super::ClientBuilder; - use crate::erase::DynConnector; - use crate::hyper_ext::Adapter; - - #[test] - fn builder_connection_helpers_are_dyn() { - #[cfg(feature = "rustls")] - let _builder: ClientBuilder = ClientBuilder::new().rustls(); - #[cfg(feature = "native-tls")] - let _builder: ClientBuilder = ClientBuilder::new().native_tls(); - } - #[tokio::test] async fn hyper_io_error() { let connector = TestConnection { diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index a5b44ae484..b6efb53969 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -62,7 +62,12 @@ pub mod conns { #[cfg(feature = "rustls")] lazy_static::lazy_static! { static ref HTTPS_NATIVE_ROOTS: Https = { - hyper_rustls::HttpsConnector::with_native_roots() + hyper_rustls::HttpsConnectorBuilder::new() + .with_native_roots() + .https_or_http() + .enable_http1() + .enable_http2() + .build() }; } @@ -93,7 +98,7 @@ use aws_smithy_http::retry::ClassifyRetry; use aws_smithy_http_tower::dispatch::DispatchLayer; use aws_smithy_http_tower::parse_response::ParseResponseLayer; use aws_smithy_types::retry::ProvideErrorKind; -use aws_smithy_types::timeout::TimeoutConfig; +use aws_smithy_types::timeout::OperationTimeoutConfig; use std::error::Error; use std::sync::Arc; use timeout::ClientTimeoutParams; @@ -127,7 +132,7 @@ pub struct Client< connector: Connector, middleware: Middleware, retry_policy: RetryPolicy, - timeout_config: TimeoutConfig, + operation_timeout_config: OperationTimeoutConfig, sleep_impl: Option>, } @@ -203,7 +208,7 @@ where let connector = self.connector.clone(); let timeout_params = - ClientTimeoutParams::new(&self.timeout_config, self.sleep_impl.clone()); + ClientTimeoutParams::new(&self.operation_timeout_config, self.sleep_impl.clone()); let svc = ServiceBuilder::new() .layer(TimeoutLayer::new(timeout_params.operation_timeout)) diff --git a/rust-runtime/aws-smithy-client/src/timeout.rs b/rust-runtime/aws-smithy-client/src/timeout.rs index 489160a4b8..a3d7db5241 100644 --- a/rust-runtime/aws-smithy-client/src/timeout.rs +++ b/rust-runtime/aws-smithy-client/src/timeout.rs @@ -14,7 +14,7 @@ use crate::SdkError; use aws_smithy_async::future::timeout::Timeout; use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; use aws_smithy_http::operation::Operation; -use aws_smithy_types::timeout::TimeoutConfig; +use aws_smithy_types::timeout::OperationTimeoutConfig; use pin_project_lite::pin_project; use std::future::Future; use std::pin::Pin; @@ -64,25 +64,26 @@ pub(crate) struct ClientTimeoutParams { } impl ClientTimeoutParams { - pub fn new(timeout_config: &TimeoutConfig, async_sleep: Option>) -> Self { + pub fn new( + timeout_config: &OperationTimeoutConfig, + async_sleep: Option>, + ) -> Self { if let Some(async_sleep) = async_sleep { Self { - operation_timeout: timeout_config - .operation_timeout() - .map(|duration| TimeoutServiceParams { + operation_timeout: timeout_config.operation_timeout().map(|duration| { + TimeoutServiceParams { duration, kind: "operation timeout (all attempts including retries)", async_sleep: async_sleep.clone(), - }) - .into(), - operation_attempt_timeout: timeout_config - .operation_attempt_timeout() - .map(|duration| TimeoutServiceParams { + } + }), + operation_attempt_timeout: timeout_config.operation_attempt_timeout().map( + |duration| TimeoutServiceParams { duration, kind: "operation attempt timeout (single attempt)", async_sleep: async_sleep.clone(), - }) - .into(), + }, + ), } } else { Default::default() @@ -249,9 +250,11 @@ mod test { let req = Request::new(http::Request::new(SdkBody::empty())); let op = Operation::new(req, ()); let never_service: NeverService<_, (), _> = NeverService::new(); - let timeout_config = TimeoutConfig::builder() - .operation_timeout(Duration::from_secs_f32(0.25)) - .build(); + let timeout_config = OperationTimeoutConfig::from( + TimeoutConfig::builder() + .operation_timeout(Duration::from_secs_f32(0.25)) + .build(), + ); let sleep_impl: Arc = Arc::new(TokioSleep::new()); let timeout_service_params = ClientTimeoutParams::new(&timeout_config, Some(sleep_impl)); let mut svc = ServiceBuilder::new() diff --git a/rust-runtime/aws-smithy-types/src/timeout/config.rs b/rust-runtime/aws-smithy-types/src/timeout/config.rs index 9cbc6b4e0b..68d1296695 100644 --- a/rust-runtime/aws-smithy-types/src/timeout/config.rs +++ b/rust-runtime/aws-smithy-types/src/timeout/config.rs @@ -271,3 +271,51 @@ impl TimeoutConfig { || self.operation_attempt_timeout.is_some() } } + +/// Configuration subset of [`TimeoutConfig`] for operation timeouts +#[non_exhaustive] +#[derive(Clone, PartialEq, Debug)] +pub struct OperationTimeoutConfig { + operation_timeout: Option, + operation_attempt_timeout: Option, +} + +impl OperationTimeoutConfig { + /// Returns this config's operation timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// The operation timeout is a limit on the total amount of time it takes for an operation to be + /// fully serviced, including the time for all retries that may have been attempted for it. + pub fn operation_timeout(&self) -> Option { + self.operation_timeout + } + + /// Returns this config's operation attempt timeout. + /// + /// An operation represents the full request/response lifecycle of a call to a service. + /// When retries are enabled, then this setting makes it possible to set a timeout for individual + /// retry attempts (including the initial attempt) for an operation. + pub fn operation_attempt_timeout(&self) -> Option { + self.operation_attempt_timeout + } + + /// Returns true if any of the possible timeouts are set. + pub fn has_timeouts(&self) -> bool { + self.operation_timeout.is_some() || self.operation_attempt_timeout.is_some() + } +} + +impl From<&TimeoutConfig> for OperationTimeoutConfig { + fn from(cfg: &TimeoutConfig) -> Self { + OperationTimeoutConfig { + operation_timeout: cfg.operation_timeout, + operation_attempt_timeout: cfg.operation_attempt_timeout, + } + } +} + +impl From for OperationTimeoutConfig { + fn from(cfg: TimeoutConfig) -> Self { + OperationTimeoutConfig::from(&cfg) + } +} diff --git a/rust-runtime/aws-smithy-types/src/timeout/mod.rs b/rust-runtime/aws-smithy-types/src/timeout/mod.rs index e0a7c34a92..b32eaac637 100644 --- a/rust-runtime/aws-smithy-types/src/timeout/mod.rs +++ b/rust-runtime/aws-smithy-types/src/timeout/mod.rs @@ -9,5 +9,5 @@ mod config; mod error; -pub use config::{TimeoutConfig, TimeoutConfigBuilder}; +pub use config::{OperationTimeoutConfig, TimeoutConfig, TimeoutConfigBuilder}; pub use error::ConfigError; From d833d17d7ed0b7710f64c46d85d14cf35b639708 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 20 Sep 2022 13:29:53 -0700 Subject: [PATCH 10/15] Fix tests --- aws/rust-runtime/aws-config/external-types.toml | 5 +++-- .../aws-inlineable/tests/middleware_e2e_test.rs | 6 +++++- .../smithy/generators/client/FluentClientDecorator.kt | 7 ++++--- .../smithy/rust/codegen/client/testutil/Rust.kt | 11 +++++++++-- .../ResiliencyConfigCustomizationTest.kt | 8 ++++++-- rust-runtime/aws-smithy-client/src/builder.rs | 6 ++++-- rust-runtime/aws-smithy-client/src/lib.rs | 4 ++-- .../examples/pokemon-service-test/tests/helpers.rs | 6 +++--- .../examples/pokemon-service/tests/helpers.rs | 6 +++--- 9 files changed, 39 insertions(+), 20 deletions(-) diff --git a/aws/rust-runtime/aws-config/external-types.toml b/aws/rust-runtime/aws-config/external-types.toml index 40a56765f6..74fb4e49fe 100644 --- a/aws/rust-runtime/aws-config/external-types.toml +++ b/aws/rust-runtime/aws-config/external-types.toml @@ -11,9 +11,10 @@ allowed_external_types = [ "aws_smithy_client::http_connector::HttpSettings", "aws_smithy_http::body::SdkBody", "aws_smithy_http::result::SdkError", - "aws_smithy_types::retry::RetryConfig*", + "aws_smithy_types::retry", + "aws_smithy_types::retry::*", "aws_smithy_types::timeout", - "aws_smithy_types::timeout::config::Config", + "aws_smithy_types::timeout::config::TimeoutConfig", "aws_smithy_types::timeout::error::ConfigError", "aws_types::*", "http::response::Response", diff --git a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs index 23932395db..a5900ac1cb 100644 --- a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs +++ b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs @@ -20,6 +20,7 @@ use aws_http::user_agent::AwsUserAgent; use aws_inlineable::middleware::DefaultMiddleware; use aws_sig_auth::signer::OperationSigningConfig; +use aws_smithy_client::http_connector::HttpSettings; use aws_smithy_client::test_connection::TestConnection; use aws_smithy_http::body::SdkBody; use aws_smithy_http::endpoint::ResolveEndpoint; @@ -117,7 +118,10 @@ fn test_operation() -> Operation { - val testDir = TestWorkspace.subproject() +fun generatePluginContext( + model: Model, + additionalSettings: ObjectNode = ObjectNode.builder().build(), + addModuleToEventStreamAllowList: Boolean = false, + service: String? = null, + runtimeConfig: RuntimeConfig? = null, + overrideTestDir: File? = null, +): Pair { + val testDir = overrideTestDir ?: TestWorkspace.subproject() val moduleName = "test_${testDir.nameWithoutExtension}" val testPath = testDir.toPath() val manifest = FileManifest.create(testPath) diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/customizations/ResiliencyConfigCustomizationTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/customizations/ResiliencyConfigCustomizationTest.kt index cbf7917856..8cb3506f70 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/customizations/ResiliencyConfigCustomizationTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/customizations/ResiliencyConfigCustomizationTest.kt @@ -7,13 +7,15 @@ package software.amazon.smithy.rust.codegen.client.customizations import org.junit.jupiter.api.Test 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.transformers.OperationNormalizer import software.amazon.smithy.rust.codegen.client.smithy.transformers.RecursiveShapeBoxer import software.amazon.smithy.rust.codegen.client.testutil.TestWorkspace import software.amazon.smithy.rust.codegen.client.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.client.testutil.compileAndTest import software.amazon.smithy.rust.codegen.client.testutil.rustSettings +import software.amazon.smithy.rust.codegen.client.testutil.stubConfigProject import software.amazon.smithy.rust.codegen.client.testutil.testCodegenContext -import software.amazon.smithy.rust.codegen.client.testutil.validateConfigCustomizations internal class ResiliencyConfigCustomizationTest { private val baseModel = """ @@ -38,6 +40,8 @@ internal class ResiliencyConfigCustomizationTest { val project = TestWorkspace.testProject() val codegenContext = testCodegenContext(model, settings = project.rustSettings()) - validateConfigCustomizations(ResiliencyConfigCustomization(codegenContext), project) + stubConfigProject(ResiliencyConfigCustomization(codegenContext), project) + ResiliencyReExportCustomization(codegenContext.runtimeConfig).extras(project) + project.compileAndTest() } } diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index e19985d783..b893894987 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -5,7 +5,6 @@ use crate::erase::DynConnector; use crate::http_connector::HttpSettings; -use crate::hyper_ext::Adapter as HyperAdapter; use crate::{bounds, erase, retry, Client}; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_http::body::SdkBody; @@ -83,6 +82,9 @@ where } } +#[cfg(any(feature = "rustls", feature = "native-tls"))] +use crate::hyper_ext::Adapter as HyperAdapter; + #[cfg(feature = "rustls")] impl Builder<(), M, R> { /// Connect to the service over HTTPS using Rustls using dynamic dispatch. @@ -211,7 +213,7 @@ impl Builder { /// use aws_smithy_client::never::NeverConnector; /// use aws_smithy_http::body::SdkBody; /// let my_connector = DynConnector::new( - /// // Your own connector here or use `dyn_https()` + /// // Your own connector here or use `dyn_https_connector()` /// # NeverConnector::new() /// ); /// let client = Builder::new() diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index b6efb53969..c33186c395 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -121,8 +121,8 @@ use tower::{Layer, Service, ServiceBuilder, ServiceExt}; /// /// With the `hyper` feature enabled, you can construct a `Client` directly from a /// [`hyper::Client`] using [`hyper_ext::Adapter::builder`]. You can also enable the `rustls` or `native-tls` -/// features to construct a Client against a standard HTTPS endpoint using [`Builder::rustls`] and -/// `Builder::native_tls` respectively. +/// features to construct a Client against a standard HTTPS endpoint using [`Builder::rustls_connector`] and +/// `Builder::native_tls_connector` respectively. #[derive(Debug)] pub struct Client< Connector = erase::DynConnector, diff --git a/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs b/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs index f294100ed2..df7832d67e 100644 --- a/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs +++ b/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs @@ -4,7 +4,7 @@ */ use command_group::{CommandGroup, GroupChild}; -use pokemon_service_client::{Builder, Client, Config}; +use pokemon_service_client::{Client, Config}; use std::{process::Command, thread, time::Duration}; pub(crate) struct PokemonService { @@ -43,8 +43,8 @@ pub fn client() -> Client< aws_smithy_client::erase::DynConnector, aws_smithy_client::erase::DynMiddleware, > { - let raw_client = Builder::new() - .rustls() + let raw_client = Client::builder() + .rustls_connector(Default::default()) .middleware_fn(|mut req| { let http_req = req.http_mut(); let uri = format!("http://localhost:13734{}", http_req.uri().path()); diff --git a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs index 62322ba085..e232f74840 100644 --- a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs +++ b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs @@ -11,7 +11,7 @@ use std::time::Duration; use assert_cmd::prelude::*; use aws_smithy_client::{erase::DynConnector, hyper_ext::Adapter}; use aws_smithy_http::operation::Request; -use pokemon_service_client::{Builder, Client, Config}; +use pokemon_service_client::{Client, Config}; use tokio::time; const TEST_KEY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/testdata/localhost.key"); @@ -81,8 +81,8 @@ pub fn client() -> Client< aws_smithy_client::erase::DynMiddleware, > { let base_url = PokemonServiceVariant::Http.base_url(); - let raw_client = Builder::new() - .rustls() + let raw_client = Config::builder() + .rustls_connector(Default::default()) .middleware_fn(rewrite_base_url(base_url)) .build_dyn(); let config = Config::builder().build(); From 87051701a26875afecdcabb0245af8b143b1095b Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 20 Sep 2022 16:25:58 -0700 Subject: [PATCH 11/15] Fix unused imports --- .../aws-inlineable/tests/middleware_e2e_test.rs | 3 +-- rust-runtime/aws-smithy-client/src/builder.rs | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs index a5900ac1cb..e52667290b 100644 --- a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs +++ b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs @@ -20,7 +20,6 @@ use aws_http::user_agent::AwsUserAgent; use aws_inlineable::middleware::DefaultMiddleware; use aws_sig_auth::signer::OperationSigningConfig; -use aws_smithy_client::http_connector::HttpSettings; use aws_smithy_client::test_connection::TestConnection; use aws_smithy_http::body::SdkBody; use aws_smithy_http::endpoint::ResolveEndpoint; @@ -119,7 +118,7 @@ fn test_operation() -> Operation Date: Tue, 20 Sep 2022 17:27:49 -0700 Subject: [PATCH 12/15] Rename `HttpSettings` to `ConnectorSettings` --- .../aws-config/external-types.toml | 2 +- aws/rust-runtime/aws-config/src/connector.rs | 18 +++++----- aws/rust-runtime/aws-config/src/ecs.rs | 6 ++-- .../src/http_credential_provider.rs | 14 ++++---- .../aws-config/src/imds/client.rs | 6 ++-- aws/rust-runtime/aws-config/src/lib.rs | 4 +-- .../aws-config/src/provider_config.rs | 15 ++++---- aws/rust-runtime/aws-config/src/sso.rs | 3 +- aws/rust-runtime/aws-config/src/sts.rs | 3 +- .../aws-config/src/sts/assume_role.rs | 3 +- aws/rust-runtime/aws-config/src/test_case.rs | 4 +-- .../rustsdk/AwsFluentClientDecorator.kt | 6 ++-- rust-runtime/aws-smithy-client/src/builder.rs | 27 +++++++++----- .../aws-smithy-client/src/http_connector.rs | 26 +++++++------- .../aws-smithy-client/src/hyper_ext.rs | 36 ++++++++++--------- rust-runtime/aws-smithy-client/src/timeout.rs | 5 --- 16 files changed, 90 insertions(+), 88 deletions(-) diff --git a/aws/rust-runtime/aws-config/external-types.toml b/aws/rust-runtime/aws-config/external-types.toml index 74fb4e49fe..94b43e4d03 100644 --- a/aws/rust-runtime/aws-config/external-types.toml +++ b/aws/rust-runtime/aws-config/external-types.toml @@ -8,7 +8,7 @@ allowed_external_types = [ "aws_smithy_client::erase::DynConnector", "aws_smithy_client::erase::boxclone::BoxCloneService", "aws_smithy_client::http_connector::HttpConnector", - "aws_smithy_client::http_connector::HttpSettings", + "aws_smithy_client::http_connector::ConnectorSettings", "aws_smithy_http::body::SdkBody", "aws_smithy_http::result::SdkError", "aws_smithy_types::retry", diff --git a/aws/rust-runtime/aws-config/src/connector.rs b/aws/rust-runtime/aws-config/src/connector.rs index ae826a7b71..4ea5ee77ff 100644 --- a/aws/rust-runtime/aws-config/src/connector.rs +++ b/aws/rust-runtime/aws-config/src/connector.rs @@ -7,7 +7,7 @@ use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_client::erase::DynConnector; -use aws_smithy_client::http_connector::HttpSettings; +use aws_smithy_client::http_connector::ConnectorSettings; use std::sync::Arc; // unused when all crate features are disabled @@ -18,41 +18,41 @@ pub(crate) fn expect_connector(connector: Option) -> DynConnector #[cfg(any(feature = "rustls", feature = "native-tls"))] fn base( - settings: &HttpSettings, + settings: &ConnectorSettings, sleep: Option>, ) -> aws_smithy_client::hyper_ext::Builder { let mut hyper = - aws_smithy_client::hyper_ext::Adapter::builder().http_settings(settings.clone()); + aws_smithy_client::hyper_ext::Adapter::builder().connector_settings(settings.clone()); if let Some(sleep) = sleep { hyper = hyper.sleep_impl(sleep); } hyper } -/// Given `HttpSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. +/// Given `ConnectorSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. #[cfg(feature = "rustls")] pub fn default_connector( - settings: &HttpSettings, + settings: &ConnectorSettings, sleep: Option>, ) -> Option { let hyper = base(settings, sleep).build(aws_smithy_client::conns::https()); Some(DynConnector::new(hyper)) } -/// Given `HttpSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. +/// Given `ConnectorSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. #[cfg(all(not(feature = "rustls"), feature = "native-tls"))] pub fn default_connector( - settings: &HttpSettings, + settings: &ConnectorSettings, sleep: Option>, ) -> Option { let hyper = base(settings, sleep).build(aws_smithy_client::conns::native_tls()); Some(DynConnector::new(hyper)) } -/// Given `HttpSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. +/// Given `ConnectorSettings` and an `AsyncSleep`, create a `DynConnector` from defaults depending on what cargo features are activated. #[cfg(not(any(feature = "rustls", feature = "native-tls")))] pub fn default_connector( - _settings: &HttpSettings, + _settings: &ConnectorSettings, _sleep: Option>, ) -> Option { None diff --git a/aws/rust-runtime/aws-config/src/ecs.rs b/aws/rust-runtime/aws-config/src/ecs.rs index 920772d8eb..45a4a16992 100644 --- a/aws/rust-runtime/aws-config/src/ecs.rs +++ b/aws/rust-runtime/aws-config/src/ecs.rs @@ -61,7 +61,7 @@ use tower::{Service, ServiceExt}; use crate::http_credential_provider::HttpCredentialProvider; use crate::provider_config::ProviderConfig; -use aws_smithy_client::http_connector::HttpSettings; +use aws_smithy_client::http_connector::ConnectorSettings; use aws_types::os_shim_internal::Env; use http::header::InvalidHeaderValue; use std::time::Duration; @@ -168,8 +168,8 @@ impl Provider { }; let http_provider = HttpCredentialProvider::builder() .configure(&provider_config) - .http_settings( - HttpSettings::builder() + .connector_settings( + ConnectorSettings::builder() .connect_timeout(DEFAULT_CONNECT_TIMEOUT) .read_timeout(DEFAULT_READ_TIMEOUT) .build(), 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 b219332cae..45010b6aee 100644 --- a/aws/rust-runtime/aws-config/src/http_credential_provider.rs +++ b/aws/rust-runtime/aws-config/src/http_credential_provider.rs @@ -9,7 +9,7 @@ //! Future work will stabilize this interface and enable it to be used directly. use aws_smithy_client::erase::DynConnector; -use aws_smithy_client::http_connector::HttpSettings; +use aws_smithy_client::http_connector::ConnectorSettings; use aws_smithy_http::body::SdkBody; use aws_smithy_http::operation::{Operation, Request}; use aws_smithy_http::response::ParseStrictResponse; @@ -78,7 +78,7 @@ impl HttpCredentialProvider { #[derive(Default)] pub(crate) struct Builder { provider_config: Option, - http_settings: Option, + connector_settings: Option, } impl Builder { @@ -87,20 +87,20 @@ impl Builder { self } - pub(crate) fn http_settings(mut self, http_settings: HttpSettings) -> Self { - self.http_settings = Some(http_settings); + pub(crate) fn connector_settings(mut self, connector_settings: ConnectorSettings) -> Self { + self.connector_settings = Some(connector_settings); self } pub(crate) fn build(self, provider_name: &'static str, uri: Uri) -> HttpCredentialProvider { let provider_config = self.provider_config.unwrap_or_default(); - let http_settings = self.http_settings.unwrap_or_else(|| { - HttpSettings::builder() + let connector_settings = self.connector_settings.unwrap_or_else(|| { + ConnectorSettings::builder() .connect_timeout(DEFAULT_CONNECT_TIMEOUT) .read_timeout(DEFAULT_READ_TIMEOUT) .build() }); - let connector = expect_connector(provider_config.connector(&http_settings)); + let connector = expect_connector(provider_config.connector(&connector_settings)); let mut client_builder = aws_smithy_client::Client::builder() .connector(connector) .middleware(Identity::new()); diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 75d9f507b4..a1e39def07 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -41,7 +41,7 @@ use crate::profile::ProfileParseError; use crate::provider_config::ProviderConfig; use crate::{profile, PKG_VERSION}; use aws_sdk_sso::config::timeout::TimeoutConfig; -use aws_smithy_client::http_connector::HttpSettings; +use aws_smithy_client::http_connector::ConnectorSettings; mod token; @@ -559,8 +559,8 @@ impl Builder { .connect_timeout(DEFAULT_CONNECT_TIMEOUT) .read_timeout(DEFAULT_READ_TIMEOUT) .build(); - let http_settings = HttpSettings::from_timeout_config(&timeout_config); - let connector = expect_connector(config.connector(&http_settings)); + let connector_settings = ConnectorSettings::from_timeout_config(&timeout_config); + let connector = expect_connector(config.connector(&connector_settings)); let endpoint_source = self .endpoint .unwrap_or_else(|| EndpointSource::Env(config.env(), config.fs())); diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index fe795005fd..6d4506fc4e 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -170,7 +170,7 @@ mod loader { use crate::connector::default_connector; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; - use aws_smithy_client::http_connector::{HttpConnector, HttpSettings}; + use aws_smithy_client::http_connector::{ConnectorSettings, HttpConnector}; use aws_smithy_types::retry::RetryConfig; use aws_smithy_types::timeout::TimeoutConfig; use aws_types::app_name::AppName; @@ -413,7 +413,7 @@ mod loader { http_connector } else { HttpConnector::Prebuilt(default_connector( - &HttpSettings::from_timeout_config(&timeout_config), + &ConnectorSettings::from_timeout_config(&timeout_config), sleep_impl.clone(), )) }; diff --git a/aws/rust-runtime/aws-config/src/provider_config.rs b/aws/rust-runtime/aws-config/src/provider_config.rs index e32cc74f49..553be2645a 100644 --- a/aws/rust-runtime/aws-config/src/provider_config.rs +++ b/aws/rust-runtime/aws-config/src/provider_config.rs @@ -9,7 +9,7 @@ use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_client::erase::DynConnector; use aws_types::os_shim_internal::{Env, Fs, TimeSource}; use aws_types::{ - http_connector::{HttpConnector, HttpSettings}, + http_connector::{ConnectorSettings, HttpConnector}, region::Region, }; @@ -50,7 +50,7 @@ impl Debug for ProviderConfig { impl Default for ProviderConfig { fn default() -> Self { let connector = HttpConnector::ConnectorFn(Arc::new( - |settings: &HttpSettings, sleep: Option>| { + |settings: &ConnectorSettings, sleep: Option>| { default_connector(settings, sleep) }, )); @@ -161,11 +161,11 @@ impl ProviderConfig { #[allow(dead_code)] pub(crate) fn default_connector(&self) -> Option { self.connector - .connector(&HttpSettings::default(), self.sleep.clone()) + .connector(&Default::default(), self.sleep.clone()) } #[allow(dead_code)] - pub(crate) fn connector(&self, settings: &HttpSettings) -> Option { + pub(crate) fn connector(&self, settings: &ConnectorSettings) -> Option { self.connector.connector(settings, self.sleep.clone()) } @@ -249,9 +249,10 @@ impl ProviderConfig { C::Future: Unpin + Send + 'static, C::Error: Into>, { - let connector_fn = move |settings: &HttpSettings, sleep: Option>| { - let mut builder = - aws_smithy_client::hyper_ext::Adapter::builder().http_settings(settings.clone()); + let connector_fn = move |settings: &ConnectorSettings, + sleep: Option>| { + let mut builder = aws_smithy_client::hyper_ext::Adapter::builder() + .connector_settings(settings.clone()); if let Some(sleep) = sleep { builder = builder.sleep_impl(sleep); }; diff --git a/aws/rust-runtime/aws-config/src/sso.rs b/aws/rust-runtime/aws-config/src/sso.rs index 8c3b4ea948..54bcc07b7f 100644 --- a/aws/rust-runtime/aws-config/src/sso.rs +++ b/aws/rust-runtime/aws-config/src/sso.rs @@ -39,10 +39,9 @@ impl crate::provider_config::ProviderConfig { &self, ) -> aws_smithy_client::Client { use crate::connector::expect_connector; - use aws_smithy_client::http_connector::HttpSettings; let mut client_builder = aws_smithy_client::Client::builder() - .connector(expect_connector(self.connector(&HttpSettings::default()))) + .connector(expect_connector(self.connector(&Default::default()))) .middleware(SsoMiddleware::default()); client_builder.set_sleep_impl(self.sleep()); client_builder.build() diff --git a/aws/rust-runtime/aws-config/src/sts.rs b/aws/rust-runtime/aws-config/src/sts.rs index 802850c008..e6e02a43cb 100644 --- a/aws/rust-runtime/aws-config/src/sts.rs +++ b/aws/rust-runtime/aws-config/src/sts.rs @@ -8,7 +8,6 @@ use crate::connector::expect_connector; use aws_sdk_sts::middleware::DefaultMiddleware; use aws_smithy_client::erase::DynConnector; -use aws_smithy_client::http_connector::HttpSettings; use aws_smithy_client::Client; pub(crate) mod util; @@ -20,7 +19,7 @@ mod assume_role; impl crate::provider_config::ProviderConfig { pub(crate) fn sts_client(&self) -> Client { let mut builder = Client::builder() - .connector(expect_connector(self.connector(&HttpSettings::default()))) + .connector(expect_connector(self.connector(&Default::default()))) .middleware(DefaultMiddleware::default()); builder.set_sleep_impl(self.sleep()); builder.build() diff --git a/aws/rust-runtime/aws-config/src/sts/assume_role.rs b/aws/rust-runtime/aws-config/src/sts/assume_role.rs index c33f3a11d7..fd576a0425 100644 --- a/aws/rust-runtime/aws-config/src/sts/assume_role.rs +++ b/aws/rust-runtime/aws-config/src/sts/assume_role.rs @@ -9,7 +9,6 @@ use aws_sdk_sts::error::AssumeRoleErrorKind; use aws_sdk_sts::middleware::DefaultMiddleware; use aws_sdk_sts::operation::AssumeRole; use aws_smithy_client::erase::DynConnector; -use aws_smithy_client::http_connector::HttpSettings; use aws_smithy_http::result::SdkError; use aws_types::credentials::{ self, future, CredentialsError, ProvideCredentials, SharedCredentialsProvider, @@ -173,7 +172,7 @@ impl AssumeRoleProviderBuilder { .build(); let conn = conf - .connector(&HttpSettings::default()) + .connector(&Default::default()) .expect("A connector must be provided"); let mut client_builder = aws_smithy_client::Client::builder() .connector(conn) diff --git a/aws/rust-runtime/aws-config/src/test_case.rs b/aws/rust-runtime/aws-config/src/test_case.rs index bf5fc83851..9543995503 100644 --- a/aws/rust-runtime/aws-config/src/test_case.rs +++ b/aws/rust-runtime/aws-config/src/test_case.rs @@ -8,7 +8,6 @@ use crate::provider_config::ProviderConfig; use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep, TokioSleep}; use aws_smithy_client::dvr::{NetworkTraffic, RecordingConnection, ReplayingConnection}; use aws_smithy_client::erase::DynConnector; -use aws_smithy_client::http_connector::HttpSettings; use aws_types::credentials::{self, ProvideCredentials}; use aws_types::os_shim_internal::{Env, Fs}; @@ -182,9 +181,8 @@ impl TestEnvironment { P: ProvideCredentials, { // swap out the connector generated from `http-traffic.json` for a real connector: - let settings = HttpSettings::default(); let (_test_connector, config) = self.provider_config().await; - let live_connector = default_connector(&settings, config.sleep()).unwrap(); + let live_connector = default_connector(&Default::default(), config.sleep()).unwrap(); let live_connector = RecordingConnection::new(live_connector); let config = config.with_http_connector(DynConnector::new(live_connector.clone())); let provider = make_provider(config).await; diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt index d15bed7ce6..40ba2c6cf8 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt @@ -46,10 +46,10 @@ private class Types(runtimeConfig: RuntimeConfig) { val smithyClientRetry = RuntimeType("retry", smithyClientDep, "aws_smithy_client") val awsSmithyClient = smithyClientDep.asType() + val connectorSettings = RuntimeType("ConnectorSettings", smithyClientDep, "aws_smithy_client::http_connector") val defaultMiddleware = runtimeConfig.defaultMiddleware() val dynConnector = RuntimeType("DynConnector", smithyClientDep, "aws_smithy_client::erase") val dynMiddleware = RuntimeType("DynMiddleware", smithyClientDep, "aws_smithy_client::erase") - val httpSettings = RuntimeType("HttpSettings", smithyClientDep, "aws_smithy_client::http_connector") val retryConfig = RuntimeType("RetryConfig", smithyTypesDep, "aws_smithy_types::retry") val smithyConnector = RuntimeType("SmithyConnector", smithyClientDep, "aws_smithy_client::bounds") val timeoutConfig = RuntimeType("TimeoutConfig", smithyTypesDep, "aws_smithy_types::timeout") @@ -138,7 +138,7 @@ private class AwsFluentClientExtensions(types: Types) { "ConnectorError" to types.connectorError, "DynConnector" to types.dynConnector, "DynMiddleware" to types.dynMiddleware, - "HttpSettings" to types.httpSettings, + "ConnectorSettings" to types.connectorSettings, "Middleware" to types.defaultMiddleware, "RetryConfig" to types.retryConfig, "SmithyConnector" to types.smithyConnector, @@ -187,7 +187,7 @@ private class AwsFluentClientExtensions(types: Types) { Set the `sleep_impl` on the Config passed into this function to fix this panic."); } let mut builder = #{aws_smithy_client}::Builder::new() - .dyn_https_connector(#{HttpSettings}::from_timeout_config(&timeout_config)) + .dyn_https_connector(#{ConnectorSettings}::from_timeout_config(&timeout_config)) .middleware(#{DynMiddleware}::new(#{Middleware}::new())) .retry_config(retry_config.into()) .operation_timeout_config(timeout_config.into()); diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index e4dea96fa0..017ae04e7b 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -83,17 +83,20 @@ where #[cfg(any(feature = "rustls", feature = "native-tls"))] use crate::erase::DynConnector; #[cfg(any(feature = "rustls", feature = "native-tls"))] -use crate::http_connector::HttpSettings; +use crate::http_connector::ConnectorSettings; #[cfg(any(feature = "rustls", feature = "native-tls"))] use crate::hyper_ext::Adapter as HyperAdapter; #[cfg(feature = "rustls")] impl Builder<(), M, R> { /// Connect to the service over HTTPS using Rustls using dynamic dispatch. - pub fn rustls_connector(self, http_settings: HttpSettings) -> Builder { + pub fn rustls_connector( + self, + connector_settings: ConnectorSettings, + ) -> Builder { self.connector(DynConnector::new( HyperAdapter::builder() - .http_settings(http_settings) + .connector_settings(connector_settings) .build(crate::conns::https()), )) } @@ -103,10 +106,13 @@ impl Builder<(), M, R> { impl Builder<(), M, R> { /// Connect to the service over HTTPS using the native TLS library on your /// platform using dynamic dispatch. - pub fn native_tls_connector(self, http_settings: HttpSettings) -> Builder { + pub fn native_tls_connector( + self, + connector_settings: ConnectorSettings, + ) -> Builder { self.connector(DynConnector::new( HyperAdapter::builder() - .http_settings(http_settings) + .connector_settings(connector_settings) .build(crate::conns::native_tls()), )) } @@ -121,9 +127,12 @@ impl Builder<(), M, R> { /// dynamic dispatch. This comes at a slight runtime performance cost. See /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use /// [`Builder::rustls_connector`] or [`Builder::native_tls_connector`] instead. - pub fn dyn_https_connector(self, http_settings: HttpSettings) -> Builder { + pub fn dyn_https_connector( + self, + connector_settings: ConnectorSettings, + ) -> Builder { #[cfg(feature = "rustls")] - let with_https = |b: Builder<_, M, R>| b.rustls_connector(http_settings); + let with_https = |b: Builder<_, M, R>| b.rustls_connector(connector_settings); // If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled #[cfg(not(feature = "rustls"))] let with_https = |b: Builder<_, M, R>| b.native_tls_connector(); @@ -509,9 +518,9 @@ mod tests { fn builder_connection_helpers_are_dyn() { #[cfg(feature = "rustls")] let _builder: Builder = - Builder::new().rustls_connector(HttpSettings::default()); + Builder::new().rustls_connector(Default::default()); #[cfg(feature = "native-tls")] let _builder: Builder = - Builder::new().native_tls_connector(HttpSettings::default()); + Builder::new().native_tls_connector(Default::default()); } } diff --git a/rust-runtime/aws-smithy-client/src/http_connector.rs b/rust-runtime/aws-smithy-client/src/http_connector.rs index 8112f8b96d..67db6e6918 100644 --- a/rust-runtime/aws-smithy-client/src/http_connector.rs +++ b/rust-runtime/aws-smithy-client/src/http_connector.rs @@ -14,7 +14,7 @@ use std::{fmt::Debug, sync::Arc}; /// Type alias for a Connector factory function. pub type MakeConnectorFn = - dyn Fn(&HttpSettings, Option>) -> Option + Send + Sync; + dyn Fn(&ConnectorSettings, Option>) -> Option + Send + Sync; /// Enum for describing the two "kinds" of HTTP Connectors in smithy-rs. #[derive(Clone)] @@ -46,7 +46,7 @@ impl HttpConnector { /// If `HttpConnector` is `ConnectorFn`, generate a new connector from settings and return it. pub fn connector( &self, - settings: &HttpSettings, + settings: &ConnectorSettings, sleep: Option>, ) -> Option { match self { @@ -56,15 +56,15 @@ impl HttpConnector { } } -/// Builder for [`HttpSettings`]. +/// Builder for [`ConnectorSettings`]. #[non_exhaustive] #[derive(Default, Debug)] -pub struct HttpSettingsBuilder { +pub struct ConnectorSettingsBuilder { connect_timeout: Option, read_timeout: Option, } -impl HttpSettingsBuilder { +impl ConnectorSettingsBuilder { /// Creates a new builder. pub fn new() -> Self { Default::default() @@ -104,9 +104,9 @@ impl HttpSettingsBuilder { self } - /// Builds the [`HttpSettings`]. - pub fn build(self) -> HttpSettings { - HttpSettings { + /// Builds the [`ConnectorSettings`]. + pub fn build(self) -> ConnectorSettings { + ConnectorSettings { connect_timeout: self.connect_timeout, read_timeout: self.read_timeout, } @@ -116,14 +116,14 @@ impl HttpSettingsBuilder { /// Settings for HTTP Connectors #[non_exhaustive] #[derive(Clone, Default, Debug)] -pub struct HttpSettings { +pub struct ConnectorSettings { connect_timeout: Option, read_timeout: Option, } -impl HttpSettings { - /// Returns a builder for `HttpSettings`. - pub fn builder() -> HttpSettingsBuilder { +impl ConnectorSettings { + /// Returns a builder for `ConnectorSettings`. + pub fn builder() -> ConnectorSettingsBuilder { Default::default() } @@ -143,7 +143,7 @@ impl HttpSettings { } // This function may be removed/refactored in the future if other non-timeout - // properties are added to the `HttpSettings` struct. + // properties are added to the `ConnectorSettings` struct. #[doc(hidden)] pub fn from_timeout_config(timeout_config: &TimeoutConfig) -> Self { Self { diff --git a/rust-runtime/aws-smithy-client/src/hyper_ext.rs b/rust-runtime/aws-smithy-client/src/hyper_ext.rs index 20e9af5a0c..18acf75526 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -19,10 +19,9 @@ //! //! ```no_run //! use aws_smithy_client::Client; -//! use aws_smithy_client::http_connector::HttpSettings; //! //! let client = Client::builder() -//! .dyn_https_connector(HttpSettings::default()) +//! .dyn_https_connector(Default::default()) //! .middleware( //! // Replace this with your middleware type //! tower::layer::util::Identity::default() @@ -39,7 +38,7 @@ //! use std::time::Duration; //! use aws_smithy_client::{Client, conns, hyper_ext}; //! use aws_smithy_client::erase::DynConnector; -//! use aws_smithy_client::http_connector::HttpSettings; +//! use aws_smithy_client::http_connector::ConnectorSettings; //! //! let https_connector = hyper_rustls::HttpsConnectorBuilder::new() //! .with_webpki_roots() @@ -49,8 +48,8 @@ //! .build(); //! let smithy_connector = hyper_ext::Adapter::builder() //! // Optionally set things like timeouts as well -//! .http_settings( -//! HttpSettings::builder() +//! .connector_settings( +//! ConnectorSettings::builder() //! .connect_timeout(Duration::from_secs(5)) //! .build() //! ) @@ -63,7 +62,7 @@ //! .build(); //! ``` -use crate::http_connector::HttpSettings; +use crate::http_connector::ConnectorSettings; use crate::hyper_ext::timeout_middleware::{ConnectTimeout, HttpReadTimeout, HttpTimeoutError}; use crate::never::stream::EmptyStream; use aws_smithy_async::future::timeout::TimedOutError; @@ -199,7 +198,7 @@ fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option /// ``` #[derive(Default, Debug)] pub struct Builder { - http_settings: Option, + connector_settings: Option, sleep_impl: Option>, client_builder: Option, } @@ -217,7 +216,7 @@ impl Builder { let client_builder = self.client_builder.unwrap_or_default(); let sleep_impl = self.sleep_impl.or_else(default_async_sleep); let (connect_timeout, read_timeout) = self - .http_settings + .connector_settings .map(|c| (c.connect_timeout(), c.read_timeout())) .unwrap_or((None, None)); @@ -268,14 +267,17 @@ impl Builder { } /// Configure the HTTP settings for the `HyperAdapter` - pub fn http_settings(mut self, http_settings: HttpSettings) -> Self { - self.http_settings = Some(http_settings); + pub fn connector_settings(mut self, connector_settings: ConnectorSettings) -> Self { + self.connector_settings = Some(connector_settings); self } /// Configure the HTTP settings for the `HyperAdapter` - pub fn set_http_settings(&mut self, http_settings: Option) -> &mut Self { - self.http_settings = http_settings; + pub fn set_connector_settings( + &mut self, + connector_settings: Option, + ) -> &mut Self { + self.connector_settings = connector_settings; self } @@ -505,7 +507,7 @@ mod timeout_middleware { #[cfg(test)] mod test { - use crate::http_connector::HttpSettings; + use crate::http_connector::ConnectorSettings; use crate::hyper_ext::Adapter; use crate::never::{NeverConnected, NeverReplies}; use aws_smithy_async::assert_elapsed; @@ -527,13 +529,13 @@ mod timeout_middleware { #[tokio::test] async fn http_connect_timeout_works() { let inner = NeverConnected::new(); - let http_settings = HttpSettings::from_timeout_config( + let connector_settings = ConnectorSettings::from_timeout_config( &TimeoutConfig::builder() .connect_timeout(Duration::from_secs(1)) .build(), ); let mut hyper = Adapter::builder() - .http_settings(http_settings) + .connector_settings(connector_settings) .sleep_impl(Arc::new(TokioSleep::new())) .build(inner); let now = tokio::time::Instant::now(); @@ -562,14 +564,14 @@ mod timeout_middleware { #[tokio::test] async fn http_read_timeout_works() { let inner = NeverReplies::new(); - let http_settings = HttpSettings::from_timeout_config( + let connector_settings = ConnectorSettings::from_timeout_config( &TimeoutConfig::builder() .connect_timeout(Duration::from_secs(1)) .read_timeout(Duration::from_secs(2)) .build(), ); let mut hyper = Adapter::builder() - .http_settings(http_settings) + .connector_settings(connector_settings) .sleep_impl(Arc::new(TokioSleep::new())) .build(inner); let now = tokio::time::Instant::now(); diff --git a/rust-runtime/aws-smithy-client/src/timeout.rs b/rust-runtime/aws-smithy-client/src/timeout.rs index a3d7db5241..e535678370 100644 --- a/rust-runtime/aws-smithy-client/src/timeout.rs +++ b/rust-runtime/aws-smithy-client/src/timeout.rs @@ -4,11 +4,6 @@ */ //! Timeout Configuration -//! -//! While timeout configuration is unstable, this module is in aws-smithy-client. -//! -//! As timeout and HTTP configuration stabilizes, this will move to aws-types and become a part of -//! HttpSettings. use crate::SdkError; use aws_smithy_async::future::timeout::Timeout; From cefd83f7cd8cc7c6a8135fa20ad01fc9e2cfbb7d Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 20 Sep 2022 18:15:19 -0700 Subject: [PATCH 13/15] Incorporate feedback --- .../smithy/rust/codegen/client/smithy/CodegenDelegator.kt | 8 +++++++- .../customizations/ResiliencyConfigCustomization.kt | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt index 7032d17cfc..db33989787 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenDelegator.kt @@ -139,7 +139,13 @@ open class RustCrate( } /** - * Create a new non-root module directly. + * Create a new non-root module directly. For example, if given the namespace `crate::foo::bar`, + * this will create `src/foo/bar.rs` with the contents from the given `moduleWriter`. + * Multiple calls to this with the same namespace are additive, so new code can be added + * by various customizations. + * + * Caution: this does not automatically add the required Rust `mod` statements to make this + * file an official part of the generated crate. This step needs to be done manually. */ fun withNonRootModule( namespace: String, diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt index 75fcdcf1ee..bb4e5f88d6 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt @@ -225,11 +225,16 @@ class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) writer.rustTemplate( """ pub use #{sleep}::{AsyncSleep, Sleep}; + /// Retry configuration + /// + /// These are re-exported from `aws-smithy-types` for convenience. pub mod retry { pub use #{types_retry}::{RetryConfig, RetryConfigBuilder, RetryMode}; } /// Timeout configuration + /// + /// These are re-exported from `aws-smithy-types` for convenience. pub mod timeout { pub use #{timeout}::{TimeoutConfig, TimeoutConfigBuilder}; } From f77648fcf07554a57c30029055975bd73d6c2309 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 21 Sep 2022 12:51:16 -0700 Subject: [PATCH 14/15] Fix more test failures --- aws/rust-runtime/aws-types/external-types.toml | 2 +- .../examples/pokemon-service-test/tests/helpers.rs | 4 ++-- .../examples/pokemon-service/tests/helpers.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/aws/rust-runtime/aws-types/external-types.toml b/aws/rust-runtime/aws-types/external-types.toml index df0f95847d..273584842e 100644 --- a/aws/rust-runtime/aws-types/external-types.toml +++ b/aws/rust-runtime/aws-types/external-types.toml @@ -5,6 +5,6 @@ allowed_external_types = [ "aws_smithy_http::endpoint::Endpoint", "aws_smithy_http::endpoint::EndpointPrefix", "aws_smithy_types::retry::RetryConfig", - "aws_smithy_types::timeout::config::Config", + "aws_smithy_types::timeout::config::TimeoutConfig", "http::uri::Uri", ] diff --git a/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs b/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs index df7832d67e..a2c9763167 100644 --- a/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs +++ b/rust-runtime/aws-smithy-http-server-python/examples/pokemon-service-test/tests/helpers.rs @@ -4,7 +4,7 @@ */ use command_group::{CommandGroup, GroupChild}; -use pokemon_service_client::{Client, Config}; +use pokemon_service_client::{Builder, Client, Config}; use std::{process::Command, thread, time::Duration}; pub(crate) struct PokemonService { @@ -43,7 +43,7 @@ pub fn client() -> Client< aws_smithy_client::erase::DynConnector, aws_smithy_client::erase::DynMiddleware, > { - let raw_client = Client::builder() + let raw_client = Builder::new() .rustls_connector(Default::default()) .middleware_fn(|mut req| { let http_req = req.http_mut(); diff --git a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs index e232f74840..1d632a27ed 100644 --- a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs +++ b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/tests/helpers.rs @@ -11,7 +11,7 @@ use std::time::Duration; use assert_cmd::prelude::*; use aws_smithy_client::{erase::DynConnector, hyper_ext::Adapter}; use aws_smithy_http::operation::Request; -use pokemon_service_client::{Client, Config}; +use pokemon_service_client::{Builder, Client, Config}; use tokio::time; const TEST_KEY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/testdata/localhost.key"); @@ -81,7 +81,7 @@ pub fn client() -> Client< aws_smithy_client::erase::DynMiddleware, > { let base_url = PokemonServiceVariant::Http.base_url(); - let raw_client = Config::builder() + let raw_client = Builder::new() .rustls_connector(Default::default()) .middleware_fn(rewrite_base_url(base_url)) .build_dyn(); From ae3251e0fbfa24ce13c18359a1daeeb4d8991159 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 21 Sep 2022 17:51:07 -0700 Subject: [PATCH 15/15] Update the changelog --- CHANGELOG.next.toml | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index fc4c4c2578..1f4e21c37f 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -9,4 +9,36 @@ # message = "Fix typos in module documentation for generated crates" # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} -# author = "rcoh" \ No newline at end of file +# author = "rcoh" + +[[aws-sdk-rust]] +message = """ +The SDK, by default, now times out if socket connect or time to first byte read takes longer than +3.1 seconds. There are a large number of breaking changes that come with this change that may +affect you if you customize the client configuration at all. +See [the upgrade guide](https://github.com/awslabs/aws-sdk-rust/issues/622) for information +on how to configure timeouts, and how to resolve compilation issues after upgrading. +""" +references = ["smithy-rs#1740", "smithy-rs#256"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "jdisanti" + +[[aws-sdk-rust]] +message = """ +Setting connect/read timeouts with `SdkConfig` now works. Previously, these timeout config values +were lost during connector creation, so the only reliable way to set them was to manually override +the HTTP connector. +""" +references = ["smithy-rs#1740", "smithy-rs#256"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "jdisanti" + +[[smithy-rs]] +message = """ +A large list of breaking changes were made to accomodate default timeouts in the AWS SDK. +See [the smithy-rs upgrade guide](https://github.com/awslabs/smithy-rs/issues/1760) for a full list +of breaking changes and how to resolve them. +""" +references = ["smithy-rs#1740", "smithy-rs#256"] +meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } +author = "jdisanti"