diff --git a/dspace_rest_client/client.py b/dspace_rest_client/client.py index 061680f..259db32 100644 --- a/dspace_rest_client/client.py +++ b/dspace_rest_client/client.py @@ -17,6 +17,7 @@ import json import logging import os +from urllib.parse import urlparse from uuid import UUID import requests @@ -26,6 +27,9 @@ __all__ = ['DSpaceClient'] +# cap on the response body echoed into an error log line +_ERR_BODY_MAX = 500 + _logger = logging.getLogger("dspace.client") # A library must not configure the root logger - that is the consuming # application's job. Attach a NullHandler (once) so records are dropped unless @@ -522,7 +526,20 @@ def fetch_resource(self, url, params=None): # record the failing response so callers can tell a 404 (the # resource is gone) from a transient 5xx before we drop the body self._last_err = r - _logger.error(f'Error encountered fetching resource: {r.text}') + # keep the query: on search endpoints it holds the only resource id + u = urlparse(r.url) + path = f'{u.path}?{u.query}' if u.query else u.path + if r.status_code == 401: + _logger.warning( + f'DSpace returned 401 for [{path}] - authentication required ' + '(session expired or not logged in)') + elif r.status_code == 404: + _logger.warning( + f'DSpace returned 404 for [{path}] - resource not found (deleted?)') + else: + _logger.error( + f'Error encountered fetching resource: {r.status_code} for ' + f'[{path}]: {r.text[:_ERR_BODY_MAX]}') return None # ValueError / JSON handling moved to static method return parse_json(r) diff --git a/tests/test_client_read.py b/tests/test_client_read.py index f48b04a..ffa058e 100644 --- a/tests/test_client_read.py +++ b/tests/test_client_read.py @@ -337,6 +337,50 @@ def test_404_returns_none_and_records_last_err(self): self.assertIsNone(c.fetch_resource(url)) self.assertEqual(c.last_err.status_code, 404) + def test_401_logs_warning_with_path_not_body(self): + # expired session token or a client that never logged in + c = make_client() + url = f"{API}/core/items/{ITEM_UUID}/bundles" + with requests_mock.Mocker() as m: + m.get(url, status_code=401, + json={"status": 401, "message": "Authentication is required"}) + with self.assertLogs("dspace.client", level="WARNING") as logs: + self.assertIsNone(c.fetch_resource(url, params={"size": 10})) + self.assertEqual(c.last_err.status_code, 401) + self.assertEqual(logs.output, [ + "WARNING:dspace.client:DSpace returned 401 for " + f"[/server/api/core/items/{ITEM_UUID}/bundles?size=10] - " + "authentication required (session expired or not logged in)"]) + + def test_404_logs_warning_with_path_and_query_not_body(self): + # on search endpoints the query carries the only id of the resource + c = make_client() + url = f"{API}/authz/resourcepolicies/search/resource" + with requests_mock.Mocker() as m: + m.get(url, status_code=404, json={"timestamp": "2026-01-01"}) + with self.assertLogs("dspace.client", level="WARNING") as logs: + self.assertIsNone(c.fetch_resource(url, params={"uuid": ITEM_UUID})) + self.assertEqual(logs.output, [ + "WARNING:dspace.client:DSpace returned 404 for " + f"[/server/api/authz/resourcepolicies/search/resource?uuid={ITEM_UUID}]" + " - resource not found (deleted?)"]) + + def test_other_error_logs_error_with_truncated_body(self): + c = make_client() + url = f"{API}/core/items/{ITEM_UUID}/bundles" + with requests_mock.Mocker() as m: + m.get(url, status_code=500, text="boom" + "x" * 5000) + with self.assertLogs("dspace.client", level="WARNING") as logs: + self.assertIsNone(c.fetch_resource(url, params={"size": 10})) + self.assertEqual(c.last_err.status_code, 500) + self.assertEqual(len(logs.records), 1) + rec = logs.records[0] + self.assertEqual(rec.levelname, "ERROR") + msg = rec.getMessage() + self.assertIn("500", msg) + self.assertIn(f"[/server/api/core/items/{ITEM_UUID}/bundles?size=10]", msg) + self.assertIn("boom", msg) + self.assertLess(len(msg), 1000) if __name__ == "__main__": unittest.main()