From f68a51ba7c9479787910d0bdf0dd690163e132e9 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Sun, 13 Sep 2026 00:05:47 +0200 Subject: [PATCH 1/5] Fix file transfers ignoring connection 'max_retries' Uploads and downloads used 'get_default_max_retries()' (env variable or class default) instead of the connection's 'max_retries', and did not ensure at least one attempt. With 'AYON_SERVER_RETRIES=0' an upload crashed with AttributeError on 'None' response and a download silently did not download anything. Co-Authored-By: Claude Opus 5 --- ayon_api/server_api.py | 4 +-- tests/fake_transfer.py | 41 ++++++++++++++++++++++++ tests/test_transfer_max_retries.py | 50 ++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 tests/fake_transfer.py create mode 100644 tests/test_transfer_max_retries.py diff --git a/ayon_api/server_api.py b/ayon_api/server_api.py index e42add68c..2fce18fc0 100644 --- a/ayon_api/server_api.py +++ b/ayon_api/server_api.py @@ -1660,7 +1660,7 @@ def _download_file_to_stream( url = self._endpoint_to_url(endpoint, use_rest=False) progress.set_source_url(url) - retries = self.get_default_max_retries() + retries = max(self.max_retries, 1) api_prepended = False for attempt in range(retries): # Continue in download @@ -2095,7 +2095,7 @@ def _upload_file( headers.pop(orig_key) headers[key] = value - retries = self.get_default_max_retries() + retries = max(self.max_retries, 1) response = None # Get size of file diff --git a/tests/fake_transfer.py b/tests/fake_transfer.py new file mode 100644 index 000000000..536c62307 --- /dev/null +++ b/tests/fake_transfer.py @@ -0,0 +1,41 @@ +"""Helpers to test requests and file transfers without AYON server.""" +import requests +from requests.structures import CaseInsensitiveDict + + +class FakeResponse: + """Minimal stand-in for 'requests.Response'.""" + def __init__( + self, status_code=200, content=b"", headers=None, json_data=None + ): + self.status_code = status_code + self.content = content + self.headers = CaseInsensitiveDict(headers or {}) + self._json_data = json_data + self.reason = "Reason" + self.text = content.decode(errors="ignore") + + @property + def ok(self): + return self.status_code < 400 + + def raise_for_status(self): + if not self.ok: + raise requests.exceptions.HTTPError( + f"{self.status_code} Error", response=self + ) + + def json(self): + if self._json_data is None: + raise ValueError("No json") + return self._json_data + + def iter_content(self, chunk_size=1): + for idx in range(0, len(self.content), chunk_size): + yield self.content[idx:idx + chunk_size] + + def __enter__(self): + return self + + def __exit__(self, *args): + return False diff --git a/tests/test_transfer_max_retries.py b/tests/test_transfer_max_retries.py new file mode 100644 index 000000000..a431e591f --- /dev/null +++ b/tests/test_transfer_max_retries.py @@ -0,0 +1,50 @@ +"""Retries of file transfers. Does not require running AYON server.""" +import io + +import pytest +import requests + +from ayon_api.server_api import ServerAPI +from ayon_api.utils import RequestTypes + +from .fake_transfer import FakeResponse + + +@pytest.fixture +def con(monkeypatch): + monkeypatch.setattr("time.sleep", lambda *args, **kwargs: None) + return ServerAPI("http://localhost:0", create_session=False, max_retries=3) + + +def test_upload_respects_connection_max_retries(con): + con.set_max_retries(1) + attempts = [] + + def put_func(url, data=None, **kwargs): + attempts.append(b"".join(data)) + raise requests.exceptions.ConnectionError("broken pipe") + + con._base_functions_mapping[RequestTypes.put] = put_func + with pytest.raises(requests.exceptions.ConnectionError): + con.upload_file_from_stream("api/upload", io.BytesIO(b"x")) + assert len(attempts) == 1 + + +def test_zero_retries_still_transfers(con, monkeypatch): + monkeypatch.setenv("AYON_SERVER_RETRIES", "0") + con.set_max_retries(None) + con._base_functions_mapping[RequestTypes.put] = ( + lambda url, data=None, **kwargs: (list(data), FakeResponse(204))[1] + ) + con._base_functions_mapping[RequestTypes.get] = ( + lambda url, **kwargs: FakeResponse( + 200, b"abc", {"Content-Length": "3"} + ) + ) + + response = con.upload_file_from_stream("api/upload", io.BytesIO(b"x")) + assert response.status_code == 204 + + stream = io.BytesIO() + con.download_file_to_stream("api/file", stream) + assert stream.getvalue() == b"abc" From 17aa22c8c8ea421041396001ce8d8070a487d9dc Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Sun, 13 Sep 2026 00:05:49 +0200 Subject: [PATCH 2/5] Fix corrupted or incomplete files after interrupted download - Download continuation with 'Range' header did not check the response is partial content (206). A server ignoring 'Range' sends the whole file, which was appended to already downloaded content. - Downloaded size was never compared to 'Content-Length', so a download closed before all content was received was accepted as complete. It is now continued as another attempt, also on ChunkedEncodingError. - 'Content-Length' was stored as string and missing header raised KeyError. Co-Authored-By: Claude Opus 5 --- ayon_api/server_api.py | 26 +++++++++++++-- tests/test_download_resume.py | 60 +++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 tests/test_download_resume.py diff --git a/ayon_api/server_api.py b/ayon_api/server_api.py index 2fce18fc0..5c469e30d 100644 --- a/ayon_api/server_api.py +++ b/ayon_api/server_api.py @@ -1686,19 +1686,41 @@ def _download_file_to_stream( progress.set_destination_url(url) continue response.raise_for_status() - if progress.get_content_size() is None: + if offset > 0 and response.status_code != 206: + # Server ignored 'Range' and sends whole file again, + # already downloaded content must be discarded + stream.seek(0) + stream.truncate() + progress.reset_transferred() + + content_length = response.headers.get("Content-Length") + if ( + progress.get_content_size() is None + and content_length is not None + ): progress.set_content_size( - response.headers["Content-length"] + int(content_length) + + progress.get_transferred_size() ) for chunk in response.iter_content(chunk_size=chunk_size): stream.write(chunk) progress.add_transferred_chunk(len(chunk)) + + content_size = progress.get_content_size() + transferred = progress.get_transferred_size() + if content_size is not None and transferred != content_size: + # Connection was closed before all content was received + raise requests.exceptions.ConnectionError( + f"Downloaded {transferred} out of {content_size}" + f" bytes from '{url}'." + ) break except ( requests.exceptions.Timeout, requests.exceptions.ConnectionError, + requests.exceptions.ChunkedEncodingError, ): if attempt == retries - 1: raise diff --git a/tests/test_download_resume.py b/tests/test_download_resume.py new file mode 100644 index 000000000..9c353cbcb --- /dev/null +++ b/tests/test_download_resume.py @@ -0,0 +1,60 @@ +"""Interrupted downloads. Does not require running AYON server.""" +import io + +import pytest + +from ayon_api.server_api import ServerAPI +from ayon_api.utils import RequestTypes + +from .fake_transfer import FakeResponse + +CONTENT = b"0123456789" +SIZE_HEADER = {"Content-Length": str(len(CONTENT))} + + +@pytest.fixture +def con(monkeypatch): + monkeypatch.setattr("time.sleep", lambda *args, **kwargs: None) + return ServerAPI("http://localhost:0", create_session=False, max_retries=3) + + +def _download(con, get_func): + con._base_functions_mapping[RequestTypes.get] = get_func + stream = io.BytesIO() + progress = con.download_file_to_stream("api/file", stream) + return stream.getvalue(), progress + + +def test_incomplete_download_is_continued(con): + def get_func(url, **kwargs): + if "Range" not in kwargs["headers"]: + # Connection closed after 4 bytes without an exception + return FakeResponse(200, CONTENT[:4], SIZE_HEADER) + assert kwargs["headers"]["Range"] == "bytes=4-" + return FakeResponse(206, CONTENT[4:], {"Content-Length": "6"}) + + content, progress = _download(con, get_func) + assert content == CONTENT + assert progress.transferred_size == len(CONTENT) + + +def test_resume_without_range_support_does_not_duplicate(con): + calls = [] + + def get_func(url, **kwargs): + calls.append(url) + if len(calls) == 1: + return FakeResponse(200, CONTENT[:4], SIZE_HEADER) + # Server ignores 'Range' header and sends whole file again + return FakeResponse(200, CONTENT, SIZE_HEADER) + + content, progress = _download(con, get_func) + assert content == CONTENT + assert progress.transferred_size == len(CONTENT) + + +def test_download_without_content_length(con): + content, _ = _download( + con, lambda url, **kwargs: FakeResponse(200, CONTENT) + ) + assert content == CONTENT From 631b2fd32ab712b49b423ba4f549fb55b14a5ce5 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Mon, 14 Sep 2026 16:40:04 +0200 Subject: [PATCH 3/5] Remove redundant tests --- tests/fake_transfer.py | 41 ------------------------ tests/test_transfer_max_retries.py | 50 ------------------------------ 2 files changed, 91 deletions(-) delete mode 100644 tests/fake_transfer.py delete mode 100644 tests/test_transfer_max_retries.py diff --git a/tests/fake_transfer.py b/tests/fake_transfer.py deleted file mode 100644 index 536c62307..000000000 --- a/tests/fake_transfer.py +++ /dev/null @@ -1,41 +0,0 @@ -"""Helpers to test requests and file transfers without AYON server.""" -import requests -from requests.structures import CaseInsensitiveDict - - -class FakeResponse: - """Minimal stand-in for 'requests.Response'.""" - def __init__( - self, status_code=200, content=b"", headers=None, json_data=None - ): - self.status_code = status_code - self.content = content - self.headers = CaseInsensitiveDict(headers or {}) - self._json_data = json_data - self.reason = "Reason" - self.text = content.decode(errors="ignore") - - @property - def ok(self): - return self.status_code < 400 - - def raise_for_status(self): - if not self.ok: - raise requests.exceptions.HTTPError( - f"{self.status_code} Error", response=self - ) - - def json(self): - if self._json_data is None: - raise ValueError("No json") - return self._json_data - - def iter_content(self, chunk_size=1): - for idx in range(0, len(self.content), chunk_size): - yield self.content[idx:idx + chunk_size] - - def __enter__(self): - return self - - def __exit__(self, *args): - return False diff --git a/tests/test_transfer_max_retries.py b/tests/test_transfer_max_retries.py deleted file mode 100644 index a431e591f..000000000 --- a/tests/test_transfer_max_retries.py +++ /dev/null @@ -1,50 +0,0 @@ -"""Retries of file transfers. Does not require running AYON server.""" -import io - -import pytest -import requests - -from ayon_api.server_api import ServerAPI -from ayon_api.utils import RequestTypes - -from .fake_transfer import FakeResponse - - -@pytest.fixture -def con(monkeypatch): - monkeypatch.setattr("time.sleep", lambda *args, **kwargs: None) - return ServerAPI("http://localhost:0", create_session=False, max_retries=3) - - -def test_upload_respects_connection_max_retries(con): - con.set_max_retries(1) - attempts = [] - - def put_func(url, data=None, **kwargs): - attempts.append(b"".join(data)) - raise requests.exceptions.ConnectionError("broken pipe") - - con._base_functions_mapping[RequestTypes.put] = put_func - with pytest.raises(requests.exceptions.ConnectionError): - con.upload_file_from_stream("api/upload", io.BytesIO(b"x")) - assert len(attempts) == 1 - - -def test_zero_retries_still_transfers(con, monkeypatch): - monkeypatch.setenv("AYON_SERVER_RETRIES", "0") - con.set_max_retries(None) - con._base_functions_mapping[RequestTypes.put] = ( - lambda url, data=None, **kwargs: (list(data), FakeResponse(204))[1] - ) - con._base_functions_mapping[RequestTypes.get] = ( - lambda url, **kwargs: FakeResponse( - 200, b"abc", {"Content-Length": "3"} - ) - ) - - response = con.upload_file_from_stream("api/upload", io.BytesIO(b"x")) - assert response.status_code == 204 - - stream = io.BytesIO() - con.download_file_to_stream("api/file", stream) - assert stream.getvalue() == b"abc" From 4fce5ae3cc78a632a34fa7853ed328a9d802d082 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Mon, 14 Sep 2026 16:58:47 +0200 Subject: [PATCH 4/5] Apply suggestions --- ayon_api/server_api.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/ayon_api/server_api.py b/ayon_api/server_api.py index 5c469e30d..6c98ecdf9 100644 --- a/ayon_api/server_api.py +++ b/ayon_api/server_api.py @@ -1692,15 +1692,11 @@ def _download_file_to_stream( stream.seek(0) stream.truncate() progress.reset_transferred() + headers.pop("Range", None) - content_length = response.headers.get("Content-Length") - if ( - progress.get_content_size() is None - and content_length is not None - ): + if progress.get_content_size() is None: progress.set_content_size( - int(content_length) - + progress.get_transferred_size() + response.headers["Content-length"] ) for chunk in response.iter_content(chunk_size=chunk_size): From 867b1dee26fc7a9028cb040c7d96ea4e7afcdfd4 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Mon, 14 Sep 2026 17:01:04 +0200 Subject: [PATCH 5/5] Safeguard against potential `str` Content-Length (edge case) --- ayon_api/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ayon_api/utils.py b/ayon_api/utils.py index ab2e4c7dd..f5769e7d8 100644 --- a/ayon_api/utils.py +++ b/ayon_api/utils.py @@ -955,7 +955,7 @@ def get_content_size(self) -> int | None: """ return self._content_size - def set_content_size(self, content_size: int) -> None: + def set_content_size(self, content_size: int | str) -> None: """Set content size in bytes. Args: @@ -967,7 +967,7 @@ def set_content_size(self, content_size: int) -> None: """ if self._content_size is not None: raise ValueError("Content size was set more then once") - self._content_size = content_size + self._content_size = int(content_size) def get_started(self) -> bool: """Transfer was started.