diff --git a/esphome/platformio/runner.py b/esphome/platformio/runner.py index 976979dc57..caab47dcc2 100644 --- a/esphome/platformio/runner.py +++ b/esphome/platformio/runner.py @@ -51,9 +51,13 @@ def patch_file_downloader() -> None: """Retry PlatformIO package downloads with exponential backoff. PlatformIO's ``FileDownloader`` uses an ``HTTPSession`` without built-in - retry for 502/503 errors. We wrap ``__init__`` to retry on - ``PackageException`` and close the session between attempts so a new - TCP connection can route to a different CDN edge node. + retry. We wrap ``__init__`` to retry on transient failures and close the + session between attempts so a new TCP connection can route to a different + CDN edge node. We catch both ``PackageException`` (raised when the server + returns a non-200 status such as 502/503) and ``OSError`` -- which covers + ``requests.exceptions.ConnectionError``, ``ReadTimeout``, and + ``ChunkedEncodingError`` (all subclasses of ``OSError``) that get raised + when the connection is aborted before a response is parsed. """ from platformio.package.download import FileDownloader from platformio.package.exception import PackageException @@ -70,7 +74,7 @@ def patch_file_downloader() -> None: try: original_init(self, *args, **kwargs) return - except PackageException as e: + except (PackageException, OSError) as e: if attempt < max_retries - 1: delay = 2 ** (attempt + 1) _LOGGER.warning( diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index f771437dd4..64f82ba74f 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -867,6 +867,56 @@ def test_patch_file_downloader_closes_session_and_response_between_retries() -> mock_session.close.assert_called_once() +def test_patch_file_downloader_retries_on_connection_error() -> None: + """Test patch_file_downloader retries on transport-layer errors (OSError subclasses). + + ``requests.exceptions.ConnectionError`` and ``ReadTimeout`` subclass + ``OSError`` and are raised when the connection is aborted before any HTTP + response is parsed -- e.g. ``RemoteDisconnected`` mid-download. These must + retry too, not just ``PackageException``. + """ + mock_exception_cls = type("PackageException", (Exception,), {}) + call_count = 0 + + def failing_init(self, *args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count < 3: + raise ConnectionError( + f"Connection aborted attempt {call_count}: RemoteDisconnected" + ) + + with ( + patch.dict( + "sys.modules", + { + "platformio": MagicMock(), + "platformio.package": MagicMock(), + "platformio.package.download": SimpleNamespace( + FileDownloader=type( + "FileDownloader", (), {"__init__": failing_init} + ) + ), + "platformio.package.exception": SimpleNamespace( + PackageException=mock_exception_cls + ), + }, + ), + patch("time.sleep") as mock_sleep, + ): + runner.patch_file_downloader() + + from platformio.package.download import FileDownloader + + instance = object.__new__(FileDownloader) + FileDownloader.__init__(instance, "http://example.com/file.zip") + + assert call_count == 3 + assert mock_sleep.call_count == 2 + mock_sleep.assert_any_call(2) + mock_sleep.assert_any_call(4) + + def test_patch_file_downloader_idempotent() -> None: """Test patch_file_downloader does not stack wrappers when called multiple times.""" mock_exception_cls = type("PackageException", (Exception,), {})