Timeouts refactor (#1246) (107194f1) · Commits · Public Repositories / Smithy Rust

CHANGELOG.next.toml

+40 −0

Original line number	Diff line number	Diff line
		@@ -93,3 +93,43 @@ message = "Enable presigning for S3 operations UploadPart and DeleteObject"
		references = ["aws-sdk-rust#475", "aws-sdk-rust#473"]
		meta = { "breaking" = false, "tada" = true, "bug" = false }
		author = "rcoh"

		[[smithy-rs]]
		message = """
		Timeout configuration has been refactored a bit. If you were setting timeouts through environment variables or an AWS
		profile, then you shouldn't need to change anything. Take note, however, that we don't currently support HTTP connect,
		read, write, or TLS negotiation timeouts. If you try to set any of those timeouts in your profile or environment, we'll
		log a warning explaining that those timeouts don't currently do anything.

		If you were using timeouts programmatically,
		you'll need to update your code. In previous versions, timeout configuration was stored in a single `TimeoutConfig`
		struct. In this new version, timeouts have been broken up into several different config structs that are then collected
		in a `timeout::Config` struct. As an example, to get the API per-attempt timeout in previous versions you would access
		it with `<your TimeoutConfig>.api_call_attempt_timeout()` and in this new version you would access it with
		`<your timeout::Config>.api.call_attempt_timeout()`. We also made some unimplemented timeouts inaccessible in order to
		avoid giving users the impression that setting them had an effect. We plan to re-introduce them once they're made
		functional in a future update.
		"""
		references = ["smithy-rs#724"]
		meta = { "breaking" = true, "tada" = false, "bug" = false }
		author = "Velfi"

		[[aws-sdk-rust]]
		message = """
		Timeout configuration has been refactored a bit. If you were setting timeouts through environment variables or an AWS
		profile, then you shouldn't need to change anything. Take note, however, that we don't currently support HTTP connect,
		read, write, or TLS negotiation timeouts. If you try to set any of those timeouts in your profile or environment, we'll
		log a warning explaining that those timeouts don't currently do anything.

		If you were using timeouts programmatically,
		you'll need to update your code. In previous versions, timeout configuration was stored in a single `TimeoutConfig`
		struct. In this new version, timeouts have been broken up into several different config structs that are then collected
		in a `timeout::Config` struct. As an example, to get the API per-attempt timeout in previous versions you would access
		it with `<your TimeoutConfig>.api_call_attempt_timeout()` and in this new version you would access it with
		`<your timeout::Config>.api.call_attempt_timeout()`. We also made some unimplemented timeouts inaccessible in order to
		avoid giving users the impression that setting them had an effect. We plan to re-introduce them once they're made
		functional in a future update.
		"""
		references = ["smithy-rs#724"]
		meta = { "breaking" = true, "tada" = false, "bug" = false }
		author = "Velfi"

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

+1 −1

Original line number	Diff line number	Diff line
		@@ -23,7 +23,7 @@ fn base(
		sleep: Option<Arc<dyn AsyncSleep>>,
		) -> aws_smithy_client::hyper_ext::Builder {
		let mut hyper =
		aws_smithy_client::hyper_ext::Adapter::builder().timeout(&settings.timeout_config);
		aws_smithy_client::hyper_ext::Adapter::builder().timeout(&settings.http_timeout_config);
		if let Some(sleep) = sleep {
		hyper = hyper.sleep_impl(sleep);
		}

aws/rust-runtime/aws-config/src/default_provider/timeout_config.rs

+7 −27

Original line number	Diff line number	Diff line
		@@ -3,15 +3,15 @@
		* SPDX-License-Identifier: Apache-2.0.
		*/

		use aws_smithy_types::timeout::TimeoutConfig;
		use aws_smithy_types::timeout;

		use crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider;
		use crate::profile;
		use crate::provider_config::ProviderConfig;

		/// Default [`TimeoutConfig`] Provider chain
		/// Default [`timeout::Config`] Provider chain
		///
		/// Unlike other credentials and region, [`TimeoutConfig`] has no related `TimeoutConfigProvider` trait. Instead,
		/// Unlike other credentials and region, [`timeout::Config`] 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:
		@@ -44,7 +44,7 @@ pub fn default_provider() -> Builder {
		Builder::default()
		}

		/// Builder for [`TimeoutConfig`] that checks the environment variables and AWS profile files for configuration
		/// Builder for [`timeout::Config`](aws_smithy_types::timeout::Config) that checks the environment variables and AWS profile files for configuration
		#[derive(Default)]
		pub struct Builder {
		env_provider: EnvironmentVariableTimeoutConfigProvider,
		@@ -68,7 +68,7 @@ impl Builder {
		self
		}

		/// Attempt to create a [`TimeoutConfig`](aws_smithy_types::timeout::TimeoutConfig) from following sources in order:
		/// Attempt to create a [`timeout::Config`](aws_smithy_types::timeout::Config) from following sources in order:
		/// 1. [Environment variables](crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider)
		/// 2. [Profile file](crate::profile::timeout_config::ProfileFileTimeoutConfigProvider)
		///
		@@ -79,7 +79,7 @@ impl Builder {
		/// This will panic if:
		/// - a timeout is set to `NaN`, a negative number, or infinity
		/// - a timeout can't be parsed as a floating point number
		pub async fn timeout_config(self) -> TimeoutConfig {
		pub async fn timeout_config(self) -> timeout::Config {
		// Both of these can return errors due to invalid config settings and we want to surface those as early as possible
		// hence, we'll panic if any config values are invalid (missing values are OK though)
		// We match this instead of unwrapping so we can print the error with the `Display` impl instead of the `Debug` impl that unwrap uses
		@@ -92,26 +92,6 @@ impl 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
		builder_from_env.take_unset_from(builder_from_profile)
		}
		}

aws/rust-runtime/aws-config/src/environment/timeout_config.rs

+54 −41

Original line number	Diff line number	Diff line
		@@ -5,23 +5,28 @@

		//! Load timeout configuration properties from environment variables

		use aws_smithy_types::timeout::{parse_str_as_timeout, TimeoutConfig, TimeoutConfigError};
		use crate::parsing::parse_str_as_timeout;

		use aws_smithy_types::timeout;
		use aws_smithy_types::tristate::TriState;
		use aws_types::os_shim_internal::Env;

		use std::time::Duration;

		// Currently unsupported timeouts
		const ENV_VAR_CONNECT_TIMEOUT: &str = "AWS_CONNECT_TIMEOUT";
		const ENV_VAR_TLS_NEGOTIATION_TIMEOUT: &str = "AWS_TLS_NEGOTIATION_TIMEOUT";
		const ENV_VAR_READ_TIMEOUT: &str = "AWS_READ_TIMEOUT";

		// Supported timeouts
		const ENV_VAR_API_CALL_ATTEMPT_TIMEOUT: &str = "AWS_API_CALL_ATTEMPT_TIMEOUT";
		const ENV_VAR_API_CALL_TIMEOUT: &str = "AWS_API_CALL_TIMEOUT";

		/// Load a timeout_config from environment variables
		///
		/// This provider will check the values of the following variables in order to build a `TimeoutConfig`
		/// This provider will check the values of the following variables in order to build a
		/// [`timeout::Config`](aws_smithy_types::timeout::Config)
		///
		/// - `AWS_CONNECT_TIMEOUT`
		/// - `AWS_TLS_NEGOTIATION_TIMEOUT`
		/// - `AWS_READ_TIMEOUT`
		/// - `AWS_API_CALL_ATTEMPT_TIMEOUT`
		/// - `AWS_API_CALL_TIMEOUT`
		///
		@@ -46,34 +51,48 @@ impl EnvironmentVariableTimeoutConfigProvider {
		EnvironmentVariableTimeoutConfigProvider { env }
		}

		/// Attempt to create a new [`TimeoutConfig`] from environment variables
		pub fn timeout_config(&self) -> Result<TimeoutConfig, TimeoutConfigError> {
		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)?;
		/// Attempt to create a new [`timeout::Config`](aws_smithy_types::timeout::Config) from environment variables
		pub fn timeout_config(&self) -> Result<timeout::Config, timeout::ConfigError> {
		// Warn users that set unsupported timeouts in their profile
		for timeout in [
		ENV_VAR_CONNECT_TIMEOUT,
		ENV_VAR_TLS_NEGOTIATION_TIMEOUT,
		ENV_VAR_READ_TIMEOUT,
		] {
		warn_if_unsupported_timeout_is_set(&self.env, timeout);
		}

		let api_call_attempt_timeout =
		construct_timeout_from_env_var(&self.env, ENV_VAR_API_CALL_ATTEMPT_TIMEOUT)?;
		let api_call_timeout = construct_timeout_from_env_var(&self.env, ENV_VAR_API_CALL_TIMEOUT)?;

		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))
		let api_timeouts = timeout::Api::new()
		.with_call_timeout(api_call_timeout)
		.with_call_attempt_timeout(api_call_attempt_timeout);

		// Only API-related timeouts are currently supported
		Ok(timeout::Config::new().with_api_timeouts(api_timeouts))
		}
		}

		fn construct_timeout_from_env_var(
		env: &Env,
		var: &'static str,
		) -> Result<Option<Duration>, TimeoutConfigError> {
		) -> Result<TriState<Duration>, timeout::ConfigError> {
		match env.get(var).ok() {
		Some(timeout) => {
		parse_str_as_timeout(&timeout, var.into(), "environment variable".into()).map(Some)
		Some(timeout) => parse_str_as_timeout(&timeout, var.into(), "environment variable".into())
		.map(TriState::Set),
		None => Ok(TriState::Unset),
		}
		None => Ok(None),
		}

		fn warn_if_unsupported_timeout_is_set(env: &Env, var: &'static str) {
		if env.get(var).is_ok() {
		tracing::warn!(
		"Environment variable for '{}' timeout was set but that feature is currently unimplemented so the setting will be ignored. \
		To help us prioritize support for this feature, please upvote aws-sdk-rust#151 (https://github.com/awslabs/aws-sdk-rust/issues/151)",
		var
		)
		}
		}

		@@ -81,10 +100,10 @@ fn construct_timeout_from_env_var(
		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,
		ENV_VAR_API_CALL_TIMEOUT,
		};
		use aws_smithy_types::timeout::TimeoutConfig;
		use aws_smithy_types::timeout;
		use aws_smithy_types::tristate::TriState;
		use aws_types::os_shim_internal::Env;
		use std::time::Duration;

		@@ -96,33 +115,27 @@ mod 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);
		assert_eq!(built.api.call_timeout(), TriState::Unset);
		assert_eq!(built.api.call_attempt_timeout(), TriState::Unset);
		}

		#[test]
		fn all_fields_can_be_set_at_once() {
		let expected_api_timeouts = timeout::Api::new()
		.with_call_attempt_timeout(TriState::Set(Duration::from_secs_f32(4.0)))
		// Some floats can't be represented as f32 so this duration will end up equalling the
		// duration from the env.
		.with_call_timeout(TriState::Set(Duration::from_secs_f32(900012350.0)));
		let expected_timeouts = timeout::Config::new().with_api_timeouts(expected_api_timeouts);

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

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

+16 −9

Original line number	Diff line number	Diff line
		@@ -16,7 +16,8 @@ use aws_smithy_http::response::ParseStrictResponse;
		use aws_smithy_http::result::{SdkError, SdkSuccess};
		use aws_smithy_http::retry::ClassifyResponse;
		use aws_smithy_types::retry::{ErrorKind, RetryKind};
		use aws_smithy_types::timeout::TimeoutConfig;
		use aws_smithy_types::timeout;
		use aws_smithy_types::tristate::TriState;
		use aws_types::credentials::CredentialsError;
		use aws_types::{credentials, Credentials};

		@@ -79,7 +80,7 @@ impl HttpCredentialProvider {
		#[derive(Default)]
		pub(crate) struct Builder {
		provider_config: Option<ProviderConfig>,
		timeout_config: TimeoutConfig,
		http_timeout_config: timeout::Http,
		}

		impl Builder {
		@@ -91,22 +92,28 @@ impl Builder {
		// read_timeout and connect_timeout accept options to enable easy pass through from
		// other builders
		pub(crate) fn read_timeout(mut self, read_timeout: Option<Duration>) -> Self {
		self.timeout_config = self.timeout_config.with_read_timeout(read_timeout);
		self.http_timeout_config = self
		.http_timeout_config
		.with_read_timeout(read_timeout.into());
		self
		}

		pub(crate) fn connect_timeout(mut self, connect_timeout: Option<Duration>) -> Self {
		self.timeout_config = self.timeout_config.with_connect_timeout(connect_timeout);
		self.http_timeout_config = self
		.http_timeout_config
		.with_connect_timeout(connect_timeout.into());
		self
		}

		pub(crate) fn build(self, provider_name: &'static str, uri: Uri) -> HttpCredentialProvider {
		let provider_config = self.provider_config.unwrap_or_default();
		let default_timeout_config = TimeoutConfig::new()
		.with_connect_timeout(Some(DEFAULT_CONNECT_TIMEOUT))
		.with_read_timeout(Some(DEFAULT_READ_TIMEOUT));
		let timeout_config = self.timeout_config.take_unset_from(default_timeout_config);
		let http_settings = HttpSettings::default().with_timeout_config(timeout_config);
		let default_timeout_config = timeout::Http::new()
		.with_connect_timeout(TriState::Set(DEFAULT_CONNECT_TIMEOUT))
		.with_read_timeout(TriState::Set(DEFAULT_READ_TIMEOUT));
		let http_timeout_config = self
		.http_timeout_config
		.take_unset_from(default_timeout_config);
		let http_settings = HttpSettings::default().with_http_timeout_config(http_timeout_config);
		let connector = expect_connector(provider_config.connector(&http_settings));
		let client = aws_smithy_client::Builder::new()
		.connector(connector)