From 107cb995afa484d65ae118cefe93c78bf7a01388 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 28 Nov 2015 02:06:53 -0800 Subject: [PATCH 1/4] Reduce verbosity of error tracebacks Counteracting #1413 and going a little further. --- letsencrypt/cli.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 2fae5fe3e..729979f39 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1150,9 +1150,9 @@ def _handle_exception(exc_type, exc_value, trace, args): else: # Tell the user a bit about what happened, without overwhelming # them with a full traceback - msg = ("An unexpected error occurred.\n" + - traceback.format_exception_only(exc_type, exc_value)[0] + - "Please see the ") + err = traceback.format_exception_only(exc_type, exc_value)[0] + _code, _sep, err = err.partition(":: ") + msg = "An unexpected error occurred:\n" + err + "Please see the " if args is None: msg += "logfile '{0}' for more details.".format(logfile) else: From dce0f6bf16762284ccf3e95083c11fa8df313c29 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 28 Nov 2015 02:09:12 -0800 Subject: [PATCH 2/4] Comment string manipulation --- letsencrypt/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 729979f39..566ed42c5 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1151,6 +1151,7 @@ def _handle_exception(exc_type, exc_value, trace, args): # Tell the user a bit about what happened, without overwhelming # them with a full traceback err = traceback.format_exception_only(exc_type, exc_value)[0] + # prune ACME error code, we have a human description _code, _sep, err = err.partition(":: ") msg = "An unexpected error occurred:\n" + err + "Please see the " if args is None: From 29c3cc8647d965e79147f684496f8a737205bfb7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 28 Nov 2015 15:27:33 -0800 Subject: [PATCH 3/4] Only prune error message when non-verbose --- letsencrypt/cli.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 566ed42c5..cf1251f0c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1151,8 +1151,14 @@ def _handle_exception(exc_type, exc_value, trace, args): # Tell the user a bit about what happened, without overwhelming # them with a full traceback err = traceback.format_exception_only(exc_type, exc_value)[0] - # prune ACME error code, we have a human description - _code, _sep, err = err.partition(":: ") + # Typical error from the ACME module: + # acme.messages.Error: urn:acme:error:malformed :: The request message was + # malformed :: Error creating new registration :: Validation of contact + # mailto:none@longrandomstring.biz failed: Server failure at resolver + if ("urn:acme" in err and ":: " in err + and args.verbose_count <= flag_default("verbose_count")): + # prune ACME error code, we have a human description + _code, _sep, err = err.partition(":: ") msg = "An unexpected error occurred:\n" + err + "Please see the " if args is None: msg += "logfile '{0}' for more details.".format(logfile) From 48104cded91d92e94e986a75602a403cfafd87d6 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 28 Nov 2015 17:09:24 -0800 Subject: [PATCH 4/4] Tests for error simplification --- letsencrypt/cli.py | 1 + letsencrypt/tests/cli_test.py | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index cf1251f0c..42e9e252e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1148,6 +1148,7 @@ def _handle_exception(exc_type, exc_value, trace, args): if issubclass(exc_type, errors.Error): sys.exit(exc_value) else: + # Here we're passing a client or ACME error out to the client at the shell # Tell the user a bit about what happened, without overwhelming # them with a full traceback err = traceback.format_exception_only(exc_type, exc_value)[0] diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index e512668c5..b8c67696f 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -450,9 +450,14 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods @mock.patch('letsencrypt.cli.sys') def test_handle_exception(self, mock_sys): # pylint: disable=protected-access + from acme import messages + + args = mock.MagicMock() mock_open = mock.mock_open() + with mock.patch('letsencrypt.cli.open', mock_open, create=True): exception = Exception('detail') + args.verbose_count = 1 cli._handle_exception( Exception, exc_value=exception, trace=None, args=None) mock_open().write.assert_called_once_with(''.join( @@ -469,11 +474,23 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_sys.exit.assert_any_call(''.join( traceback.format_exception_only(errors.Error, error))) - args = mock.MagicMock(debug=False) + exception = messages.Error(detail='alpha', typ='urn:acme:error:triffid', + title='beta') + args = mock.MagicMock(debug=False, verbose_count=-3) cli._handle_exception( - Exception, exc_value=Exception('detail'), trace=None, args=args) + messages.Error, exc_value=exception, trace=None, args=args) error_msg = mock_sys.exit.call_args_list[-1][0][0] self.assertTrue('unexpected error' in error_msg) + self.assertTrue('acme:error' not in error_msg) + self.assertTrue('alpha' in error_msg) + self.assertTrue('beta' in error_msg) + args = mock.MagicMock(debug=False, verbose_count=1) + cli._handle_exception( + messages.Error, exc_value=exception, trace=None, args=args) + error_msg = mock_sys.exit.call_args_list[-1][0][0] + self.assertTrue('unexpected error' in error_msg) + self.assertTrue('acme:error' in error_msg) + self.assertTrue('alpha' in error_msg) interrupt = KeyboardInterrupt('detail') cli._handle_exception(