Skip to content

Commit 092de58

Browse files
committed
fix(serverless): fail file() on HTTP error responses
file() saved whatever body came back, so a 4xx/5xx error page was written as the downloaded file and, when the URL ended in .zip, handed to the extractor. download_files_from_urls already called raise_for_status(); file() never did. Behavior change: file() now raises requests.RequestException on an error status instead of returning a dict pointing at the error page. It has no in-tree callers and is not re-exported from serverless.utils, so this is limited to direct importers.
1 parent 831a1f5 commit 092de58

2 files changed

Lines changed: 24 additions & 0 deletions

File tree

‎runpod/serverless/utils/rp_download.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ def file(file_url: str) -> dict:
134134
os.makedirs("job_files", exist_ok=True)
135135

136136
with safe_get(file_url, stream=True, timeout=30, headers=HEADERS) as download_response:
137+
# Fail on an error response rather than saving the error page as the
138+
# file; for a .zip that body would go straight to the extractor.
139+
download_response.raise_for_status()
140+
137141
content_disposition = download_response.headers.get("Content-Disposition")
138142

139143
original_file_name = ""

‎tests/test_serverless/test_utils/test_download.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,26 @@ def test_download_file_with_malformed_content_length(self, mock_file, mock_get):
234234
self.assertEqual(result["type"], "txt")
235235
mock_file().write.assert_called_once_with(b"file content")
236236

237+
@patch("runpod.serverless.utils.rp_download.safe_get")
238+
@patch("builtins.open", new_callable=mock_open)
239+
@patch("runpod.serverless.utils.rp_download.zipfile.ZipFile")
240+
def test_download_file_raises_on_http_error(self, mock_zip, mock_file, mock_get):
241+
"""
242+
An error response must not be saved as if it were the file: a 404 body
243+
written to a .zip would then be handed to the extractor.
244+
"""
245+
mock_get.return_value = MockResponse(
246+
b"<html>404 Not Found</html>",
247+
404,
248+
{"Content-Disposition": "filename=test_file.zip"},
249+
)
250+
251+
with self.assertRaises(RequestException):
252+
file("http://test.com/test_file.zip")
253+
254+
mock_file().write.assert_not_called()
255+
mock_zip.assert_not_called() # and the error page never reaches the extractor
256+
237257
@patch("runpod.serverless.utils.rp_download.safe_get")
238258
@patch("builtins.open", new_callable=mock_open)
239259
@patch("runpod.serverless.utils.rp_download.zipfile.ZipFile")

0 commit comments

Comments
 (0)