diff --git a/aws/rust-runtime/aws-config/Cargo.lock b/aws/rust-runtime/aws-config/Cargo.lock index ded620f2fdf..f7f5f73e8e9 100644 --- a/aws/rust-runtime/aws-config/Cargo.lock +++ b/aws/rust-runtime/aws-config/Cargo.lock @@ -291,7 +291,7 @@ dependencies = [ [[package]] name = "aws-smithy-json" -version = "0.61.8" +version = "0.61.9" dependencies = [ "aws-smithy-types", ] @@ -330,7 +330,7 @@ dependencies = [ [[package]] name = "aws-smithy-runtime" -version = "1.9.5" +version = "1.9.6" dependencies = [ "aws-smithy-async", "aws-smithy-http", diff --git a/aws/sdk/integration-tests/s3/tests/timeouts.rs b/aws/sdk/integration-tests/s3/tests/timeouts.rs index 14e6f0d54b9..c037eca575b 100644 --- a/aws/sdk/integration-tests/s3/tests/timeouts.rs +++ b/aws/sdk/integration-tests/s3/tests/timeouts.rs @@ -166,25 +166,95 @@ async fn test_read_timeout() { server_handle.await.unwrap(); } -#[tokio::test] -async fn test_connect_timeout() { - let config = Config::builder() +async fn run_connect_timeout_test(timeout_config: Option, expected_timeout_ms: u64) { + let mut config_builder = Config::builder() .with_test_defaults() .region(Region::new("us-east-1")) - .timeout_config( + .endpoint_url("http://172.255.255.0:18104"); + + if let Some(tc) = timeout_config { + config_builder = config_builder.timeout_config(tc); + } + + let client = Client::from_conf(config_builder.build()); + + if let Ok(result) = timeout( + Duration::from_millis(expected_timeout_ms + 1000), + client.get_object().bucket("test").key("test").send(), + ) + .await + { + match result { + Ok(_) => panic!("should not have succeeded"), + Err(err) => { + let message = format!("{}", DisplayErrorContext(&err)); + let expected = + "timeout: client error (Connect): HTTP connect timeout occurred after"; + assert!( + message.contains(expected), + "expected '{message}' to contain '{expected}'" + ); + } + } + } else { + panic!("the client didn't timeout"); + } +} + +#[tokio::test] +async fn test_connect_timeout_explicit() { + run_connect_timeout_test( + Some( TimeoutConfig::builder() .connect_timeout(Duration::from_millis(300)) .build(), - ) - .endpoint_url( - // Emulate a connect timeout error by hitting an unroutable IP - "http://172.255.255.0:18104", - ) + ), + 300, + ) + .await; +} + +#[tokio::test] +async fn test_connect_timeout_default() { + run_connect_timeout_test(None, 3100).await; +} + +#[tokio::test] +async fn test_connect_timeout_disabled() { + let config = Config::builder() + .with_test_defaults() + .region(Region::new("us-east-1")) + .timeout_config(TimeoutConfig::disabled()) + .endpoint_url("http://172.255.255.0:18104") .build(); let client = Client::from_conf(config); + if let Err(_) = timeout( + Duration::from_secs(5), + client.get_object().bucket("test").key("test").send(), + ) + .await + { + // Expected: the operation should not complete within 5 seconds when timeout is disabled + } else { + panic!("operation completed unexpectedly when timeout was disabled"); + } +} + +// this behavior is surprising—I would have expected this to fail, but it seems like the default +// runtime plugin always is providing a sleep impl. In any case, this pattern does not seem to exist +// in real life +#[tokio::test] +async fn test_connect_timeout_with_sleep_impl_none() { + let mut builder = Config::builder() + .with_test_defaults() + .region(Region::new("us-east-1")) + .endpoint_url("http://172.255.255.0:18104"); + builder.set_sleep_impl(None); + let client = Client::from_conf(builder.build()); + if let Ok(result) = timeout( - Duration::from_millis(1000), + Duration::from_millis(4100), client.get_object().bucket("test").key("test").send(), ) .await @@ -194,7 +264,7 @@ async fn test_connect_timeout() { Err(err) => { let message = format!("{}", DisplayErrorContext(&err)); let expected = - "timeout: client error (Connect): HTTP connect timeout occurred after 300ms"; + "timeout: client error (Connect): HTTP connect timeout occurred after"; assert!( message.contains(expected), "expected '{message}' to contain '{expected}'" diff --git a/codegen-server-test/integration-tests/Cargo.lock b/codegen-server-test/integration-tests/Cargo.lock index be271443640..f893b571903 100644 --- a/codegen-server-test/integration-tests/Cargo.lock +++ b/codegen-server-test/integration-tests/Cargo.lock @@ -148,7 +148,7 @@ dependencies = [ [[package]] name = "aws-smithy-json" -version = "0.61.8" +version = "0.61.9" dependencies = [ "aws-smithy-types", ] @@ -179,7 +179,7 @@ dependencies = [ [[package]] name = "aws-smithy-runtime" -version = "1.9.5" +version = "1.9.6" dependencies = [ "aws-smithy-async", "aws-smithy-http", diff --git a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs index a0a2b4d17dc..5a247760ec7 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs @@ -181,6 +181,30 @@ pub fn default_timeout_config_plugin() -> Option { ) } +/// Runtime plugin that sets the default timeout config (no timeouts). +pub fn default_timeout_config_plugin_v2() -> Option { + Some( + default_plugin("default_timeout_config_plugin", |components| { + components.with_config_validator(SharedConfigValidator::base_client_config_fn( + validate_timeout_config, + )) + }) + .with_config(layer("default_timeout_config", |layer| { + let timeout_config = if default_sleep_impl_plugin().is_some() { + TimeoutConfig::builder() + .connect_timeout(Duration::from_millis(3100)) + .disable_operation_attempt_timeout() + .disable_operation_timeout() + .build() + } else { + TimeoutConfig::disabled() + }; + layer.store_put(timeout_config); + })) + .into_shared(), + ) +} + fn validate_timeout_config( components: &RuntimeComponentsBuilder, cfg: &ConfigBag, @@ -327,7 +351,7 @@ pub fn default_plugins( ), default_sleep_impl_plugin(), default_time_source_plugin(), - default_timeout_config_plugin(), + default_timeout_config_plugin_v2(), enforce_content_length_runtime_plugin(), default_stalled_stream_protection_config_plugin_v2(behavior_version), ]