Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions dspace_rest_client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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']:
Expand Down
22 changes: 18 additions & 4 deletions tests/test_client_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down