Make sure fs exceptions are raised if not MissingFs exceptions (clone)

docs/release.rst

@@ -49,6 +49,9 @@ Docs
 Maintenance
 ~~~~~~~~~~~
 
+* FSStore now raises rather than return bad data.
+  By :user:`Martin Durant <martindurant>` and :user:`Ian Carroll <itcarroll>` :issue:`1604`.
+
 * Cache result of ``FSStore._fsspec_installed()``.
   By :user:`Janick Martinez Esturo <ph03>` :issue:`1581`.
 

zarr/storage.py

@@ -1413,11 +1413,23 @@ def _normalize_key(self, key):
     def getitems(
         self, keys: Sequence[str], *, contexts: Mapping[str, Context]
     ) -> Mapping[str, Any]:
-        keys_transformed = [self._normalize_key(key) for key in keys]
-        results = self.map.getitems(keys_transformed, on_error="omit")
-        # The function calling this method may not recognize the transformed keys
-        # So we send the values returned by self.map.getitems back into the original key space.
-        return {keys[keys_transformed.index(rk)]: rv for rk, rv in results.items()}
+        keys_transformed = {self._normalize_key(key): key for key in keys}
+        results_transformed = self.map.getitems(list(keys_transformed), on_error="return")
+        results = {}
+        for k, v in results_transformed.items():
+            if isinstance(v, self.exceptions):
+                # Cause recognized exceptions to prompt a KeyError in the
+                # function calling this method
+                continue
+            elif isinstance(v, Exception):
+                # Raise any other exception
+                raise v
+            else:
+                # The function calling this method may not recognize the transformed
+                # keys, so we send the values returned by self.map.getitems back into
+                # the original key space.
+                results[keys_transformed[k]] = v
+        return results
 
     def __getitem__(self, key):
         key = self._normalize_key(key)

zarr/tests/test_storage.py

@@ -1098,6 +1098,12 @@ def mock_walker_no_slash(_path):
 
 @pytest.mark.skipif(have_fsspec is False, reason="needs fsspec")
 class TestFSStore(StoreTests):
+    @pytest.fixture
+    def memory_store(self):
+        store = FSStore("memory://test/out.zarr")
+        yield store
+        store.map.fs.store.clear()
+
     def create_store(self, normalize_keys=False, dimension_separator=".", path=None, **kwargs):
         if path is None:
             path = tempfile.mkdtemp()
@@ -1337,6 +1343,21 @@ def test_s3_complex(self):
         )
         assert (a[:] == -np.ones((8, 8, 8))).all()
 
+    def test_exceptions(self, memory_store):
+        path = memory_store.path
+        m = memory_store.fs
+        g = zarr.open_group(memory_store, mode="w")
+        arr = g.create_dataset("data", data=[1, 2, 3, 4], dtype="i4", compression=None, chunks=[2])
+        m.store[path + "/data/0"] = None
+        del m.store[path + "/data/1"]
+        assert g.store.getitems(["data/1"], contexts={}) == {}  # not found
+        with pytest.raises(Exception):
+            # None is bad data, as opposed to missing
+            g.store.getitems(["data/0", "data/1"], contexts={})
+        with pytest.raises(Exception):
+            # None is bad data, as opposed to missing
+            arr[:]
+
 
 @pytest.mark.skipif(have_fsspec is False, reason="needs fsspec")
 class TestFSStoreWithKeySeparator(StoreTests):