Skip to content

[deployer]: remove unbounded layered S3 retries#3279

Open
GarmashAlex wants to merge 1 commit intocommonwarexyz:mainfrom
GarmashAlex:fix/deployer-s3-retry-layering
Open

[deployer]: remove unbounded layered S3 retries#3279
GarmashAlex wants to merge 1 commit intocommonwarexyz:mainfrom
GarmashAlex:fix/deployer-s3-retry-layering

Conversation

@GarmashAlex
Copy link
Contributor

Why were these changes needed?

S3 uploads had two unbounded retry layers at the same time:

  1. SDK retry config used with_max_attempts(u32::MAX),
  2. upload_with_retry also had an infinite manual retry loop.

This made failures hard to diagnose and could cause very long or effectively unbounded wait times.

What does this change do?

  • Caps S3 SDK retries to a finite value (max_attempts = 5).
  • Removes the manual infinite retry loop from upload_with_retry.
  • Keeps retry responsibility in one place (AWS SDK policy).
  • Propagates upload errors to callers instead of retrying forever at the wrapper level.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

.await
.map_err(|e| Error::AwsS3 {
bucket: bucket.to_string(),
operation: super::S3Operation::HeadObject,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong S3Operation variant used for PutObject error

Medium Severity

The upload_with_retry function performs a put_object call but tags the error with S3Operation::HeadObject. This causes any upload failure to be reported as a HeadObject error, making it very difficult to diagnose S3 upload issues. The S3Operation enum also lacks a PutObject variant, so one needs to be added.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant