Restore persistent directory when a download fails

custom_components/hacs/repositories/base.py

@@ -965,35 +965,54 @@ async def async_install_repository(self, *, version: str | None = None, **_) ->
             {"repository": self.data.full_name, "progress": 50},
         )
 
-        if self.repository_manifest.zip_release and self.repository_manifest.filename:
-            await self.download_zip_files(self.validate)
-        else:
-            await self.download_content(version_to_install)
+        download_exception: Exception | None = None
+        try:
+            try:
+                if self.repository_manifest.zip_release and self.repository_manifest.filename:
+                    await self.download_zip_files(self.validate)
+                else:
+                    await self.download_content(version_to_install)
+            except Exception as exception:  # pylint: disable=broad-except
+                download_exception = exception
 
-        self.hacs.async_dispatch(
-            HacsDispatchEvent.REPOSITORY_DOWNLOAD_PROGRESS,
-            {"repository": self.data.full_name, "progress": 70},
-        )
+            self.hacs.async_dispatch(
+                HacsDispatchEvent.REPOSITORY_DOWNLOAD_PROGRESS,
+                {"repository": self.data.full_name, "progress": 70},
+            )
 
-        if self.validate.errors:
-            for error in self.validate.errors:
-                self.logger.error("%s %s", self.string, error)
-            if self.data.installed and not self.content.single:
-                await self.hacs.hass.async_add_executor_job(backup.restore)
-                await self.hacs.hass.async_add_executor_job(backup.cleanup)
-            raise HacsException("Could not download, see log for details")
+            if download_exception is not None or self.validate.errors:
+                for error in self.validate.errors:
+                    self.logger.error("%s %s", self.string, error)
 
-        self.hacs.async_dispatch(
-            HacsDispatchEvent.REPOSITORY_DOWNLOAD_PROGRESS,
-            {"repository": self.data.full_name, "progress": 80},
-        )
+                if self.data.installed and not self.content.single:
+                    await self.hacs.hass.async_add_executor_job(backup.restore)
+                    await self.hacs.hass.async_add_executor_job(backup.cleanup)
 
-        if self.data.installed and not self.content.single:
-            await self.hacs.hass.async_add_executor_job(backup.cleanup)
+                if download_exception is not None:
+                    raise download_exception
+
+                raise HacsException("Could not download, see log for details")
+
+            self.hacs.async_dispatch(
+                HacsDispatchEvent.REPOSITORY_DOWNLOAD_PROGRESS,
+                {"repository": self.data.full_name, "progress": 80},
+            )
 
-        if persistent_directory is not None:
-            await self.hacs.hass.async_add_executor_job(persistent_directory.restore)
-            await self.hacs.hass.async_add_executor_job(persistent_directory.cleanup)
+            if self.data.installed and not self.content.single:
+                await self.hacs.hass.async_add_executor_job(backup.cleanup)
+        finally:
+            # The persistent directory is moved out of the way before the download
+            # and is not part of the backup of the old install, so it needs to be
+            # moved back in, also when the download failed. Restore errors are
+            # only logged, to not mask the failure that got us here.
+            if persistent_directory is not None:
+                try:
+                    await self.hacs.hass.async_add_executor_job(persistent_directory.restore)
+                    await self.hacs.hass.async_add_executor_job(persistent_directory.cleanup)
+                except Exception as exception:  # pylint: disable=broad-except
+                    self.logger.error(
+                        "%s Could not restore persistent directory: %s", self.string, exception
+                    )
 
         if self.validate.success:
             self.data.installed = True

tests/repositories/test_update_repository.py

@@ -1,5 +1,6 @@
 from collections.abc import Generator
 import json
+from pathlib import Path
 import re
 from unittest.mock import patch
 
@@ -8,6 +9,7 @@
 import pytest
 
 from custom_components.hacs.const import DOMAIN
