Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 68 additions & 1 deletion tuf/ngclient/urllib3_fetcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

# Imports
import urllib3
from urllib3.util.retry import Retry

import tuf
from tuf.api import exceptions
Expand Down Expand Up @@ -50,7 +51,19 @@
if app_user_agent is not None:
ua = f"{app_user_agent} {ua}"

self._proxy_env = ProxyEnvironment(headers={"User-Agent": ua})
# Configure retry strategy: retry on read timeouts and connection errors
# This enables retries for streaming failures, not just initial connection

Check failure on line 55 in tuf/ngclient/urllib3_fetcher.py

View workflow job for this annotation

GitHub Actions / test / Lint Test

Ruff (E501)

tuf/ngclient/urllib3_fetcher.py:55:81: E501 Line too long (82 > 80)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am I correct that retry_strategy variable does not actually affect the streaming retries at all? it seems to be a completely separate implementation

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is useful if the connection fails before streaming, although this shouldn't be a part of this PR, I could remove it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it seems to be codifying the default urllib3 behaviour here. I'm asking about the comment that seems to claim that setting retry_strategy enables retries for streaming failures

retry_strategy = Retry(
total=3,
read=3,
connect=3,
status_forcelist=[500, 502, 503, 504],
raise_on_status=False,
)

self._proxy_env = ProxyEnvironment(
headers={"User-Agent": ua}, retries=retry_strategy
)

def _fetch(self, url: str) -> Iterator[bytes]:
"""Fetch the contents of HTTP/HTTPS url from a remote server.
Expand Down Expand Up @@ -82,6 +95,7 @@
except urllib3.exceptions.MaxRetryError as e:
if isinstance(e.reason, urllib3.exceptions.TimeoutError):
raise exceptions.SlowRetrievalError from e
raise

if response.status >= 400:
response.close()
Expand All @@ -106,6 +120,59 @@
except urllib3.exceptions.MaxRetryError as e:
if isinstance(e.reason, urllib3.exceptions.TimeoutError):
raise exceptions.SlowRetrievalError from e
raise
except (
urllib3.exceptions.ReadTimeoutError,
urllib3.exceptions.ProtocolError,
) as e:
raise exceptions.SlowRetrievalError from e

finally:
response.release_conn()

def download_bytes(self, url: str, max_length: int) -> bytes:
"""Download bytes from given ``url`` with retry on streaming failures.

This override adds retry logic for mid-stream timeout and connection
errors that are not automatically retried by urllib3.

Args:
url: URL string that represents the location of the file.
max_length: Upper bound of data size in bytes.

Raises:
exceptions.DownloadError: An error occurred during download.
exceptions.DownloadLengthMismatchError: Downloaded bytes exceed
``max_length``.
exceptions.DownloadHTTPError: An HTTP error code was received.

Returns:
Content of the file in bytes.
"""
max_retries = 3
last_exception: Exception | None = None

for attempt in range(max_retries):
try:
return super().download_bytes(url, max_length)
except exceptions.SlowRetrievalError as e:
last_exception = e
if attempt < max_retries - 1:
logger.debug(
"Retrying download after streaming error "
"(attempt %d/%d): %s",
attempt + 1,
max_retries,
url,
)
continue
raise
except (
exceptions.DownloadHTTPError,
exceptions.DownloadLengthMismatchError,
):
raise

if last_exception:
raise last_exception
raise exceptions.DownloadError(f"Failed to download {url}")

Check failure on line 178 in tuf/ngclient/urllib3_fetcher.py

View workflow job for this annotation

GitHub Actions / test / Lint Test

Ruff (W292)

tuf/ngclient/urllib3_fetcher.py:178:68: W292 No newline at end of file
Loading