versione con tab

action_handler.py

@@ -4,7 +4,7 @@ import os
 
 # Import dependencies
 from git_commands import GitCommands, GitCommandError
-from backup_handler import BackupHandler # To perform backups
+from backup_handler import BackupHandler
 
 class ActionHandler:
     """Handles the execution logic for core application actions."""
@@ -20,44 +20,47 @@ class ActionHandler:
         """
         self.logger = logger
         self.git_commands = git_commands
-        self.backup_handler = backup_handler # Store backup handler instance
-
+        self.backup_handler = backup_handler
 
     def _perform_backup_if_enabled(self, svn_path, profile_name,
                                    autobackup_enabled, backup_base_dir,
                                    excluded_extensions, excluded_dirs):
         """
-        Performs backup if enabled.
+        Performs backup if enabled. Raises IOError on backup failure.
 
         Args:
             svn_path (str): Path to the repository.
             profile_name (str): Name of the profile (for backup naming).
             autobackup_enabled (bool): Flag from settings.
             backup_base_dir (str): Destination directory for backups.
-            excluded_extensions (set): Set of file extensions to exclude.
-            excluded_dirs (set): Set of directory names to exclude.
+            excluded_extensions (set): File extensions to exclude.
+            excluded_dirs (set): Directory names to exclude.
 
-        Returns:
-            bool: True if backup succeeded or was not needed, False on failure.
+        Raises:
+            IOError: If the backup process fails.
         """
         if not autobackup_enabled:
             self.logger.debug("Autobackup disabled, skipping backup.")
-            return True # Not an error, just skipped
+            return # Backup not needed, proceed
 
         self.logger.info("Autobackup enabled. Starting backup...")
         try:
-            self.backup_handler.create_zip_backup(
-                svn_path, backup_base_dir, profile_name,
-                excluded_extensions, excluded_dirs
+            # Delegate backup creation to the BackupHandler instance
+            backup_path = self.backup_handler.create_zip_backup(
+                svn_path,
+                backup_base_dir,
+                profile_name,
+                excluded_extensions,
+                excluded_dirs
             )
-            self.logger.info("Backup completed successfully.")
-            return True # Indicate backup success
+            # Log success if backup handler doesn't raise an error
+            self.logger.info(f"Backup completed successfully: {backup_path}")
+            # No explicit return value needed on success, failure indicated by exception
+
         except Exception as backup_e:
-             # Log error and indicate backup failure
+             # Log error and re-raise as IOError to signal critical failure
              self.logger.error(f"Backup failed: {backup_e}", exc_info=True)
-             # Let the caller decide how to handle backup failure (e.g., show error)
-             # For now, just return False
-             return False
+             raise IOError(f"Autobackup failed: {backup_e}") from backup_e
 
 
     def execute_prepare_repo(self, svn_path):
@@ -68,17 +71,17 @@ class ActionHandler:
             svn_path (str): Validated path to the repository.
 
         Returns:
-            bool: True on success.
+            bool: True on success. (Always True if no exception raised)
         Raises:
             ValueError: If repository is already prepared.
             GitCommandError/IOError/Exception: If preparation fails.
         """
         self.logger.info(f"Executing preparation for: {svn_path}")
-        # Check if already prepared first
+        # Check if already prepared first to provide specific feedback
         git_dir = os.path.join(svn_path, ".git")
         if os.path.exists(git_dir):
             self.logger.warning("Repository is already prepared.")
-            # Raise specific error that UI can interpret
+            # Raise ValueError to signal this specific status to the UI layer
             raise ValueError("Repository is already prepared.")
 
         # Attempt preparation using GitCommands
@@ -87,8 +90,8 @@ class ActionHandler:
             self.logger.info("Repository prepared successfully.")
             return True
         except (GitCommandError, ValueError, IOError) as e:
-            # Log and re-raise known errors
-            self.logger.error(f"Failed to prepare repository: {e}")
+            # Log and re-raise known errors for the UI layer to handle
+            self.logger.error(f"Failed to prepare repository: {e}", exc_info=True)
             raise
         except Exception as e:
             # Log and re-raise unexpected errors