+from custom_components.hacs.exceptions import HacsException
 
 from tests.common import (
     CategoryTestData,
@@ -266,6 +268,119 @@ async def test_update_repository_entity_download_failure(
         )
 
 
+async def test_update_repository_entity_download_exception_restores_backup(
+    hass: HomeAssistant,
+    setup_integration: Generator,
+):
+    """Ensure the old install is restored when the download step raises."""
+    hacs = get_hacs(hass)
+    repo = hacs.repositories.get_by_full_name(
+        "hacs-test-org/integration-basic")
+
+    assert repo is not None
+
+    repo.data.installed = True
+    repo.data.installed_version = "1.0.0"
+
+    await hass.config_entries.async_reload(hacs.configuration.config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    # Get a new HACS instance after reload
+    hacs = get_hacs(hass)
+    repo = hacs.repositories.get_by_full_name(
+        "hacs-test-org/integration-basic")
+
+    installed_file = Path(repo.localpath) / "__init__.py"
+    installed_file.parent.mkdir(parents=True, exist_ok=True)
+    installed_file.write_text("old install")
+
+    er = async_get_entity_registry(hacs.hass)
+    entity_id = er.async_get_entity_id("update", DOMAIN, repo.data.id)
+
+    with patch(
+        "custom_components.hacs.repositories.base.HacsRepository.download_content",
+        side_effect=HacsException("No content to download"),
+    ), pytest.raises(
+        HomeAssistantError,
+        match=re.escape(
+            "Downloading hacs-test-org/integration-basic with version 2.0.0 failed with (No content to download)",
+        ),
+    ):
+        await hass.services.async_call(
+            "update",
+            "install",
+            service_data={"entity_id": entity_id, "version": "2.0.0"},
+            blocking=True,
+        )
+
+    assert installed_file.exists()
+    assert installed_file.read_text() == "old install"
+
+
+async def test_update_repository_entity_download_failure_keeps_persistent_directory(
+    hass: HomeAssistant,
+    setup_integration: Generator,
+    response_mocker: ResponseMocker,
+):
+    """Ensure the persistent directory survives a failed download."""
+    hacs = get_hacs(hass)
+    repo = hacs.repositories.get_by_full_name(
+        "hacs-test-org/integration-basic")
+
+    assert repo is not None
+
+    repo.data.installed = True
+    repo.data.installed_version = "1.0.0"
+
+    await hass.config_entries.async_reload(hacs.configuration.config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    # Get a new HACS instance after reload
+    hacs = get_hacs(hass)
+    repo = hacs.repositories.get_by_full_name(
+        "hacs-test-org/integration-basic")
+
+    # Set a persistent directory on the manifest, and 404 the hacs.json
+    # fetch so the update flow keeps the manifest set here.
+    repo.repository_manifest.persistent_directory = "userfiles"
+    response_mocker.add(
+        "https://api.github.com/repos/hacs-test-org/integration-basic/contents/hacs.json",
+        MockedResponse(status=404, keep=True),
+    )
+
+    response_mocker.add(
+        "https://github.com/hacs-test-org/integration-basic/archive/refs/tags/2.0.0.zip",
+        MockedResponse(status=503),
+    )
+    response_mocker.add(
+        "https://github.com/hacs-test-org/integration-basic/archive/refs/heads/2.0.0.zip",
+        MockedResponse(status=503),
+    )
+
+    persistent_file = Path(repo.localpath) / "userfiles" / "data.txt"
+    persistent_file.parent.mkdir(parents=True, exist_ok=True)
+    persistent_file.write_text("Important user data")
+
+    er = async_get_entity_registry(hacs.hass)
+    entity_id = er.async_get_entity_id("update", DOMAIN, repo.data.id)
+
+    with pytest.raises(
+        HomeAssistantError,
+        match=re.escape(
+            "Downloading hacs-test-org/integration-basic with version 2.0.0 failed with (Could not download, see log for details)",
+        ),
+    ):
+        await hass.services.async_call(
+            "update",
+            "install",
+            service_data={"entity_id": entity_id, "version": "2.0.0"},
+            blocking=True,
+        )
+
+    assert persistent_file.exists()
+    assert persistent_file.read_text() == "Important user data"
+
+
 async def test_update_repository_entity_same_provided_version(
     hass: HomeAssistant, setup_integration: Generator
 ):

tests/snapshots/api-usage/tests/repositories/test_update_repositorytest-update-repository-entity-download-exception-restores-backup.json

@@ -0,0 +1,25 @@
+{
+    "tests/repositories/test_update_repository.py::test_update_repository_entity_download_exception_restores_backup": {
+        "https://api.github.com/repos/hacs-test-org/integration-basic": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/branches/main": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/contents/custom_components/example/manifest.json": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/contents/hacs.json": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/git/trees/1.0.0": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/releases": 1,
+        "https://api.github.com/repos/hacs/integration": 1,
+        "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1,
+        "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1,
+        "https://api.github.com/repos/hacs/integration/git/trees/main": 1,
+        "https://api.github.com/repos/hacs/integration/releases": 1,
+        "https://data-v2.hacs.xyz/appdaemon/data.json": 1,
+        "https://data-v2.hacs.xyz/critical/data.json": 1,
+        "https://data-v2.hacs.xyz/integration/data.json": 1,
+        "https://data-v2.hacs.xyz/plugin/data.json": 1,
+        "https://data-v2.hacs.xyz/python_script/data.json": 1,
+        "https://data-v2.hacs.xyz/removed/data.json": 1,
+        "https://data-v2.hacs.xyz/template/data.json": 1,
+        "https://data-v2.hacs.xyz/theme/data.json": 1,
+        "https://raw.githubusercontent.com/hacs-test-org/integration-basic/1.0.0/README.md": 1,
+        "https://raw.githubusercontent.com/hacs-test-org/integration-basic/2.0.0/hacs.json": 1
+    }
+}

tests/snapshots/api-usage/tests/repositories/test_update_repositorytest-update-repository-entity-download-failure-keeps-persistent-directory.json

@@ -0,0 +1,28 @@
+{
+    "tests/repositories/test_update_repository.py::test_update_repository_entity_download_failure_keeps_persistent_directory": {
+        "https://api.github.com/repos/hacs-test-org/integration-basic": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/branches/main": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/contents/custom_components/example/manifest.json": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/contents/hacs.json": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/git/trees/1.0.0": 1,
+        "https://api.github.com/repos/hacs-test-org/integration-basic/releases": 1,
+        "https://api.github.com/repos/hacs/integration": 1,
+        "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1,
+        "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1,
+        "https://api.github.com/repos/hacs/integration/git/trees/main": 1,
+        "https://api.github.com/repos/hacs/integration/releases": 1,
+        "https://data-v2.hacs.xyz/appdaemon/data.json": 1,
+        "https://data-v2.hacs.xyz/critical/data.json": 1,
+        "https://data-v2.hacs.xyz/integration/data.json": 1,
+        "https://data-v2.hacs.xyz/plugin/data.json": 1,
+        "https://data-v2.hacs.xyz/python_script/data.json": 1,
+        "https://data-v2.hacs.xyz/removed/data.json": 1,
+        "https://data-v2.hacs.xyz/template/data.json": 1,
+        "https://data-v2.hacs.xyz/theme/data.json": 1,
+        "https://github.com/hacs-test-org/integration-basic/archive/refs/heads/2.0.0.zip": 1,
+        "https://github.com/hacs-test-org/integration-basic/archive/refs/tags/2.0.0.zip": 1,
+        "https://raw.githubusercontent.com/hacs-test-org/integration-basic/1.0.0/README.md": 1,
+        "https://raw.githubusercontent.com/hacs-test-org/integration-basic/1.0.0/custom_components/example/manifest.json": 1,
+        "https://raw.githubusercontent.com/hacs-test-org/integration-basic/2.0.0/hacs.json": 1
+    }
+}