From cd3c06f8536377b977b438e31c594da193246790 Mon Sep 17 00:00:00 2001 From: Tobias Fischer Date: Wed, 7 Oct 2026 07:38:46 +1000 Subject: [PATCH 1/2] Retry transient failures when fetching package.xml raw.githubusercontent.com regularly answers CI with errors like "503 Server Error: first byte timeout", which failed the whole recipe generation. Retry connection errors, timeouts, 429 and 5xx responses up to five times with exponential backoff (2, 4, 8, 16 s); other errors still fail at once. Co-Authored-By: Claude Opus 5.5 --- vinca/distro.py | 36 ++++++++++++++++++++++-- vinca/test_fetch_retry.py | 58 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 vinca/test_fetch_retry.py diff --git a/vinca/distro.py b/vinca/distro.py index cdb3420..7a980be 100644 --- a/vinca/distro.py +++ b/vinca/distro.py @@ -2,6 +2,7 @@ import os import posixpath import tarfile +import time import urllib.parse import zipfile from concurrent.futures import ThreadPoolExecutor @@ -12,6 +13,10 @@ from rosdistro.dependency_walker import DependencyWalker from rosdistro.manifest_provider import get_release_tag +# Retries for fetching package.xml files (raw.githubusercontent.com is flaky in CI). +_FETCH_ATTEMPTS = 5 +_RETRY_STATUS = {429, 500, 502, 503, 504} + # Source archive suffixes an additional recipe may point at instead of a git repository. # Such a package is fetched by URL (checksummed with sha256) rather than cloned. # Zstandard is left out on purpose: tarfile only reads it from Python 3.14 on, while @@ -477,10 +482,35 @@ def _get(self, url): requests also drops the Authorization header when a redirect leaves the original host, which matters because an Artifactory server usually redirects a download to a presigned URL on a separate storage host. + + Transient failures (connection errors, timeouts, 429 and 5xx responses, e.g. + raw.githubusercontent.com's "503 first byte timeout") are retried with + exponential backoff before giving up. """ - response = requests.get(url, headers=self._get_auth_headers(url), timeout=60) - response.raise_for_status() - return response + for attempt in range(1, _FETCH_ATTEMPTS + 1): + try: + response = requests.get( + url, headers=self._get_auth_headers(url), timeout=60 + ) + status = getattr(response, "status_code", 200) + if status in _RETRY_STATUS and attempt < _FETCH_ATTEMPTS: + raise requests.HTTPError( + f"{response.status_code} for url: {url}", response=response + ) + response.raise_for_status() + return response + except ( + requests.ConnectionError, + requests.Timeout, + requests.HTTPError, + ) as error: + status = getattr(getattr(error, "response", None), "status_code", None) + retryable = status is None or status in _RETRY_STATUS + if not retryable or attempt == _FETCH_ATTEMPTS: + raise + delay = 2**attempt + print(f"Fetching {url} failed ({error}); retrying in {delay}s") + time.sleep(delay) def _download_raw_pkg_xml_or_cached(self, url): if url in self._additional_xml_cache: diff --git a/vinca/test_fetch_retry.py b/vinca/test_fetch_retry.py new file mode 100644 index 0000000..ca62093 --- /dev/null +++ b/vinca/test_fetch_retry.py @@ -0,0 +1,58 @@ +from types import SimpleNamespace +from unittest.mock import Mock + +import pytest +import requests + +from vinca import distro as distro_module +from vinca.distro import Distro + + +def fake_distro(): + return SimpleNamespace(_get_auth_headers=lambda url: {}) + + +def response(status): + r = Mock(status_code=status) + if status >= 400: + r.raise_for_status.side_effect = requests.HTTPError(f"{status}", response=r) + return r + + +def test_retries_transient_failures(monkeypatch): + calls = [requests.ConnectionError("reset"), response(503), response(200)] + get = Mock( + side_effect=lambda *a, **k: (_ for _ in ()).throw(c) + if isinstance(c := calls.pop(0), Exception) + else c + ) + monkeypatch.setattr(distro_module.requests, "get", get) + sleep = Mock() + monkeypatch.setattr(distro_module.time, "sleep", sleep) + + assert ( + Distro._get(fake_distro(), "https://raw.githubusercontent.com/x").status_code + == 200 + ) + assert get.call_count == 3 + assert [c.args[0] for c in sleep.call_args_list] == [2, 4] + + +def test_does_not_retry_client_errors(monkeypatch): + get = Mock(return_value=response(404)) + monkeypatch.setattr(distro_module.requests, "get", get) + monkeypatch.setattr(distro_module.time, "sleep", Mock()) + + with pytest.raises(requests.HTTPError): + Distro._get(fake_distro(), "https://raw.githubusercontent.com/x") + assert get.call_count == 1 + + +def test_gives_up_after_the_last_attempt(monkeypatch): + get = Mock(return_value=response(503)) + monkeypatch.setattr(distro_module.requests, "get", get) + monkeypatch.setattr(distro_module.time, "sleep", Mock()) + + with pytest.raises(requests.HTTPError): + Distro._get(fake_distro(), "https://raw.githubusercontent.com/x") + assert get.call_count == distro_module._FETCH_ATTEMPTS From 83696471f791fe5a9911b1cce5e4f553016febe4 Mon Sep 17 00:00:00 2001 From: Tobias Fischer Date: Wed, 7 Oct 2026 07:54:54 +1000 Subject: [PATCH 2/2] Make the retry loop's return type explicit for pyrefly Co-Authored-By: Claude Opus 5.5 --- vinca/distro.py | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/vinca/distro.py b/vinca/distro.py index 7a980be..727f906 100644 --- a/vinca/distro.py +++ b/vinca/distro.py @@ -487,30 +487,23 @@ def _get(self, url): raw.githubusercontent.com's "503 first byte timeout") are retried with exponential backoff before giving up. """ - for attempt in range(1, _FETCH_ATTEMPTS + 1): + headers = self._get_auth_headers(url) + for attempt in range(1, _FETCH_ATTEMPTS): try: - response = requests.get( - url, headers=self._get_auth_headers(url), timeout=60 - ) + response = requests.get(url, headers=headers, timeout=60) status = getattr(response, "status_code", 200) - if status in _RETRY_STATUS and attempt < _FETCH_ATTEMPTS: - raise requests.HTTPError( - f"{response.status_code} for url: {url}", response=response - ) - response.raise_for_status() - return response - except ( - requests.ConnectionError, - requests.Timeout, - requests.HTTPError, - ) as error: - status = getattr(getattr(error, "response", None), "status_code", None) - retryable = status is None or status in _RETRY_STATUS - if not retryable or attempt == _FETCH_ATTEMPTS: - raise - delay = 2**attempt - print(f"Fetching {url} failed ({error}); retrying in {delay}s") - time.sleep(delay) + if status not in _RETRY_STATUS: + response.raise_for_status() + return response + error = f"{status} for url: {url}" + except (requests.ConnectionError, requests.Timeout) as exc: + error = exc + delay = 2**attempt + print(f"Fetching {url} failed ({error}); retrying in {delay}s") + time.sleep(delay) + response = requests.get(url, headers=headers, timeout=60) + response.raise_for_status() + return response def _download_raw_pkg_xml_or_cached(self, url): if url in self._additional_xml_cache: