diff --git a/dspace_rest_client/client.py b/dspace_rest_client/client.py index 7049228..c298378 100644 --- a/dspace_rest_client/client.py +++ b/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']: diff --git a/tests/test_client_read.py b/tests/test_client_read.py index 2ddceb7..f48b04a 100644 --- a/tests/test_client_read.py +++ b/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