From a9a691b6b4d53f373ef1d19e68aed997f68f4134 Mon Sep 17 00:00:00 2001 From: Zelda Hessler Date: Fri, 19 Nov 2021 15:16:24 -0600 Subject: [PATCH] Feature: Fine-grained Timeout Configuration (#831) * add: TimeoutConfig add: provider to fetch timeout from profile add: provider to fetch timeout from environment add: default provider for TimeoutConfigs update: aws_config::Config to support timeout conf update: rustdoc lint * add: TimeoutConfigBuilder::merge_with test * update: changelogs * add: timeout_config to client and builder add: generic timeout service add: non-working timeout layer * add: timeout layer/service with configurable duration add: test that ensures timeout service works * fix: eliminate useless clones * feature: Kotlin decorator for TimeoutConfig add: tests for timeout-related codegen update: incorrect package path in RetryConfigDecorator.kt fix: outdated aws-config timeout config code * remove: link to struct in external crate fix: outdated doc test fix: add missing import to doc test fix: copypaste error in doc test update: outdated lint name * update: comment on Builder::timeout_config panics add: test ensuring timeouts can't be infinite update: use a floating point number in a timeout doc update: message for failed profile load to mention that profile will be skipped remove: commented out code update: attempt to make difference between api_call_timeout and api_call_attempt_timeout clearer update: outdated doc comments update: TimeoutConfigError descriptions * formatting: arrange imports in aws_smithy_client lib.rs add: todo for improving timeout error categorization * update: var parsers to work without needless allocations update: improve example for default timeout config provider update: improve example for default retry config provider fix: unhelpful doc comment for TimeoutLayerFuture remove: outdated TODO * fix: various typos in docs add: PR links to changelogs format: doc references to structs to look nicer add: note about expected form and unit of timeout config data update: expand ProfileFileTimeoutConfigProvider example * fix: relocate provider config tests to the correct package/directory remove: unimplemented timeouts from `Settings` * add: S3 integration test for timeouts update: re-enable TimeoutLayers add: "list_of_set_timeouts" logging helper to TimeoutConfig refactor: the way TimeoutService handles futures so that it can work better with no timeout set add: helper structs to make creating timeout services easier * update: split timeout_config example into multiple lines * fix: Clippy lints * fix: outdated test * fix: more clippy lints * fix: typo add: TimeoutConfig example * Update CHANGELOG.md Co-authored-by: Russell Cohen * Update aws/rust-runtime/aws-config/src/default_provider.rs Co-authored-by: Russell Cohen * Update aws/rust-runtime/aws-config/src/default_provider.rs Co-authored-by: Russell Cohen * Update aws/rust-runtime/aws-config/src/default_provider.rs Co-authored-by: Russell Cohen * Update aws/rust-runtime/aws-config/src/default_provider.rs Co-authored-by: Russell Cohen * Update aws/rust-runtime/aws-config/src/default_provider.rs Co-authored-by: Russell Cohen * Update aws/rust-runtime/aws-config/src/default_provider.rs Co-authored-by: Russell Cohen * feature: user-configurable AsyncSleep impls update: fallback to sleep impl that sleeps forever instead of sleep impl being optional * Update rust-runtime/aws-smithy-types/src/timeout.rs Co-authored-by: John DiSanti * update: changelogs format: SleepImplDecorator.kt update: TimeoutConfig doc remove: list_of_set_timeouts in favor of Debug impl for TimeoutConfig * Apply suggestions from code review Co-authored-by: John DiSanti * fix: broken macro doc test by ignoring it fix: outdated struct ref in doc fix: outdated generated doc * fix: mode broken doc tests * fix: broken doc test * attempt to fix CI-only doc test error * add: moduleUseName method to CodegenContext remove: pub use reexports from timeout and sleep impl decorators add: pub use reexports to aws_config for timeout and retry configs undo: default_sleep_impl changes attempt to add tokio time feature to S3 integration test * Update timeout.rs * Apply suggestions from code review Co-authored-by: John DiSanti * refactor: consolidate timeout parsing logic rename: `RetryConfigBuilder::merge_with` to `RetryConfigBuilder::take_unset_from` refactor: move timeout parsing tests to timeouts.rs add: entry to SDK changelog noting the renaming remove: redundant feature from s3 integration test Cargo.toml update: various setters added by this PR to have the same form as our preexisting setters add: extra info to the warning emitted when ConfigLoader calls default_async_sleep and gets None * fix: tests broken when implementing suggestions * update: doc hide sleep_impl for aws_types::Config remove: leftover comment Co-authored-by: Russell Cohen Co-authored-by: John DiSanti --- CHANGELOG.md | 6 +- aws/SDK_CHANGELOG.md | 20 +- .../aws-config/src/default_provider.rs | 140 ++++++- .../aws-config/src/environment/mod.rs | 5 + .../src/environment/retry_config.rs | 5 +- .../src/environment/timeout_config.rs | 128 +++++++ .../aws-config/src/imds/client.rs | 6 +- .../aws-config/src/imds/client/token.rs | 7 +- aws/rust-runtime/aws-config/src/lib.rs | 72 +++- .../aws-config/src/profile/mod.rs | 1 + .../aws-config/src/profile/retry_config.rs | 4 +- .../aws-config/src/profile/timeout_config.rs | 162 ++++++++ aws/rust-runtime/aws-inlineable/src/lib.rs | 2 +- aws/rust-runtime/aws-sigv4/src/lib.rs | 2 +- aws/rust-runtime/aws-types/src/config.rs | 137 ++++++- .../smithy/rustsdk/AwsCodegenDecorator.kt | 6 +- .../rustsdk/AwsFluentClientDecorator.kt | 16 +- .../smithy/rustsdk/SharedConfigDecorator.kt | 2 + aws/sdk/integration-tests/s3/Cargo.toml | 4 +- .../integration-tests/s3/tests/timeouts.rs | 67 ++++ .../rust/codegen/smithy/CodegenContext.kt | 8 + .../customizations/RetryConfigDecorator.kt | 9 +- .../customizations/SleepImplDecorator.kt | 163 ++++++++ .../customizations/TimeoutConfigDecorator.kt | 146 ++++++++ .../generators/FluentClientDecorator.kt | 3 +- .../RetryConfigProviderConfigTest.kt | 4 +- .../SleepImplProviderConfigTest.kt | 43 +++ .../TimeoutConfigProviderConfigTest.kt | 43 +++ rust-runtime/aws-smithy-async/src/lib.rs | 27 ++ rust-runtime/aws-smithy-async/src/rt/sleep.rs | 20 +- rust-runtime/aws-smithy-client/src/builder.rs | 38 +- rust-runtime/aws-smithy-client/src/erase.rs | 4 + .../aws-smithy-client/src/hyper_ext.rs | 59 ++- rust-runtime/aws-smithy-client/src/lib.rs | 58 ++- rust-runtime/aws-smithy-client/src/retry.rs | 17 +- rust-runtime/aws-smithy-client/src/timeout.rs | 263 ++++++++++++- .../src/parse_response.rs | 23 +- rust-runtime/aws-smithy-http/src/result.rs | 3 + .../aws-smithy-types-convert/src/lib.rs | 2 +- rust-runtime/aws-smithy-types/src/lib.rs | 3 +- rust-runtime/aws-smithy-types/src/retry.rs | 6 +- rust-runtime/aws-smithy-types/src/timeout.rs | 350 ++++++++++++++++++ 42 files changed, 1944 insertions(+), 140 deletions(-) create mode 100644 aws/rust-runtime/aws-config/src/environment/timeout_config.rs create mode 100644 aws/rust-runtime/aws-config/src/profile/timeout_config.rs create mode 100644 aws/sdk/integration-tests/s3/tests/timeouts.rs create mode 100644 codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/SleepImplDecorator.kt create mode 100644 codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/TimeoutConfigDecorator.kt rename codegen/src/test/kotlin/software/amazon/smithy/rust/{ => codegen/customizations}/RetryConfigProviderConfigTest.kt (90%) create mode 100644 codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/SleepImplProviderConfigTest.kt create mode 100644 codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/TimeoutConfigProviderConfigTest.kt create mode 100644 rust-runtime/aws-smithy-types/src/timeout.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 67b4a4cba..0d1465bc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,13 +7,17 @@ vNext (Month Day, Year) **Breaking Changes** - (aws-smithy-client): Extraneous `pub use SdkSuccess` removed from `aws_smithy_client::hyper_ext`. (smithy-rs#855) +**New this week** + +- Timeouts for requests are now configurable. You can set separate timeouts for each individual request attempt and all attempts made for a request. (smithy-rs#831) + v0.29.0-alpha (November 11th, 2021) =================================== **Breaking Changes** Several breaking changes around `aws_smithy_types::Instant` were introduced by smithy-rs#849: -- `aws_smithy_types::Instant` from was renamed to `DateTime` to avoid confusion with the standard library's monotonically nondecreasing `Instant` type. +- `aws_smithy_types::Instant` from was renamed to `DateTime` to avoid confusion with the standard library's monotonically non-decreasing `Instant` type. - `DateParseError` in `aws_smithy_types` has been renamed to `DateTimeParseError` to match the type that's being parsed. - The `chrono-conversions` feature and associated functions have been moved to the `aws-smithy-types-convert` crate. - Calls to `Instant::from_chrono` should be changed to: diff --git a/aws/SDK_CHANGELOG.md b/aws/SDK_CHANGELOG.md index cac190afb..5424ed388 100644 --- a/aws/SDK_CHANGELOG.md +++ b/aws/SDK_CHANGELOG.md @@ -1,18 +1,24 @@ vNext (Month Day, Year) ======================= -**New this release** -- Improve docs on `aws-smithy-client` (smithy-rs#855) +**TODO Upon release** +- Update README & aws-sdk-rust CI for MSRV upgrade to 1.54 -**Breaking Changes** -- (aws-smithy-client): Extraneous `pub use SdkSuccess` removed from `aws_smithy_client::hyper_ext`. (smithy-rs#855) +**New this week** + +- :tada: Timeouts for requests are now configurable. You can set a timeout for each individual request attempt or for all attempts made for a request. (smithy-rs#831) + +**Breaking changes** +- `RetryConfigBuilder::merge_with` has been renamed to `RetryConfigBuilder::take_unset_from` v0.0.26-alpha (TBD) -======================= +=================================== +**New this release** +- Improve docs on `aws-smithy-client` (smithy-rs#855) -**TODO Upon release** -- Update README & aws-sdk-rust CI for MSRV upgrade to 1.54 +**Breaking Changes** +- (aws-smithy-client): Extraneous `pub use SdkSuccess` removed from `aws_smithy_client::hyper_ext`. (smithy-rs#855) **Breaking Changes** diff --git a/aws/rust-runtime/aws-config/src/default_provider.rs b/aws/rust-runtime/aws-config/src/default_provider.rs index 72bc6c712..303856f2e 100644 --- a/aws/rust-runtime/aws-config/src/default_provider.rs +++ b/aws/rust-runtime/aws-config/src/default_provider.rs @@ -104,15 +104,26 @@ pub mod retry_config { /// /// # Example /// + /// When running [`aws_config::from_env()`](crate::from_env()), a [`ConfigLoader`](crate::ConfigLoader) + /// is created that will then create a [`RetryConfig`] from the default_provider. There is no + /// need to call `default_provider` and the example below is only for illustration purposes. + /// /// ```no_run /// # use std::error::Error; /// # #[tokio::main] /// # async fn main() -> Result<(), Box> { /// use aws_config::default_provider::retry_config; - /// // Creating a RetryConfig from the default_provider already happens when loading a config from_env - /// // This is only for illustration purposes - /// let retry_config = retry_config::default_provider().retry_config().await; - /// let config = aws_config::from_env().retry_config(retry_config).load().await; + /// + /// // Load a retry config from a specific profile + /// let retry_config = retry_config::default_provider() + /// .profile_name("other_profile") + /// .retry_config() + /// .await; + /// let config = aws_config::from_env() + /// // Override the retry config set by the default profile + /// .retry_config(retry_config) + /// .load() + /// .await; /// // instantiate a service client: /// // ::Client::new(&config); /// # Ok(()) @@ -130,7 +141,6 @@ pub mod retry_config { } impl Builder { - #[doc(hidden)] /// Configure the default chain /// /// Exposed for overriding the environment when unit-testing providers @@ -172,7 +182,9 @@ pub mod retry_config { Err(err) => panic!("{}", err), }; - builder_from_env.merge_with(builder_from_profile).build() + builder_from_env + .take_unset_from(builder_from_profile) + .build() } } } @@ -272,6 +284,122 @@ pub mod app_name { } } +/// Default timeout configuration provider chain +pub mod timeout_config { + use aws_smithy_types::timeout::TimeoutConfig; + + use crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider; + use crate::profile; + use crate::provider_config::ProviderConfig; + + /// Default [`TimeoutConfig`] Provider chain + /// + /// 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 [`TimeoutConfig`] that checks the environment variables and AWS profile files for configuration + #[derive(Default)] + pub struct Builder { + env_provider: EnvironmentVariableTimeoutConfigProvider, + profile_file: profile::timeout_config::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); + self + } + + /// Attempt to create a [`TimeoutConfig`](aws_smithy_types::timeout::TimeoutConfig) 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 + 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), + }; + + let conf = builder_from_env.take_unset_from(builder_from_profile); + + if conf.tls_negotiation_timeout().is_some() { + tracing::warn!( + "A TLS negotiation 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)") + } + + if conf.connect_timeout().is_some() { + tracing::warn!( + "A connect 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)") + } + + if conf.read_timeout().is_some() { + tracing::warn!( + "A read 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)") + } + + conf + } + } +} + /// Default credentials provider chain pub mod credentials { use std::borrow::Cow; diff --git a/aws/rust-runtime/aws-config/src/environment/mod.rs b/aws/rust-runtime/aws-config/src/environment/mod.rs index d7c9342cf..f6cddbdb4 100644 --- a/aws/rust-runtime/aws-config/src/environment/mod.rs +++ b/aws/rust-runtime/aws-config/src/environment/mod.rs @@ -9,6 +9,7 @@ pub use app_name::EnvironmentVariableAppNameProvider; /// Load credentials from the environment pub mod credentials; pub use credentials::EnvironmentVariableCredentialsProvider; + /// Load regions from the environment pub mod region; pub use region::EnvironmentVariableRegionProvider; @@ -16,3 +17,7 @@ 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/retry_config.rs b/aws/rust-runtime/aws-config/src/environment/retry_config.rs index 0cfdd2be7..bb459af5d 100644 --- a/aws/rust-runtime/aws-config/src/environment/retry_config.rs +++ b/aws/rust-runtime/aws-config/src/environment/retry_config.rs @@ -14,15 +14,14 @@ const ENV_VAR_RETRY_MODE: &str = "AWS_RETRY_MODE"; /// Load a retry_config from environment variables /// /// This provider will check the values of `AWS_RETRY_MODE` and `AWS_MAX_ATTEMPTS` -/// in order to build a retry config. If at least one is set to a valid value, -/// construction will succeed +/// in order to build a retry config. #[derive(Debug, Default)] pub struct EnvironmentVariableRetryConfigProvider { env: Env, } impl EnvironmentVariableRetryConfigProvider { - /// Create a new `EnvironmentVariableRetryConfigProvider` + /// Create a new [`EnvironmentVariableRetryConfigProvider`] pub fn new() -> Self { EnvironmentVariableRetryConfigProvider { env: Env::real() } } diff --git a/aws/rust-runtime/aws-config/src/environment/timeout_config.rs b/aws/rust-runtime/aws-config/src/environment/timeout_config.rs new file mode 100644 index 000000000..64691b441 --- /dev/null +++ b/aws/rust-runtime/aws-config/src/environment/timeout_config.rs @@ -0,0 +1,128 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +//! Load timeout configuration properties from environment variables + +use aws_smithy_types::timeout::{parse_str_as_timeout, TimeoutConfig, TimeoutConfigError}; +use aws_types::os_shim_internal::Env; +use std::time::Duration; + +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"; +const ENV_VAR_API_CALL_ATTEMPT_TIMEOUT: &str = "AWS_API_CALL_ATTEMPT_TIMEOUT"; +const ENV_VAR_API_CALL_TIMEOUT: &str = "AWS_API_CALL_TIMEOUT"; + +/// Load a 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_TLS_NEGOTIATION_TIMEOUT` +/// - `AWS_READ_TIMEOUT` +/// - `AWS_API_CALL_ATTEMPT_TIMEOUT` +/// - `AWS_API_CALL_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 [`TimeoutConfig`] from environment variables + pub fn timeout_config(&self) -> Result { + let connect_timeout = construct_timeout_from_env_var(&self.env, ENV_VAR_CONNECT_TIMEOUT)?; + let tls_negotiation_timeout = + construct_timeout_from_env_var(&self.env, ENV_VAR_TLS_NEGOTIATION_TIMEOUT)?; + let read_timeout = construct_timeout_from_env_var(&self.env, ENV_VAR_READ_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)?; + + Ok(TimeoutConfig::new() + .with_connect_timeout(connect_timeout) + .with_tls_negotiation_timeout(tls_negotiation_timeout) + .with_read_timeout(read_timeout) + .with_api_call_attempt_timeout(api_call_attempt_timeout) + .with_api_call_timeout(api_call_timeout)) + } +} + +fn construct_timeout_from_env_var( + env: &Env, + var: &'static str, +) -> Result, TimeoutConfigError> { + match env.get(var).ok() { + Some(timeout) => { + parse_str_as_timeout(&timeout, var.into(), "environment variable".into()).map(Some) + } + None => Ok(None), + } +} + +#[cfg(test)] +mod test { + use super::{ + EnvironmentVariableTimeoutConfigProvider, ENV_VAR_API_CALL_ATTEMPT_TIMEOUT, + ENV_VAR_API_CALL_TIMEOUT, ENV_VAR_CONNECT_TIMEOUT, ENV_VAR_READ_TIMEOUT, + ENV_VAR_TLS_NEGOTIATION_TIMEOUT, + }; + use aws_smithy_types::timeout::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(); + + assert_eq!(built.read_timeout(), None); + assert_eq!(built.connect_timeout(), None); + assert_eq!(built.tls_negotiation_timeout(), None); + assert_eq!(built.api_call_attempt_timeout(), None); + assert_eq!(built.api_call_timeout(), None); + } + + #[test] + fn all_fields_can_be_set_at_once() { + assert_eq!( + test_provider(&[ + (ENV_VAR_READ_TIMEOUT, "1.0"), + (ENV_VAR_CONNECT_TIMEOUT, "2"), + (ENV_VAR_TLS_NEGOTIATION_TIMEOUT, "3.0000"), + (ENV_VAR_API_CALL_ATTEMPT_TIMEOUT, "04.000"), + (ENV_VAR_API_CALL_TIMEOUT, "900012345.0") + ]) + .timeout_config() + .unwrap(), + TimeoutConfig::new() + .with_read_timeout(Some(Duration::from_secs_f32(1.0))) + .with_connect_timeout(Some(Duration::from_secs_f32(2.0))) + .with_tls_negotiation_timeout(Some(Duration::from_secs_f32(3.0))) + .with_api_call_attempt_timeout(Some(Duration::from_secs_f32(4.0))) + // Some floats can't be represented as f32 so this duration will be equal to the + // duration from the env. + .with_api_call_timeout(Some(Duration::from_secs_f32(900012350.0))) + ); + } +} diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index b8fb07175..253dc31cc 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -27,6 +27,7 @@ use aws_smithy_http_tower::map_request::{ AsyncMapRequestLayer, AsyncMapRequestService, MapRequestLayer, MapRequestService, }; use aws_smithy_types::retry::{ErrorKind, RetryKind}; +use aws_smithy_types::timeout::TimeoutConfig; use aws_types::os_shim_internal::{Env, Fs}; use bytes::Bytes; use http::uri::InvalidUri; @@ -543,19 +544,22 @@ 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 = TimeoutConfig::default(); let token_loader = token::TokenMiddleware::new( connector.clone(), config.time_source(), endpoint.clone(), self.token_ttl.unwrap_or(DEFAULT_TOKEN_TTL), retry_config.clone(), + timeout_config.clone(), ); let middleware = ImdsMiddleware { token_loader }; let inner_client = aws_smithy_client::Builder::new() .connector(connector.clone()) .middleware(middleware) .build() - .with_retry_config(retry_config); + .with_retry_config(retry_config) + .with_timeout_config(timeout_config); let client = Client { endpoint, inner: inner_client, 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 b2d89b1a5..16d23031d 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/token.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/token.rs @@ -35,6 +35,7 @@ use http::{HeaderValue, Uri}; use crate::cache::ExpiringCache; use crate::imds::client::{ImdsError, ImdsErrorPolicy, TokenError}; use aws_smithy_client::retry; +use aws_smithy_types::timeout::TimeoutConfig; use std::fmt::{Debug, Formatter}; /// Token Refresh Buffer @@ -82,9 +83,11 @@ impl TokenMiddleware { endpoint: Endpoint, token_ttl: Duration, retry_config: retry::Config, + timeout_config: TimeoutConfig, ) -> Self { - let inner_client = - aws_smithy_client::Client::new(connector).with_retry_config(retry_config); + let inner_client = aws_smithy_client::Client::new(connector) + .with_retry_config(retry_config) + .with_timeout_config(timeout_config); 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 c225e7877..9ae979d1e 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -89,6 +89,10 @@ mod json_credentials; #[cfg(feature = "http-provider")] mod http_provider; +// Re-export types from smithy-types +pub use aws_smithy_types::retry::RetryConfig; +pub use aws_smithy_types::timeout::TimeoutConfig; + // Re-export types from aws-types pub use aws_types::app_name::{AppName, InvalidAppName}; pub use aws_types::config::Config; @@ -121,13 +125,18 @@ pub use loader::ConfigLoader; #[cfg(feature = "default-provider")] mod loader { - use crate::default_provider::{app_name, credentials, region, retry_config}; - use crate::meta::region::ProvideRegion; + use std::sync::Arc; + + use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_types::retry::RetryConfig; + use aws_smithy_types::timeout::TimeoutConfig; use aws_types::app_name::AppName; use aws_types::config::Config; use aws_types::credentials::{ProvideCredentials, SharedCredentialsProvider}; + use crate::default_provider::{app_name, credentials, region, retry_config, timeout_config}; + use crate::meta::region::ProvideRegion; + /// Load a cross-service [`Config`](aws_types::config::Config) from the environment /// /// This builder supports overriding individual components of the generated config. Overriding a component @@ -136,10 +145,12 @@ mod loader { /// chain will not be used. #[derive(Default, Debug)] pub struct ConfigLoader { - region: Option>, - retry_config: Option, app_name: Option, credentials_provider: Option, + region: Option>, + retry_config: Option, + sleep: Option>, + timeout_config: Option, } impl ConfigLoader { @@ -175,6 +186,35 @@ mod loader { self } + /// Override the timeout config used to build [`Config`](aws_types::config::Config). + /// **Note: This only sets timeouts for calls to AWS services.** Timeouts for the credentials + /// provider chain are configured separately. + /// + /// # Examples + /// ```rust + /// # use std::time::Duration; + /// # use aws_smithy_types::timeout::TimeoutConfig; + /// # async fn create_config() { + /// let timeout_config = TimeoutConfig::new().with_api_call_timeout(Some(Duration::from_secs(1))); + /// let config = aws_config::from_env() + /// .timeout_config(timeout_config) + /// .load() + /// .await; + /// # } + /// ``` + pub fn timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { + self.timeout_config = Some(timeout_config); + self + } + + /// Override the sleep implementation for this [`ConfigLoader`]. The sleep implementation + /// is used to create timeout futures. + pub fn sleep_impl(mut self, sleep: impl AsyncSleep + 'static) -> Self { + // it's possible that we could wrapping an `Arc in an `Arc` and that's OK + self.sleep = Some(Arc::new(sleep)); + self + } + /// Override the credentials provider used to build [`Config`](aws_types::config::Config). /// # Examples /// Override the credentials provider but load the default value for region: @@ -222,6 +262,27 @@ mod loader { app_name::default_provider().app_name().await }; + let timeout_config = if let Some(timeout_config) = self.timeout_config { + timeout_config + } else { + timeout_config::default_provider().timeout_config().await + }; + + let sleep_impl = if self.sleep.is_none() { + if default_async_sleep().is_none() { + tracing::warn!( + "An implementation of AsyncSleep was requested by calling default_async_sleep \ + but no default was set. + This happened when ConfigLoader::load was called during Config construction. \ + You can fix this by setting a sleep_impl on the ConfigLoader before calling \ + load or by enabling the rt-tokio feature" + ); + } + default_async_sleep() + } else { + self.sleep + }; + let credentials_provider = if let Some(provider) = self.credentials_provider { provider } else { @@ -233,8 +294,11 @@ mod loader { let mut builder = Config::builder() .region(region) .retry_config(retry_config) + .timeout_config(timeout_config) .credentials_provider(credentials_provider); + builder.set_app_name(app_name); + builder.set_sleep_impl(sleep_impl); builder.build() } } diff --git a/aws/rust-runtime/aws-config/src/profile/mod.rs b/aws/rust-runtime/aws-config/src/profile/mod.rs index ad27dd9be..fc7cb666f 100644 --- a/aws/rust-runtime/aws-config/src/profile/mod.rs +++ b/aws/rust-runtime/aws-config/src/profile/mod.rs @@ -16,6 +16,7 @@ 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/retry_config.rs b/aws/rust-runtime/aws-config/src/profile/retry_config.rs index 83b05196f..5e5db34f1 100644 --- a/aws/rust-runtime/aws-config/src/profile/retry_config.rs +++ b/aws/rust-runtime/aws-config/src/profile/retry_config.rs @@ -19,13 +19,13 @@ use crate::provider_config::ProviderConfig; /// /// # Examples /// -/// **Loads 2 as the `max_attempts` to make when sending a request +/// **Loads 2 as the `max_attempts` to make when sending a request** /// ```ini /// [default] /// max_attempts = 2 /// ``` /// -/// **Loads `standard` as the `retry_mode` _if and only if_ the `other` profile is selected. +/// **Loads `standard` as the `retry_mode` _if and only if_ the `other` profile is selected.** /// /// ```ini /// [profile other] diff --git a/aws/rust-runtime/aws-config/src/profile/timeout_config.rs b/aws/rust-runtime/aws-config/src/profile/timeout_config.rs new file mode 100644 index 000000000..38282ddda --- /dev/null +++ b/aws/rust-runtime/aws-config/src/profile/timeout_config.rs @@ -0,0 +1,162 @@ +/* + * 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::profile::Profile; +use crate::provider_config::ProviderConfig; +use aws_smithy_types::timeout::{parse_str_as_timeout, TimeoutConfig, TimeoutConfigError}; +use aws_types::os_shim_internal::{Env, Fs}; +use std::time::Duration; + +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"; +const PROFILE_VAR_API_CALL_ATTEMPT_TIMEOUT: &str = "api_call_attempt_timeout"; +const PROFILE_VAR_API_CALL_TIMEOUT: &str = "api_call_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] +/// connect_timeout = 1.0 +/// read_timeout = 1.0 +/// tls_negotiation_timeout = 0.5 +/// api_call_attempt_timeout = 2 +/// api_call_timeout = 3 +/// ``` +/// +/// **Sets the `connect_timeout` to 0.5 seconds _if and only if_ the `other` profile is selected.** +/// +/// ```ini +/// [profile other] +/// connect_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(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 [`TimeoutConfig`] 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 => { + tracing::warn!( + "failed to get selected '{}' profile, skipping it", + selected_profile + ); + // return an empty config + return Ok(TimeoutConfig::new()); + } + }; + + let connect_timeout = + construct_timeout_from_profile_var(selected_profile, PROFILE_VAR_CONNECT_TIMEOUT)?; + let tls_negotiation_timeout = construct_timeout_from_profile_var( + selected_profile, + PROFILE_VAR_TLS_NEGOTIATION_TIMEOUT, + )?; + let read_timeout = + construct_timeout_from_profile_var(selected_profile, PROFILE_VAR_READ_TIMEOUT)?; + let api_call_attempt_timeout = construct_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)?; + + Ok(TimeoutConfig::new() + .with_connect_timeout(connect_timeout) + .with_tls_negotiation_timeout(tls_negotiation_timeout) + .with_read_timeout(read_timeout) + .with_api_call_attempt_timeout(api_call_attempt_timeout) + .with_api_call_timeout(api_call_timeout)) + } +} + +fn construct_timeout_from_profile_var( + profile: &Profile, + var: &'static str, +) -> Result, TimeoutConfigError> { + 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), + } +} diff --git a/aws/rust-runtime/aws-inlineable/src/lib.rs b/aws/rust-runtime/aws-inlineable/src/lib.rs index a71000507..439a6dea6 100644 --- a/aws/rust-runtime/aws-inlineable/src/lib.rs +++ b/aws/rust-runtime/aws-inlineable/src/lib.rs @@ -12,7 +12,7 @@ #![warn( missing_docs, - missing_crate_level_docs, + rustdoc::missing_crate_level_docs, missing_debug_implementations, rust_2018_idioms, unreachable_pub diff --git a/aws/rust-runtime/aws-sigv4/src/lib.rs b/aws/rust-runtime/aws-sigv4/src/lib.rs index 75fc46135..232c2bc3b 100644 --- a/aws/rust-runtime/aws-sigv4/src/lib.rs +++ b/aws/rust-runtime/aws-sigv4/src/lib.rs @@ -8,7 +8,7 @@ #![warn( missing_docs, - missing_crate_level_docs, + rustdoc::missing_crate_level_docs, missing_debug_implementations, rust_2018_idioms, unreachable_pub diff --git a/aws/rust-runtime/aws-types/src/config.rs b/aws/rust-runtime/aws-types/src/config.rs index 485ce70d6..447641861 100644 --- a/aws/rust-runtime/aws-types/src/config.rs +++ b/aws/rust-runtime/aws-types/src/config.rs @@ -9,26 +9,35 @@ //! //! This module contains an shared configuration representation that is agnostic from a specific service. +use std::sync::Arc; + +use aws_smithy_async::rt::sleep::AsyncSleep; +use aws_smithy_types::retry::RetryConfig; +use aws_smithy_types::timeout::TimeoutConfig; + use crate::app_name::AppName; use crate::credentials::SharedCredentialsProvider; use crate::region::Region; -use aws_smithy_types::retry::RetryConfig; /// AWS Shared Configuration pub struct Config { + app_name: Option, + credentials_provider: Option, region: Option, retry_config: Option, - credentials_provider: Option, - app_name: Option, + sleep_impl: Option>, + timeout_config: Option, } /// Builder for AWS Shared Configuration #[derive(Default)] pub struct Builder { + app_name: Option, + credentials_provider: Option, region: Option, retry_config: Option, - credentials_provider: Option, - app_name: Option, + sleep_impl: Option>, + timeout_config: Option, } impl Builder { @@ -102,6 +111,107 @@ impl Builder { self } + /// Set the [`TimeoutConfig`] for the builder + /// + /// # Examples + /// + /// ```rust + /// # use std::time::Duration; + /// use aws_types::config::Config; + /// use aws_smithy_types::timeout::TimeoutConfig; + /// + /// let timeout_config = TimeoutConfig::new() + /// .with_api_call_attempt_timeout(Some(Duration::from_secs(1))); + /// let config = Config::builder().timeout_config(timeout_config).build(); + /// ``` + pub fn timeout_config(mut self, timeout_config: TimeoutConfig) -> Self { + self.set_timeout_config(Some(timeout_config)); + self + } + + /// Set the [`TimeoutConfig`] for the builder + /// + /// # Examples + /// ```rust + /// # use std::time::Duration; + /// use aws_types::config::{Config, Builder}; + /// use aws_smithy_types::timeout::TimeoutConfig; + /// + /// fn set_preferred_timeouts(builder: &mut Builder) { + /// let timeout_config = TimeoutConfig::new() + /// .with_api_call_attempt_timeout(Some(Duration::from_secs(2))) + /// .with_api_call_timeout(Some(Duration::from_secs(5))); + /// builder.set_timeout_config(Some(timeout_config)); + /// } + /// + /// let mut builder = Config::builder(); + /// set_preferred_timeouts(&mut builder); + /// let config = builder.build(); + /// ``` + pub fn set_timeout_config(&mut self, timeout_config: Option) -> &mut Self { + self.timeout_config = timeout_config; + self + } + + #[doc(hidden)] + /// Set the sleep implementation for the builder. The sleep implementation is used to create + /// timeout futures. + /// + /// # Examples + /// + /// ```rust + /// use std::sync::Arc; + /// use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; + /// use aws_types::config::Config; + /// + /// ##[derive(Debug)] + /// pub struct ForeverSleep; + /// + /// impl AsyncSleep for ForeverSleep { + /// fn sleep(&self, duration: std::time::Duration) -> Sleep { + /// Sleep::new(std::future::pending()) + /// } + /// } + /// + /// let sleep_impl = Arc::new(ForeverSleep); + /// let config = Config::builder().sleep_impl(sleep_impl).build(); + /// ``` + pub fn sleep_impl(mut self, sleep_impl: Arc) -> Self { + self.set_sleep_impl(Some(sleep_impl)); + self + } + + #[doc(hidden)] + /// Set the sleep implementation for the builder. The sleep implementation is used to create + /// timeout futures. + /// + /// # Examples + /// ```rust + /// # use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; + /// # use aws_types::config::{Builder, Config}; + /// #[derive(Debug)] + /// pub struct ForeverSleep; + /// + /// impl AsyncSleep for ForeverSleep { + /// fn sleep(&self, duration: std::time::Duration) -> Sleep { + /// Sleep::new(std::future::pending()) + /// } + /// } + /// + /// fn set_never_ending_sleep_impl(builder: &mut Builder) { + /// let sleep_impl = std::sync::Arc::new(ForeverSleep); + /// builder.set_sleep_impl(Some(sleep_impl)); + /// } + /// + /// let mut builder = Config::builder(); + /// set_never_ending_sleep_impl(&mut builder); + /// let config = builder.build(); + /// ``` + pub fn set_sleep_impl(&mut self, sleep_impl: Option>) -> &mut Self { + self.sleep_impl = sleep_impl; + self + } + /// Set the credentials provider for the builder /// /// # Examples @@ -175,10 +285,12 @@ impl Builder { /// Build a [`Config`](Config) from this builder pub fn build(self) -> Config { Config { + app_name: self.app_name, + credentials_provider: self.credentials_provider, region: self.region, retry_config: self.retry_config, - credentials_provider: self.credentials_provider, - app_name: self.app_name, + sleep_impl: self.sleep_impl, + timeout_config: self.timeout_config, } } } @@ -194,6 +306,17 @@ impl Config { self.retry_config.as_ref() } + /// Configured timeout config + pub fn timeout_config(&self) -> Option<&TimeoutConfig> { + self.timeout_config.as_ref() + } + + #[doc(hidden)] + /// Configured sleep implementation + pub fn sleep_impl(&self) -> Option> { + self.sleep_impl.clone() + } + /// Configured credentials provider pub fn credentials_provider(&self) -> Option<&SharedCredentialsProvider> { self.credentials_provider.as_ref() diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt index 0553b4379..108c06f37 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt @@ -5,9 +5,11 @@ package software.amazon.smithy.rustsdk -import software.amazon.smithy.rust.codegen.smithy.RetryConfigDecorator import software.amazon.smithy.rust.codegen.smithy.customizations.DocsRsMetadataDecorator import software.amazon.smithy.rust.codegen.smithy.customizations.DocsRsMetadataSettings +import software.amazon.smithy.rust.codegen.smithy.customizations.RetryConfigDecorator +import software.amazon.smithy.rust.codegen.smithy.customizations.SleepImplDecorator +import software.amazon.smithy.rust.codegen.smithy.customizations.TimeoutConfigDecorator import software.amazon.smithy.rust.codegen.smithy.customize.CombinedCodegenDecorator import software.amazon.smithy.rustsdk.customize.apigateway.ApiGatewayDecorator import software.amazon.smithy.rustsdk.customize.auth.DisabledAuthDecorator @@ -33,6 +35,8 @@ val DECORATORS = listOf( // Smithy specific decorators RetryConfigDecorator(), + SleepImplDecorator(), + TimeoutConfigDecorator(), // Service specific decorators DisabledAuthDecorator(), 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 5c905e433..46e13a1ac 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 @@ -101,7 +101,13 @@ private class AwsFluentClientExtensions(private val types: Types) { /// Creates a client with the given service config and connector override. pub fn from_conf_conn(conf: crate::Config, conn: C) -> Self { let retry_config = conf.retry_config.as_ref().cloned().unwrap_or_default(); - let client = #{aws_hyper}::Client::new(conn).with_retry_config(retry_config.into()); + let timeout_config = conf.timeout_config.as_ref().cloned().unwrap_or_default(); + let sleep_impl = conf.sleep_impl.clone(); + let mut client = #{aws_hyper}::Client::new(conn) + .with_retry_config(retry_config.into()) + .with_timeout_config(timeout_config); + + client.set_sleep_impl(sleep_impl); Self { handle: std::sync::Arc::new(Handle { client, conf }) } } """, @@ -121,7 +127,13 @@ private class AwsFluentClientExtensions(private val types: Types) { ##[cfg(any(feature = "rustls", feature = "native-tls"))] pub fn from_conf(conf: crate::Config) -> Self { let retry_config = conf.retry_config.as_ref().cloned().unwrap_or_default(); - let client = #{aws_hyper}::Client::https().with_retry_config(retry_config.into()); + let timeout_config = conf.timeout_config.as_ref().cloned().unwrap_or_default(); + let sleep_impl = conf.sleep_impl.clone(); + let mut client = #{aws_hyper}::Client::https() + .with_retry_config(retry_config.into()) + .with_timeout_config(timeout_config); + + client.set_sleep_impl(sleep_impl); Self { handle: std::sync::Arc::new(Handle { client, conf }) } } """, diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt index 0fdcb2290..0591d1652 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt @@ -47,6 +47,8 @@ class SharedConfigDecorator : RustCodegenDecorator { let mut builder = Builder::default(); builder = builder.region(input.region().cloned()); builder.set_retry_config(input.retry_config().cloned()); + builder.set_timeout_config(input.timeout_config().cloned()); + builder.set_sleep_impl(input.sleep_impl().clone()); builder.set_credentials_provider(input.credentials_provider().cloned()); builder.set_app_name(input.app_name().cloned()); builder diff --git a/aws/sdk/integration-tests/s3/Cargo.toml b/aws/sdk/integration-tests/s3/Cargo.toml index 1719287f8..714b13213 100644 --- a/aws/sdk/integration-tests/s3/Cargo.toml +++ b/aws/sdk/integration-tests/s3/Cargo.toml @@ -11,7 +11,10 @@ edition = "2018" aws-sdk-s3 = { path = "../../build/aws-sdk/sdk/s3" } aws-smithy-client = { path = "../../build/aws-sdk/sdk/aws-smithy-client", features = ["test-util"] } aws-smithy-http = { path = "../../build/aws-sdk/sdk/aws-smithy-http" } +aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async" } +aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types" } tracing-subscriber = "0.2.18" +tokio = { version = "1", features = ["full"]} [dev-dependencies] aws-http = { path = "../../build/aws-sdk/sdk/aws-http"} @@ -19,4 +22,3 @@ aws-hyper = { path = "../../build/aws-sdk/sdk/aws-hyper"} bytes = "1" http = "0.2.3" serde_json = "1" -tokio = { version = "1", features = ["full"]} diff --git a/aws/sdk/integration-tests/s3/tests/timeouts.rs b/aws/sdk/integration-tests/s3/tests/timeouts.rs new file mode 100644 index 000000000..242f92712 --- /dev/null +++ b/aws/sdk/integration-tests/s3/tests/timeouts.rs @@ -0,0 +1,67 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +use aws_sdk_s3::model::{ + CompressionType, CsvInput, CsvOutput, ExpressionType, FileHeaderInfo, InputSerialization, + OutputSerialization, +}; +use aws_sdk_s3::{Client, Config, Credentials, Region}; +use aws_smithy_async::assert_elapsed; +use aws_smithy_async::rt::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 std::sync::Arc; +use std::time::Duration; + +#[tokio::test] +async fn test_timeout_service_ends_request_that_never_completes() { + let conn: NeverService, http::Response, ConnectorError> = + NeverService::new(); + let region = Region::from_static("us-east-2"); + let credentials = Credentials::from_keys("test", "test", None); + let timeout_config = + TimeoutConfig::new().with_api_call_timeout(Some(Duration::from_secs_f32(0.5))); + let sleep_impl: Arc = Arc::new(TokioSleep::new()); + let config = Config::builder() + .region(region) + .credentials_provider(credentials) + .timeout_config(timeout_config) + .sleep_impl(sleep_impl) + .build(); + let client = Client::from_conf_conn(config, conn.clone()); + + let now = tokio::time::Instant::now(); + tokio::time::pause(); + + let err = client + .select_object_content() + .bucket("aws-rust-sdk") + .key("sample_data.csv") + .expression_type(ExpressionType::Sql) + .expression("SELECT * FROM s3object s WHERE s.\"Name\" = 'Jane'") + .input_serialization( + InputSerialization::builder() + .csv( + CsvInput::builder() + .file_header_info(FileHeaderInfo::Use) + .build(), + ) + .compression_type(CompressionType::None) + .build(), + ) + .output_serialization( + OutputSerialization::builder() + .csv(CsvOutput::builder().build()) + .build(), + ) + .send() + .await + .unwrap_err(); + + assert_eq!(format!("{:?}", err), "ConstructionFailure(TimedOutError)"); + assert_elapsed!(now, std::time::Duration::from_secs_f32(0.5)); +} diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenContext.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenContext.kt index 3a3293f0f..e3fa3599c 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenContext.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenContext.kt @@ -64,4 +64,12 @@ data class CodegenContext( settings: RustSettings, mode: CodegenMode, ) : this(model, symbolProvider, settings.runtimeConfig, serviceShape, protocol, settings.moduleName, settings, mode) + + /** + * A moduleName for a crate uses kebab-case. When you want to `use` a crate in Rust code, + * it must be in snake-case. Call this method to get this crate's name in snake-case. + */ + fun moduleUseName(): String { + return this.moduleName.replace("-", "_") + } } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt index 2cde87f4b..0e935e97c 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt @@ -3,12 +3,15 @@ * SPDX-License-Identifier: Apache-2.0. */ -package software.amazon.smithy.rust.codegen.smithy +package software.amazon.smithy.rust.codegen.smithy.customizations import software.amazon.smithy.rust.codegen.rustlang.Writable import software.amazon.smithy.rust.codegen.rustlang.rust import software.amazon.smithy.rust.codegen.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.rustlang.writable +import software.amazon.smithy.rust.codegen.smithy.CodegenContext +import software.amazon.smithy.rust.codegen.smithy.RuntimeConfig +import software.amazon.smithy.rust.codegen.smithy.RuntimeType import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization import software.amazon.smithy.rust.codegen.smithy.generators.LibRsSection @@ -61,7 +64,6 @@ fn test_1() { fn assert_send_sync() {} assert_send_sync::(); } - */ class RetryConfigDecorator : RustCodegenDecorator { @@ -85,8 +87,7 @@ class RetryConfigDecorator : RustCodegenDecorator { class RetryConfigProviderConfig(codegenContext: CodegenContext) : ConfigCustomization() { private val retryConfig = smithyTypesRetry(codegenContext.runtimeConfig) - private val moduleName = codegenContext.moduleName - private val moduleUseName = moduleName.replace("-", "_") + private val moduleUseName = codegenContext.moduleUseName() private val codegenScope = arrayOf("RetryConfig" to retryConfig.member("RetryConfig")) override fun section(section: ServiceConfig) = writable { when (section) { diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/SleepImplDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/SleepImplDecorator.kt new file mode 100644 index 000000000..972188741 --- /dev/null +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/SleepImplDecorator.kt @@ -0,0 +1,163 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.rust.codegen.smithy.customizations + +import software.amazon.smithy.rust.codegen.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.rustlang.writable +import software.amazon.smithy.rust.codegen.smithy.CodegenContext +import software.amazon.smithy.rust.codegen.smithy.RuntimeConfig +import software.amazon.smithy.rust.codegen.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator +import software.amazon.smithy.rust.codegen.smithy.generators.config.ConfigCustomization +import software.amazon.smithy.rust.codegen.smithy.generators.config.ServiceConfig + +/* Example Generated Code */ +/* +pub struct Config { + pub(crate) sleep_impl: Option>, +} +impl std::fmt::Debug for Config { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut config = f.debug_struct("Config"); + config.finish() + } +} +impl Config { + pub fn builder() -> Builder { + Builder::default() + } +} +#[derive(Default)] +pub struct Builder { + sleep_impl: Option>, +} +impl Builder { + pub fn new() -> Self { + Self::default() + } + pub fn sleep_impl(mut self, sleep_impl: Arc) -> Self { + self.set_sleep_impl(Some(sleep_impl)); + self + } + pub fn set_sleep_impl( + &mut self, + sleep_impl: Option>, + ) -> &mut Self { + self.sleep_impl = sleep_impl; + self + } + pub fn build(self) -> Config { + Config { + sleep_impl: self.sleep_impl, + } + } +} +#[test] +fn test_1() { + fn assert_send_sync() {} + assert_send_sync::(); +} + */ + +class SleepImplDecorator : RustCodegenDecorator { + override val name: String = "AsyncSleep" + override val order: Byte = 0 + + override fun configCustomizations( + codegenContext: CodegenContext, + baseCustomizations: List + ): List { + return baseCustomizations + SleepImplProviderConfig(codegenContext) + } +} + +class SleepImplProviderConfig(codegenContext: CodegenContext) : ConfigCustomization() { + private val sleepModule = smithyAsyncRtSleep(codegenContext.runtimeConfig) + private val moduleUseName = codegenContext.moduleUseName() + private val codegenScope = + arrayOf("AsyncSleep" to sleepModule.member("AsyncSleep"), "Sleep" to sleepModule.member("Sleep")) + + override fun section(section: ServiceConfig) = writable { + when (section) { + is ServiceConfig.ConfigStruct -> rustTemplate( + "pub(crate) sleep_impl: Option>,", + *codegenScope + ) + is ServiceConfig.ConfigImpl -> emptySection + is ServiceConfig.BuilderStruct -> + rustTemplate("sleep_impl: Option>,", *codegenScope) + ServiceConfig.BuilderImpl -> + rustTemplate( + """ + /// Set the sleep_impl for the builder + /// + /// ## Examples + /// ```rust + /// use $moduleUseName::config::Config; + /// use #{AsyncSleep}; + /// use #{Sleep}; + /// + /// ##[derive(Debug)] + /// pub struct ForeverSleep; + /// + /// impl AsyncSleep for ForeverSleep { + /// fn sleep(&self, duration: std::time::Duration) -> Sleep { + /// Sleep::new(std::future::pending()) + /// } + /// } + /// + /// let sleep_impl = std::sync::Arc::new(ForeverSleep); + /// let config = Config::builder().sleep_impl(sleep_impl).build(); + /// ``` + pub fn sleep_impl(mut self, sleep_impl: std::sync::Arc) -> Self { + self.set_sleep_impl(Some(sleep_impl)); + self + } + + /// Set the sleep_impl for the builder + /// + /// ## Examples + /// ```rust + /// use $moduleUseName::config::{Builder, Config}; + /// use #{AsyncSleep}; + /// use #{Sleep}; + /// + /// ##[derive(Debug)] + /// pub struct ForeverSleep; + /// + /// impl AsyncSleep for ForeverSleep { + /// fn sleep(&self, duration: std::time::Duration) -> Sleep { + /// Sleep::new(std::future::pending()) + /// } + /// } + /// + /// fn set_never_ending_sleep_impl(builder: &mut Builder) { + /// let sleep_impl = std::sync::Arc::new(ForeverSleep); + /// builder.set_sleep_impl(Some(sleep_impl)); + /// } + /// + /// let mut builder = Config::builder(); + /// set_never_ending_sleep_impl(&mut builder); + /// let config = builder.build(); + /// ``` + pub fn set_sleep_impl(&mut self, sleep_impl: Option>) -> &mut Self { + self.sleep_impl = sleep_impl; + self + } + """, + *codegenScope + ) + ServiceConfig.BuilderBuild -> rustTemplate( + """sleep_impl: self.sleep_impl,""", + *codegenScope + ) + } + } +} + +// Generate path to the root module in aws_smithy_async +fun smithyAsyncRtSleep(runtimeConfig: RuntimeConfig) = + RuntimeType("sleep", runtimeConfig.runtimeCrate("async"), "aws_smithy_async::rt") diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/TimeoutConfigDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/TimeoutConfigDecorator.kt new file mode 100644 index 000000000..c4b062a53 --- /dev/null +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/TimeoutConfigDecorator.kt @@ -0,0 +1,146 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.rust.codegen.smithy.customizations + +import software.amazon.smithy.rust.codegen.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.rustlang.writable +import software.amazon.smithy.rust.codegen.smithy.CodegenContext +import software.amazon.smithy.rust.codegen.smithy.RuntimeConfig +import software.amazon.smithy.rust.codegen.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator +import software.amazon.smithy.rust.codegen.smithy.generators.config.ConfigCustomization +import software.amazon.smithy.rust.codegen.smithy.generators.config.ServiceConfig + +/* Example Generated Code */ +/* +pub struct Config { + pub(crate) timeout_config: Option, +} +impl std::fmt::Debug for Config { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut config = f.debug_struct("Config"); + config.finish() + } +} +impl Config { + pub fn builder() -> Builder { + Builder::default() + } +} +#[derive(Default)] +pub struct Builder { + timeout_config: Option, +} +impl Builder { + pub fn new() -> Self { + Self::default() + } + pub fn timeout_config(mut self, timeout_config: aws_smithy_types::timeout::TimeoutConfig) -> Self { + self.set_timeout_config(Some(timeout_config)); + self + } + pub fn set_timeout_config( + &mut self, + timeout_config: Option, + ) -> &mut Self { + self.timeout_config = timeout_config; + self + } + pub fn build(self) -> Config { + Config { + timeout_config: self.timeout_config, + } + } +} +#[test] +fn test_1() { + fn assert_send_sync() {} + assert_send_sync::(); +} + + */ + +class TimeoutConfigDecorator : RustCodegenDecorator { + override val name: String = "TimeoutConfig" + override val order: Byte = 0 + + override fun configCustomizations( + codegenContext: CodegenContext, + baseCustomizations: List + ): List { + return baseCustomizations + TimeoutConfigProviderConfig(codegenContext) + } +} + +class TimeoutConfigProviderConfig(codegenContext: CodegenContext) : ConfigCustomization() { + private val timeoutConfig = smithyTypesTimeout(codegenContext.runtimeConfig) + private val moduleUseName = codegenContext.moduleUseName() + private val codegenScope = arrayOf("TimeoutConfig" to timeoutConfig.member("TimeoutConfig")) + override fun section(section: ServiceConfig) = writable { + when (section) { + is ServiceConfig.ConfigStruct -> rustTemplate( + "pub(crate) timeout_config: Option<#{TimeoutConfig}>,", + *codegenScope + ) + is ServiceConfig.ConfigImpl -> emptySection + is ServiceConfig.BuilderStruct -> + rustTemplate("timeout_config: Option<#{TimeoutConfig}>,", *codegenScope) + ServiceConfig.BuilderImpl -> + rustTemplate( + """ + /// Set the timeout_config for the builder + /// + /// ## Examples + /// ```rust + /// ## use std::time::Duration; + /// use $moduleUseName::config::Config; + /// use #{TimeoutConfig}; + /// + /// let timeout_config = TimeoutConfig::new() + /// .with_api_call_attempt_timeout(Some(Duration::from_secs(1))); + /// let config = Config::builder().timeout_config(timeout_config).build(); + /// ``` + pub fn timeout_config(mut self, timeout_config: #{TimeoutConfig}) -> Self { + self.set_timeout_config(Some(timeout_config)); + self + } + + /// Set the timeout_config for the builder + /// + /// ## Examples + /// ```rust + /// ## use std::time::Duration; + /// use $moduleUseName::config::{Builder, Config}; + /// use #{TimeoutConfig}; + /// + /// fn set_request_timeout(builder: &mut Builder) { + /// let timeout_config = TimeoutConfig::new() + /// .with_api_call_timeout(Some(Duration::from_secs(3))); + /// builder.set_timeout_config(Some(timeout_config)); + /// } + /// + /// let mut builder = Config::builder(); + /// set_request_timeout(&mut builder); + /// let config = builder.build(); + /// ``` + pub fn set_timeout_config(&mut self, timeout_config: Option<#{TimeoutConfig}>) -> &mut Self { + self.timeout_config = timeout_config; + self + } + """, + *codegenScope + ) + ServiceConfig.BuilderBuild -> rustTemplate( + """timeout_config: self.timeout_config,""", + *codegenScope + ) + } + } +} + +// Generate path to the timeout module in aws_smithy_types +fun smithyTypesTimeout(runtimeConfig: RuntimeConfig) = + RuntimeType("timeout", runtimeConfig.runtimeCrate("types"), "aws_smithy_types") diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index e03bc31de..83b203f9c 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -148,8 +148,7 @@ class FluentClientGenerator( private val model = codegenContext.model private val clientDep = CargoDependency.SmithyClient(codegenContext.runtimeConfig).copy(optional = true) private val runtimeConfig = codegenContext.runtimeConfig - private val moduleName = codegenContext.moduleName - private val moduleUseName = moduleName.replace("-", "_") + private val moduleUseName = codegenContext.moduleUseName() private val humanName = serviceShape.id.name private val core = FluentClientCore(model) diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/RetryConfigProviderConfigTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/RetryConfigProviderConfigTest.kt similarity index 90% rename from codegen/src/test/kotlin/software/amazon/smithy/rust/RetryConfigProviderConfigTest.kt rename to codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/RetryConfigProviderConfigTest.kt index 812834cd7..b5165199f 100644 --- a/codegen/src/test/kotlin/software/amazon/smithy/rust/RetryConfigProviderConfigTest.kt +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/RetryConfigProviderConfigTest.kt @@ -3,10 +3,10 @@ * SPDX-License-Identifier: Apache-2.0. */ -package software.amazon.smithy.rust +package software.amazon.smithy.rust.codegen.customizations import org.junit.jupiter.api.Test -import software.amazon.smithy.rust.codegen.smithy.RetryConfigProviderConfig +import software.amazon.smithy.rust.codegen.smithy.customizations.RetryConfigProviderConfig import software.amazon.smithy.rust.codegen.smithy.transformers.OperationNormalizer import software.amazon.smithy.rust.codegen.smithy.transformers.RecursiveShapeBoxer import software.amazon.smithy.rust.codegen.testutil.TestWorkspace diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/SleepImplProviderConfigTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/SleepImplProviderConfigTest.kt new file mode 100644 index 000000000..9ab7c434f --- /dev/null +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/SleepImplProviderConfigTest.kt @@ -0,0 +1,43 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.rust.codegen.customizations + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.smithy.customizations.SleepImplProviderConfig +import software.amazon.smithy.rust.codegen.smithy.transformers.OperationNormalizer +import software.amazon.smithy.rust.codegen.smithy.transformers.RecursiveShapeBoxer +import software.amazon.smithy.rust.codegen.testutil.TestWorkspace +import software.amazon.smithy.rust.codegen.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.testutil.rustSettings +import software.amazon.smithy.rust.codegen.testutil.testCodegenContext +import software.amazon.smithy.rust.codegen.testutil.validateConfigCustomizations + +internal class SleepImplProviderConfigTest { + private val baseModel = """ + namespace test + use aws.protocols#awsQuery + + structure SomeOutput { + @xmlAttribute + someAttribute: Long, + + someVal: String + } + + operation SomeOperation { + output: SomeOutput + } + """.asSmithyModel() + + @Test + fun `generates a valid config`() { + val model = RecursiveShapeBoxer.transform(OperationNormalizer.transform(baseModel)) + val project = TestWorkspace.testProject() + val codegenContext = testCodegenContext(model, settings = project.rustSettings(model)) + + validateConfigCustomizations(SleepImplProviderConfig(codegenContext), project) + } +} diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/TimeoutConfigProviderConfigTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/TimeoutConfigProviderConfigTest.kt new file mode 100644 index 000000000..bc9479861 --- /dev/null +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/customizations/TimeoutConfigProviderConfigTest.kt @@ -0,0 +1,43 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.rust.codegen.customizations + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.smithy.customizations.TimeoutConfigProviderConfig +import software.amazon.smithy.rust.codegen.smithy.transformers.OperationNormalizer +import software.amazon.smithy.rust.codegen.smithy.transformers.RecursiveShapeBoxer +import software.amazon.smithy.rust.codegen.testutil.TestWorkspace +import software.amazon.smithy.rust.codegen.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.testutil.rustSettings +import software.amazon.smithy.rust.codegen.testutil.testCodegenContext +import software.amazon.smithy.rust.codegen.testutil.validateConfigCustomizations + +internal class TimeoutConfigProviderConfigTest { + private val baseModel = """ + namespace test + use aws.protocols#awsQuery + + structure SomeOutput { + @xmlAttribute + someAttribute: Long, + + someVal: String + } + + operation SomeOperation { + output: SomeOutput + } + """.asSmithyModel() + + @Test + fun `generates a valid config`() { + val model = RecursiveShapeBoxer.transform(OperationNormalizer.transform(baseModel)) + val project = TestWorkspace.testProject() + val codegenContext = testCodegenContext(model, settings = project.rustSettings(model)) + + validateConfigCustomizations(TimeoutConfigProviderConfig(codegenContext), project) + } +} diff --git a/rust-runtime/aws-smithy-async/src/lib.rs b/rust-runtime/aws-smithy-async/src/lib.rs index f6a8dfb5c..13a1115ae 100644 --- a/rust-runtime/aws-smithy-async/src/lib.rs +++ b/rust-runtime/aws-smithy-async/src/lib.rs @@ -10,3 +10,30 @@ pub mod future; pub mod rt; + +/// Given an `Instant` and a `Duration`, assert time elapsed since `Instant` is equal to `Duration`. +/// This macro allows for a 5ms margin of error. +/// +/// # Example +/// +/// ```rust,ignore +/// let now = std::time::Instant::now(); +/// let _ = some_function_that_always_takes_five_seconds_to_run().await; +/// assert_elapsed!(now, std::time::Duration::from_secs(5)); +/// ``` +#[macro_export] +macro_rules! assert_elapsed { + ($start:expr, $dur:expr) => {{ + let elapsed = $start.elapsed(); + // type ascription improves compiler error when wrong type is passed + let lower: std::time::Duration = $dur; + + // Handles ms rounding + assert!( + elapsed >= lower && elapsed <= lower + std::time::Duration::from_millis(5), + "actual = {:?}, expected = {:?}", + elapsed, + lower + ); + }}; +} diff --git a/rust-runtime/aws-smithy-async/src/rt/sleep.rs b/rust-runtime/aws-smithy-async/src/rt/sleep.rs index 5bd7e38b7..a9bbc37f2 100644 --- a/rust-runtime/aws-smithy-async/src/rt/sleep.rs +++ b/rust-runtime/aws-smithy-async/src/rt/sleep.rs @@ -38,10 +38,15 @@ where } } -/// Returns a default sleep implementation based on the features enabled, or `None` if -/// there isn't one available from this crate. +#[cfg(feature = "rt-tokio")] +/// Returns a default sleep implementation based on the features enabled +pub fn default_async_sleep() -> Option> { + Some(sleep_tokio()) +} + +#[cfg(not(feature = "rt-tokio"))] pub fn default_async_sleep() -> Option> { - sleep_tokio() + None } /// Future returned by [`AsyncSleep`]. @@ -83,11 +88,6 @@ impl AsyncSleep for TokioSleep { } #[cfg(feature = "rt-tokio")] -fn sleep_tokio() -> Option> { - Some(Arc::new(TokioSleep::new())) -} - -#[cfg(not(feature = "rt-tokio"))] -fn sleep_tokio() -> Option> { - None +fn sleep_tokio() -> Arc { + Arc::new(TokioSleep::new()) } diff --git a/rust-runtime/aws-smithy-client/src/builder.rs b/rust-runtime/aws-smithy-client/src/builder.rs index dbb79f116..18b27d6c4 100644 --- a/rust-runtime/aws-smithy-client/src/builder.rs +++ b/rust-runtime/aws-smithy-client/src/builder.rs @@ -3,9 +3,13 @@ * SPDX-License-Identifier: Apache-2.0. */ +use std::sync::Arc; + use crate::{bounds, erase, retry, Client}; +use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; +use aws_smithy_types::timeout::TimeoutConfig; /// A builder that provides more customization options when constructing a [`Client`]. /// @@ -17,6 +21,8 @@ pub struct Builder { connector: C, middleware: M, retry_policy: R, + timeout_config: TimeoutConfig, + sleep_impl: Option>, } // It'd be nice to include R where R: Default here, but then the caller ends up always having to @@ -32,13 +38,9 @@ where C: Default, M: Default, { - /// Construct a new builder. - /// - /// This will - /// - /// You will likely want to , as it does not specify a [connector](Builder::connector) - /// or [middleware](Builder::middleware). It uses the [standard retry - /// mechanism](retry::Standard). + /// Construct a new builder. This does not specify a [connector](Builder::connector) + /// or [middleware](Builder::middleware). + /// It uses the [standard retry mechanism](retry::Standard). pub fn new() -> Self { Self::default() } @@ -58,6 +60,8 @@ impl Builder<(), M, R> { connector, retry_policy: self.retry_policy, middleware: self.middleware, + timeout_config: self.timeout_config, + sleep_impl: self.sleep_impl, } } @@ -111,7 +115,9 @@ impl Builder { Builder { connector: self.connector, retry_policy: self.retry_policy, + timeout_config: self.timeout_config, middleware, + sleep_impl: self.sleep_impl, } } @@ -154,7 +160,9 @@ impl Builder { Builder { connector: self.connector, retry_policy, + timeout_config: self.timeout_config, middleware: self.middleware, + sleep_impl: self.sleep_impl, } } } @@ -164,6 +172,16 @@ impl Builder { pub fn set_retry_config(&mut self, config: retry::Config) { self.retry_policy.with_config(config); } + + /// Set a timeout config for the builder + pub fn set_timeout_config(&mut self, timeout_config: TimeoutConfig) { + self.timeout_config = timeout_config; + } + + /// 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>) { + self.sleep_impl = async_sleep; + } } impl Builder { @@ -176,6 +194,8 @@ impl Builder { connector: map(self.connector), middleware: self.middleware, retry_policy: self.retry_policy, + timeout_config: self.timeout_config, + sleep_impl: self.sleep_impl, } } @@ -188,6 +208,8 @@ impl Builder { connector: self.connector, middleware: map(self.middleware), retry_policy: self.retry_policy, + timeout_config: self.timeout_config, + sleep_impl: self.sleep_impl, } } @@ -197,6 +219,8 @@ impl Builder { connector: self.connector, retry_policy: self.retry_policy, middleware: self.middleware, + timeout_config: self.timeout_config, + sleep_impl: self.sleep_impl, } } } diff --git a/rust-runtime/aws-smithy-client/src/erase.rs b/rust-runtime/aws-smithy-client/src/erase.rs index b698eb0c3..f688533f9 100644 --- a/rust-runtime/aws-smithy-client/src/erase.rs +++ b/rust-runtime/aws-smithy-client/src/erase.rs @@ -56,6 +56,8 @@ where connector: self.connector, middleware: DynMiddleware::new(self.middleware), retry_policy: self.retry_policy, + timeout_config: self.timeout_config, + sleep_impl: self.sleep_impl, } } } @@ -94,6 +96,8 @@ where connector: DynConnector::new(self.connector), middleware: self.middleware, retry_policy: self.retry_policy, + timeout_config: self.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 2b0699687..12547bd46 100644 --- a/rust-runtime/aws-smithy-client/src/hyper_ext.rs +++ b/rust-runtime/aws-smithy-client/src/hyper_ext.rs @@ -38,23 +38,24 @@ //! let client = Client::::new(DynConnector::new(connector)); //! ``` +use std::error::Error; use std::sync::Arc; use http::Uri; use hyper::client::connect::Connection; - use tokio::io::{AsyncRead, AsyncWrite}; use tower::{BoxError, Service}; +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 std::error::Error; +pub use aws_smithy_http::result::{SdkError, SdkSuccess}; +use aws_smithy_types::retry::ErrorKind; -use crate::hyper_ext::timeout_middleware::{ConnectTimeout, HttpReadTimeout, TimeoutError}; use crate::{timeout, Builder as ClientBuilder}; -use aws_smithy_async::future::timeout::TimedOutError; -use aws_smithy_types::retry::ErrorKind; + +use self::timeout_middleware::{ConnectTimeout, HttpReadTimeout, TimeoutError}; /// Adapter from a [`hyper::Client`](hyper::Client) to a connector usable by a Smithy [`Client`](crate::Client). /// @@ -303,6 +304,8 @@ impl ClientBuilder<(), M, R> { } mod timeout_middleware { + use std::error::Error; + use std::fmt::Formatter; use std::future::Future; use std::pin::Pin; use std::sync::Arc; @@ -310,16 +313,13 @@ mod timeout_middleware { use std::time::Duration; use http::Uri; - use pin_project_lite::pin_project; + use tower::BoxError; use aws_smithy_async::future; use aws_smithy_async::future::timeout::{TimedOutError, Timeout}; use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_async::rt::sleep::Sleep; - use std::error::Error; - use std::fmt::Formatter; - use tower::BoxError; #[derive(Debug)] pub(crate) struct TimeoutError { @@ -507,29 +507,17 @@ mod timeout_middleware { #[cfg(test)] mod test { - use crate::hyper_ext::Adapter; - use crate::never::{NeverConnected, NeverReplies}; - use crate::timeout; - use aws_smithy_async::rt::sleep::TokioSleep; - use aws_smithy_http::body::SdkBody; use std::time::Duration; + use tower::Service; - macro_rules! assert_elapsed { - ($start:expr, $dur:expr) => {{ - let elapsed = $start.elapsed(); - // type ascription improves compiler error when wrong type is passed - let lower: std::time::Duration = $dur; - - // Handles ms rounding - assert!( - elapsed >= lower && elapsed <= lower + std::time::Duration::from_millis(5), - "actual = {:?}, expected = {:?}", - elapsed, - lower - ); - }}; - } + use aws_smithy_async::assert_elapsed; + use aws_smithy_async::rt::sleep::TokioSleep; + use aws_smithy_http::body::SdkBody; + + use crate::hyper_ext::Adapter; + use crate::never::{NeverConnected, NeverReplies}; + use crate::timeout; #[allow(unused)] fn connect_timeout_is_correct() { @@ -595,18 +583,19 @@ mod timeout_middleware { #[cfg(test)] mod test { - use crate::hyper_ext::Adapter; - use http::Uri; - use hyper::client::connect::{Connected, Connection}; - - use aws_smithy_http::body::SdkBody; use std::io::{Error, ErrorKind}; use std::pin::Pin; use std::task::{Context, Poll}; - use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; + 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 crate::hyper_ext::Adapter; + #[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 e77c98f65..b253435c9 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -36,6 +36,7 @@ pub mod static_tests; #[cfg(feature = "hyper")] pub mod never; pub mod timeout; +pub use timeout::TimeoutLayer; /// Type aliases for standard connection types. #[cfg(feature = "hyper")] @@ -72,6 +73,12 @@ 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::{default_async_sleep, AsyncSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::operation::Operation; use aws_smithy_http::response::ParseHttpResponse; @@ -80,13 +87,11 @@ use aws_smithy_http::retry::ClassifyResponse; use aws_smithy_http_tower::dispatch::DispatchLayer; use aws_smithy_http_tower::parse_response::ParseResponseLayer; use aws_smithy_types::retry::ProvideErrorKind; -use std::error::Error; - -use tower::{Layer, Service, ServiceBuilder, ServiceExt}; +use aws_smithy_types::timeout::TimeoutConfig; /// Smithy service client. /// -/// The service client is customizeable in a number of ways (see [`Builder`]), but most customers +/// The service client is customizable in a number of ways (see [`Builder`]), but most customers /// can stick with the standard constructor provided by [`Client::new`]. It takes only a single /// argument, which is the middleware that fills out the [`http::Request`] for each higher-level /// operation so that it can ultimately be sent to the remote host. The middleware is responsible @@ -112,6 +117,8 @@ pub struct Client< connector: Connector, middleware: Middleware, retry_policy: RetryPolicy, + timeout_config: TimeoutConfig, + sleep_impl: Option>, } // Quick-create for people who just want "the default". @@ -119,13 +126,17 @@ impl Client where M: Default, { - /// Create a Smithy client that the given connector, a middleware default, and the [standard - /// retry policy](crate::retry::Standard). + /// Create a Smithy client that the given connector, a middleware default, the [standard + /// retry policy](crate::retry::Standard), and the [`default_async_sleep`] sleep implementation. pub fn new(connector: C) -> Self { - Builder::new() + let mut client = Builder::new() .connector(connector) .middleware(M::default()) - .build() + .build(); + + client.set_sleep_impl(default_async_sleep()); + + client } } @@ -140,6 +151,28 @@ impl Client { self.set_retry_config(config); self } + + /// Set the client's timeout configuration. + pub fn set_timeout_config(&mut self, config: TimeoutConfig) { + self.timeout_config = config; + } + + /// Set the client's timeout configuration. + pub fn with_timeout_config(mut self, config: TimeoutConfig) -> Self { + self.set_timeout_config(config); + self + } + + /// Set the [`AsyncSleep`] function that the client will use to create things like timeout futures. + 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 { @@ -189,9 +222,16 @@ where Service, Response = SdkSuccess, Error = SdkError> + Clone, { let connector = self.connector.clone(); + + let timeout_servic_params = generate_timeout_service_params_from_timeout_config( + &self.timeout_config, + self.sleep_impl.clone(), + ); + let svc = ServiceBuilder::new() - // Create a new request-scoped policy + .layer(TimeoutLayer::new(timeout_servic_params.api_call)) .retry(self.retry_policy.new_request_policy()) + .layer(TimeoutLayer::new(timeout_servic_params.api_call_attempt)) .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/retry.rs b/rust-runtime/aws-smithy-client/src/retry.rs index e94b923a6..34b51655a 100644 --- a/rust-runtime/aws-smithy-client/src/retry.rs +++ b/rust-runtime/aws-smithy-client/src/retry.rs @@ -14,17 +14,18 @@ //! Its sole purpose in life is to create a [`RetryHandler`] for individual requests. //! - [`RetryHandler`]: A request-scoped retry policy, backed by request-local state and shared //! state contained within [`Standard`]. -//! - [`Config`]: Static configuration (max retries, max backoff etc.) +//! - [`Config`]: Static configuration (max attempts, max backoff etc.) + +use std::future::Future; +use std::pin::Pin; +use std::sync::{Arc, Mutex}; +use std::time::Duration; use crate::{SdkError, SdkSuccess}; use aws_smithy_http::operation; use aws_smithy_http::operation::Operation; use aws_smithy_http::retry::ClassifyResponse; use aws_smithy_types::retry::{ErrorKind, RetryKind}; -use std::future::Future; -use std::pin::Pin; -use std::sync::{Arc, Mutex}; -use std::time::Duration; use tracing::Instrument; /// A policy instantiator. @@ -334,15 +335,15 @@ mod test { use aws_smithy_types::retry::ErrorKind; use std::time::Duration; - fn assert_send_sync() {} - fn test_config() -> Config { Config::default().with_base(|| 1_f64) } #[test] fn retry_handler_send_sync() { - assert_send_sync::() + fn must_be_send_sync() {} + + must_be_send_sync::() } #[test] diff --git a/rust-runtime/aws-smithy-client/src/timeout.rs b/rust-runtime/aws-smithy-client/src/timeout.rs index 64ed1b3ea..22811c6d8 100644 --- a/rust-runtime/aws-smithy-client/src/timeout.rs +++ b/rust-runtime/aws-smithy-client/src/timeout.rs @@ -9,25 +9,33 @@ //! //! 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::{TimedOutError, 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 tower::Layer; + /// Timeout Configuration #[derive(Default, Debug, Clone)] #[non_exhaustive] pub struct Settings { connect_timeout: Option, - http_read_timeout: Option, + read_timeout: Option, + tls_negotiation_timeout: Option, } impl Settings { - /// Create a new timeout configuration - /// - /// The current default (subject to change) is no timeouts + /// Create a new timeout configuration with no timeouts set pub fn new() -> Self { - Self { - connect_timeout: None, - http_read_timeout: None, - } + Default::default() } /// The configured TCP-connect timeout @@ -37,7 +45,7 @@ impl Settings { /// The configured HTTP-read timeout pub fn read(&self) -> Option { - self.http_read_timeout + self.read_timeout } /// Sets the connect timeout @@ -51,8 +59,243 @@ impl Settings { /// Sets the read timeout pub fn with_read_timeout(self, read_timeout: Duration) -> Self { Self { - http_read_timeout: Some(read_timeout), + read_timeout: Some(read_timeout), ..self } } } + +#[derive(Clone, Debug)] +/// A struct containing everything needed to create a new [`TimeoutService`] +pub struct TimeoutServiceParams { + /// The duration of timeouts created from these params + duration: Duration, + /// The kind of timeouts created from these params + kind: &'static str, + /// The AsyncSleep impl that will be used to create time-limited futures + async_sleep: Arc, +} + +#[derive(Clone, Debug, Default)] +/// A struct of structs containing everything needed to create new [`TimeoutService`]s +pub struct ClientTimeoutParams { + /// Params used to create a new API call [`TimeoutService`] + pub(crate) api_call: Option, + /// Params used to create a new API call attempt [`TimeoutService`] + pub(crate) api_call_attempt: Option, +} + +/// Convert a [`TimeoutConfig`] 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( + timeout_config: &TimeoutConfig, + async_sleep: Option>, +) -> ClientTimeoutParams { + if let Some(async_sleep) = async_sleep { + ClientTimeoutParams { + api_call: timeout_config + .api_call_timeout() + .map(|duration| TimeoutServiceParams { + duration, + kind: "API call (all attempts including retries)", + async_sleep: async_sleep.clone(), + }), + api_call_attempt: timeout_config.api_call_attempt_timeout().map(|duration| { + TimeoutServiceParams { + duration, + kind: "API call (single attempt)", + async_sleep: async_sleep.clone(), + } + }), + } + } else { + tracing::warn!( + "One or more timeouts were set but no async_sleep fn was passed. No timeouts will occur.\n{:?}", + timeout_config + ); + + Default::default() + } +} + +/// A service that wraps another service, adding the ability to set a timeout for requests +/// handled by the inner service. +#[derive(Clone, Debug)] +pub struct TimeoutService { + inner: S, + params: Option, +} + +impl TimeoutService { + /// Create a new `TimeoutService` that will timeout after the duration specified in `params` elapses + pub fn new(inner: S, params: Option) -> Self { + Self { inner, params } + } + + /// Create a new `TimeoutService` that will never timeout + pub fn no_timeout(inner: S) -> Self { + Self { + inner, + params: None, + } + } +} + +/// A layer that wraps services in a timeout service +#[non_exhaustive] +#[derive(Debug)] +pub struct TimeoutLayer(Option); + +impl TimeoutLayer { + /// Create a new `TimeoutLayer` + pub fn new(params: Option) -> Self { + TimeoutLayer(params) + } +} + +impl Layer for TimeoutLayer { + type Service = TimeoutService; + + fn layer(&self, inner: S) -> Self::Service { + TimeoutService { + inner, + params: self.0.clone(), + } + } +} + +pin_project! { + #[non_exhaustive] + #[must_use = "futures do nothing unless you `.await` or poll them"] + // This allow is needed because otherwise Clippy will get mad we didn't document the + // generated TimeoutServiceFutureProj + #[allow(missing_docs)] + #[project = TimeoutServiceFutureProj] + /// A future generated by a [`TimeoutService`] that may or may not have a timeout depending on + /// whether or not one was set. Because `TimeoutService` can be used at multiple levels of the + /// service stack, a `kind` can be set so that when a timeout occurs, you can know which kind of + /// timeout it was. + pub enum TimeoutServiceFuture { + /// A wrapper around an inner future that will output an [`SdkError`] if it runs longer than + /// the given duration + Timeout { + #[pin] + future: Timeout, + kind: &'static str, + duration: Duration, + }, + /// A thin wrapper around an inner future that will never time out + NoTimeout { + #[pin] + future: F + } + } +} + +impl TimeoutServiceFuture { + /// Given a `future`, an implementor of `AsyncSleep`, a `kind` for this timeout, and a `duration`, + /// wrap the `future` inside a [`Timeout`] future and create a new [`TimeoutServiceFuture`] that + /// will output an [`SdkError`] if `future` doesn't complete before `duration` has elapsed. + pub fn new(future: F, params: &TimeoutServiceParams) -> Self { + Self::Timeout { + future: Timeout::new(future, params.async_sleep.sleep(params.duration)), + kind: params.kind, + duration: params.duration, + } + } + + /// Create a [`TimeoutServiceFuture`] that will never time out. + pub fn no_timeout(future: F) -> Self { + Self::NoTimeout { future } + } +} + +impl Future for TimeoutServiceFuture +where + InnerFuture: Future>>, +{ + type Output = Result>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // TODO duration will be used in the error message once a Timeout variant is added to SdkError + let (future, _kind, _duration) = match self.project() { + TimeoutServiceFutureProj::NoTimeout { future } => return future.poll(cx), + TimeoutServiceFutureProj::Timeout { + future, + kind, + duration, + } => (future, kind, duration), + }; + match future.poll(cx) { + Poll::Ready(Ok(response)) => Poll::Ready(response), + Poll::Ready(Err(_timeout)) => { + // TODO update SdkError to include a variant specifically for timeouts + Poll::Ready(Err(SdkError::ConstructionFailure(Box::new(TimedOutError)))) + } + Poll::Pending => Poll::Pending, + } + } +} + +impl tower::Service> for TimeoutService +where + InnerService: tower::Service, Error = SdkError>, +{ + type Response = InnerService::Response; + type Error = aws_smithy_http::result::SdkError; + type Future = TimeoutServiceFuture; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, req: Operation) -> Self::Future { + let future = self.inner.call(req); + + if let Some(params) = &self.params { + Self::Future::new(future, params) + } else { + Self::Future::no_timeout(future) + } + } +} + +#[cfg(test)] +mod test { + use std::sync::Arc; + use std::time::Duration; + + 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::timeout::TimeoutConfig; + use tower::{Service, ServiceBuilder, ServiceExt}; + + #[tokio::test] + async fn test_timeout_service_ends_request_that_never_completes() { + let req = Request::new(http::Request::new(SdkBody::empty())); + let op = Operation::new(req, ()); + let never_service: NeverService<_, (), _> = NeverService::new(); + let timeout_config = + TimeoutConfig::new().with_api_call_timeout(Some(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 mut svc = ServiceBuilder::new() + .layer(TimeoutLayer::new(timeout_service_params.api_call)) + .service(never_service); + + let now = tokio::time::Instant::now(); + tokio::time::pause(); + + let err: SdkError> = + svc.ready().await.unwrap().call(op).await.unwrap_err(); + + assert_eq!(format!("{:?}", err), "ConstructionFailure(TimedOutError)"); + assert_elapsed!(now, Duration::from_secs_f32(0.25)); + } +} diff --git a/rust-runtime/aws-smithy-http-tower/src/parse_response.rs b/rust-runtime/aws-smithy-http-tower/src/parse_response.rs index 34f010aa2..4259b9910 100644 --- a/rust-runtime/aws-smithy-http-tower/src/parse_response.rs +++ b/rust-runtime/aws-smithy-http-tower/src/parse_response.rs @@ -9,7 +9,6 @@ use aws_smithy_http::operation; use aws_smithy_http::operation::Operation; use aws_smithy_http::response::ParseHttpResponse; use aws_smithy_http::result::SdkError; -use std::error::Error; use std::future::Future; use std::marker::PhantomData; use std::pin::Pin; @@ -67,22 +66,28 @@ type BoxedResultFuture = Pin> + Send> /// `T`: The happy path return of the response parser /// `E`: The error path return of the response parser /// `R`: The type of the retry policy -impl tower::Service> for ParseResponseService +impl + tower::Service> + for ParseResponseService where - S: Service, - S::Future: Send + 'static, - O: ParseHttpResponse> + Send + Sync + 'static, - E: Error, + InnerService: + Service, + InnerService::Future: Send + 'static, + ResponseHandler: ParseHttpResponse> + + Send + + Sync + + 'static, + FailureResponse: std::error::Error, { - type Response = aws_smithy_http::result::SdkSuccess; - type Error = aws_smithy_http::result::SdkError; + type Response = aws_smithy_http::result::SdkSuccess; + type Error = aws_smithy_http::result::SdkError; type Future = BoxedResultFuture; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { self.inner.poll_ready(cx).map_err(|err| err.into()) } - fn call(&mut self, req: Operation) -> Self::Future { + fn call(&mut self, req: Operation) -> Self::Future { let (req, parts) = req.into_request_response(); let handler = parts.response_handler; // send_operation records the full request-response lifecycle. diff --git a/rust-runtime/aws-smithy-http/src/result.rs b/rust-runtime/aws-smithy-http/src/result.rs index e55e73438..ffc0cff82 100644 --- a/rust-runtime/aws-smithy-http/src/result.rs +++ b/rust-runtime/aws-smithy-http/src/result.rs @@ -33,6 +33,9 @@ pub struct SdkSuccess { /// Failed SDK Result #[derive(Debug)] pub enum SdkError { + // TODO Request failures due to a timeout currently report this error type even though + // they're not really a construction failure. Add a new variant for timeouts or update + // DispatchFailure to accept more than just ConnectorErrors /// The request failed during construction. It was not dispatched over the network. ConstructionFailure(BoxError), diff --git a/rust-runtime/aws-smithy-types-convert/src/lib.rs b/rust-runtime/aws-smithy-types-convert/src/lib.rs index fffcfa252..5f614f2ef 100644 --- a/rust-runtime/aws-smithy-types-convert/src/lib.rs +++ b/rust-runtime/aws-smithy-types-convert/src/lib.rs @@ -7,7 +7,7 @@ #![warn( missing_docs, - missing_crate_level_docs, + rustdoc::missing_crate_level_docs, missing_debug_implementations, rust_2018_idioms, unreachable_pub diff --git a/rust-runtime/aws-smithy-types/src/lib.rs b/rust-runtime/aws-smithy-types/src/lib.rs index 356edae96..3dc67cc22 100644 --- a/rust-runtime/aws-smithy-types/src/lib.rs +++ b/rust-runtime/aws-smithy-types/src/lib.rs @@ -7,7 +7,7 @@ #![warn( missing_docs, - missing_crate_level_docs, + rustdoc::missing_crate_level_docs, missing_debug_implementations, rust_2018_idioms, unreachable_pub @@ -19,6 +19,7 @@ pub mod base64; pub mod date_time; pub mod primitive; pub mod retry; +pub mod timeout; pub use crate::date_time::DateTime; diff --git a/rust-runtime/aws-smithy-types/src/retry.rs b/rust-runtime/aws-smithy-types/src/retry.rs index c6fb1ebb1..f67d15bf6 100644 --- a/rust-runtime/aws-smithy-types/src/retry.rs +++ b/rust-runtime/aws-smithy-types/src/retry.rs @@ -173,13 +173,13 @@ impl RetryConfigBuilder { /// # use aws_smithy_types::retry::{RetryMode, RetryConfigBuilder}; /// let a = RetryConfigBuilder::new().max_attempts(1); /// let b = RetryConfigBuilder::new().max_attempts(5).mode(RetryMode::Adaptive); - /// let retry_config = a.merge_with(b).build(); + /// let retry_config = a.take_unset_from(b).build(); /// // A's value take precedence over B's value /// assert_eq!(retry_config.max_attempts(), 1); /// // A never set a retry mode so B's value was used /// assert_eq!(retry_config.mode(), RetryMode::Adaptive); /// ``` - pub fn merge_with(self, other: Self) -> Self { + pub fn take_unset_from(self, other: Self) -> Self { Self { mode: self.mode.or(other.mode), max_attempts: self.max_attempts.or(other.max_attempts), @@ -324,7 +324,7 @@ mod tests { let other_builder = RetryConfigBuilder::new() .max_attempts(5) .mode(RetryMode::Standard); - let retry_config = self_builder.merge_with(other_builder).build(); + let retry_config = self_builder.take_unset_from(other_builder).build(); assert_eq!(retry_config.max_attempts, 1); assert_eq!(retry_config.mode, RetryMode::Adaptive); diff --git a/rust-runtime/aws-smithy-types/src/timeout.rs b/rust-runtime/aws-smithy-types/src/timeout.rs new file mode 100644 index 000000000..291694037 --- /dev/null +++ b/rust-runtime/aws-smithy-types/src/timeout.rs @@ -0,0 +1,350 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +//! This module defines types that describe timeouts for the various stages of an HTTP request. + +use std::borrow::Cow; +use std::fmt::{Debug, Display, Formatter}; +use std::time::Duration; + +/// Configuration for timeouts +/// +/// # Example +/// +/// ```rust +/// # use std::time::Duration; +/// +/// # fn main() { +/// use aws_smithy_types::timeout::TimeoutConfig; +/// let timeout_config = TimeoutConfig::new() +/// .with_api_call_timeout(Some(Duration::from_secs(2))) +/// .with_api_call_attempt_timeout(Some(Duration::from_secs_f32(0.5))); +/// +/// assert_eq!( +/// timeout_config.api_call_timeout(), +/// Some(Duration::from_secs(2)) +/// ); +/// +/// assert_eq!( +/// timeout_config.api_call_attempt_timeout(), +/// Some(Duration::from_secs_f32(0.5)) +/// ); +/// # } +/// ``` +#[derive(Clone, PartialEq, Default)] +pub struct TimeoutConfig { + connect_timeout: Option, + tls_negotiation_timeout: Option, + read_timeout: Option, + api_call_attempt_timeout: Option, + api_call_timeout: Option, +} + +impl Debug for TimeoutConfig { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + r#"Timeouts: +Connect (time to first byte):{} +TLS negotiation:{} +HTTP read:{} +API requests:{} +HTTP requests:{} +"#, + format_timeout(self.connect_timeout), + format_timeout(self.tls_negotiation_timeout), + format_timeout(self.read_timeout), + format_timeout(self.api_call_timeout), + format_timeout(self.api_call_attempt_timeout), + ) + } +} + +fn format_timeout(timeout: Option) -> String { + timeout + .map(|d| format!("\t{}s", d.as_secs_f32())) + .unwrap_or_else(|| "(unset)".to_owned()) +} + +/// 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 +/// +/// ```should_panic +/// # use std::time::Duration; +/// # use aws_smithy_types::timeout::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 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(TimeoutConfigError::InvalidTimeout { + set_by, + name, + reason: "timeout must not be less than or equal to zero".into(), + }), + Ok(timeout) if timeout.is_nan() => Err(TimeoutConfigError::InvalidTimeout { + set_by, + name, + reason: "timeout must not be NaN".into(), + }), + Ok(timeout) if timeout.is_infinite() => Err(TimeoutConfigError::InvalidTimeout { + set_by, + name, + reason: "timeout must not be infinite".into(), + }), + Ok(timeout) => Ok(Duration::from_secs_f32(timeout)), + Err(err) => Err(TimeoutConfigError::ParseError { + set_by, + name, + source: Box::new(err), + }), + } +} + +impl TimeoutConfig { + /// Create a new `TimeoutConfig` with no timeouts set + pub fn new() -> Self { + Default::default() + } + + /// 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) -> Option { + self.connect_timeout + } + + /// A limit on the amount of time a TLS handshake takes from when the `CLIENT HELLO` message is + /// sent to the time the client and server have fully negotiated ciphers and exchanged keys. + pub fn tls_negotiation_timeout(&self) -> Option { + self.tls_negotiation_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) -> Option { + self.read_timeout + } + + /// 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 [`TimeoutConfig::api_call_timeout`] instead. + pub fn api_call_attempt_timeout(&self) -> Option { + self.api_call_attempt_timeout + } + + /// A limit on the amount of time it takes for request to complete. A single request may be + /// comprised of several attemps depending on an app's [`RetryConfig`](super::retry::RetryConfig). If you want + /// to control timeouts for a single attempt, use [`TimeoutConfig::api_call_attempt_timeout`]. + pub fn api_call_timeout(&self) -> Option { + self.api_call_timeout + } + + /// Consume a `TimeoutConfig` to create a new one, setting the connect timeout + pub fn with_connect_timeout(mut self, timeout: Option) -> Self { + self.connect_timeout = timeout; + self + } + + /// Consume a `TimeoutConfig` to create a new one, setting the TLS negotiation timeout + pub fn with_tls_negotiation_timeout(mut self, timeout: Option) -> Self { + self.tls_negotiation_timeout = timeout; + self + } + + /// Consume a `TimeoutConfig` to create a new one, setting the read timeout + pub fn with_read_timeout(mut self, timeout: Option) -> Self { + self.read_timeout = timeout; + self + } + + /// Consume a `TimeoutConfig` to create a new one, setting the API call attempt timeout + pub fn with_api_call_attempt_timeout(mut self, timeout: Option) -> Self { + self.api_call_attempt_timeout = timeout; + self + } + + /// Consume a `TimeoutConfig` to create a new one, setting the API call timeout + pub fn with_api_call_timeout(mut self, timeout: Option) -> Self { + self.api_call_timeout = timeout; + self + } + + /// Merges two 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 + /// handle "precedence" per value instead of at the config level + /// + /// # Example + /// + /// ```rust + /// # use std::time::Duration; + /// # use aws_smithy_types::timeout::TimeoutConfig; + /// let a = TimeoutConfig::new().with_read_timeout(Some(Duration::from_secs(2))); + /// let b = TimeoutConfig::new() + /// .with_read_timeout(Some(Duration::from_secs(10))) + /// .with_connect_timeout(Some(Duration::from_secs(3))); + /// let timeout_config = a.take_unset_from(b); + /// // A's value take precedence over B's value + /// assert_eq!(timeout_config.read_timeout(), Some(Duration::from_secs(2))); + /// // A never set a connect timeout so B's value was used + /// assert_eq!(timeout_config.connect_timeout(), Some(Duration::from_secs(3))); + /// ``` + pub fn take_unset_from(self, other: Self) -> Self { + Self { + connect_timeout: self.connect_timeout.or(other.connect_timeout), + tls_negotiation_timeout: self + .tls_negotiation_timeout + .or(other.tls_negotiation_timeout), + read_timeout: self.read_timeout.or(other.read_timeout), + api_call_attempt_timeout: self + .api_call_attempt_timeout + .or(other.api_call_attempt_timeout), + api_call_timeout: self.api_call_timeout.or(other.api_call_timeout), + } + } +} + +#[non_exhaustive] +#[derive(Debug)] +/// An error that occurs during construction of a `TimeoutConfig` +pub enum TimeoutConfigError { + /// A timeout value was set to an invalid value: + /// - Any number less than 0 + /// - Infinity or negative infinity + /// - `NaN` + InvalidTimeout { + /// The name of the invalid value + name: Cow<'static, str>, + /// The reason that why the timeout was considered invalid + reason: Cow<'static, str>, + /// Where the invalid value originated from + set_by: Cow<'static, str>, + }, + /// The timeout value couln't be parsed as an `f32` + ParseError { + /// The name of the invalid value + name: Cow<'static, str>, + /// Where the invalid value originated from + set_by: Cow<'static, str>, + /// The source of this error + source: Box, + }, +} + +impl Display for TimeoutConfigError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + use TimeoutConfigError::*; + match self { + InvalidTimeout { + name, + set_by, + reason, + } => { + write!( + f, + "invalid timeout '{}' set by {} is invalid: {}", + name, set_by, reason + ) + } + ParseError { + name, + set_by, + source, + } => { + write!( + f, + "timeout '{}' set by {} could not be parsed as an f32: {}", + name, set_by, source + ) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::{parse_str_as_timeout, TimeoutConfig}; + use std::time::Duration; + + #[test] + fn retry_config_builder_merge_with_favors_self_values_over_other_values() { + let one_second = Some(Duration::from_secs(1)); + let two_seconds = Some(Duration::from_secs(2)); + + let self_config = TimeoutConfig::new() + .with_connect_timeout(one_second) + .with_read_timeout(one_second) + .with_tls_negotiation_timeout(one_second) + .with_api_call_timeout(one_second) + .with_api_call_attempt_timeout(one_second); + let other_config = TimeoutConfig::new() + .with_connect_timeout(two_seconds) + .with_read_timeout(two_seconds) + .with_tls_negotiation_timeout(two_seconds) + .with_api_call_timeout(two_seconds) + .with_api_call_attempt_timeout(two_seconds); + let timeout_config = self_config.take_unset_from(other_config); + + assert_eq!(timeout_config.connect_timeout(), one_second); + assert_eq!(timeout_config.read_timeout(), one_second); + assert_eq!(timeout_config.tls_negotiation_timeout(), one_second); + assert_eq!(timeout_config.api_call_timeout(), one_second); + assert_eq!(timeout_config.api_call_attempt_timeout(), one_second); + } + + #[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(); + } +} -- GitLab