Fix a regression in which soft failure exit codes in command hooks were not respected (#1059).

NEWS

@@ -1,3 +1,6 @@
+2.0.2.dev0
+ * #1059: Fix a regression in which soft failure exit codes in command hooks were not respected.
+
 2.0.1
  * #1057: Fix argument parsing to avoid using Python 3.12+ string features. Now borgmatic will
    work with Python 3.9, 3.10, and 3.11 again.

borgmatic/hooks/command.py

@@ -1,3 +1,4 @@
+import functools
 import logging
 import os
 import re
@@ -194,7 +195,7 @@ class Before_after_hooks:
             )
         except (OSError, subprocess.CalledProcessError) as error:
             if considered_soft_failure(error):
-                return
+                raise
 
             # Trigger the after hook manually, since raising here will prevent it from being run
             # otherwise.
@@ -221,16 +222,20 @@ class Before_after_hooks:
             )
         except (OSError, subprocess.CalledProcessError) as error:
             if considered_soft_failure(error):
-                return
+                raise
 
             raise ValueError(f'Error running after {self.before_after} hook: {error}')
 
 
+@functools.cache
 def considered_soft_failure(error):
     '''
     Given a configuration filename and an exception object, return whether the exception object
     represents a subprocess.CalledProcessError with a return code of SOFT_FAIL_EXIT_CODE. If so,
     that indicates that the error is a "soft failure", and should not result in an error.
+
+    The results of this function are cached so that it can be called multiple times without logging
+    multiple times.
     '''
     exit_code = getattr(error, 'returncode', None)
     if exit_code is None:

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "borgmatic"
-version = "2.0.1"
+version = "2.0.2.dev0"
 authors = [
   { name="Dan Helfman", email="witten@torsion.org" },
 ]

tests/unit/hooks/test_command.py

@@ -396,7 +396,7 @@ def test_before_after_hooks_with_before_error_runs_after_hook_and_raises():
             assert False  # This should never get called.
 
 
-def test_before_after_hooks_with_before_soft_failure_does_not_raise():
+def test_before_after_hooks_with_before_soft_failure_raises():
     commands = [
         {'before': 'repository', 'run': ['foo', 'bar']},
         {'after': 'repository', 'run': ['baz']},
@@ -412,22 +412,23 @@ def test_before_after_hooks_with_before_soft_failure_does_not_raise():
         after='action',
         hook_name='myhook',
         action_names=['create'],
-    ).and_return(flexmock()).once()
+    ).never()
     flexmock(module).should_receive('execute_hooks').and_raise(OSError)
     flexmock(module).should_receive('considered_soft_failure').and_return(True)
 
-    with module.Before_after_hooks(
-        command_hooks=commands,
-        before_after='action',
-        umask=1234,
-        working_directory='/working',
-        dry_run=False,
-        hook_name='myhook',
-        action_names=['create'],
-        context1='stuff',
-        context2='such',
-    ):
-        pass
+    with pytest.raises(OSError):
+        with module.Before_after_hooks(
+            command_hooks=commands,
+            before_after='action',
+            umask=1234,
+            working_directory='/working',
+            dry_run=False,
+            hook_name='myhook',
+            action_names=['create'],
+            context1='stuff',
+            context2='such',
+        ):
+            pass
 
 
 def test_before_after_hooks_with_after_error_raises():
@@ -465,7 +466,7 @@ def test_before_after_hooks_with_after_error_raises():
             pass
 
 
-def test_before_after_hooks_with_after_soft_failure_does_not_raise():
+def test_before_after_hooks_with_after_soft_failure_raises():
     commands = [
         {'before': 'repository', 'run': ['foo', 'bar']},
         {'after': 'repository', 'run': ['baz']},
@@ -485,18 +486,19 @@ def test_before_after_hooks_with_after_soft_failure_does_not_raise():
     flexmock(module).should_receive('execute_hooks').and_return(None).and_raise(OSError)
     flexmock(module).should_receive('considered_soft_failure').and_return(True)
 
-    with module.Before_after_hooks(
-        command_hooks=commands,
-        before_after='action',
-        umask=1234,
-        working_directory='/working',
-        dry_run=False,
-        hook_name='myhook',
-        action_names=['create'],
-        context1='stuff',
-        context2='such',
-    ):
-        pass
+    with pytest.raises(OSError):
+        with module.Before_after_hooks(
+            command_hooks=commands,
+            before_after='action',
+            umask=1234,
+            working_directory='/working',
+            dry_run=False,
+            hook_name='myhook',
+            action_names=['create'],
+            context1='stuff',
+            context2='such',
+        ):
+            pass
 
 
 def test_considered_soft_failure_treats_soft_fail_exit_code_as_soft_fail():
@@ -513,3 +515,11 @@ def test_considered_soft_failure_does_not_treat_other_exit_code_as_soft_fail():
 
 def test_considered_soft_failure_does_not_treat_other_exception_type_as_soft_fail():
     assert not module.considered_soft_failure(Exception())
+
+
+def test_considered_soft_failure_caches_results_and_only_logs_once():
+    error = subprocess.CalledProcessError(module.SOFT_FAIL_EXIT_CODE, 'try again')
+    flexmock(module.logger).should_receive('info').once()
+
+    assert module.considered_soft_failure(error)
+    assert module.considered_soft_failure(error)