From b5a3589471aabd4bbc3680d09e82b2a3ea34fa4c Mon Sep 17 00:00:00 2001
From: Dan Helfman <witten@torsion.org>
Date: Sat, 23 Nov 2024 18:09:59 -0800
Subject: [PATCH] A little more error handling (#261).

---
 borgmatic/hooks/zfs.py       | 11 +++++++--
 tests/unit/hooks/test_zfs.py | 48 ++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/borgmatic/hooks/zfs.py b/borgmatic/hooks/zfs.py
index 766a5a01..ed5c44c1 100644
--- a/borgmatic/hooks/zfs.py
+++ b/borgmatic/hooks/zfs.py
@@ -159,7 +159,7 @@ def dump_data_sources(
         snapshot_mount_path_for_borg = os.path.join(
             os.path.normpath(borgmatic_runtime_directory),
             'zfs_snapshots',
-            '.',
+            '.',  # Borg 1.4+ "slashdot" hack.
             mount_point.lstrip(os.path.sep),
         )
         snapshot_mount_path = os.path.normpath(snapshot_mount_path_for_borg)
@@ -282,7 +282,14 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
             )
 
             if not dry_run:
-                unmount_snapshot(umount_command, snapshot_mount_path)
+                try:
+                    unmount_snapshot(umount_command, snapshot_mount_path)
+                except FileNotFoundError:
+                    logger.debug(f'{log_prefix}: Could not find "{umount_command}" command')
+                    return
+                except subprocess.CalledProcessError as error:
+                    logger.debug(f'{log_prefix}: {error}')
+                    return
 
         if not dry_run:
             shutil.rmtree(snapshots_directory)
diff --git a/tests/unit/hooks/test_zfs.py b/tests/unit/hooks/test_zfs.py
index 4567f826..680e5166 100644
--- a/tests/unit/hooks/test_zfs.py
+++ b/tests/unit/hooks/test_zfs.py
@@ -248,6 +248,54 @@ def test_remove_data_source_dumps_bails_for_zfs_command_error():
     )
 
 
+def test_remove_data_source_dumps_bails_for_missing_umount_command():
+    flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),))
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).and_return('/run/borgmatic')
+    flexmock(module.glob).should_receive('glob').replace_with(lambda path: [path])
+    flexmock(module.os.path).should_receive('isdir').and_return(True)
+    flexmock(module.shutil).should_receive('rmtree')
+    flexmock(module).should_receive('unmount_snapshot').with_args(
+        '/usr/local/bin/umount', '/run/borgmatic/zfs_snapshots/mnt/dataset'
+    ).and_raise(FileNotFoundError)
+    flexmock(module).should_receive('get_all_snapshots').never()
+    flexmock(module).should_receive('destroy_snapshot').never()
+    hook_config = {'zfs_command': '/usr/local/bin/zfs', 'umount_command': '/usr/local/bin/umount'}
+
+    module.remove_data_source_dumps(
+        hook_config=hook_config,
+        config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
+def test_remove_data_source_dumps_bails_for_umount_command_error():
+    flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),))
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).and_return('/run/borgmatic')
+    flexmock(module.glob).should_receive('glob').replace_with(lambda path: [path])
+    flexmock(module.os.path).should_receive('isdir').and_return(True)
+    flexmock(module.shutil).should_receive('rmtree')
+    flexmock(module).should_receive('unmount_snapshot').with_args(
+        '/usr/local/bin/umount', '/run/borgmatic/zfs_snapshots/mnt/dataset'
+    ).and_raise(module.subprocess.CalledProcessError(1, 'wtf'))
+    flexmock(module).should_receive('get_all_snapshots').never()
+    flexmock(module).should_receive('destroy_snapshot').never()
+    hook_config = {'zfs_command': '/usr/local/bin/zfs', 'umount_command': '/usr/local/bin/umount'}
+
+    module.remove_data_source_dumps(
+        hook_config=hook_config,
+        config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
 def test_remove_data_source_dumps_skips_unmount_snapshot_directories_that_are_not_actually_directories():
     flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/mnt/dataset'),))
     flexmock(module.borgmatic.config.paths).should_receive(