From 9f457c03743fde70f65be82d7d62e639ce7bcfb5 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 15 May 2026 01:23:55 -0700 Subject: [PATCH 1/4] raise when presigned URL response is missing or malformed --- .../pytexera/storage/dataset_file_document.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/amber/src/main/python/pytexera/storage/dataset_file_document.py b/amber/src/main/python/pytexera/storage/dataset_file_document.py index 3d077735833..98956cdbfd7 100644 --- a/amber/src/main/python/pytexera/storage/dataset_file_document.py +++ b/amber/src/main/python/pytexera/storage/dataset_file_document.py @@ -76,7 +76,22 @@ def get_presigned_url(self) -> str: f"Failed to get presigned URL: {response.status_code} {response.text}" ) - return response.json().get("presignedUrl") + try: + payload = response.json() + except ValueError as e: + raise RuntimeError( + f"Failed to get presigned URL: invalid JSON response: " + f"{response.text}" + ) from e + + presigned_url = payload.get("presignedUrl") + if not isinstance(presigned_url, str) or not presigned_url: + raise RuntimeError( + f"Failed to get presigned URL: 'presignedUrl' missing from " + f"response: {response.text}" + ) + + return presigned_url def read_file(self) -> io.BytesIO: """ From 00e77e41f31c68657c9f8c453096a3e21e9921a7 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 15 May 2026 01:53:29 -0700 Subject: [PATCH 2/4] ran lint --- .../src/main/python/pytexera/storage/dataset_file_document.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/amber/src/main/python/pytexera/storage/dataset_file_document.py b/amber/src/main/python/pytexera/storage/dataset_file_document.py index 98956cdbfd7..31f95d3fc72 100644 --- a/amber/src/main/python/pytexera/storage/dataset_file_document.py +++ b/amber/src/main/python/pytexera/storage/dataset_file_document.py @@ -80,8 +80,7 @@ def get_presigned_url(self) -> str: payload = response.json() except ValueError as e: raise RuntimeError( - f"Failed to get presigned URL: invalid JSON response: " - f"{response.text}" + f"Failed to get presigned URL: invalid JSON response: {response.text}" ) from e presigned_url = payload.get("presignedUrl") From 44b386caeecb3abb954b970bc312e194aa72e917 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 15 May 2026 02:45:07 -0700 Subject: [PATCH 3/4] fixed old test case --- .../python/pytexera/storage/test_dataset_file_document.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/amber/src/test/python/pytexera/storage/test_dataset_file_document.py b/amber/src/test/python/pytexera/storage/test_dataset_file_document.py index ecf9dd5b8ca..9d48702b235 100644 --- a/amber/src/test/python/pytexera/storage/test_dataset_file_document.py +++ b/amber/src/test/python/pytexera/storage/test_dataset_file_document.py @@ -137,13 +137,12 @@ def test_raises_runtime_error_with_status_and_body_on_failure(self, monkeypatch) with pytest.raises(RuntimeError, match=r"403.*forbidden"): doc.get_presigned_url() - def test_returns_none_when_response_body_lacks_presigned_url_key(self, monkeypatch): - # Pins current behavior: a 200 with no "presignedUrl" key yields None - # rather than raising. read_file() will then call requests.get(None). + def test_raises_when_response_body_lacks_presigned_url_key(self, monkeypatch): doc = self._make_doc(monkeypatch) with patch("pytexera.storage.dataset_file_document.requests.get") as mock_get: mock_get.return_value = make_response(200, body={"other": "value"}) - assert doc.get_presigned_url() is None + with pytest.raises(RuntimeError, match="'presignedUrl' missing"): + doc.get_presigned_url() class TestReadFile: From 032e0069b1dc46de9aa23bfa38604f360248a2ed Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 15 May 2026 05:56:56 -0700 Subject: [PATCH 4/4] Added test --- .../storage/test_dataset_file_document.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/amber/src/test/python/pytexera/storage/test_dataset_file_document.py b/amber/src/test/python/pytexera/storage/test_dataset_file_document.py index 9d48702b235..680f512072b 100644 --- a/amber/src/test/python/pytexera/storage/test_dataset_file_document.py +++ b/amber/src/test/python/pytexera/storage/test_dataset_file_document.py @@ -144,6 +144,31 @@ def test_raises_when_response_body_lacks_presigned_url_key(self, monkeypatch): with pytest.raises(RuntimeError, match="'presignedUrl' missing"): doc.get_presigned_url() + def test_raises_when_response_body_is_not_valid_json(self, monkeypatch): + doc = self._make_doc(monkeypatch) + with patch("pytexera.storage.dataset_file_document.requests.get") as mock_get: + response = MagicMock() + response.status_code = 200 + response.json.side_effect = ValueError("Expecting value") + response.text = "not json" + mock_get.return_value = response + with pytest.raises(RuntimeError, match="invalid JSON response"): + doc.get_presigned_url() + + def test_raises_when_presigned_url_is_empty_string(self, monkeypatch): + doc = self._make_doc(monkeypatch) + with patch("pytexera.storage.dataset_file_document.requests.get") as mock_get: + mock_get.return_value = make_response(200, body={"presignedUrl": ""}) + with pytest.raises(RuntimeError, match="'presignedUrl' missing"): + doc.get_presigned_url() + + def test_raises_when_presigned_url_is_not_a_string(self, monkeypatch): + doc = self._make_doc(monkeypatch) + with patch("pytexera.storage.dataset_file_document.requests.get") as mock_get: + mock_get.return_value = make_response(200, body={"presignedUrl": None}) + with pytest.raises(RuntimeError, match="'presignedUrl' missing"): + doc.get_presigned_url() + class TestReadFile: def _make_doc(self, monkeypatch):