display acme.Errors less verbosely (#9255)

* display acme.Errors less verbosely

* remove superfluous import

certbot/certbot/_internal/display/util.py

@@ -4,6 +4,7 @@ import textwrap
 from typing import List
 from typing import Optional
 
+from acme import messages as acme_messages
 from certbot.compat import misc
 
 
@@ -105,3 +106,18 @@ def summarize_domain_list(domains: List[str]) -> str:
         return " and ".join(domains)
     else:
         return "{0} and {1} more domains".format(domains[0], length-1)
+
+
+def describe_acme_error(error: acme_messages.Error) -> str:
+    """Returns a human-readable description of an RFC7807 error.
+
+    :param error: The ACME error
+    :returns: a string describing the error, suitable for human consumption.
+    :rtype: str
+    """
+    parts = (error.title, error.detail)
+    if any(parts):
+        return ' :: '.join(part for part in parts if part is not None)
+    if error.description:
+        return error.description
+    return error.typ

certbot/certbot/_internal/log.py

@@ -30,6 +30,7 @@ import tempfile
 import traceback
 from types import TracebackType
 from typing import Any
+from typing import cast
 from typing import IO
 from typing import Optional
 from typing import Tuple
@@ -40,6 +41,7 @@ from certbot import configuration
 from certbot import errors
 from certbot import util
 from certbot._internal import constants
+from certbot._internal.display import util as display_util
 from certbot.compat import os
 
 # Logging format
@@ -365,9 +367,7 @@ def post_arg_parse_except_hook(exc_type: Type[BaseException], exc_value: BaseExc
             exit_func()
         logger.error('An unexpected error occurred:')
         if messages.is_acme_error(exc_value):
-            # Remove the ACME error prefix from the exception
-            _, _, exc_str = str(exc_value).partition(':: ')
-            logger.error(exc_str)
+            logger.error(display_util.describe_acme_error(cast(messages.Error, exc_value)))
         else:
             output = traceback.format_exception_only(exc_type, exc_value)
             # format_exception_only returns a list of strings each

certbot/certbot/_internal/main.py

@@ -5,6 +5,7 @@ from contextlib import contextmanager
 import functools
 import logging.handlers
 import sys
+from typing import cast
 from typing import Generator
 from typing import IO
 from typing import Iterable
@@ -730,7 +731,9 @@ def _determine_account(config: configuration.NamespaceConfig
             except (errors.Error, acme_messages.Error) as err:
                 logger.debug("", exc_info=True)
                 if acme_messages.is_acme_error(err):
-                    err_msg = f"Error returned by the ACME server: {str(err)}"
+                    err_msg = internal_display_util.describe_acme_error(
+                        cast(acme_messages.Error, err))
+                    err_msg = f"Error returned by the ACME server: {err_msg}"
                 else:
                     err_msg = str(err)
                 raise errors.Error(

certbot/tests/display/internal_util_test.py

@@ -4,6 +4,7 @@ import socket
 import tempfile
 import unittest
 
+from acme import messages as acme_messages
 from certbot import errors
 
 try:
@@ -124,6 +125,29 @@ class SummarizeDomainListTest(unittest.TestCase):
         self.assertEqual("", self._call([]))
 
 
+class DescribeACMEErrorTest(unittest.TestCase):
+    @classmethod
+    def _call(cls, typ: str = "urn:ietf:params:acme:error:badCSR",
+              title: str = "Unacceptable CSR",
+              detail: str = "CSR contained unknown extensions"):
+        from certbot._internal.display.util import describe_acme_error
+        return describe_acme_error(
+            acme_messages.Error(typ=typ, title=title, detail=detail))
+
+    def test_title_and_detail(self):
+        self.assertEqual("Unacceptable CSR :: CSR contained unknown extensions", self._call())
+
+    def test_detail(self):
+        self.assertEqual("CSR contained unknown extensions", self._call(title=None))
+
+    def test_description(self):
+        self.assertEqual(acme_messages.ERROR_CODES["badCSR"], self._call(title=None, detail=None))
+
+    def test_unknown_type(self):
+        self.assertEqual(
+            "urn:ietf:params:acme:error:unknownErrorType",
+            self._call(typ="urn:ietf:params:acme:error:unknownErrorType", title=None, detail=None))
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot/tests/main_test.py

@@ -668,16 +668,13 @@ class DetermineAccountTest(test_util.ConfigTestCase):
         self._register_error_common(err_msg, errors.Error(err_msg))
 
     def test_register_error_acme_type_and_detail(self):
-        err_msg = ("Error returned by the ACME server: urn:ietf:params:acme:"
-                   "error:malformed :: The request message was malformed :: "
-                   "must agree to terms of service")
+        err_msg = ("Error returned by the ACME server: must agree to terms of service")
         exception = acme_error(typ = "urn:ietf:params:acme:error:malformed",
                                detail = "must agree to terms of service")
         self._register_error_common(err_msg, exception)
 
     def test_register_error_acme_type_only(self):
-        err_msg = ("Error returned by the ACME server: urn:ietf:params:acme:"
-                   "error:serverInternal :: The server experienced an internal error")
+        err_msg = ("Error returned by the ACME server: The server experienced an internal error")
         exception = acme_error(typ = "urn:ietf:params:acme:error:serverInternal")
         self._register_error_common(err_msg, exception)