cli: improve error messages for enhance errors (#8884)

* cli: improve error messages for enhance errors

* remove status message after enhance config revert

certbot-nginx/certbot_nginx/_internal/configurator.py

@@ -769,9 +769,6 @@ class NginxConfigurator(common.Installer):
         except (KeyError, ValueError):
             raise errors.PluginError(
                 "Unsupported enhancement: {0}".format(enhancement))
-        except errors.PluginError:
-            logger.error("Failed %s for %s", enhancement, domain)
-            raise
 
     def _has_certbot_redirect(self, vhost, domain):
         test_redirect_block = _test_block_from_block(_redirect_block_for_domain(domain))
@@ -791,6 +788,10 @@ class NginxConfigurator(common.Installer):
         :raises .errors.PluginError: If no viable HTTPS host can be created or
             set with header header_substring.
         """
+        if not header_substring in constants.HEADER_ARGS:
+            raise errors.NotSupportedError(
+                f"{header_substring} is not supported by the nginx plugin.")
+
         vhosts = self.choose_vhosts(domain)
         if not vhosts:
             raise errors.PluginError(

certbot/certbot/_internal/client.py

@@ -2,7 +2,7 @@
 import datetime
 import logging
 import platform
-from typing import Optional
+from typing import List, Optional, Union
 
 from cryptography.hazmat.backends import default_backend
 # See https://github.com/pyca/cryptography/issues/4275
@@ -598,7 +598,8 @@ class Client:
             with error_handler.ErrorHandler(self._rollback_and_restart, msg):
                 self.installer.restart()
 
-    def apply_enhancement(self, domains, enhancement, options=None):
+    def apply_enhancement(self, domains: List[str], enhancement: str,
+                          options: Optional[Union[List[str], str]] = None) -> None:
         """Applies an enhancement on all domains.
 
         :param list domains: list of ssl_vhosts (as strings)
@@ -612,33 +613,28 @@ class Client:
 
 
         """
-        msg = f"Could not set up {enhancement} enhancement"
-        with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg):
+        enh_label = options if enhancement == "ensure-http-header" else enhancement
+        with error_handler.ErrorHandler(self._recovery_routine_with_msg, None):
             for dom in domains:
                 try:
                     self.installer.enhance(dom, enhancement, options)
                 except errors.PluginEnhancementAlreadyPresent:
-                    if enhancement == "ensure-http-header":
-                        logger.info("Enhancement %s was already set.",
-                                options)
-                    else:
-                        logger.info("Enhancement %s was already set.",
-                                enhancement)
+                    logger.info("Enhancement %s was already set.", enh_label)
                 except errors.PluginError:
-                    logger.error("Unable to set enhancement %s for %s",
-                            enhancement, dom)
+                    logger.error("Unable to set the %s enhancement for %s.", enh_label, dom)
                     raise
 
-            self.installer.save("Add enhancement %s" % (enhancement))
+            self.installer.save(f"Add enhancement {enh_label}")
 
-    def _recovery_routine_with_msg(self, success_msg):
+    def _recovery_routine_with_msg(self, success_msg: Optional[str]) -> None:
         """Calls the installer's recovery routine and prints success_msg
 
         :param str success_msg: message to show on successful recovery
 
         """
         self.installer.recovery_routine()
-        display_util.notify(success_msg)
+        if success_msg:
+            display_util.notify(success_msg)
 
     def _rollback_and_restart(self, success_msg):
         """Rollback the most recent checkpoint and restart the webserver

certbot/tests/client_test.py

@@ -712,7 +712,7 @@ class EnhanceConfigTest(ClientTestCommon):
 
         if enhance_error:
             self.assertEqual(mock_logger.error.call_count, 1)
-            self.assertIn('Unable to set enhancement', mock_logger.error.call_args_list[0][0][0])
+            self.assertEqual('Unable to set the %s enhancement for %s.', mock_logger.error.call_args_list[0][0][0])
         if restart_error:
             mock_logger.critical.assert_called_with(
                 'Rolling back to previous server configuration...')