DEVX-156: Add retry with backoff to install_tools download for transient network failures #255

Merged
emil merged 1 commits from DEVX-156-install-tools-retry into master 2026-08-12 18:15:27 +00:00
2 changed files with 105 additions and 2 deletions
+43 -2
View File
@@ -22,18 +22,37 @@ Usage::
from __future__ import annotations
import logging
import os
import platform
import shutil
import tarfile
import tempfile
import time
import urllib.error
import urllib.request
from pathlib import Path
import click
from tenacity import (
Retrying,
before_sleep_log,
retry_if_exception_type,
stop_after_attempt,
wait_exponential,
)
TARGET_DIR = Path.home() / ".local" / "bin"
logger = logging.getLogger(__name__)
# Retry configuration for transient network failures during download.
# GitHub releases occasionally drops connections ("Remote end closed
# connection without response"). Retrying with backoff before falling
# through to the next fallback URL makes the build resilient to
# momentary network blips.
MAX_DOWNLOAD_RETRIES = 3
ACTIONLINT_VERSION = "1.7.12"
GIT_CLIFF_VERSION = "2.13.1"
@@ -64,8 +83,30 @@ def _ensure_target_dir() -> Path:
return TARGET_DIR
def _download(url: str, dest: Path) -> None:
"""Download a file from ``url`` to ``dest`` with a 60s timeout."""
def _download(url: str, dest: Path, *, _sleep=None) -> None:
"""Download a file from ``url`` to ``dest`` with retry and 60s timeout.
Retries up to ``MAX_DOWNLOAD_RETRIES`` times on transient network
errors (``URLError``, ``OSError`` from connection resets) using
exponential backoff. This handles momentary GitHub releases
connection drops that were causing CI image builds to fail.
The ``_sleep`` kwarg is for tests to avoid real sleeping; production
code should leave it as ``None`` (uses ``time.sleep``).
"""
retrying = Retrying(
stop=stop_after_attempt(MAX_DOWNLOAD_RETRIES),
wait=wait_exponential(multiplier=2, min=2, max=10),
retry=retry_if_exception_type((urllib.error.URLError, OSError, ConnectionError)),
before_sleep=before_sleep_log(logger, logging.WARNING),
sleep=_sleep if _sleep is not None else time.sleep,
reraise=True,
)
retrying(_do_download, url, dest)
def _do_download(url: str, dest: Path) -> None:
"""Single download attempt — called by :func:`_download` retry wrapper."""
with urllib.request.urlopen(url, timeout=60) as resp, open(dest, "wb") as f: # nosec B310
shutil.copyfileobj(resp, f)
+62
View File
@@ -1,6 +1,7 @@
from __future__ import annotations
import platform
import urllib.error
from pathlib import Path
from unittest.mock import patch
@@ -68,6 +69,67 @@ class TestDownload:
mock_urlopen.assert_called_once()
assert dest.read_bytes() == b"data"
def test_download_retries_on_transient_error(self, tmp_path: Path) -> None:
"""Download retries on URLError then succeeds."""
dest = tmp_path / "file.bin"
call_count = [0]
class _FakeResponse:
def __init__(self) -> None:
self._sent = False
def __enter__(self) -> _FakeResponse:
return self
def __exit__(self, *args: object) -> None:
pass
def read(self, n: int = -1) -> bytes:
if self._sent:
return b""
self._sent = True
return b"data"
def _flaky_urlopen(url: str, timeout: int = 60):
call_count[0] += 1
if call_count[0] < 2:
raise urllib.error.URLError("Remote end closed connection")
return _FakeResponse()
with patch("urllib.request.urlopen", side_effect=_flaky_urlopen):
install_tools._download("https://example.com/file", dest, _sleep=lambda _: None)
assert call_count[0] == 2
assert dest.read_bytes() == b"data"
def test_download_fails_after_max_retries(self, tmp_path: Path) -> None:
"""Download raises after MAX_DOWNLOAD_RETRIES attempts."""
dest = tmp_path / "file.bin"
call_count = [0]
def _always_fail(url: str, timeout: int = 60):
call_count[0] += 1
raise urllib.error.URLError("Remote end closed connection")
with patch("urllib.request.urlopen", side_effect=_always_fail):
with pytest.raises(urllib.error.URLError):
install_tools._download("https://example.com/file", dest, _sleep=lambda _: None)
assert call_count[0] == install_tools.MAX_DOWNLOAD_RETRIES
assert not dest.exists()
def test_download_no_retry_on_non_transient_error(self, tmp_path: Path) -> None:
"""Download does not retry on non-network errors (e.g. ValueError)."""
dest = tmp_path / "file.bin"
call_count = [0]
def _fail_with_value_error(url: str, timeout: int = 60):
call_count[0] += 1
raise ValueError("not a network error")
with patch("urllib.request.urlopen", side_effect=_fail_with_value_error):
with pytest.raises(ValueError):
install_tools._download("https://example.com/file", dest, _sleep=lambda _: None)
assert call_count[0] == 1
class TestDownloadBinary:
def test_download(self, tmp_path: Path) -> None: