Improve timeout config ergonomics and add SDK default timeouts

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<DynConnector>`](aws_smithy_client::erase::DynConnector), and panic with a helpful error message if it's `None`
@@ -23,7 +22,7 @@ fn base(
     sleep: Option<Arc<dyn AsyncSleep>>,
 ) -> 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);
     }

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()
     }
 }

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)
     }

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<timeout::Config, timeout::ConfigError> {
+    pub fn timeout_config(&self) -> Result<TimeoutConfigBuilder, timeout::ConfigError> {
         // 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<TriState<Duration>, timeout::ConfigError> {
+) -> Result<Option<Duration>, 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
         );
     }

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<ProviderConfig>,
-    http_timeout_config: timeout::Http,
+    http_settings: Option<HttpSettings>,
 }
 
 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<Duration>) -> 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<Duration>) -> 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,

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,15 +40,16 @@ 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;
 
 // 6 hours
 const DEFAULT_TOKEN_TTL: Duration = Duration::from_secs(21_600);
 const DEFAULT_ATTEMPTS: u32 = 4;
-const DEFAULT_CONNECT_TIMEOUT: Option<Duration> = Some(Duration::from_secs(1));
-const DEFAULT_READ_TIMEOUT: Option<Duration> = 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<Client, BuildError> {
         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 {