Merge pull request #19 from dataquest-dev/fix/explicit-fetch-errors

fix(client): raise informative error on non-404 fetch failure

Author: vidiecan

dspace_rest_client/client.py

@@ -733,12 +733,17 @@ def get_bundles(self, parent=None, uuid=None, page=0, size=20, sort=None):
         if sort is not None:
             params['sort'] = sort
         r_json = self.fetch_resource(url, params=params)
-        if r_json is None and getattr(self._last_err, 'status_code', None) == 404:
-            # the item (or bundle) no longer exists - a deleted item simply has
-            # no bundles, which is a clean empty result, not a crash. any other
-            # failure falls through and still surfaces to the caller.
-            _logger.info(f'No bundles: resource not found (404) [{url}]')
-            return bundles
+        if r_json is None:
+            status = getattr(self._last_err, 'status_code', None)
+            if status == 404:
+                # a deleted item (or bundle) simply has no bundles, which is a
+                # clean empty result, not a crash.
+                _logger.info(f'No bundles: resource not found (404) [{url}]')
+                return bundles
+            # any other failure surfaces with its status + url, not as an opaque
+            # 'NoneType is not subscriptable' further down, so the caller can
+            # see what failed and retry.
+            raise RuntimeError(f'Failed to fetch bundles: HTTP {status} [{url}]')
         try:
             if single_result:
                 bundles.append(Bundle(r_json))
@@ -800,13 +805,16 @@ def get_bitstreams(self, uuid=None, bundle=None, page=0, size=20, sort=None):
         if sort is not None:
             params['sort'] = sort
         r_json = self.fetch_resource(url, params=params)
-        if r_json is None and getattr(self._last_err, 'status_code', None) == 404:
-            # the bundle (or item) is gone - no bitstreams, a clean empty result
-            # rather than a crash. Mirrors get_bundles (#16). Any other failure
-            # (a transient 5xx, say) falls through and still surfaces to the
-            # caller so it is retried, not silently recorded as "no bitstreams".
-            _logger.info(f'No bitstreams: resource not found (404) [{url}]')
-            return list()
+        if r_json is None:
+            status = getattr(self._last_err, 'status_code', None)
+            if status == 404:
+                # the bundle (or item) is gone - no bitstreams, a clean empty
+                # result rather than a crash. Mirrors get_bundles.
+                _logger.info(f'No bitstreams: resource not found (404) [{url}]')
+                return list()
+            # a transient 5xx must NOT masquerade as "no bitstreams"; surface it
+            # with status + url so the caller can retry, not an opaque TypeError.
+            raise RuntimeError(f'Failed to fetch bitstreams: HTTP {status} [{url}]')
         bitstreams = list()
         if '_embedded' in r_json and 'bitstreams' in r_json['_embedded']:
             for bitstream_resource in r_json['_embedded']['bitstreams']:

tests/test_client_read.py

@@ -128,6 +128,18 @@ def test_deleted_item_404_returns_empty_list(self):
                   json={"timestamp": "2026-01-01"})
             self.assertEqual(c.get_bundles(parent=parent), [])
 
+    def test_non_404_error_raises_informative_error(self):
+        # a non-404 fetch failure surfaces with its status + url so the caller
+        # can retry, rather than an opaque 'NoneType is not subscriptable'.
+        c = make_client()
+        parent = Item(item_json(ITEM_UUID))
+        with requests_mock.Mocker() as m:
+            m.get(f"{API}/core/items/{ITEM_UUID}/bundles",
+                  status_code=500, text="boom")
+            with self.assertRaises(RuntimeError) as ctx:
+                c.get_bundles(parent=parent)
+            self.assertIn("500", str(ctx.exception))
+
     def test_no_args_returns_empty_without_request(self):
         c = make_client()
         with requests_mock.Mocker() as m:
@@ -181,16 +193,18 @@ def test_deleted_bundle_404_returns_empty_list(self):
                   status_code=404, json={"timestamp": "2026-01-01"})
             self.assertEqual(c.get_bitstreams(bundle=bundle), [])
 
-    def test_non_404_error_still_surfaces(self):
-        # a transient 5xx must NOT masquerade as "no bitstreams"; it surfaces so
-        # the caller can retry, exactly as get_bundles does for non-404 errors.
+    def test_non_404_error_raises_informative_error(self):
+        # a transient 5xx must NOT masquerade as "no bitstreams"; it surfaces
+        # with its status + url (not a bare TypeError) so the caller can retry.
         c = make_client()
         bundle = Bundle(bundle_json("bnd2"))
         with requests_mock.Mocker() as m:
             m.get(f"{API}/core/bundles/bnd2/bitstreams",
                   status_code=500, text="boom")
-            with self.assertRaises(Exception):
+            with self.assertRaises(RuntimeError) as ctx:
                 c.get_bitstreams(bundle=bundle)
+            self.assertIn("500", str(ctx.exception))
+            self.assertIn("/core/bundles/bnd2/bitstreams", str(ctx.exception))
 
     def test_200_without_bitstreams_returns_empty_list(self):
         # a well-formed response with no bitstreams -> [] (not None), so callers