diff --git a/src/fwl_io/mirror.py b/src/fwl_io/mirror.py index b244dd4..3079d56 100644 --- a/src/fwl_io/mirror.py +++ b/src/fwl_io/mirror.py @@ -151,20 +151,32 @@ def _request(self, method: str, path: str, **kwargs) -> dict: timeout=self.timeout, **kwargs, ) - if not response.ok: - raise DataverseError( - f'Dataverse {method} {path} failed ({response.status_code}): ' - f'{response.text[:500]}' - ) - return response.json() except requests.RequestException as exc: - # A transport failure (connection error, timeout, DNS) or an - # unparseable response body (JSONDecodeError is a requests error) is - # a failed native-API request too; surface it as a DataverseError so - # every Dataverse-side failure is one error type for callers to catch. - # The not-ok DataverseError above is not a RequestException, so it - # passes through this handler unchanged. + # A transport failure (connection error, timeout, DNS) is a failed + # native-API request too; surface it as a DataverseError so every + # Dataverse-side failure is one error type for callers to catch. raise DataverseError(f'Dataverse {method} {path} failed: {exc}') from exc + if not response.ok: + raise DataverseError( + f'Dataverse {method} {path} failed ({response.status_code}): {response.text[:500]}' + ) + if not response.content: + # A 2xx with an empty body (routine for a DELETE) is a success + # with nothing to parse. + return {} + try: + body = response.json() + except ValueError: + raise DataverseError( + f'Dataverse {method} {path} returned {response.status_code} with a ' + f'non-JSON body: {response.text[:500]}' + ) from None + if not isinstance(body, dict): + raise DataverseError( + f'Dataverse {method} {path} returned {response.status_code} with a ' + f'non-object JSON body: {response.text[:500]}' + ) + return body def _post(self, path: str, **kwargs) -> dict: return self._request('POST', path, **kwargs) @@ -294,9 +306,10 @@ def mirror_to_dataverse( DataverseError If a Dataverse native-API request fails: the server rejects it (for example an unknown subject in the citation metadata), the HTTP transport - fails (connection error or timeout), or the response body is unparseable. - A failure during upload or publish can leave a draft that the rollback - then tries to delete. + fails (connection error or timeout), or a 2xx response body is not a + JSON object (a non-empty body that fails to parse, or that parses to + something other than a JSON object). A failure during upload or publish + can leave a draft that the rollback then tries to delete. DownloadError If a Zenodo file fails its checksum or cannot be downloaded; raised by the fetcher (``fwl_io.fetch``) before any Dataverse write. diff --git a/tests/test_mirror.py b/tests/test_mirror.py index 9cc5362..dc4e0a7 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -352,27 +352,162 @@ def boom(*args, **kwargs): requests.request = orig +def _fake_response(status_code: int, content: bytes): + import requests + + response = requests.Response() + response.status_code = status_code + response._content = content + return response + + @pytest.mark.unit -def test_unparseable_success_body_becomes_a_dataverse_error(): - """A 2xx response with a non-JSON body is wrapped as DataverseError too. +def test_create_with_an_empty_success_body_raises_for_the_missing_persistent_id(): + """A 2xx create response with an empty body still fails, for the missing id.""" + import requests - JSONDecodeError is a requests error, so decoding a garbled success body would - otherwise leak a bare requests exception from a Dataverse call; every - Dataverse-side failure must surface as the one DataverseError type. + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'') + try: + with pytest.raises(DataverseError, match='no persistentId'): + client.create_dataset('coll', {'datasetVersion': {}}) + finally: + requests.request = orig + + +@pytest.mark.unit +def test_create_with_a_non_json_success_body_raises_with_the_status_and_body(): + """A 2xx create response with a non-JSON body raises, naming the status and body. + + This is the exact shape a live Dataverse.nl call returned: a 2xx status with + a body that failed to parse as JSON. The status code and raw body must be in + the exception message, not just discoverable by decoding the JSON error text. """ import requests - response = requests.Response() - response.status_code = 200 # ok, so it reaches the body decode - response._content = b'this is not json' client = DataverseClient('http://unused', 'tok') orig = requests.request - requests.request = lambda *args, **kwargs: response + requests.request = lambda *args, **kwargs: _fake_response(200, b'this is not json') try: - with pytest.raises(DataverseError, match='failed') as exc_info: + with pytest.raises(DataverseError, match='this is not json') as exc_info: client.create_dataset('coll', {'datasetVersion': {}}) - # The decode error is a requests error, chained as the cause. - assert isinstance(exc_info.value.__cause__, requests.RequestException) + assert '200' in str(exc_info.value) + finally: + requests.request = orig + + +@pytest.mark.unit +def test_create_with_a_non_object_json_body_raises_with_the_status_and_body(): + """A 2xx create response whose body parses to a non-object (e.g. null) raises. + + ``response.json()`` succeeds here, so this must be checked separately from + the decode failure above; a caller must never see a bare AttributeError + from treating a non-dict body as a dict. + """ + import requests + + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'null') + try: + with pytest.raises(DataverseError, match='non-object JSON body') as exc_info: + client.create_dataset('coll', {'datasetVersion': {}}) + assert '200' in str(exc_info.value) + finally: + requests.request = orig + + +@pytest.mark.unit +def test_add_file_accepts_an_empty_success_body(tmp_path): + """add_file does not raise when Dataverse returns 2xx with an empty body.""" + import requests + + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'') + target = tmp_path / 'f.dat' + target.write_bytes(b'data') + try: + client.add_file('doi:10.34894/DEMO01', target) # must not raise + finally: + requests.request = orig + + +@pytest.mark.unit +def test_add_file_raises_with_the_status_and_body_on_a_non_json_success_body(tmp_path): + """add_file raises on a 2xx non-JSON body, naming the status and body.""" + import requests + + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'this is not json') + target = tmp_path / 'f.dat' + target.write_bytes(b'data') + try: + with pytest.raises(DataverseError, match='this is not json') as exc_info: + client.add_file('doi:10.34894/DEMO01', target) + assert '200' in str(exc_info.value) + finally: + requests.request = orig + + +@pytest.mark.unit +def test_publish_accepts_an_empty_success_body(): + """publish does not raise when Dataverse returns 2xx with an empty body.""" + import requests + + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'') + try: + client.publish('doi:10.34894/DEMO01') # must not raise + finally: + requests.request = orig + + +@pytest.mark.unit +def test_publish_raises_with_the_status_and_body_on_a_non_json_success_body(): + """publish raises on a 2xx non-JSON body, naming the status and body.""" + import requests + + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'this is not json') + try: + with pytest.raises(DataverseError, match='this is not json') as exc_info: + client.publish('doi:10.34894/DEMO01') + assert '200' in str(exc_info.value) + finally: + requests.request = orig + + +@pytest.mark.unit +def test_delete_draft_accepts_an_empty_success_body(): + """delete_draft (the rollback path) does not raise when Dataverse returns an empty 2xx.""" + import requests + + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'') + try: + client.delete_draft('doi:10.34894/DEMO01') # must not raise + finally: + requests.request = orig + + +@pytest.mark.unit +def test_delete_draft_raises_with_the_status_and_body_on_a_non_json_success_body(): + """delete_draft (the rollback path) raises on a 2xx non-JSON body, naming status and body.""" + import requests + + client = DataverseClient('http://unused', 'tok') + orig = requests.request + requests.request = lambda *args, **kwargs: _fake_response(200, b'this is not json') + try: + with pytest.raises(DataverseError, match='this is not json') as exc_info: + client.delete_draft('doi:10.34894/DEMO01') + assert '200' in str(exc_info.value) finally: requests.request = orig