Skip to content

Restore persistent directory when a download fails #5378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
frenck wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Restore persistent directory when a download fails #5378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
612da79
Restore persistent directory when a download fails
frenck Jul 12, 2026
796a52c
Restore backup on download exceptions and guard persistent directory …
frenck Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 43 additions & 24 deletions custom_components/hacs/repositories/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
115 changes: 115 additions & 0 deletions tests/repositories/test_update_repository.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections.abc import Generator
import json
from pathlib import Path
import re
from unittest.mock import patch

Expand All @@ -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,
Expand Down Expand Up @@ -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)

@MartinHjelmare MartinHjelmare Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the repo.localpath inside the configuration directory which we've set to be inside a tmp directory for the tests?

@frenck frenck Aug 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. For an integration localpath is {hacs.core.config_path}/custom_components/{domain}, and hacs.core.config_path is set from hass.config.path(), which the hass fixture points at pytest's tmp dir. Printed during a run it resolves to /tmp/pytest-of-<user>/pytest-<n>/test_update_repository_entity_0/custom_components/example, so the test only writes inside the temporary configuration directory.

The backup and persistent directory that the code moves things through do land in the real system temp dir (tempfile.gettempdir(), so /tmp/hacs_backup/ and /tmp/hacs_persistent_integration/), but that is existing behavior of utils/backup.py that all the other download tests share, not something this test introduces.

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
):
Expand Down
25 changes: 25 additions & 0 deletions ...st_update_repositorytest-update-repository-entity-download-exception-restores-backup.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
}

@MartinHjelmare MartinHjelmare Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

@frenck frenck Aug 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These snapshots are generated, not hand written. track_api_usage in tests/conftest.py passes safe_json_dumps(...) to snapshots.assert_match, and that output has no trailing newline, so none of the 96 existing api-usage snapshots have one either. Adding it makes the next run fail:

E  AssertionError: value does not match the expected value in snapshot
E  assert '{\n    "test...: 1\n    }\n}' == '{\n    "test...1\n    }\n}\n'

Leaving these as the fixture writes them.

28 changes: 28 additions & 0 deletions ..._repositorytest-update-repository-entity-download-failure-keeps-persistent-directory.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
}

@MartinHjelmare MartinHjelmare Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

@frenck frenck Aug 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These snapshots are generated, not hand written. track_api_usage in tests/conftest.py passes safe_json_dumps(...) to snapshots.assert_match, and that output has no trailing newline, so none of the 96 existing api-usage snapshots have one either. Adding it makes the next run fail:

E  AssertionError: value does not match the expected value in snapshot
E  assert '{\n    "test...: 1\n    }\n}' == '{\n    "test...1\n    }\n}\n'

Leaving these as the fixture writes them.

Loading