Simplify error handling in BackupAgent when a backup is not found (#139754)

Simplify error handling in BackupAgent when backup is not found

homeassistant/components/backup/agent.py

@@ -41,6 +41,8 @@ class BackupAgent(abc.ABC):
     ) -> AsyncIterator[bytes]:
         """Download a backup file.
 
+        Raises BackupNotFound if the backup does not exist.
+
         :param backup_id: The ID of the backup that was returned in async_list_backups.
         :return: An async iterator that yields bytes.
         """
@@ -67,6 +69,8 @@ class BackupAgent(abc.ABC):
     ) -> None:
         """Delete a backup file.
 
+        Raises BackupNotFound if the backup does not exist.
+
         :param backup_id: The ID of the backup that was returned in async_list_backups.
         """
 
@@ -80,7 +84,10 @@ class BackupAgent(abc.ABC):
         backup_id: str,
         **kwargs: Any,
     ) -> AgentBackup | None:
-        """Return a backup."""
+        """Return a backup.
+
+        Raises BackupNotFound if the backup does not exist.
+        """
 
 
 class LocalBackupAgent(BackupAgent):

homeassistant/components/backup/backup.py

@@ -88,13 +88,13 @@ class CoreLocalBackupAgent(LocalBackupAgent):
         self,
         backup_id: str,
         **kwargs: Any,
-    ) -> AgentBackup | None:
+    ) -> AgentBackup:
         """Return a backup."""
         if not self._loaded_backups:
             await self._load_backups()
 
         if backup_id not in self._backups:
-            return None
+            raise BackupNotFound(f"Backup {backup_id} not found")
 
         backup, backup_path = self._backups[backup_id]
         if not await self._hass.async_add_executor_job(backup_path.exists):
@@ -107,7 +107,7 @@ class CoreLocalBackupAgent(LocalBackupAgent):
                 backup_path,
             )
             self._backups.pop(backup_id)
-            return None
+            raise BackupNotFound(f"Backup {backup_id} not found")
 
         return backup
 
@@ -130,10 +130,7 @@ class CoreLocalBackupAgent(LocalBackupAgent):
         if not self._loaded_backups:
             await self._load_backups()
 
-        try:
-            backup_path = self.get_backup_path(backup_id)
-        except BackupNotFound:
-            return
+        backup_path = self.get_backup_path(backup_id)
         await self._hass.async_add_executor_job(backup_path.unlink, True)
         LOGGER.debug("Deleted backup located at %s", backup_path)
         self._backups.pop(backup_id)

homeassistant/components/backup/http.py

@@ -59,10 +59,13 @@ class DownloadBackupView(HomeAssistantView):
         if agent_id not in manager.backup_agents:
             return Response(status=HTTPStatus.BAD_REQUEST)
         agent = manager.backup_agents[agent_id]
-        backup = await agent.async_get_backup(backup_id)
+        try:
+            backup = await agent.async_get_backup(backup_id)
+        except BackupNotFound:
+            return Response(status=HTTPStatus.NOT_FOUND)
 
-        # We don't need to check if the path exists, aiohttp.FileResponse will handle
-        # that
+        # Check for None to be backwards compatible with the old BackupAgent API,
+        # this can be removed in HA Core 2025.10
         if backup is None:
             return Response(status=HTTPStatus.NOT_FOUND)
 
@@ -92,6 +95,8 @@ class DownloadBackupView(HomeAssistantView):
     ) -> StreamResponse | FileResponse | Response:
         if agent_id in manager.local_backup_agents:
             local_agent = manager.local_backup_agents[agent_id]
+            # We don't need to check if the path exists, aiohttp.FileResponse will
+            # handle that
             path = local_agent.get_backup_path(backup_id)
             return FileResponse(path=path.as_posix(), headers=headers)
 

homeassistant/components/backup/manager.py

@@ -64,6 +64,7 @@ from .models import (
     AgentBackup,
     BackupError,
     BackupManagerError,
+    BackupNotFound,
     BackupReaderWriterError,
     BaseBackup,
     Folder,
@@ -648,6 +649,8 @@ class BackupManager:
         )
         for idx, result in enumerate(get_backup_results):
             agent_id = agent_ids[idx]
