apache: expose aug_save errors in the debug log (#9169)

Fixes #9168.

* apache: expose aug_save errors in the debug log

* logger arguments wrong way around

* log formatting

Co-authored-by: Brad Warren <bmw@users.noreply.github.com>

2 files changed, 29 insertions, 7 deletions

diff --git a/certbot-apache/certbot_apache/_internal/parser.py b/certbot-apache/certbot_apache/_internal/parser.py
index f52cb2416..fbb22a116 100644
--- a/certbot-apache/certbot_apache/_internal/parser.py
+++ b/certbot-apache/certbot_apache/_internal/parser.py
@@ -199,7 +199,13 @@ class ApacheParser:
 
         """
         self.configurator.save_notes = ""
-        self.aug.save()
+
+        ex_errs = self.aug.match("/augeas//error")
+        try:
+            self.aug.save()
+        except IOError:
+            self._log_save_errors(ex_errs)
+            raise
 
         # Force reload if files were modified
         # This is needed to recalculate augeas directive span
@@ -215,12 +221,15 @@ class ApacheParser:
 
         """
         # Check for the root of save problems
-        new_errs = self.aug.match("/augeas//error")
-        # logger.error("During Save - %s", mod_conf)
-        logger.error("Unable to save files: %s. Attempted Save Notes: %s",
-                     ", ".join(err[13:len(err) - 6] for err in new_errs
-                               # Only new errors caused by recent save
-                               if err not in ex_errs), self.configurator.save_notes)
+        new_errs = [e for e in self.aug.match("/augeas//error") if e not in ex_errs]
+
+        for err in new_errs:
+            logger.debug(
+                "Error %s saving %s: %s", self.aug.get(err), err[13:len(err) - 6],
+                self.aug.get(f"{err}/message"))
+        logger.error(
+            "Unable to save files: %s.%s", ", ".join(err[13:len(err) - 6] for err in new_errs),
+            f" Save Notes: {self.configurator.save_notes}" if self.configurator.save_notes else "")
 
     def add_include(self, main_config: str, inc_path: str) -> None:
         """Add Include for a new configuration file if one does not exist
diff --git a/certbot-apache/tests/parser_test.py b/certbot-apache/tests/parser_test.py
index 8607eef11..1062156e3 100644
--- a/certbot-apache/tests/parser_test.py
+++ b/certbot-apache/tests/parser_test.py
@@ -35,6 +35,19 @@ class BasicParserTest(util.ParserTest):
         self.parser.aug.save = mock_save
         self.assertRaises(errors.PluginError, self.parser.unsaved_files)
 
+    @mock.patch("certbot_apache._internal.parser.logger")
+    def test_bad_save_errors(self, mock_logger):
+        nx_path = "/non/existent/path.conf"
+        self.parser.aug.set("/augeas/load/Httpd/incl[last()]", nx_path)
+        self.parser.add_dir(f"/files{nx_path}", "AddDirective", "test")
+
+        self.assertRaises(IOError, self.parser.save, {})
+        mock_logger.error.assert_called_with(
+            'Unable to save files: %s.%s', '/non/existent/path.conf', mock.ANY)
+        mock_logger.debug.assert_called_with(
+            "Error %s saving %s: %s", "mk_augtemp",
+            "/non/existent/path.conf", "No such file or directory")
+
     def test_aug_version(self):
         mock_match = mock.Mock(return_value=["something"])
         self.parser.aug.match = mock_match