Honor strict=False when a collection's own load fails

.github/workflows/contraqctor.yml

@@ -28,9 +28,9 @@ jobs:
         python-version: [ 3.11, 3.12, 3.13 ]
       fail-fast: false
     steps:
-      - uses: actions/checkout@v6
+      - uses: actions/checkout@v7
 
-      - uses: astral-sh/setup-uv@v8.1.0
+      - uses: astral-sh/setup-uv@v8.2.0
         with:
           enable-cache: true
 
@@ -81,12 +81,12 @@ jobs:
       version: ${{ steps.get_version.outputs.version }}
 
     steps:
-      - uses: actions/checkout@v6
+      - uses: actions/checkout@v7
         with:
           fetch-depth: 0
           ref: main
 
-      - uses: astral-sh/setup-uv@v8.1.0
+      - uses: astral-sh/setup-uv@v8.2.0
         with:
           enable-cache: true
 
@@ -108,7 +108,7 @@ jobs:
         run: uv build
 
       - name: Upload wheels as artifact
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v7
         with:
           name: dist
           path: dist/
@@ -133,12 +133,12 @@ jobs:
     needs: prepare-release
     steps:
       - name: Download wheels artifact
-        uses: actions/download-artifact@v4
+        uses: actions/download-artifact@v8.0.1
         with:
           name: dist
           path: dist/
 
-      - uses: astral-sh/setup-uv@v8.1.0
+      - uses: astral-sh/setup-uv@v8.2.0
         with:
           enable-cache: true
 
@@ -165,12 +165,12 @@ jobs:
     if: github.event_name == 'release' && !github.event.release.prerelease
     steps:
       - name: Checkout
-        uses: actions/checkout@v6
+        uses: actions/checkout@v7
         with:
           ref: main
 
       - name: Install uv
-        uses: astral-sh/setup-uv@v8.1.0
+        uses: astral-sh/setup-uv@v8.2.0
         with:
           enable-cache: true
 

src/contraqctor/contract/base.py

@@ -386,7 +386,10 @@ def collect_errors(self) -> List[_typing.ErrorOnLoad]:
         """
         errors = []
         if self.has_error:
+            # A stream that errored on load has no valid data to traverse into,
+            # so we report its own error and stop here (iterating would re-raise).
             errors.append(cast(_typing.ErrorOnLoad, self._data))
+            return errors
         for stream in self:
             if stream is None:
                 continue
@@ -418,6 +421,13 @@ def load_all(self, strict: bool = False) -> Self:
             ```
         """
         self.load()
+        if self.has_error:
+            if strict:
+                cast(_typing.ErrorOnLoad, self._data).raise_from_error()
+            # In non-strict mode the collection failed to load its own children,
+            # so there is nothing further to recurse into; the error is preserved
+            # and surfaced via collect_errors().
+            return self
         for stream in self:
             if stream is None:
                 continue
@@ -539,16 +549,21 @@ def load(self) -> Self:
 
         Overrides the base method to add validation that loaded data is a list of DataStreams.
 
+        If the underlying read raised, the error is preserved as an ``ErrorOnLoad`` (as in the
+        base implementation) rather than re-raised, so that non-strict loading can collect it.
+        A non-list result from a *successful* read is a genuine contract violation and is captured
+        as an ``ErrorOnLoad`` as well.
+
         Returns:
             Self: The collection instance for method chaining.
-
-        Raises:
-            ValueError: If loaded data is not a list of DataStreams.
         """
         super().load()
+        if self.has_error:
+            # read() raised and was captured as an ErrorOnLoad; leave it in place.
+            return self
         if not isinstance(self._data, list):
-            self._data = _typing.UnsetData
-            raise ValueError("Data must be a list of DataStreams.")
+            self._data = _typing.ErrorOnLoad(self, exception=ValueError("Data must be a list of DataStreams."))
+            return self
         self._update_data_stream_mapping()
         return self
 

tests/test_contract/test_core.py

@@ -2,7 +2,24 @@
 from conftest import SimpleDataStream, SimpleParams
 
 from contraqctor import _typing
-from contraqctor.contract.base import DataStream, DataStreamCollection, implicit_loading
+from contraqctor.contract.base import (
+    DataStream,
+    DataStreamCollection,
+    DataStreamCollectionBase,
+    implicit_loading,
+)
+
+
+class _FailingCollection(DataStreamCollectionBase[DataStream, SimpleParams]):
+    """A collection whose own reader raises, mimicking e.g. a HarpDevice pointed at a missing folder."""
+
+    @staticmethod
+    def _reader(params: SimpleParams) -> list:
+        with open(params.path, "r") as f:
+            f.read()
+        return []
+
+    make_params = SimpleParams
 
 
 class TestDataStream:
@@ -435,3 +452,53 @@ def test_load_all_strict(self, text_file, temp_dir):
 
         with pytest.raises(FileNotFoundError):
             collection.load_all(strict=True)
+
+    def test_load_all_non_strict_with_failing_collection(self, text_file, temp_dir):
+        """Non-strict load_all should not raise when a collection's own read() fails."""
+        working = SimpleDataStream(name="working", reader_params=SimpleParams(path=text_file))
+
+        nonexistent_path = temp_dir / "nonexistent_folder" / "device.yml"
+        failing_collection = _FailingCollection(name="failing", reader_params=SimpleParams(path=nonexistent_path))
+
+        root = DataStreamCollection(name="root", data_streams=[working, failing_collection])
+
+        # Should not raise, even though the sub-collection's own read() fails.
+        result = root.load_all(strict=False)
+
+        errors = result.collect_errors()
+        assert len(errors) == 1
+        assert errors[0].data_stream == failing_collection
+        assert isinstance(errors[0].exception, FileNotFoundError)
+
+        assert failing_collection.has_error
+        assert root.at("working").has_data
+
+    def test_load_all_strict_with_failing_collection(self, temp_dir):
+        """Strict load_all should raise the original error when a collection's own read() fails."""
+        nonexistent_path = temp_dir / "nonexistent_folder" / "device.yml"
+        failing_collection = _FailingCollection(name="failing", reader_params=SimpleParams(path=nonexistent_path))
+
+        root = DataStreamCollection(name="root", data_streams=[failing_collection])
+
+        with pytest.raises(FileNotFoundError):
+            root.load_all(strict=True)
+
+    def test_load_collection_non_list_result_is_captured(self, temp_dir):
+        """A successful read returning a non-list is captured as an ErrorOnLoad, not raised."""
+        bad_file = temp_dir / "bad.txt"
+        bad_file.write_text("not a list")
+
+        class _BadResultCollection(DataStreamCollectionBase[DataStream, SimpleParams]):
+            @staticmethod
+            def _reader(params: SimpleParams) -> list:
+                with open(params.path, "r") as f:
+                    return f.read()  # returns a str, not a list
+
+            make_params = SimpleParams
+
+        collection = _BadResultCollection(name="bad", reader_params=SimpleParams(path=bad_file))
+
+        collection.load()
+        assert collection.has_error
+        with pytest.raises(ValueError, match="Data must be a list of DataStreams."):
+            collection.collect_errors()[0].raise_from_error()

tests/test_contract/test_mux.py

@@ -61,9 +61,10 @@ def test_multiple_paths(self, temp_dir):  # grab temp_dir from conftest
             ),
         )
 
+        mux.load()
+        assert mux.has_error
         with pytest.raises(ValueError):
-            # Should throw since we have duplicate file names
-            mux.load()
+            _ = mux.data
 
     def test_include_exclude_patterns(self, temp_dir):
         """Test include and exclude patterns."""