+            if isinstance(result, BackupNotFound):
+                continue
             if isinstance(result, BackupAgentError):
                 agent_errors[agent_id] = result
                 continue
@@ -659,6 +662,8 @@ class BackupManager:
                 continue
             if isinstance(result, BaseException):
                 raise result  # unexpected error
+            # Check for None to be backwards compatible with the old BackupAgent API,
+            # this can be removed in HA Core 2025.10
             if not result:
                 continue
             if backup is None:
@@ -723,6 +728,8 @@ class BackupManager:
         )
         for idx, result in enumerate(delete_backup_results):
             agent_id = agent_ids[idx]
+            if isinstance(result, BackupNotFound):
+                continue
             if isinstance(result, BackupAgentError):
                 agent_errors[agent_id] = result
                 continue
@@ -832,7 +839,7 @@ class BackupManager:
         agent_errors = {
             backup_id: error
             for backup_id, error in zip(backup_ids, delete_results, strict=True)
-            if error
+            if error and not isinstance(error, BackupNotFound)
         }
         if agent_errors:
             LOGGER.error(
@@ -1264,7 +1271,15 @@ class BackupManager:
     ) -> None:
         """Initiate restoring a backup."""
         agent = self.backup_agents[agent_id]
-        if not await agent.async_get_backup(backup_id):
+        try:
+            backup = await agent.async_get_backup(backup_id)
+        except BackupNotFound as err:
+            raise BackupManagerError(
+                f"Backup {backup_id} not found in agent {agent_id}"
+            ) from err
+        # Check for None to be backwards compatible with the old BackupAgent API,
+        # this can be removed in HA Core 2025.10
+        if not backup:
             raise BackupManagerError(
                 f"Backup {backup_id} not found in agent {agent_id}"
             )
@@ -1352,7 +1367,15 @@ class BackupManager:
             agent = self.backup_agents[agent_id]
         except KeyError as err:
             raise BackupManagerError(f"Invalid agent selected: {agent_id}") from err
-        if not await agent.async_get_backup(backup_id):
+        try:
+            backup = await agent.async_get_backup(backup_id)
+        except BackupNotFound as err:
+            raise BackupManagerError(
+                f"Backup {backup_id} not found in agent {agent_id}"
+            ) from err
+        # Check for None to be backwards compatible with the old BackupAgent API,
+        # this can be removed in HA Core 2025.10
+        if not backup:
             raise BackupManagerError(
                 f"Backup {backup_id} not found in agent {agent_id}"
             )

tests/components/backup/common.py

@@ -67,15 +67,20 @@ async def aiter_from_iter(iterable: Iterable) -> AsyncIterator:
 def mock_backup_agent(name: str, backups: list[AgentBackup] | None = None) -> Mock:
     """Create a mock backup agent."""
 
+    async def delete_backup(backup_id: str, **kwargs: Any) -> None:
+        """Mock delete."""
+        get_backup(backup_id)
+
     async def download_backup(backup_id: str, **kwargs: Any) -> AsyncIterator[bytes]:
         """Mock download."""
-        if not await get_backup(backup_id):
-            raise BackupNotFound
         return aiter_from_iter((backups_data.get(backup_id, b"backup data"),))
 
-    async def get_backup(backup_id: str, **kwargs: Any) -> AgentBackup | None:
+    async def get_backup(backup_id: str, **kwargs: Any) -> AgentBackup:
         """Get a backup."""
-        return next((b for b in backups if b.backup_id == backup_id), None)
+        backup = next((b for b in backups if b.backup_id == backup_id), None)
+        if backup is None:
+            raise BackupNotFound
+        return backup
 
     async def upload_backup(
         *,
@@ -99,7 +104,7 @@ def mock_backup_agent(name: str, backups: list[AgentBackup] | None = None) -> Mo
     mock_agent.unique_id = name
     type(mock_agent).agent_id = BackupAgent.agent_id
     mock_agent.async_delete_backup = AsyncMock(
-        spec_set=[BackupAgent.async_delete_backup]
+        side_effect=delete_backup, spec_set=[BackupAgent.async_delete_backup]
     )
     mock_agent.async_download_backup = AsyncMock(
         side_effect=download_backup, spec_set=[BackupAgent.async_download_backup]