@@ -117,44 +120,45 @@ class ActionHandler:
         Returns:
             str or None: Path to created bundle on success, None if empty/not created.
         Raises:
-            Exception: Relays exceptions from backup, commit, or bundle creation.
+            IOError: If backup fails.
+            GitCommandError/ValueError/Exception: If commit or bundle creation fails.
         """
         # --- Backup Step ---
-        if autobackup_enabled:
-            backup_success = self._perform_backup_if_enabled(
-                svn_path, profile_name, True, backup_base_dir,
-                excluded_extensions, excluded_dirs
-            )
-            if not backup_success:
-                # Raise specific error if backup fails? Or just log and continue?
-                # For now, let's raise an error to stop the process.
-                raise IOError("Autobackup failed. Bundle creation aborted.")
+        # _perform_backup_if_enabled raises IOError on failure
+        self._perform_backup_if_enabled(
+            svn_path, profile_name, autobackup_enabled, backup_base_dir,
+            excluded_extensions, excluded_dirs
+        )
 
         # --- Autocommit Step ---
         if autocommit_enabled:
             self.logger.info("Autocommit before bundle is enabled.")
             try:
+                # Check for changes before attempting commit
                 has_changes = self.git_commands.git_status_has_changes(svn_path)
                 if has_changes:
                     self.logger.info("Changes detected, performing autocommit...")
+                    # Use provided message or generate default
                     commit_msg_to_use = commit_message if commit_message else \
                                      f"Autocommit '{profile_name}' before bundle"
                     self.logger.debug(f"Using autocommit message: '{commit_msg_to_use}'")
-                    # Perform commit (logs success/nothing internally)
+                    # Perform commit (raises error on failure)
                     self.git_commands.git_commit(svn_path, commit_msg_to_use)
+                    # Log based on return value? git_commit already logs detail.
+                    self.logger.info("Autocommit attempt finished.")
                 else:
                     self.logger.info("No changes detected for autocommit.")
             except Exception as commit_e:
-                # Let caller handle commit errors
+                # Log and re-raise commit error to stop the process
                 self.logger.error(f"Autocommit failed: {commit_e}", exc_info=True)
-                raise commit_e # Re-raise to abort bundle creation
+                raise commit_e
 
         # --- Create Bundle Step ---
         self.logger.info(f"Creating bundle file: {bundle_full_path}")
         try:
-            # Execute command via GitCommands
+            # Execute command (raises error on failure)
             self.git_commands.create_git_bundle(svn_path, bundle_full_path)
-            # Check result after command execution
+            # Check result: file exists and is not empty
             bundle_exists = os.path.exists(bundle_full_path)
             bundle_not_empty = bundle_exists and os.path.getsize(bundle_full_path) > 0
             if bundle_exists and bundle_not_empty:
@@ -164,12 +168,14 @@ class ActionHandler:
                  # Bundle empty or not created (logged by GitCommands)
                  self.logger.warning("Bundle file not created or is empty.")
                  if bundle_exists and not bundle_not_empty:
-                     try: os.remove(bundle_full_path) # Clean up
-                     except OSError: pass
+                     # Clean up empty file
+                     try:
+                         os.remove(bundle_full_path)
+                     except OSError:
+                         self.logger.warning(f"Could not remove empty bundle file.")
                  return None # Indicate non-fatal issue (empty bundle)
-
         except Exception as bundle_e:
-             # Let caller handle bundle creation errors
+             # Log and re-raise bundle creation errors
              self.logger.error(f"Bundle creation failed: {bundle_e}", exc_info=True)
              raise bundle_e
 
@@ -183,27 +189,25 @@ class ActionHandler:
         Args: See execute_create_bundle args, excluding commit related ones.
 
         Raises:
-            Exception: Relays exceptions from backup or fetch/merge operations.
+            IOError: If backup fails.
+            GitCommandError/Exception: If fetch or merge fails (incl. conflicts).
         """
         # --- Backup Step ---
-        if autobackup_enabled:
-            backup_success = self._perform_backup_if_enabled(
-                svn_path, profile_name, True, backup_base_dir,
-                excluded_extensions, excluded_dirs
-            )
-            if not backup_success:
-                raise IOError("Autobackup failed. Fetch operation aborted.")
+        # Raises IOError on failure
+        self._perform_backup_if_enabled(
+            svn_path, profile_name, autobackup_enabled, backup_base_dir,
+            excluded_extensions, excluded_dirs
+        )
 
         # --- Fetch and Merge Step ---
         self.logger.info(f"Fetching into '{svn_path}' from: {bundle_full_path}")
         try:
-            # Execute fetch/merge via GitCommands
-            # This method might raise GitCommandError for conflicts
+            # Delegate to GitCommands; it raises GitCommandError on conflict/failure
             self.git_commands.fetch_from_git_bundle(svn_path, bundle_full_path)
-            self.logger.info("Fetch/merge process completed.")
-            # Return True or some status? For now, rely on exceptions for errors.
+            self.logger.info("Fetch/merge process completed successfully.")
+            # No return value needed, success indicated by no exception
         except Exception as fetch_e:
-             # Let caller handle fetch/merge errors (incl. conflicts)
+             # Log and re-raise any error from fetch/merge
              self.logger.error(f"Fetch/merge failed: {fetch_e}", exc_info=True)
              raise fetch_e
 
@@ -214,45 +218,46 @@ class ActionHandler:
 
         Args:
             svn_path (str): Validated path to the repository.
-            commit_message (str): The commit message (checked not empty by caller).
+            commit_message (str): The commit message (must not be empty).
 
         Returns:
-            bool: True if a commit was made, False if no changes were committed.
+            bool: True if commit made, False if no changes.
         Raises:
-            GitCommandError: If the commit command fails.
-            Exception: For other unexpected errors.
+            ValueError: If commit_message is empty.
+            GitCommandError/Exception: If commit command fails.
         """
         if not commit_message:
-             # This validation should ideally happen before calling this method
-             self.logger.error("Manual commit attempted with empty message.")
+             # Should be validated by caller (UI layer)
+             self.logger.error("Manual commit attempt with empty message.")
              raise ValueError("Commit message cannot be empty.")
 
         self.logger.info(f"Executing manual commit for: {svn_path}")
         try:
-            # git_commit handles staging and committing
+            # git_commit handles staging and commit attempt
+            # It returns True/False and raises GitCommandError on failure
             commit_made = self.git_commands.git_commit(svn_path, commit_message)
-            if commit_made:
-                self.logger.info("Manual commit successful.")
-            else:
-                self.logger.info("Manual commit: Nothing to commit.")
-            return commit_made # Return status
+            return commit_made
         except Exception as e:
+            # Catch and re-raise errors from git_commit
             self.logger.error(f"Manual commit failed: {e}", exc_info=True)
-            raise # Re-raise for caller to handle
+            raise
 
 
     def execute_create_tag(self, svn_path, commit_message, tag_name, tag_message):
         """
         Executes tag creation, including pre-commit using commit_message if needed.
 
-        Args: See execute_manual_commit and add tag_name, tag_message.
+        Args:
+            svn_path (str): Validated path to repository.
+            commit_message (str): Message for pre-tag commit (if needed). Can be empty.
+            tag_name (str): Name for the new tag.
+            tag_message (str): Annotation message for the new tag.
 
         Returns:
             bool: True on successful tag creation.
         Raises:
-            ValueError: If validation fails (e.g., changes exist but no commit message).
-            GitCommandError: If commit or tag command fails.
-            Exception: For unexpected errors.
+            ValueError: If validation fails (e.g., changes exist, no commit message).
+            GitCommandError/Exception: If commit or tag command fails.
         """
         self.logger.info(f"Executing create tag '{tag_name}' for: {svn_path}")
 
@@ -265,31 +270,34 @@ class ActionHandler:
                     # Block if changes exist but no message provided
                     msg = "Changes exist. Commit message required before tagging."
                     self.logger.error(f"Tag creation blocked: {msg}")
-                    raise ValueError(msg)
+                    raise ValueError(msg) # Raise error for UI layer
 
                 # Perform the pre-tag commit with the provided message
                 self.logger.debug(f"Performing pre-tag commit: '{commit_message}'")
+                # git_commit raises error on failure, returns bool on success/no changes
                 self.git_commands.git_commit(svn_path, commit_message)
-                # Log success/nothing based on return value if needed
                 self.logger.info("Pre-tag commit attempt finished.")
             else:
-                self.logger.info("No uncommitted changes detected.")
+                self.logger.info("No uncommitted changes detected before tagging.")
 
         except Exception as e:
             # Catch errors during status check or commit
-            self.logger.error(f"Error during pre-tag commit step: {e}", exc_info=True)
+            self.logger.error(f"Error during pre-tag commit step: {e}",
+                              exc_info=True)
             raise # Re-raise commit-related errors
 
         # --- Create Tag Step ---
         self.logger.info(f"Proceeding to create tag '{tag_name}'...")
         try:
-            # git_commands.create_tag handles validation and execution
+            # git_commands.create_tag handles its own validation and execution
+            # It raises ValueError for invalid name, GitCommandError for exists/fail
             self.git_commands.create_tag(svn_path, tag_name, tag_message)
             self.logger.info(f"Tag '{tag_name}' created successfully.")
             return True # Indicate success
         except Exception as e:
-            # Catch errors during tag creation (e.g., exists, invalid name)
-            self.logger.error(f"Failed to create tag '{tag_name}': {e}", exc_info=True)
+            # Catch errors during tag creation
+            self.logger.error(f"Failed to create tag '{tag_name}': {e}",
+                              exc_info=True)
             raise # Re-raise tag creation errors
 
 
@@ -299,47 +307,155 @@ class ActionHandler:
 
         Args:
             svn_path (str): Validated path to repository.
-            tag_name (str): The tag name to check out (already validated).
+            tag_name (str): The tag name to check out (already validated by caller).
 
         Returns:
             bool: True on successful checkout.
         Raises:
             ValueError: If uncommitted changes exist.
-            GitCommandError: If status check or checkout command fails.
-            Exception: For unexpected errors.
+            GitCommandError/Exception: If status check or checkout fails.
         """
         if not tag_name:
-            # Should be validated by caller, but double-check
-            raise ValueError("Tag name required for checkout.")
+            raise ValueError("Tag name required for checkout.") # Should be caught earlier
 
-        self.logger.info(f"Executing checkout for tag '{tag_name}' in: {svn_path}")
+        self.logger.info(f"Executing checkout tag '{tag_name}' in: {svn_path}")
 
         # --- Check for Uncommitted Changes ---
         try:
             has_changes = self.git_commands.git_status_has_changes(svn_path)
             if has_changes:
-                self.logger.error("Checkout blocked: Uncommitted changes exist.")
-                # Raise specific error for UI to handle clearly
-                raise ValueError("Uncommitted changes exist. Commit or stash first.")
+                msg = "Uncommitted changes exist. Commit or stash first."
+                self.logger.error(f"Checkout blocked: {msg}")
+                raise ValueError(msg) # Raise specific error for UI
             self.logger.debug("No uncommitted changes found.")
         except Exception as e:
              # Catch errors during status check
