From ad55f5909c1a62c53df70911118939b672641f79 Mon Sep 17 00:00:00 2001
From: Akhil Narang <me@akhilnarang.dev>
Date: Wed, 24 Apr 2024 16:23:38 +0530
Subject: [PATCH] fix(backup): delete failed backup even if something fails
 during the backup process

Not just rollback _after_ a step

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
---
 frappe/utils/backups.py | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py
index 16922ccf7a..f31a6c3965 100644
--- a/frappe/utils/backups.py
+++ b/frappe/utils/backups.py
@@ -188,14 +188,12 @@ class BackupGenerator:
 			self.set_backup_file_name()
 
 		if not (last_db and last_file and last_private_file and site_config_backup_path):
-			self.take_dump()
-			self.add_to_rollback(lambda: os.remove(self.backup_path_db))
-			self.copy_site_config()
-			self.add_to_rollback(lambda: os.remove(self.backup_path_conf))
+			self.delete_if_step_fails(self.take_dump, self.backup_path_db)
+			self.delete_if_step_fails(self.copy_site_config, self.backup_path_conf)
 			if not ignore_files:
-				self.backup_files()
-				self.add_to_rollback(lambda: os.remove(self.backup_path_files))
-				self.add_to_rollback(lambda: os.remove(self.backup_path_private_files))
+				self.delete_if_step_fails(
+					self.backup_files, self.backup_path_files, self.backup_path_private_files
+				)
 
 			if frappe.get_system_settings("encrypt_backup"):
 				self.backup_encryption()
@@ -492,6 +490,24 @@ download only after 24 hours."""
 		if self.rollback_callback:
 			self.rollback_callback.add(func)
 
+	def delete_if_step_fails(self, step: Callable, *paths: str):
+		"""
+		Deletes the given path if the given step fails
+
+		:param step: The step to execute
+		:param paths: The paths to delete
+		:return: Nothing
+		"""
+		try:
+			step()
+		except Exception as e:
+			for path in paths:
+				if os.path.exists(path):
+					os.remove(path)
+			raise e
+		for path in paths:
+			self.add_to_rollback(lambda: os.remove(path))
+
 
 def _get_tables(doctypes: list[str], existing_tables: list[str]) -> list[str]:
 	"""Return a list of tables for the given doctypes that exist in the database."""