From eb5036854f09963780cd28ea827d66e332a3e53e Mon Sep 17 00:00:00 2001 From: Erik Montnemery <erik@montnemery.com> Date: Tue, 4 Feb 2025 19:49:55 +0100 Subject: [PATCH] Improve error handling when supervisor backups are deleted (#137331) * Improve error handling when supervisor backups are deleted * Move exception definitions --- homeassistant/components/backup/__init__.py | 3 +- homeassistant/components/backup/agent.py | 14 +---- homeassistant/components/backup/backup.py | 4 +- homeassistant/components/backup/http.py | 16 +++--- homeassistant/components/backup/manager.py | 15 +++--- homeassistant/components/backup/models.py | 18 +++++++ homeassistant/components/backup/websocket.py | 6 ++- homeassistant/components/hassio/backup.py | 16 ++++-- .../backup/snapshots/test_websocket.ambr | 22 ++++++++ tests/components/backup/test_http.py | 52 ++++++++++++++++++- tests/components/backup/test_websocket.py | 37 +++++++++++++ tests/components/hassio/test_backup.py | 41 ++++++++++----- 12 files changed, 195 insertions(+), 49 deletions(-) diff --git a/homeassistant/components/backup/__init__.py b/homeassistant/components/backup/__init__.py index 449b07e7b26..71a4f5ea41a 100644 --- a/homeassistant/components/backup/__init__.py +++ b/homeassistant/components/backup/__init__.py @@ -37,7 +37,7 @@ from .manager import ( RestoreBackupState, WrittenBackup, ) -from .models import AddonInfo, AgentBackup, Folder +from .models import AddonInfo, AgentBackup, BackupNotFound, Folder from .util import suggested_filename, suggested_filename_from_name_date from .websocket import async_register_websocket_handlers @@ -48,6 +48,7 @@ __all__ = [ "BackupAgentError", "BackupAgentPlatformProtocol", "BackupManagerError", + "BackupNotFound", "BackupPlatformProtocol", "BackupReaderWriter", "BackupReaderWriterError", diff --git a/homeassistant/components/backup/agent.py b/homeassistant/components/backup/agent.py index 297ccd6f685..9530f386c7b 100644 --- a/homeassistant/components/backup/agent.py +++ b/homeassistant/components/backup/agent.py @@ -11,13 +11,7 @@ from propcache.api import cached_property from homeassistant.core import HomeAssistant, callback -from .models import AgentBackup, BackupError - - -class BackupAgentError(BackupError): - """Base class for backup agent errors.""" - - error_code = "backup_agent_error" +from .models import AgentBackup, BackupAgentError class BackupAgentUnreachableError(BackupAgentError): @@ -27,12 +21,6 @@ class BackupAgentUnreachableError(BackupAgentError): _message = "The backup agent is unreachable." -class BackupNotFound(BackupAgentError): - """Raised when a backup is not found.""" - - error_code = "backup_not_found" - - class BackupAgent(abc.ABC): """Backup agent interface.""" diff --git a/homeassistant/components/backup/backup.py b/homeassistant/components/backup/backup.py index b6282186c06..c3a46a6ab1f 100644 --- a/homeassistant/components/backup/backup.py +++ b/homeassistant/components/backup/backup.py @@ -11,9 +11,9 @@ from typing import Any from homeassistant.core import HomeAssistant from homeassistant.helpers.hassio import is_hassio -from .agent import BackupAgent, BackupNotFound, LocalBackupAgent +from .agent import BackupAgent, LocalBackupAgent from .const import DOMAIN, LOGGER -from .models import AgentBackup +from .models import AgentBackup, BackupNotFound from .util import read_backup, suggested_filename diff --git a/homeassistant/components/backup/http.py b/homeassistant/components/backup/http.py index 6b06db4601d..58f44d4a449 100644 --- a/homeassistant/components/backup/http.py +++ b/homeassistant/components/backup/http.py @@ -21,6 +21,7 @@ from . import util from .agent import BackupAgent from .const import DATA_MANAGER from .manager import BackupManager +from .models import BackupNotFound @callback @@ -69,13 +70,16 @@ class DownloadBackupView(HomeAssistantView): CONTENT_DISPOSITION: f"attachment; filename={slugify(backup.name)}.tar" } - if not password or not backup.protected: - return await self._send_backup_no_password( - request, headers, backup_id, agent_id, agent, manager + try: + if not password or not backup.protected: + return await self._send_backup_no_password( + request, headers, backup_id, agent_id, agent, manager + ) + return await self._send_backup_with_password( + hass, request, headers, backup_id, agent_id, password, agent, manager ) - return await self._send_backup_with_password( - hass, request, headers, backup_id, agent_id, password, agent, manager - ) + except BackupNotFound: + return Response(status=HTTPStatus.NOT_FOUND) async def _send_backup_no_password( self, diff --git a/homeassistant/components/backup/manager.py b/homeassistant/components/backup/manager.py index 42b5f522ecd..fa9ca956c22 100644 --- a/homeassistant/components/backup/manager.py +++ b/homeassistant/components/backup/manager.py @@ -50,7 +50,14 @@ from .const import ( EXCLUDE_FROM_BACKUP, LOGGER, ) -from .models import AgentBackup, BackupError, BackupManagerError, BaseBackup, Folder +from .models import ( + AgentBackup, + BackupError, + BackupManagerError, + BackupReaderWriterError, + BaseBackup, + Folder, +) from .store import BackupStore from .util import ( AsyncIteratorReader, @@ -274,12 +281,6 @@ class BackupReaderWriter(abc.ABC): """Get restore events after core restart.""" -class BackupReaderWriterError(BackupError): - """Backup reader/writer error.""" - - error_code = "backup_reader_writer_error" - - class IncorrectPasswordError(BackupReaderWriterError): """Raised when the password is incorrect.""" diff --git a/homeassistant/components/backup/models.py b/homeassistant/components/backup/models.py index 62118b7944f..95c5ef9809d 100644 --- a/homeassistant/components/backup/models.py +++ b/homeassistant/components/backup/models.py @@ -77,7 +77,25 @@ class BackupError(HomeAssistantError): error_code = "unknown" +class BackupAgentError(BackupError): + """Base class for backup agent errors.""" + + error_code = "backup_agent_error" + + class BackupManagerError(BackupError): """Backup manager error.""" error_code = "backup_manager_error" + + +class BackupReaderWriterError(BackupError): + """Backup reader/writer error.""" + + error_code = "backup_reader_writer_error" + + +class BackupNotFound(BackupAgentError, BackupManagerError): + """Raised when a backup is not found.""" + + error_code = "backup_not_found" diff --git a/homeassistant/components/backup/websocket.py b/homeassistant/components/backup/websocket.py index e130b9e950f..b6d092e1913 100644 --- a/homeassistant/components/backup/websocket.py +++ b/homeassistant/components/backup/websocket.py @@ -15,7 +15,7 @@ from .manager import ( IncorrectPasswordError, ManagerStateEvent, ) -from .models import Folder +from .models import BackupNotFound, Folder @callback @@ -151,6 +151,8 @@ async def handle_restore( restore_folders=msg.get("restore_folders"), restore_homeassistant=msg["restore_homeassistant"], ) + except BackupNotFound: + connection.send_error(msg["id"], "backup_not_found", "Backup not found") except IncorrectPasswordError: connection.send_error(msg["id"], "password_incorrect", "Incorrect password") else: @@ -179,6 +181,8 @@ async def handle_can_decrypt_on_download( agent_id=msg["agent_id"], password=msg.get("password"), ) + except BackupNotFound: + connection.send_error(msg["id"], "backup_not_found", "Backup not found") except IncorrectPasswordError: connection.send_error(msg["id"], "password_incorrect", "Incorrect password") except DecryptOnDowloadNotSupported: diff --git a/homeassistant/components/hassio/backup.py b/homeassistant/components/hassio/backup.py index 4103be14306..142c5fc01ce 100644 --- a/homeassistant/components/hassio/backup.py +++ b/homeassistant/components/hassio/backup.py @@ -27,6 +27,7 @@ from homeassistant.components.backup import ( AgentBackup, BackupAgent, BackupManagerError, + BackupNotFound, BackupReaderWriter, BackupReaderWriterError, CreateBackupEvent, @@ -162,10 +163,15 @@ class SupervisorBackupAgent(BackupAgent): **kwargs: Any, ) -> AsyncIterator[bytes]: """Download a backup file.""" - return await self._client.backups.download_backup( - backup_id, - options=supervisor_backups.DownloadBackupOptions(location=self.location), - ) + try: + return await self._client.backups.download_backup( + backup_id, + options=supervisor_backups.DownloadBackupOptions( + location=self.location + ), + ) + except SupervisorNotFoundError as err: + raise BackupNotFound from err async def async_upload_backup( self, @@ -528,6 +534,8 @@ class SupervisorBackupReaderWriter(BackupReaderWriter): location=restore_location, ), ) + except SupervisorNotFoundError as err: + raise BackupNotFound from err except SupervisorBadRequestError as err: # Supervisor currently does not transmit machine parsable error types message = err.args[0] diff --git a/tests/components/backup/snapshots/test_websocket.ambr b/tests/components/backup/snapshots/test_websocket.ambr index d5d15e98da6..421432fb66e 100644 --- a/tests/components/backup/snapshots/test_websocket.ambr +++ b/tests/components/backup/snapshots/test_websocket.ambr @@ -229,6 +229,28 @@ 'type': 'result', }) # --- +# name: test_can_decrypt_on_download_with_agent_error[BackupAgentError] + dict({ + 'error': dict({ + 'code': 'home_assistant_error', + 'message': 'Unknown error', + }), + 'id': 1, + 'success': False, + 'type': 'result', + }) +# --- +# name: test_can_decrypt_on_download_with_agent_error[BackupNotFound] + dict({ + 'error': dict({ + 'code': 'backup_not_found', + 'message': 'Backup not found', + }), + 'id': 1, + 'success': False, + 'type': 'result', + }) +# --- # name: test_config_info[storage_data0] dict({ 'id': 1, diff --git a/tests/components/backup/test_http.py b/tests/components/backup/test_http.py index aac39c04d31..24fd15fc4fe 100644 --- a/tests/components/backup/test_http.py +++ b/tests/components/backup/test_http.py @@ -11,7 +11,13 @@ from unittest.mock import patch from aiohttp import web import pytest -from homeassistant.components.backup import AddonInfo, AgentBackup, Folder +from homeassistant.components.backup import ( + AddonInfo, + AgentBackup, + BackupAgentError, + BackupNotFound, + Folder, +) from homeassistant.components.backup.const import DATA_MANAGER, DOMAIN from homeassistant.core import HomeAssistant @@ -141,6 +147,50 @@ async def test_downloading_remote_encrypted_backup( await _test_downloading_encrypted_backup(hass_client, "domain.test") +@pytest.mark.parametrize( + ("error", "status"), + [ + (BackupAgentError, 500), + (BackupNotFound, 404), + ], +) +@patch.object(BackupAgentTest, "async_download_backup") +async def test_downloading_remote_encrypted_backup_with_error( + download_mock, + hass: HomeAssistant, + hass_client: ClientSessionGenerator, + error: Exception, + status: int, +) -> None: + """Test downloading a local backup file.""" + await setup_backup_integration(hass) + hass.data[DATA_MANAGER].backup_agents["domain.test"] = BackupAgentTest( + "test", + [ + AgentBackup( + addons=[AddonInfo(name="Test", slug="test", version="1.0.0")], + backup_id="abc123", + database_included=True, + date="1970-01-01T00:00:00Z", + extra_metadata={}, + folders=[Folder.MEDIA, Folder.SHARE], + homeassistant_included=True, + homeassistant_version="2024.12.0", + name="Test", + protected=True, + size=13, + ) + ], + ) + + download_mock.side_effect = error + client = await hass_client() + resp = await client.get( + "/api/backup/download/abc123?agent_id=domain.test&password=blah" + ) + assert resp.status == status + + async def _test_downloading_encrypted_backup( hass_client: ClientSessionGenerator, agent_id: str, diff --git a/tests/components/backup/test_websocket.py b/tests/components/backup/test_websocket.py index 613c0b69b6b..5af6d595938 100644 --- a/tests/components/backup/test_websocket.py +++ b/tests/components/backup/test_websocket.py @@ -12,6 +12,7 @@ from homeassistant.components.backup import ( AgentBackup, BackupAgentError, BackupAgentPlatformProtocol, + BackupNotFound, BackupReaderWriterError, Folder, store, @@ -2967,3 +2968,39 @@ async def test_can_decrypt_on_download( } ) assert await client.receive_json() == snapshot + + +@pytest.mark.parametrize( + "error", + [ + BackupAgentError, + BackupNotFound, + ], +) +@pytest.mark.usefixtures("mock_backups") +async def test_can_decrypt_on_download_with_agent_error( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + snapshot: SnapshotAssertion, + error: Exception, +) -> None: + """Test can decrypt on download.""" + + await setup_backup_integration( + hass, + with_hassio=False, + backups={"test.remote": [TEST_BACKUP_ABC123]}, + remote_agents=["remote"], + ) + client = await hass_ws_client(hass) + + with patch.object(BackupAgentTest, "async_download_backup", side_effect=error): + await client.send_json_auto_id( + { + "type": "backup/can_decrypt_on_download", + "backup_id": TEST_BACKUP_ABC123.backup_id, + "agent_id": "test.remote", + "password": "hunter2", + } + ) + assert await client.receive_json() == snapshot diff --git a/tests/components/hassio/test_backup.py b/tests/components/hassio/test_backup.py index 866431d6b19..496dc93df32 100644 --- a/tests/components/hassio/test_backup.py +++ b/tests/components/hassio/test_backup.py @@ -584,22 +584,29 @@ async def test_agent_download( ) +@pytest.mark.parametrize( + ("backup_info", "backup_id", "agent_id"), + [ + (TEST_BACKUP_DETAILS_3, "unknown", "hassio.local"), + (TEST_BACKUP_DETAILS_3, TEST_BACKUP_DETAILS_3.slug, "hassio.local"), + (TEST_BACKUP_DETAILS, TEST_BACKUP_DETAILS_3.slug, "hassio.local"), + ], +) @pytest.mark.usefixtures("hassio_client", "setup_backup_integration") async def test_agent_download_unavailable_backup( hass: HomeAssistant, hass_client: ClientSessionGenerator, supervisor_client: AsyncMock, + agent_id: str, + backup_id: str, + backup_info: supervisor_backups.BackupComplete, ) -> None: """Test agent download backup which does not exist.""" client = await hass_client() - backup_id = "abc123" - supervisor_client.backups.list.return_value = [TEST_BACKUP_3] - supervisor_client.backups.backup_info.return_value = TEST_BACKUP_DETAILS_3 - supervisor_client.backups.download_backup.return_value.__aiter__.return_value = ( - iter((b"backup data",)) - ) + supervisor_client.backups.backup_info.return_value = backup_info + supervisor_client.backups.download_backup.side_effect = SupervisorNotFoundError - resp = await client.get(f"/api/backup/download/{backup_id}?agent_id=hassio.local") + resp = await client.get(f"/api/backup/download/{backup_id}?agent_id={agent_id}") assert resp.status == 404 @@ -2129,14 +2136,22 @@ async def test_reader_writer_restore_report_progress( @pytest.mark.parametrize( - ("supervisor_error_string", "expected_error_code", "expected_reason"), + ("supervisor_error", "expected_error_code", "expected_reason"), [ - ("Invalid password for backup", "password_incorrect", "password_incorrect"), ( - "Backup was made on supervisor version 2025.12.0, can't restore on 2024.12.0. Must update supervisor first.", + SupervisorBadRequestError("Invalid password for backup"), + "password_incorrect", + "password_incorrect", + ), + ( + SupervisorBadRequestError( + "Backup was made on supervisor version 2025.12.0, can't " + "restore on 2024.12.0. Must update supervisor first." + ), "home_assistant_error", "unknown_error", ), + (SupervisorNotFoundError(), "backup_not_found", "backup_not_found"), ], ) @pytest.mark.usefixtures("hassio_client", "setup_backup_integration") @@ -2144,15 +2159,13 @@ async def test_reader_writer_restore_error( hass: HomeAssistant, hass_ws_client: WebSocketGenerator, supervisor_client: AsyncMock, - supervisor_error_string: str, + supervisor_error: Exception, expected_error_code: str, expected_reason: str, ) -> None: """Test restoring a backup.""" client = await hass_ws_client(hass) - supervisor_client.backups.partial_restore.side_effect = SupervisorBadRequestError( - supervisor_error_string - ) + supervisor_client.backups.partial_restore.side_effect = supervisor_error supervisor_client.backups.list.return_value = [TEST_BACKUP] supervisor_client.backups.backup_info.return_value = TEST_BACKUP_DETAILS -- GitLab