-             self.logger.error(f"Error checking status before checkout: {e}",
+             self.logger.error(f"Status check error before checkout: {e}",
                                exc_info=True)
              raise # Re-raise status check errors
 
         # --- Execute Checkout ---
         try:
+            # git_commands.checkout_tag raises GitCommandError on failure
             checkout_success = self.git_commands.checkout_tag(svn_path, tag_name)
-            # git_commands.checkout_tag raises error on failure
-            if checkout_success:
-                 self.logger.info(f"Tag '{tag_name}' checked out successfully.")
-                 return True
-            else:
-                 # This path should theoretically not be reached if check=True used
-                 self.logger.error("Checkout command reported failure unexpectedly.")
-                 # Raise generic error if this happens?
-                 raise GitCommandError(f"Checkout failed for '{tag_name}' for unknown reasons.")
+            # If no exception, assume success
+            self.logger.info(f"Tag '{tag_name}' checked out.")
+            return True
         except Exception as e:
-            self.logger.error(f"Failed to checkout tag '{tag_name}': {e}", exc_info=True)
+            # Catch errors during checkout (e.g., tag not found)
+            self.logger.error(f"Failed to checkout tag '{tag_name}': {e}",
+                              exc_info=True)
             raise # Re-raise checkout errors
+
+
+    # --- Branch Actions ---
+    def execute_create_branch(self, svn_path, branch_name, start_point=None):
+        """
+        Executes branch creation.
+
+        Args: See git_commands.create_branch
+
+        Returns: True on success.
+        Raises: GitCommandError/ValueError/Exception on failure.
+        """
+        self.logger.info(f"Executing create branch '{branch_name}' "
+                         f"from '{start_point or 'HEAD'}'.")
+        try:
+            # Delegate to git_commands, raises error on failure
+            self.git_commands.create_branch(svn_path, branch_name, start_point)
+            return True
+        except Exception as e:
+            self.logger.error(f"Failed to create branch: {e}", exc_info=True)
+            raise
+
+
+    def execute_switch_branch(self, svn_path, branch_name):
+        """
+        Executes branch switch after checking for changes.
+
+        Args: See git_commands.checkout_branch
+
+        Returns: True on success.
+        Raises: ValueError (changes exist), GitCommandError/Exception on failure.
+        """
+        if not branch_name:
+            raise ValueError("Branch name required for switch.")
+        self.logger.info(f"Executing switch to branch '{branch_name}'.")
+
+        # --- Check for Uncommitted Changes ---
+        try:
+            has_changes = self.git_commands.git_status_has_changes(svn_path)
+            if has_changes:
+                msg = "Uncommitted changes exist. Commit or stash first."
+                self.logger.error(f"Switch blocked: {msg}")
+                raise ValueError(msg) # Raise specific error
+            self.logger.debug("No uncommitted changes found.")
+        except Exception as e:
+             self.logger.error(f"Status check error before switch: {e}",
+                               exc_info=True)
+             raise # Re-raise status check errors
+
+        # --- Execute Switch ---
+        try:
+            # Delegate to git_commands, raises error on failure
+            success = self.git_commands.checkout_branch(svn_path, branch_name)
+            return success
+        except Exception as e:
+            self.logger.error(f"Failed switch to branch '{branch_name}': {e}",
+                              exc_info=True)
+            raise
+
+
+    def execute_delete_branch(self, svn_path, branch_name, force=False):
+        """
+        Executes branch deletion.
+
+        Args: See git_commands.delete_branch
+
+        Returns: True on success.
+        Raises: GitCommandError/ValueError/Exception on failure.
+        """
+        if not branch_name:
+            raise ValueError("Branch name required for delete.")
+        # Add checks for main/master? Done in UI layer.
+        self.logger.info(f"Executing delete branch '{branch_name}' (force={force}).")
+        try:
+            # Delegate to git_commands, raises error on failure
+            success = self.git_commands.delete_branch(svn_path, branch_name, force)
+            return success
+        except Exception as e:
+            # Catch errors (like not fully merged if force=False)
+            self.logger.error(f"Failed delete branch '{branch_name}': {e}",
+                              exc_info=True)
+            raise
+
+
+    # --- ADDED: Delete Tag Action ---
+    def execute_delete_tag(self, svn_path, tag_name):
+        """
+        Executes deletion for the specified tag.
+
+        Args:
+            svn_path (str): Validated path to repository.
+            tag_name (str): The tag name to delete (validated by caller).
+
+        Returns:
+            bool: True on successful deletion.
+        Raises:
+            GitCommandError/ValueError/Exception: If delete fails.
+        """
+        if not tag_name:
+            # Should be validated by caller (UI layer)
+            raise ValueError("Tag name required for deletion.")
+
+        self.logger.info(f"Executing delete tag '{tag_name}' in: {svn_path}")
+        try:
+            # Delegate deletion to GitCommands method
+            # This raises GitCommandError if tag not found or other git error
+            success = self.git_commands.delete_tag(svn_path, tag_name)
+            return success # Should be True if no exception
+        except Exception as e:
+            # Catch and re-raise errors from git_commands.delete_tag
+            self.logger.error(f"Failed to delete tag '{tag_name}': {e}",
+                              exc_info=True)
+            raise # Re-raise for the UI layer to handle

backup_handler.py

@@ -4,10 +4,6 @@ import datetime
 import zipfile
 import logging
 
-# Note: Assumes ConfigManager is available via dependency injection or other means
-# if needed for settings beyond exclusions passed directly.
-# For now, it only needs exclusions passed to the create method.
-
 class BackupHandler:
     """Handles the creation of ZIP backups with exclusions."""
 
@@ -20,6 +16,9 @@ class BackupHandler:
         """
         self.logger = logger
 
+    # Note: _parse_exclusions was moved into GitUtilityApp as it needs direct access
+    # to ConfigManager based on the current profile selected in the UI.
+    # The create_zip_backup method now receives the parsed exclusions directly.
 
     def create_zip_backup(self, source_repo_path, backup_base_dir,
                           profile_name, excluded_extensions, excluded_dirs_base):
@@ -54,20 +53,21 @@ class BackupHandler:
         if not backup_base_dir:
             raise ValueError("Backup base directory cannot be empty.")
 
-        # Ensure backup directory exists
+        # Ensure backup directory exists, create if necessary
         if not os.path.isdir(backup_base_dir):
             self.logger.info(f"Creating backup base directory: {backup_base_dir}")
             try:
+                # exist_ok=True prevents error if directory already exists
                 os.makedirs(backup_base_dir, exist_ok=True)
             except OSError as e:
                 self.logger.error(f"Cannot create backup directory: {e}",
                                   exc_info=True)
-                # Re-raise as IOError for the caller
+                # Re-raise as IOError for the caller to potentially handle differently
                 raise IOError(f"Could not create backup directory: {e}") from e
 
         # --- 2. Construct Backup Filename ---
         now_str = datetime.datetime.now().strftime("%Y%m%d_%H%M%S")
-        # Sanitize profile name for use in filename
+        # Sanitize profile name for use in filename (remove potentially invalid chars)
         safe_profile = "".join(c for c in profile_name
                                if c.isalnum() or c in '_-').rstrip() or "profile"
         backup_filename = f"{now_str}_backup_{safe_profile}.zip"
@@ -78,20 +78,25 @@ class BackupHandler:
         files_added = 0
         files_excluded = 0
         dirs_excluded = 0
-        zip_f = None # Initialize zip file object
+        zip_f = None # Initialize zip file object outside try block
+
         try:
-            # Open ZIP file with appropriate settings
+            # Open ZIP file with settings for compression and large files
             zip_f = zipfile.ZipFile(backup_full_path, 'w',
                                     compression=zipfile.ZIP_DEFLATED,
-                                    allowZip64=True) # Support large archives
+                                    allowZip64=True)
 
-            # Walk through the source directory
+            # Walk through the source directory tree
             for root, dirs, files in os.walk(source_repo_path, topdown=True):
+
                 # --- Directory Exclusion ---
-                original_dirs = list(dirs) # Copy before modifying
-                # Exclude based on base name (case-insensitive)
+                # Keep a copy of original dirs list before modifying it in-place
+                original_dirs = list(dirs)
+                # Filter the dirs list: keep only those NOT in excluded_dirs_base
+                # Compare lowercase names for case-insensitivity
                 dirs[:] = [d for d in dirs if d.lower() not in excluded_dirs_base]
-                # Log excluded directories for this level
+
+                # Log excluded directories for this level if any were removed
                 excluded_dirs_now = set(original_dirs) - set(dirs)
                 if excluded_dirs_now:
                     dirs_excluded += len(excluded_dirs_now)
@@ -115,36 +120,37 @@ class BackupHandler:
 
                     # If not excluded, add file to ZIP
                     file_full_path = os.path.join(root, filename)
-                    # Store with relative path inside ZIP archive
+                    # Calculate relative path for storage inside ZIP archive
                     archive_name = os.path.relpath(file_full_path, source_repo_path)
 
                     try:
+                        # Write the file to the ZIP archive
                         zip_f.write(file_full_path, arcname=archive_name)
                         files_added += 1
-                        # Log progress occasionally for large backups
+                        # Log progress periodically for large backups
                         if files_added % 500 == 0:
                              self.logger.debug(f"Added {files_added} files...")
                     except Exception as write_e:
-                         # Log error writing specific file but allow backup to continue
+                         # Log error writing a specific file but allow backup to continue
                          self.logger.error(
                              f"Error writing file '{file_full_path}' to ZIP: {write_e}",
                              exc_info=True
                          )
-                         # Mark backup potentially incomplete? For now, just log.
+                         # Consider marking the backup as potentially incomplete
 
-            # Log final summary after successful walk
-            self.logger.info(f"Backup ZIP creation process finished: {backup_full_path}")
+            # Log final summary after successful walk and write attempts
+            self.logger.info(f"Backup ZIP creation finished: {backup_full_path}")
             self.logger.info(
                 f"Summary - Added: {files_added}, Excl Files: {files_excluded}, "
                 f"Excl Dirs: {dirs_excluded}"
             )
-            # Return the path of the created zip file on success
+            # Return the full path of the created ZIP file on success
             return backup_full_path
 
         except (OSError, zipfile.BadZipFile) as e:
-             # Handle OS errors and specific ZIP errors
+             # Handle OS errors (permissions, disk space) and ZIP format errors
              self.logger.error(f"Error creating backup ZIP: {e}", exc_info=True)
-             # Re-raise as specific types or a general IOError
+             # Re-raise as IOError for the caller to potentially handle specifically
              raise IOError(f"Failed to create backup ZIP: {e}") from e
         except Exception as e:
              # Catch any other unexpected error during the process
@@ -152,17 +158,18 @@ class BackupHandler:
              # Re-raise the original exception
              raise
         finally:
-            # Ensure the ZIP file is always closed
+            # Ensure the ZIP file is always closed, even if errors occurred
             if zip_f:
                 zip_f.close()
                 self.logger.debug(f"ZIP file '{backup_full_path}' closed.")
 
-            # Clean up potentially empty/failed ZIP file
+            # Clean up potentially empty or failed ZIP file
             zip_exists = os.path.exists(backup_full_path)
-            # Check if zip exists but no files were added
+            # Check if zip exists but no files were actually added
             if zip_exists and files_added == 0:
                  self.logger.warning(f"Backup ZIP is empty: {backup_full_path}")
                  try:
+                     # Attempt to remove the empty zip file
                      os.remove(backup_full_path)
                      self.logger.info("Removed empty backup ZIP file.")
                  except OSError as rm_e:
@@ -170,5 +177,6 @@ class BackupHandler:
                      self.logger.error(f"Failed remove empty backup ZIP: {rm_e}")
             elif not zip_exists and files_added > 0:
                  # This case indicates an issue if files were supposedly added
+                 # but the zip file doesn't exist at the end (perhaps deleted?)
                  self.logger.error("Backup process finished but ZIP file missing.")
-                 # Consider raising an error here?
+                 # Consider raising an error here if this state is critical