diff --git a/certbot/certbot/_internal/display/util.py b/certbot/certbot/_internal/display/util.py index d2115289e..3cb20ac7a 100644 --- a/certbot/certbot/_internal/display/util.py +++ b/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 diff --git a/certbot/certbot/_internal/log.py b/certbot/certbot/_internal/log.py index e168158d9..6d089afd4 100644 --- a/certbot/certbot/_internal/log.py +++ b/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 diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 12021c89c..85213c9d3 100644 --- a/certbot/certbot/_internal/main.py +++ b/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( diff --git a/certbot/tests/display/internal_util_test.py b/certbot/tests/display/internal_util_test.py index 8f762b1ac..86489b6a5 100644 --- a/certbot/tests/display/internal_util_test.py +++ b/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 diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index cf523abac..f168b0bca 100644 --- a/certbot/tests/main_test.py +++ b/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)