Skip to content

[Detail Bug] C0-Config timeout-only override resets client RetryConfig to defaults #26

@detail-app

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_f8be1c6c-ff1b-4620-87a8-bba9c3a1fee1

Summary

  • Context: RequestConfig provides methods to convert request configuration into AWS SDK timeout and retry configs, which are used when overriding S3 client configuration for individual requests.
  • Bug: The retry_config() method always creates a new RetryConfig::standard() even when no retry-related fields are set, inadvertently resetting any custom retry configuration from environment variables or AWS config files.
  • Actual vs. expected: When a user specifies only timeout overrides via the C0-Config header, the retry config is reset to standard defaults (3 max attempts) instead of preserving the S3 client's existing retry configuration.
  • Impact: Users who configure retry behavior via AWS_MAX_ATTEMPTS environment variable or AWS config file will have their settings silently overridden when using C0-Config header to adjust only timeout values.

Code with bug

pub fn retry_config(&self) -> aws_sdk_s3::config::retry::RetryConfig {
    let mut config = aws_sdk_s3::config::retry::RetryConfig::standard(); // <-- BUG 🔴 Always creates new config

    if let Some(max_attempts) = self.max_attempts {
        config = config.with_max_attempts(max_attempts);
    }
    if let Some(initial_backoff) = self.initial_backoff {
        config = config.with_initial_backoff(initial_backoff);
    }
    if let Some(max_backoff) = self.max_backoff {
        config = config.with_max_backoff(max_backoff);
    }

    config
}

Usage in src/object_store/downloader.rs:

request
    .customize()
    .config_override(
        self.s3
            .config()
            .to_builder()
            .timeout_config(req_config.timeout_config())
            .retry_config(req_config.retry_config()), // <-- BUG 🔴 Always replaces retry config
    )
    .send()
    .await

Failing test

#[tokio::test]
async fn test_partial_timeout_config_overwrites_retry_config() {
    let config = aws_sdk_s3::Config::builder()
        .behavior_version(aws_config::BehaviorVersion::latest())
        .credentials_provider(aws_sdk_s3::config::Credentials::new(
            "test", "test", None, None, "test",
        ))
        .region(aws_sdk_s3::config::Region::new("us-east-1"))
        .retry_config(
            aws_sdk_s3::config::retry::RetryConfig::standard()
                .with_max_attempts(10)
        )
        .build();

    let client = aws_sdk_s3::Client::from_conf(config);

    let original_max = client.config().retry_config()
        .map(|rc| rc.max_attempts())
        .expect("should have retry config");
    assert_eq!(original_max, 10);

    let req_config = RequestConfig {
        connect_timeout: Some(Duration::from_secs(5)),
        read_timeout: None,
        operation_timeout: None,
        operation_attempt_timeout: None,
        max_attempts: None,
        initial_backoff: None,
        max_backoff: None,
    };

    let new_config = client
        .config()
        .to_builder()
        .timeout_config(req_config.timeout_config())
        .retry_config(req_config.retry_config())
        .build();

    let new_client = aws_sdk_s3::Client::from_conf(new_config);

    let new_max = new_client.config().retry_config()
        .map(|rc| rc.max_attempts())
        .expect("should have retry config");

    assert_eq!(new_max, 10,
        "BUG: max_attempts changed from 10 to {} when only timeout was configured",
        new_max);
}

Output:

assertion `left == right` failed: BUG: max_attempts changed from 10 to 3 when only timeout was configured
  left: 3
 right: 10

Recommended fix

Conditionally apply overrides only when corresponding fields are set.

In src/object_store/downloader.rs:

let mut builder = self.s3.config().to_builder();

if req_config.has_timeout_fields() {
    builder = builder.timeout_config(req_config.timeout_config());
}
if req_config.has_retry_fields() {
    builder = builder.retry_config(req_config.retry_config());
}

.config_override(builder)

Add helpers to RequestConfig:

pub fn has_timeout_fields(&self) -> bool {
    self.connect_timeout.is_some()
        || self.read_timeout.is_some()
        || self.operation_timeout.is_some()
        || self.operation_attempt_timeout.is_some()
}

pub fn has_retry_fields(&self) -> bool {
    self.max_attempts.is_some()
        || self.initial_backoff.is_some()
        || self.max_backoff.is_some()
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions