Skip to content

fix(s3): return NoSuchBucket when listing missing bucket - #295

Merged
ArnavBalyan merged 2 commits into
openlake-project:mainfrom
Keda710:fix/list-missing-bucket-293
Sep 28, 2026
Merged

ArnavBalyan merged 2 commits into
openlake-project:mainfrom
Keda710:fix/list-missing-bucket-293

Conversation

@Keda710

@Keda710 Keda710 commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #293. Checks bucket existence before ListObjects V1 and V2, including max-keys=0. Adds regression tests for missing and existing buckets.

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 1 low-severity finding

Reviewed commit 8029672.

}
let engine = state.engine().clone();
let bucket_to_check = bucket.clone();
SendWrapper::new(async move { engine.stat_bucket(&bucket_to_check).await }).await?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Low · Handler-level stat adds a second cluster-wide fan-out per list page and leaves the engine bug in place

The missing-bucket bug comes from Engine::list_one_set. It drops every walk_dir error, VolumeNotFound included, and returns Ok(Vec::new()) when quorum fails. So Engine::list says a missing bucket is an empty one. This PR works around that in the handler: every ListObjects page now runs stat_bucket, which calls stat_vol on all backends in the cluster, and then runs the walk.

Two costs follow:

  • Every paginated listing makes one extra round trip to every disk.
  • Any other Engine::list caller still gets an empty result for a missing bucket.

A smaller fix is to make list_one_set count VolumeNotFound results and return StorageError::BucketNotFound when they reach quorum, the same logic stat_bucket already uses. The only case that still needs an explicit stat_bucket is the max_keys == 0 short-circuit in list_page, and it can do that check before returning the empty page.

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No issues found

Reviewed commit 17bba24.

@ArnavBalyan
ArnavBalyan merged commit 8c65819 into openlake-project:main Sep 28, 2026
5 checks passed
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.

Return NoSuchBucket when listing a nonexistent bucket

2 participants