From ef480746daa4e6876caf8e0f2113d0123439dc43 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 18:36:35 -0500 Subject: [PATCH] clawhdf5-remote: a failed first fetch is Error::Remote cached() mapped an error of its open-time prefetch (a network error, a changed file) to Error::Hdf5(Format(Storage)), misclassifying it for callers that match on the variant. It is now Error::Remote (RemoteError::Backend with the backend's message). open_object and the s3/gs/az URLs fetch the first block of an ObjectStoreStorage directly, so their errors keep their kind (FileChanged, ObjectStore). Test: cached() over a backend whose reads fail gives Error::Remote(Backend) (Error::Hdf5 before). Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-remote/src/lib.rs | 24 +++++++++++++++++++----- crates/clawhdf5-remote/src/object.rs | 5 +++++ crates/clawhdf5-remote/tests/http.rs | 24 ++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/crates/clawhdf5-remote/src/lib.rs b/crates/clawhdf5-remote/src/lib.rs index cedbfe9..2835440 100644 --- a/crates/clawhdf5-remote/src/lib.rs +++ b/crates/clawhdf5-remote/src/lib.rs @@ -126,7 +126,7 @@ fn http_storage(url: &str, _options: &Options) -> Result, Err fn cloud_storage(url: &str, _scheme: &str, options: &Options) -> Result, Error> { let (store, path) = object::store_for_url(url)?; let storage = ObjectStoreStorage::new(store, path)?; - Ok(Arc::new(cached(Box::new(storage), options)?)) + Ok(Arc::new(object_cached(storage, options)?)) } #[cfg(not(any(feature = "s3", feature = "gcs", feature = "azure")))] @@ -144,13 +144,27 @@ fn cloud_storage(url: &str, scheme: &str, _options: &Options) -> Result Result { let cache = BlockCache::new(backend, options.cache.clone()); let first = cache.config().block_size; cache .prefetch(0, first) - .map_err(|e| Error::Hdf5(clawhdf5::Error::Format(e)))?; + .map_err(|e| RemoteError::Backend(e.to_string()))?; + Ok(cache) +} + +/// [`cached`] for an [`ObjectStoreStorage`]: its first block is fetched +/// directly, so a failure keeps its kind (`FileChanged`, `ObjectStore`). +#[cfg(feature = "object-store")] +fn object_cached(storage: ObjectStoreStorage, options: &Options) -> Result { + // The block size BlockCache::new will use. + let block = options.cache.block_size.max(512); + let first = storage.fetch_first(block)?; + let cache = BlockCache::new(Box::new(storage) as Backend, options.cache.clone()); + cache.insert(0, &first); Ok(cache) } @@ -205,8 +219,8 @@ pub fn open_object( ) -> Result<(File, Arc), Error> { let path = object_store::path::Path::parse(path) .map_err(|e| RemoteError::InvalidUrl(format!("{path}: {e}")))?; - let storage = Arc::new(cached( - Box::new(ObjectStoreStorage::new(store, path)?), + let storage = Arc::new(object_cached( + ObjectStoreStorage::new(store, path)?, options, )?); let file = File::open_storage(storage.clone())?; diff --git a/crates/clawhdf5-remote/src/object.rs b/crates/clawhdf5-remote/src/object.rs index ecfb7ac..c0b7748 100644 --- a/crates/clawhdf5-remote/src/object.rs +++ b/crates/clawhdf5-remote/src/object.rs @@ -138,6 +138,11 @@ impl ObjectStoreStorage { o } + /// The object's first `n` bytes (fewer if it is shorter). + pub(crate) fn fetch_first(&self, n: u64) -> Result, RemoteError> { + Ok(self.fetch_all(&[0..n])?.pop().unwrap_or_default()) + } + fn fetch_all(&self, ranges: &[Range]) -> Result>, RemoteError> { let len = self.meta.size; let jobs: Vec<(usize, Range)> = ranges diff --git a/crates/clawhdf5-remote/tests/http.rs b/crates/clawhdf5-remote/tests/http.rs index 2bf6da1..a63fb72 100644 --- a/crates/clawhdf5-remote/tests/http.rs +++ b/crates/clawhdf5-remote/tests/http.rs @@ -834,3 +834,27 @@ fn slow_links_read_and_stalled_ones_fail() { assert!(matches!(e, Error::Remote(RemoteError::Transport(_))), "{e}"); assert!(t.elapsed() < Duration::from_secs(5), "{:?}", t.elapsed()); } + +/// `cached` reports a failed first fetch as a remote error, not as an +/// HDF5 format error. +#[test] +fn cached_reports_a_failed_prefetch_as_remote() { + use clawhdf5_format::error::FormatError; + use std::borrow::Cow; + struct Down; + impl clawhdf5_format::storage::Storage for Down { + fn read_at(&self, _: u64, _: usize) -> Result, FormatError> { + Err(RemoteError::Transport("connection refused".into()).into()) + } + fn len(&self) -> u64 { + 1 << 20 + } + } + let e = clawhdf5_remote::cached(Box::new(Down), &Options::default()) + .map(|_| ()) + .unwrap_err(); + assert!( + matches!(&e, Error::Remote(RemoteError::Backend(m)) if m.contains("connection refused")), + "{e:?}" + ); +}