diff --git a/crates/ecstore/src/error/mod.rs b/crates/ecstore/src/error/mod.rs index c29adecea..6dbe8e02c 100644 --- a/crates/ecstore/src/error/mod.rs +++ b/crates/ecstore/src/error/mod.rs @@ -890,10 +890,23 @@ pub fn is_err_io(err: &Error) -> bool { matches!(err, &StorageError::Io(_)) } +/// Strict "not found" predicate that only matches genuine object/version/volume +/// absence errors: `FileNotFound`/`VolumeNotFound`/`FileVersionNotFound`/ +/// `ObjectNotFound`/`VersionNotFound`. +/// +/// Unlike [`is_err_bucket_not_found`], this deliberately excludes +/// `DiskNotFound` so that an all-drives-unreachable error slice is treated as an +/// availability/read-quorum failure rather than "the object does not exist". +/// This mirrors MinIO's `isAllNotFound` and the sibling +/// [`crate::disk::error::DiskError::is_all_not_found`]. +pub fn is_err_strict_not_found(err: &Error) -> bool { + is_err_object_not_found(err) || is_err_version_not_found(err) || matches!(err, &StorageError::VolumeNotFound) +} + pub fn is_all_not_found(errs: &[Option]) -> bool { for err in errs.iter() { if let Some(err) = err { - if is_err_object_not_found(err) || is_err_version_not_found(err) || is_err_bucket_not_found(err) { + if is_err_strict_not_found(err) { continue; } @@ -905,10 +918,22 @@ pub fn is_all_not_found(errs: &[Option]) -> bool { !errs.is_empty() } +/// Strict "volume not found" predicate matching only `VolumeNotFound`/ +/// `BucketNotFound`. +/// +/// This is [`is_err_bucket_not_found`] minus `DiskNotFound`: an unreachable +/// drive must not be mistaken for a missing volume/bucket. Unlike +/// [`is_err_strict_not_found`] it deliberately excludes file/version level +/// not-found so that "the object is missing under an existing volume" is not +/// escalated to "the volume itself is missing". +pub fn is_err_strict_volume_not_found(err: &Error) -> bool { + matches!(err, &StorageError::VolumeNotFound) || matches!(err, &StorageError::BucketNotFound(_)) +} + pub fn is_all_volume_not_found(errs: &[Option]) -> bool { for err in errs.iter() { if let Some(err) = err { - if is_err_bucket_not_found(err) { + if is_err_strict_volume_not_found(err) { continue; } @@ -921,6 +946,22 @@ pub fn is_all_volume_not_found(errs: &[Option]) -> bool { !errs.is_empty() } +/// Returns true when every entry is `DiskNotFound` (all drives offline / +/// drive-id mismatch across all sets) and the slice is non-empty. +/// +/// Used to distinguish a full-availability failure from a genuine empty listing +/// so it can be surfaced as an error instead of being swallowed as "no entries". +pub fn is_all_disk_not_found(errs: &[Option]) -> bool { + for err in errs.iter() { + match err { + Some(err) if matches!(err, &StorageError::DiskNotFound) => continue, + _ => return false, + } + } + + !errs.is_empty() +} + pub fn to_object_err(err: Error, params: Vec<&str>) -> Error { match err { StorageError::DiskFull => StorageError::StorageFull, @@ -1135,6 +1176,75 @@ mod tests { use super::*; use std::io::{Error as IoError, ErrorKind}; + // Regression for #952 (ECA-11): an all-`DiskNotFound` slice (every drive in + // every set unreachable) must NOT be classified as "all not found", + // otherwise ListObjects silently returns an empty listing and masks a full + // availability failure as "the bucket is empty". + #[test] + fn is_all_not_found_rejects_all_disk_not_found() { + let errs = vec![Some(StorageError::DiskNotFound), Some(StorageError::DiskNotFound)]; + assert!(!is_all_not_found(&errs), "all-DiskNotFound must not be treated as all-not-found"); + assert!( + !is_all_volume_not_found(&errs), + "all-DiskNotFound must not be treated as all-volume-not-found" + ); + assert!( + is_all_disk_not_found(&errs), + "all-DiskNotFound must be recognised as a full availability failure" + ); + } + + // Genuine not-found errors must still be classified as such so real + // "object/version absent" semantics do not regress into availability errors. + #[test] + fn is_all_not_found_accepts_genuine_not_found() { + let object_errs = vec![ + Some(StorageError::FileNotFound), + Some(StorageError::ObjectNotFound("bucket".into(), "object".into())), + Some(StorageError::FileVersionNotFound), + Some(StorageError::VersionNotFound("bucket".into(), "object".into(), "vid".into())), + Some(StorageError::VolumeNotFound), + ]; + assert!( + is_all_not_found(&object_errs), + "genuine object/version/volume not-found must stay all-not-found" + ); + assert!(!is_all_disk_not_found(&object_errs)); + + let volume_errs = vec![Some(StorageError::VolumeNotFound), Some(StorageError::VolumeNotFound)]; + assert!(is_all_not_found(&volume_errs)); + assert!( + is_all_volume_not_found(&volume_errs), + "all-VolumeNotFound must remain all-volume-not-found" + ); + } + + // `is_all_volume_not_found` must escalate only volume/bucket absence, not a + // missing object under an existing volume, and must exclude DiskNotFound. + #[test] + fn is_all_volume_not_found_semantics() { + assert!(is_all_volume_not_found(&[Some(StorageError::BucketNotFound("bucket".into()))])); + assert!( + !is_all_volume_not_found(&[Some(StorageError::FileNotFound), Some(StorageError::VolumeNotFound)]), + "a missing object under an existing volume must not escalate to all-volume-not-found" + ); + assert!(!is_all_volume_not_found(&[Some(StorageError::DiskNotFound)])); + } + + // A `None` entry (a healthy set) short-circuits both predicates to false so a + // partial outage is never mistaken for an all-not-found / all-offline slice. + #[test] + fn not_found_predicates_reject_partial_and_empty() { + let partial = vec![None, Some(StorageError::DiskNotFound)]; + assert!(!is_all_not_found(&partial)); + assert!(!is_all_volume_not_found(&partial)); + assert!(!is_all_disk_not_found(&partial)); + + assert!(!is_all_not_found(&[])); + assert!(!is_all_volume_not_found(&[])); + assert!(!is_all_disk_not_found(&[])); + } + #[test] fn test_storage_error_to_u32() { // Test Io error uses 0x01 diff --git a/crates/ecstore/src/store/list_objects.rs b/crates/ecstore/src/store/list_objects.rs index d6937c777..bf2994638 100644 --- a/crates/ecstore/src/store/list_objects.rs +++ b/crates/ecstore/src/store/list_objects.rs @@ -20,7 +20,8 @@ use crate::core::sets::Sets; use crate::disk::error::DiskError; use crate::disk::{DiskAPI, DiskInfo, DiskStore, RUSTFS_META_BUCKET, WalkDirOptions}; use crate::error::{ - Error, Result, StorageError, is_all_not_found, is_all_volume_not_found, is_err_bucket_not_found, to_object_err, + Error, Result, StorageError, is_all_disk_not_found, is_all_not_found, is_all_volume_not_found, is_err_bucket_not_found, + to_object_err, }; use crate::object_api::{ObjectInfo, ObjectOptions}; use crate::set_disk::SetDisks; @@ -106,6 +107,14 @@ fn walk_result_from_set_errors(errs: &[Option]) -> Result<()> { return Ok(()); } + // Every set is unreachable (all drives offline / drive-id mismatch, with no + // healthy set contributing entries): this is an availability failure, not an + // empty listing. Surface it instead of silently swallowing DiskNotFound in + // the not-found loop below. + if is_all_disk_not_found(errs) { + return Err(StorageError::DiskNotFound); + } + for err in errs.iter().flatten() { if err == &Error::Unexpected || err.is_not_found() { continue; @@ -8842,6 +8851,24 @@ mod test { .expect("successful sets and unexpected EOF-style markers should not fail the walk"); } + // Regression for #952 (ECA-11): when every set is unreachable the walk must + // surface an availability error, not silently succeed with an empty listing. + #[test] + fn walk_result_from_set_errors_surfaces_full_offline() { + let err = walk_result_from_set_errors(&[Some(StorageError::DiskNotFound), Some(StorageError::DiskNotFound)]) + .expect_err("all sets unreachable must surface an availability error, not an empty listing"); + + assert_eq!(err, StorageError::DiskNotFound); + } + + // A partial outage (some set healthy) must still be tolerated as before: the + // healthy set's success short-circuits the all-offline detection. + #[test] + fn walk_result_from_set_errors_tolerates_partial_offline() { + walk_result_from_set_errors(&[None, Some(StorageError::DiskNotFound)]) + .expect("a partial outage with a healthy set must not fail the walk"); + } + // use std::sync::Arc; // use crate::cache_value::metacache_set::list_path_raw;