From ee82f774877bdb33ec1b043200b39b3a77b9d6d7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 02:03:08 -0500 Subject: [PATCH] Name the operation in retry warnings, share the attempts constant --- esphome/external_files.py | 7 ++++++- esphome/framework_helpers.py | 7 ++++--- esphome/net_retry.py | 11 +++++++++-- tests/unit_tests/test_external_files.py | 4 +++- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/esphome/external_files.py b/esphome/external_files.py index a3d8508957..c7ddf0ffae 100644 --- a/esphome/external_files.py +++ b/esphome/external_files.py @@ -161,12 +161,17 @@ def has_remote_file_changed( # 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. + # failure in download_content. Only connection-level failures + # are retryable here; HEAD never raises on an HTTP status + # (deliberately: servers that reject HEAD with 405/501 fall + # through to "changed" and the GET below handles them), so a + # 5xx lands in the != 304 branch and the GET's retry covers it. response = fetch_with_retry( url, lambda: requests.head( url, headers=headers, timeout=timeout, allow_redirects=True ), + what="Revalidation", ) _LOGGER.debug( diff --git a/esphome/framework_helpers.py b/esphome/framework_helpers.py index 3798f51101..105791c518 100644 --- a/esphome/framework_helpers.py +++ b/esphome/framework_helpers.py @@ -15,7 +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 +from esphome.net_retry import NETWORK_MAX_ATTEMPTS, is_transient_download_error if TYPE_CHECKING: import requests @@ -30,8 +30,9 @@ _LOGGER = logging.getLogger(__name__) _MIRROR_ATTEMPTS = 3 # Passes over the whole mirror list when a transient network error is in -# the mix; matches git.py's _NETWORK_MAX_ATTEMPTS (3 tries, 2s/4s backoff). -_MIRROR_SWEEP_ATTEMPTS = 3 +# the mix; shares net_retry's policy (3 tries, 2s/4s backoff), which in +# turn matches git.py's _NETWORK_MAX_ATTEMPTS. +_MIRROR_SWEEP_ATTEMPTS = NETWORK_MAX_ATTEMPTS def get_project_link_flags() -> list[str]: diff --git a/esphome/net_retry.py b/esphome/net_retry.py index 06f9bfa0ae..84b13e5a50 100644 --- a/esphome/net_retry.py +++ b/esphome/net_retry.py @@ -38,6 +38,11 @@ def _is_permanent_dns_failure(e: BaseException) -> bool: if id(exc) in seen: continue if isinstance(exc, socket.gaierror): + # Everything except EAI_AGAIN is treated as permanent. Codes + # like EAI_NONAME can occur during a brief network outage too, + # but a 6s backoff rarely outlives one, and permanent means + # callers fall back to their cache immediately instead of + # sleeping first (the offline-build case). return exc.errno != socket.EAI_AGAIN seen.add(id(exc)) stack.extend( @@ -83,10 +88,11 @@ def is_transient_download_error(e: Exception) -> bool: ) -def fetch_with_retry[T](url: str, fetch: Callable[[], T]) -> T: +def fetch_with_retry[T](url: str, fetch: Callable[[], T], what: str = "Download") -> 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. + ``what`` names the operation in the retry warning. """ import requests @@ -98,7 +104,8 @@ def fetch_with_retry[T](url: str, fetch: Callable[[], T]) -> T: raise delay = 2**attempt _LOGGER.warning( - "Download of %s failed: %s. Retrying in %d seconds... (attempt %d/%d)", + "%s of %s failed: %s. Retrying in %d seconds... (attempt %d/%d)", + what, url, e, delay, diff --git a/tests/unit_tests/test_external_files.py b/tests/unit_tests/test_external_files.py index 51f1befd32..cb99593fc2 100644 --- a/tests/unit_tests/test_external_files.py +++ b/tests/unit_tests/test_external_files.py @@ -647,9 +647,10 @@ def test_has_remote_file_changed_retries_transient_error( mock_requests_head: MagicMock, mock_retry_sleep: MagicMock, setup_core: Path, + caplog: pytest.LogCaptureFixture, ) -> None: """A HEAD revalidation that fails transiently then returns 304 does not - mark the cached copy stale.""" + mark the cached copy stale, and the retry warning names the operation.""" test_file = setup_core / "cached.txt" test_file.write_bytes(b"cached content") @@ -669,6 +670,7 @@ def test_has_remote_file_changed_retries_transient_error( 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)] + assert "Revalidation of" in caplog.text def test_download_content_skip_external_update_uses_cache(