diff --git a/certbot/certbot/_internal/log.py b/certbot/certbot/_internal/log.py index 90dc2cda1..04a8cb0d1 100644 --- a/certbot/certbot/_internal/log.py +++ b/certbot/certbot/_internal/log.py @@ -28,6 +28,7 @@ import shutil import sys import tempfile import traceback +from types import TracebackType from acme import messages from certbot import errors @@ -78,7 +79,9 @@ def pre_arg_parse_setup(): util.atexit_register(logging.shutdown) sys.excepthook = functools.partial( pre_arg_parse_except_hook, memory_handler, - debug='--debug' in sys.argv, log_path=temp_handler.path) + debug='--debug' in sys.argv, + quiet='--quiet' in sys.argv or '-q' in sys.argv, + log_path=temp_handler.path) def post_arg_parse_setup(config): @@ -121,10 +124,13 @@ def post_arg_parse_setup(config): level = -config.verbose_count * 10 stderr_handler.setLevel(level) logger.debug('Root logging level set at %d', level) - logger.info('Saving debug log to %s', file_path) + + if not config.quiet: + print(f'Saving debug log to {file_path}', file=sys.stderr) sys.excepthook = functools.partial( - post_arg_parse_except_hook, debug=config.debug, log_path=logs_dir) + post_arg_parse_except_hook, + debug=config.debug, quiet=config.quiet, log_path=logs_dir) def setup_log_file_handler(config, logfile, fmt): @@ -306,7 +312,8 @@ def pre_arg_parse_except_hook(memory_handler, *args, **kwargs): memory_handler.flush(force=True) -def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path): +def post_arg_parse_except_hook(exc_type: type, exc_value: BaseException, trace: TracebackType, + debug: bool, quiet: bool, log_path: str): """Logs fatal exceptions and reports them to the user. If debug is True, the full exception and traceback is shown to the @@ -317,10 +324,13 @@ def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path): :param BaseException exc_value: raised exception :param traceback trace: traceback of where the exception was raised :param bool debug: True if the traceback should be shown to the user + :param bool quiet: True if Certbot is running in quiet mode :param str log_path: path to file or directory containing the log """ exc_info = (exc_type, exc_value, trace) + # Only print human advice if not running under --quiet + exit_func = lambda: sys.exit(1) if quiet else exit_with_advice(log_path) # constants.QUIET_LOGGING_LEVEL or higher should be used to # display message the user, otherwise, a lower level like # logger.DEBUG should be used @@ -336,7 +346,7 @@ def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path): # our logger printing warnings and errors in red text. if issubclass(exc_type, errors.Error): logger.error(str(exc_value)) - sys.exit(1) + exit_func() logger.error('An unexpected error occurred:') if messages.is_acme_error(exc_value): # Remove the ACME error prefix from the exception @@ -349,11 +359,11 @@ def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path): # and remove the final newline before passing it to # logger.error. logger.error(''.join(output).rstrip()) - exit_with_log_path(log_path) + exit_func() -def exit_with_log_path(log_path): - """Print a message about the log location and exit. +def exit_with_advice(log_path: str): + """Print a link to the community forums, the debug log path, and exit The message is printed to stderr and the program will exit with a nonzero status. @@ -361,10 +371,11 @@ def exit_with_log_path(log_path): :param str log_path: path to file or directory containing the log """ - msg = 'Please see the ' + msg = ("Ask for help or search for solutions at https://community.letsencrypt.org. " + "See the ") if os.path.isdir(log_path): - msg += 'logfiles in {0} '.format(log_path) + msg += f'logfiles in {log_path} ' else: - msg += "logfile '{0}' ".format(log_path) - msg += 'for more details.' + msg += f"logfile '{log_path}' " + msg += 'or re-run Certbot with -v for more details.' sys.exit(msg) diff --git a/certbot/tests/log_test.py b/certbot/tests/log_test.py index 088faa0fb..034597a3c 100644 --- a/certbot/tests/log_test.py +++ b/certbot/tests/log_test.py @@ -57,7 +57,7 @@ class PreArgParseSetupTest(unittest.TestCase): mock_register.assert_called_once_with(logging.shutdown) mock_sys.excepthook(1, 2, 3) mock_except_hook.assert_called_once_with( - memory_handler, 1, 2, 3, debug=True, log_path=mock.ANY) + memory_handler, 1, 2, 3, debug=True, quiet=False, log_path=mock.ANY) class PostArgParseSetupTest(test_util.ConfigTestCase): @@ -109,7 +109,8 @@ class PostArgParseSetupTest(test_util.ConfigTestCase): self.assertFalse(os.path.exists(self.temp_path)) mock_sys.excepthook(1, 2, 3) mock_except_hook.assert_called_once_with( - 1, 2, 3, debug=self.config.debug, log_path=self.config.logs_dir) + 1, 2, 3, debug=self.config.debug, + quiet=self.config.quiet, log_path=self.config.logs_dir) level = self.stream_handler.level if self.config.quiet: @@ -319,6 +320,12 @@ class PostArgParseExceptHookTest(unittest.TestCase): self._assert_exception_logged(mock_logger.error, exc_type) self._assert_logfile_output(output) + def test_quiet(self): + exc_type = ValueError + mock_logger, output = self._test_common(exc_type, debug=True, quiet=True) + self._assert_exception_logged(mock_logger.error, exc_type) + self.assertNotIn('See the logfile', output) + def test_custom_error(self): exc_type = errors.PluginError mock_logger, output = self._test_common(exc_type, debug=False) @@ -349,7 +356,7 @@ class PostArgParseExceptHookTest(unittest.TestCase): mock_logger, output = self._test_common(exc_type, debug=False) mock_logger.error.assert_called_once_with('Exiting due to user request.') - def _test_common(self, error_type, debug): + def _test_common(self, error_type, debug, quiet=False): """Returns the mocked logger and stderr output.""" mock_err = io.StringIO() @@ -366,7 +373,7 @@ class PostArgParseExceptHookTest(unittest.TestCase): with mock.patch('certbot._internal.log.sys.stderr', mock_err): try: self._call( - *exc_info, debug=debug, log_path=self.log_path) + *exc_info, debug=debug, quiet=quiet, log_path=self.log_path) except SystemExit as exit_err: mock_err.write(str(exit_err)) else: # pragma: no cover @@ -385,8 +392,8 @@ class PostArgParseExceptHookTest(unittest.TestCase): self.assertEqual(actual_exc_info, expected_exc_info) def _assert_logfile_output(self, output): - self.assertTrue('Please see the logfile' in output) - self.assertTrue(self.log_path in output) + self.assertIn('See the logfile', output) + self.assertIn(self.log_path, output) def _assert_quiet_output(self, mock_logger, output): self.assertFalse(mock_logger.exception.called) @@ -394,12 +401,12 @@ class PostArgParseExceptHookTest(unittest.TestCase): self.assertTrue(self.error_msg in output) -class ExitWithLogPathTest(test_util.TempDirTestCase): - """Tests for certbot._internal.log.exit_with_log_path.""" +class ExitWithAdviceTest(test_util.TempDirTestCase): + """Tests for certbot._internal.log.exit_with_advice.""" @classmethod def _call(cls, *args, **kwargs): - from certbot._internal.log import exit_with_log_path - return exit_with_log_path(*args, **kwargs) + from certbot._internal.log import exit_with_advice + return exit_with_advice(*args, **kwargs) def test_log_file(self): log_file = os.path.join(self.tempdir, 'test.log') diff --git a/tests/lock_test.py b/tests/lock_test.py index 56399c874..2ecc8f5e8 100644 --- a/tests/lock_test.py +++ b/tests/lock_test.py @@ -215,7 +215,7 @@ def check_error(command, dir_path): if ret == 0: report_failure("Certbot didn't exit with a nonzero status!", out, err) - match = re.search("Please see the logfile '(.*)' for more details", err) + match = re.search("See the logfile '(.*)' ", err) if match is not None: # Get error output from more verbose logfile with open(match.group(1)) as f: