DEVX-156: fix: add tenacity retry to install_tools._download for transient network failures

src/devx/tools/install_tools.py

@@ -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:
+def _download(url: str, dest: Path, *, _sleep=None) -> None:
-    """Download a file from ``url`` to ``dest`` with a 60s timeout."""
+    """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)
 

tests/unit/test_install_tools.py

@@ -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: