[core] Retry transient network errors when downloading external files

esphome/external_files.py

@@ -16,6 +16,7 @@ from esphome.const import CONF_FILE, CONF_TYPE, CONF_URL, __version__
 from esphome.core import CORE, EsphomeError, TimePeriodSeconds
 from esphome.happy_eyeballs import ensure_happy_eyeballs
 from esphome.helpers import write_file
+from esphome.net_retry import fetch_with_retry
 from esphome.types import ConfigType
 
 _LOGGER = logging.getLogger(__name__)
@@ -157,8 +158,15 @@ def has_remote_file_changed(
             }
             if etag := _read_etag(local_file_path):
                 headers[IF_NONE_MATCH] = etag
-            response = requests.head(
-                url, headers=headers, timeout=timeout, allow_redirects=True
+            # Retried even though a failure degrades gracefully to the
+            # cached copy below: allow_stale=False consumers reject an
+            # unverified copy, so for them a healed flake avoids a hard
+            # failure in download_content.
+            response = fetch_with_retry(
+                url,
+                lambda: requests.head(
+                    url, headers=headers, timeout=timeout, allow_redirects=True
+                ),
             )
 
             _LOGGER.debug(
@@ -293,7 +301,7 @@ def download_content(
     _LOGGER.info("Downloading %s", url)
     _LOGGER.debug("Saving to %s", path)
 
-    try:
+    def _fetch() -> tuple[requests.Response, bytes]:
         req = requests.get(
             url,
             timeout=timeout,
@@ -304,7 +312,10 @@ def download_content(
         # and mid-stream connection errors all surface here as
         # RequestException subclasses, so this needs the same fall-back
         # treatment as the request itself.
-        data = req.content
+        return req, req.content
+
+    try:
+        req, data = fetch_with_retry(url, _fetch)
     except requests.exceptions.RequestException as e:
         if path.exists():
             # Memoized so a flaky host warns once per run, not per consumer.

esphome/framework_helpers.py

@@ -15,6 +15,7 @@ from typing import IO, TYPE_CHECKING
 
 from esphome.happy_eyeballs import ensure_happy_eyeballs
 from esphome.helpers import ProgressBar, rmtree
+from esphome.net_retry import is_transient_download_error
 
 if TYPE_CHECKING:
     import requests
@@ -903,30 +904,6 @@ def _spent_attempts_error(e: Exception, attempts: int) -> Exception:
     return err
 
 
-def _is_transient_download_error(e: Exception) -> bool:
-    """Return True when a download failure is worth retrying.
-
-    Connection-level failures and HTTP 429/5xx are transient. Other HTTP
-    errors, local errors, and exhausted-attempts EsphomeError wrappers
-    (their per-mirror retries are already spent) are permanent.
-    """
-    # Imported lazily: requests is a heavy import (~85ms) and is only
-    # needed when actually downloading, never during config validation.
-    import requests
-
-    if isinstance(e, requests.exceptions.HTTPError):
-        resp = e.response
-        return resp is not None and (resp.status_code == 429 or resp.status_code >= 500)
-    return isinstance(
-        e,
-        (
-            requests.exceptions.ConnectionError,
-            requests.exceptions.Timeout,
-            requests.exceptions.ChunkedEncodingError,
-        ),
-    )
-
-
 def _try_mirrors_once(
     urls: list[str],
     path_target: Path | None,
@@ -1131,7 +1108,7 @@ def download_from_mirrors(
         # Permanent failures (404, verification mismatch) won't heal;
         # only retry when a transient error is in the mix (as git.py does).
         transient = next(
-            ((u, e) for u, e in sweep_failures if _is_transient_download_error(e)),
+            ((u, e) for u, e in sweep_failures if is_transient_download_error(e)),
             None,
         )
         if transient is None:

esphome/net_retry.py

@@ -0,0 +1,109 @@
+"""Retry policy for HTTP downloads.
+
+Kept import-light on purpose: this module is imported at config time, so it
+must not pull in requests (a heavy import, ~85ms) at module scope.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Callable
+import logging
+import time
+
+_LOGGER = logging.getLogger(__name__)
+
+# Transient network failures are retried with 2s/4s backoff, matching
+# git.py's _NETWORK_MAX_ATTEMPTS and framework_helpers' mirror downloads.
+# Worst case per fetch is NETWORK_MAX_ATTEMPTS timeouts plus 6s of sleeps;
+# callers are expected to memoize failures so a flaky host pays that at
+# most once per file per run.
+NETWORK_MAX_ATTEMPTS = 3
+
+
+def _is_permanent_dns_failure(e: BaseException) -> bool:
+    """Whether a permanent socket.gaierror hides in ``e``'s exception chain.
+
+    EAI_AGAIN ("temporary failure in name resolution", the flaky container
+    resolver case) stays retryable; only hard resolution failures like
+    NXDOMAIN count. requests wraps urllib3's MaxRetryError, which carries
+    the underlying NameResolutionError in its ``reason`` attribute rather
+    than the __cause__ chain, so attributes and args are walked as well.
+    """
+    import socket
+
+    seen: set[int] = set()
+    stack: list[BaseException] = [e]
+    while stack:
+        exc = stack.pop()
+        if id(exc) in seen:
+            continue
+        if isinstance(exc, socket.gaierror):
+            return exc.errno != socket.EAI_AGAIN
+        seen.add(id(exc))
+        stack.extend(
+            nxt
+            for nxt in (
+                exc.__cause__,
+                exc.__context__,
+                getattr(exc, "reason", None),  # urllib3 MaxRetryError
+                *exc.args,
+            )
+            if isinstance(nxt, BaseException)
+        )
+    return False
+
+
+def is_transient_download_error(e: Exception) -> bool:
+    """Return True when a download failure is worth retrying.
+
+    Connection-level failures and HTTP 429/5xx are transient. Hard name
+    resolution failures (NXDOMAIN, offline hosts; not EAI_AGAIN), other
+    HTTP errors, local errors, and exhausted-attempts EsphomeError
+    wrappers (their per-mirror retries are already spent) are permanent.
+    """
+    # Imported lazily: requests is a heavy import (~85ms) and is only
+    # needed when actually downloading, never during config validation.
+    import requests
+
+    if isinstance(e, requests.exceptions.HTTPError):
+        resp = e.response
+        return resp is not None and (resp.status_code == 429 or resp.status_code >= 500)
+    if isinstance(e, requests.exceptions.ConnectionError) and _is_permanent_dns_failure(
+        e
+    ):
+        return False
+    return isinstance(
+        e,
+        (
+            requests.exceptions.ConnectionError,
+            requests.exceptions.Timeout,
+            requests.exceptions.ChunkedEncodingError,
+            requests.exceptions.ContentDecodingError,
+        ),
+    )
+
+
+def fetch_with_retry[T](url: str, fetch: Callable[[], T]) -> T:
+    """Run ``fetch``, retrying failures that ``is_transient_download_error``
+    classifies as transient with 2s/4s backoff. Other failures and the
+    final attempt's failure propagate to the caller's fallback handling.
+    """
+    import requests
+
+    for attempt in range(1, NETWORK_MAX_ATTEMPTS):
+        try:
+            return fetch()
+        except requests.exceptions.RequestException as e:
+            if not is_transient_download_error(e):
+                raise
+            delay = 2**attempt
+            _LOGGER.warning(
+                "Download of %s failed: %s. Retrying in %d seconds... (attempt %d/%d)",
+                url,
+                e,
+                delay,
+                attempt + 1,
+                NETWORK_MAX_ATTEMPTS,
+            )
+            time.sleep(delay)
+    return fetch()

tests/unit_tests/test_external_files.py

@@ -4,7 +4,7 @@ import os
 from pathlib import Path
 import time
 from typing import Any
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, call, patch
 
 import pytest
 import requests
@@ -81,6 +81,17 @@ def mock_download_content_many() -> MagicMock:
         yield m
 
 
+@pytest.fixture
+def mock_retry_sleep() -> MagicMock:
+    """Patch the retry backoff sleep (module-global time.sleep, so the
+    blast radius is the whole process) for tests that inject transient
+    network errors; without it they would really wait 2s/4s per retry.
+    Retry tests assert on this mock's call args.
+    """
+    with patch("esphome.net_retry.time.sleep") as m:
+        yield m
+
+
 def test_compute_local_file_dir(setup_core: Path) -> None:
     """Test compute_local_file_dir creates and returns correct path."""
     domain = "font"
@@ -495,6 +506,7 @@ class _BodyReadErrorResponse:
 def test_download_content_with_body_read_error_uses_cache(
     mock_has_remote_file_changed: MagicMock,
     mock_requests_get: MagicMock,
+    mock_retry_sleep: MagicMock,
     setup_core: Path,
 ) -> None:
     """Body-read errors (chunked-decode/gzip-decode/mid-stream connection
@@ -519,6 +531,7 @@ def test_download_content_with_body_read_error_uses_cache(
 def test_download_content_with_body_read_error_no_cache_fails(
     mock_has_remote_file_changed: MagicMock,
     mock_requests_get: MagicMock,
+    mock_retry_sleep: MagicMock,
     setup_core: Path,
 ) -> None:
     """A body-read failure with no cache available must surface as a
@@ -535,6 +548,129 @@ def test_download_content_with_body_read_error_no_cache_fails(
         external_files.download_content("https://example.com/file.txt", test_file)
 
 
+def test_download_content_retries_transient_error_then_succeeds(
+    mock_has_remote_file_changed: MagicMock,
+    mock_requests_get: MagicMock,
+    mock_retry_sleep: MagicMock,
+    setup_core: Path,
+) -> None:
+    """Transient failures (connection reset, timeout) are retried with 2s/4s
+    backoff before giving up; a late success downloads normally."""
+    test_file = setup_core / "downloads" / "file.txt"
+    mock_has_remote_file_changed.return_value = True
+
+    ok = MagicMock()
+    ok.content = b"downloaded"
+    ok.headers = {}
+    mock_requests_get.side_effect = [
+        requests.exceptions.ConnectionError("reset by peer"),
+        requests.exceptions.Timeout("timed out"),
+        ok,
+    ]
+
+    result = external_files.download_content("https://example.com/file.txt", test_file)
+
+    assert result == b"downloaded"
+    assert test_file.read_bytes() == b"downloaded"
+    assert mock_retry_sleep.call_args_list == [call(2), call(4)]
+
+
+def test_download_content_transient_error_exhausts_attempts(
+    mock_has_remote_file_changed: MagicMock,
+    mock_requests_get: MagicMock,
+    mock_retry_sleep: MagicMock,
+    setup_core: Path,
+) -> None:
+    """A persistent transient failure gives up after three attempts and then
+    follows the normal no-cache error path."""
+    test_file = setup_core / "nonexistent.txt"
+    mock_has_remote_file_changed.return_value = True
+    mock_requests_get.side_effect = requests.exceptions.ConnectionError("reset by peer")
+
+    with pytest.raises(Invalid, match="Could not download from.*reset by peer"):
+        external_files.download_content("https://example.com/file.txt", test_file)
+
+    assert mock_retry_sleep.call_args_list == [call(2), call(4)]
+
+
+def test_download_content_non_transient_error_not_retried(
+    mock_has_remote_file_changed: MagicMock,
+    mock_requests_get: MagicMock,
+    mock_retry_sleep: MagicMock,
+    setup_core: Path,
+) -> None:
+    """Permanent failures like a 404 fail on the first attempt."""
+    test_file = setup_core / "nonexistent.txt"
+    mock_has_remote_file_changed.return_value = True
+
+    response = MagicMock()
+    response.status_code = 404
+    mock_requests_get.side_effect = requests.exceptions.HTTPError(
+        "404 Client Error", response=response
+    )
+
+    with pytest.raises(Invalid, match="Could not download from.*404"):
+        external_files.download_content("https://example.com/file.txt", test_file)
+
+    assert mock_requests_get.call_count == 1
+    mock_retry_sleep.assert_not_called()
+
+
+def test_download_content_retries_body_read_error(
+    mock_has_remote_file_changed: MagicMock,
+    mock_requests_get: MagicMock,
+    mock_retry_sleep: MagicMock,
+    setup_core: Path,
+) -> None:
+    """Mid-stream failures surfacing from `.content` are retried too."""
+    test_file = setup_core / "downloads" / "file.txt"
+    mock_has_remote_file_changed.return_value = True
+
+    ok = MagicMock()
+    ok.content = b"downloaded"
+    ok.headers = {}
+    mock_requests_get.side_effect = [
+        _BodyReadErrorResponse(
+            requests.exceptions.ChunkedEncodingError("body truncated")
+        ),
+        ok,
+    ]
+
+    result = external_files.download_content("https://example.com/file.txt", test_file)
+
+    assert result == b"downloaded"
+    assert mock_requests_get.call_count == 2
+    assert mock_retry_sleep.call_args_list == [call(2)]
+
+
+def test_has_remote_file_changed_retries_transient_error(
+    mock_requests_head: MagicMock,
+    mock_retry_sleep: MagicMock,
+    setup_core: Path,
+) -> None:
+    """A HEAD revalidation that fails transiently then returns 304 does not
+    mark the cached copy stale."""
+    test_file = setup_core / "cached.txt"
+    test_file.write_bytes(b"cached content")
+
+    ok = MagicMock()
+    ok.status_code = 304
+    ok.headers = {}
+    mock_requests_head.side_effect = [
+        requests.exceptions.ConnectionError("reset by peer"),
+        ok,
+    ]
+
+    changed = external_files.has_remote_file_changed(
+        "https://example.com/file.txt", test_file
+    )
+
+    assert changed is False
+    assert test_file not in external_files._run_data().stale_paths
+    assert mock_requests_head.call_count == 2
+    assert mock_retry_sleep.call_args_list == [call(2)]
+
+
 def test_download_content_skip_external_update_uses_cache(
     mock_has_remote_file_changed: MagicMock,
     mock_requests_get: MagicMock,

tests/unit_tests/test_framework_helpers.py

@@ -23,7 +23,6 @@ from esphome.core import EsphomeError
 from esphome.framework_helpers import (
     _7z_extract_all,
     _detect_archive_root,
-    _is_transient_download_error,
     _rename_with_retry,
     _tar_extract_all,
     _zip_extract_all,
@@ -1594,43 +1593,6 @@ class TestDownloadFromMirrors:
         mock_sleep.assert_not_called()
 
 
-def _http_error(status: int) -> req.HTTPError:
-    """An HTTPError carrying a response with the given status, as raised by
-    ``raise_for_status`` on a real response."""
-    resp = MagicMock()
-    resp.status_code = status
-    return req.HTTPError(str(status), response=resp)
-
-
-class TestIsTransientDownloadError:
-    def test_connection_errors_are_transient(self) -> None:
-        assert _is_transient_download_error(req.ConnectionError("reset"))
-        assert _is_transient_download_error(req.Timeout("timed out"))
-        assert _is_transient_download_error(
-            req.exceptions.ChunkedEncodingError("dropped")
-        )
-
-    def test_http_statuses(self) -> None:
-        assert not _is_transient_download_error(_http_error(404))
-        assert not _is_transient_download_error(_http_error(403))
-        assert _is_transient_download_error(_http_error(429))
-        assert _is_transient_download_error(_http_error(503))
-
-    def test_http_error_without_response_is_permanent(self) -> None:
-        assert not _is_transient_download_error(req.HTTPError("boom"))
-
-    def test_exhausted_resume_attempts_are_permanent(self) -> None:
-        """download_with_resume already spent its own resume attempts; its
-        EsphomeError wrapper is not retried again at the sweep level."""
-        wrapped = EsphomeError("Failed to download after 3 attempts")
-        wrapped.__cause__ = req.ConnectionError("down")
-        assert not _is_transient_download_error(wrapped)
-
-    def test_unrelated_errors_are_permanent(self) -> None:
-        assert not _is_transient_download_error(OSError("disk full"))
-        assert not _is_transient_download_error(EsphomeError("size mismatch"))
-
-
 def test_importing_framework_helpers_does_not_import_requests() -> None:
     """Importing framework_helpers must not drag in requests.
 

tests/unit_tests/test_net_retry.py

@@ -0,0 +1,111 @@
+"""Tests for esphome.net_retry."""
+
+import socket
+from unittest.mock import MagicMock, call, patch
+
+import pytest
+import requests as req
+
+from esphome.core import EsphomeError
+from esphome.net_retry import fetch_with_retry, is_transient_download_error
+
+
+def _http_error(status: int) -> req.HTTPError:
+    """An HTTPError carrying a response with the given status, as raised by
+    ``raise_for_status`` on a real response."""
+    resp = MagicMock()
+    resp.status_code = status
+    return req.HTTPError(str(status), response=resp)
+
+
+class TestIsTransientDownloadError:
+    def test_connection_errors_are_transient(self) -> None:
+        assert is_transient_download_error(req.ConnectionError("reset"))
+        assert is_transient_download_error(req.Timeout("timed out"))
+        assert is_transient_download_error(
+            req.exceptions.ChunkedEncodingError("dropped")
+        )
+        assert is_transient_download_error(
+            req.exceptions.ContentDecodingError("gzip stream truncated")
+        )
+
+    def test_http_statuses(self) -> None:
+        assert not is_transient_download_error(_http_error(404))
+        assert not is_transient_download_error(_http_error(403))
+        assert is_transient_download_error(_http_error(429))
+        assert is_transient_download_error(_http_error(503))
+
+    def test_http_error_without_response_is_permanent(self) -> None:
+        assert not is_transient_download_error(req.HTTPError("boom"))
+
+    def test_hard_dns_failures_are_permanent(self) -> None:
+        """Hard name resolution failures won't heal within a retry window;
+        offline builds must fall back to their cache without sleeping first.
+        requests can surface the gaierror via the cause chain or via
+        urllib3's MaxRetryError.reason attribute."""
+        gai = socket.gaierror(socket.EAI_NONAME, "nodename nor servname provided")
+
+        chained = req.ConnectionError("resolution failed")
+        chained.__cause__ = gai
+        assert not is_transient_download_error(chained)
+
+        class _FakeMaxRetryError(Exception):
+            def __init__(self, reason: BaseException) -> None:
+                super().__init__("max retries exceeded")
+                self.reason = reason
+
+        wrapped = req.ConnectionError(_FakeMaxRetryError(gai))
+        assert not is_transient_download_error(wrapped)
+
+        # A garden-variety connection reset stays transient.
+        assert is_transient_download_error(req.ConnectionError("reset by peer"))
+
+    def test_temporary_dns_failure_stays_transient(self) -> None:
+        """EAI_AGAIN is a temporary resolver failure (flaky container DNS)
+        and does heal, matching git.py's policy of retrying DNS flakes."""
+        gai = socket.gaierror(socket.EAI_AGAIN, "temporary failure in name resolution")
+        chained = req.ConnectionError("resolution failed")
+        chained.__cause__ = gai
+
+        assert is_transient_download_error(chained)
+
+    def test_dns_walk_survives_exception_cycles(self) -> None:
+        """A cyclic cause chain must terminate (and stay transient when no
+        resolution failure is present)."""
+        outer = req.ConnectionError("a")
+        inner = ValueError("b")
+        outer.__cause__ = inner
+        inner.__cause__ = outer
+
+        assert is_transient_download_error(outer)
+
+    def test_exhausted_resume_attempts_are_permanent(self) -> None:
+        """download_with_resume already spent its own resume attempts; its
+        EsphomeError wrapper is not retried again at the sweep level."""
+        wrapped = EsphomeError("Failed to download after 3 attempts")
+        wrapped.__cause__ = req.ConnectionError("down")
+        assert not is_transient_download_error(wrapped)
+
+    def test_unrelated_errors_are_permanent(self) -> None:
+        assert not is_transient_download_error(OSError("disk full"))
+        assert not is_transient_download_error(EsphomeError("size mismatch"))
+
+
+class TestFetchWithRetry:
+    def test_logs_the_upcoming_attempt_number(
+        self, caplog: pytest.LogCaptureFixture
+    ) -> None:
+        """The warning names the attempt about to run, not the one that just
+        failed, so a user sees attempt 2/3 and 3/3 before the final failure."""
+        with (
+            patch("esphome.net_retry.time.sleep") as mock_sleep,
+            pytest.raises(req.ConnectionError),
+        ):
+            fetch_with_retry(
+                "https://example.com/f",
+                lambda: (_ for _ in ()).throw(req.ConnectionError("reset")),
+            )
+
+        assert mock_sleep.call_args_list == [call(2), call(4)]
+        assert "(attempt 2/3)" in caplog.text
+        assert "(attempt 3/3)" in caplog.text