Merge pull request #572 from bradmw/merge-fix

Fixed traceback when not run as root

.gitignore

@@ -5,6 +5,7 @@ build/
 dist/
 /venv/
 /.tox/
+letsencrypt.log
 
 # coverage
 .coverage

letsencrypt/cli.py

@@ -2,11 +2,13 @@
 # TODO: Sanity check all input.  Be sure to avoid shell code etc...
 import argparse
 import atexit
+import functools
 import logging
 import logging.handlers
 import os
 import sys
 import time
+import traceback
 
 import configargparse
 import zope.component
@@ -644,8 +646,71 @@ def _setup_logging(args):
     logger.info("Saving debug log to %s", log_file_name)
 
 
-def main2(cli_args, args, config, plugins):
+def _handle_exception(exc_type, exc_value, trace, args):
-    """Continued main script execution."""
+    """Logs exceptions and reports them to the user.
+
+    Args is used to determine how to display exceptions to the user. In
+    general, if args.debug is True, then the full exception and traceback is
+    shown to the user, otherwise it is suppressed. If args itself is None,
+    then the traceback and exception is attempted to be written to a logfile.
+    If this is successful, the traceback is suppressed, otherwise it is shown
+    to the user. sys.exit is always called with a nonzero status.
+
+    """
+    logger.debug(
+        "Exiting abnormally:\n%s",
+        "".join(traceback.format_exception(exc_type, exc_value, trace)))
+
+    if issubclass(exc_type, Exception) and (args is None or not args.debug):
+        if args is None:
+            logfile = "letsencrypt.log"
+            try:
+                with open(logfile, "w") as logfd:
+                    traceback.print_exception(
+                        exc_type, exc_value, trace, file=logfd)
+            except: # pylint: disable=bare-except
+                sys.exit("".join(
+                    traceback.format_exception(exc_type, exc_value, trace)))
+
+        if issubclass(exc_type, errors.Error):
+            sys.exit(exc_value)
+        elif args is None:
+            sys.exit(
+                "An unexpected error occurred. Please see the logfile '{0}' "
+                "for more details.".format(logfile))
+        else:
+            sys.exit(
+                "An unexpected error occurred. Please see the logfiles in {0} "
+                "for more details.".format(args.logs_dir))
+    else:
+        sys.exit("".join(
+            traceback.format_exception(exc_type, exc_value, trace)))
+
+
+def main(cli_args=sys.argv[1:]):
+    """Command line argument parsing and main script execution."""
+    sys.excepthook = functools.partial(_handle_exception, args=None)
+
+    # note: arg parser internally handles --help (and exits afterwards)
+    plugins = plugins_disco.PluginsRegistry.find_all()
+    args = create_parser(plugins, cli_args).parse_args(cli_args)
+    config = configuration.NamespaceConfig(args)
+
+    # Setup logging ASAP, otherwise "No handlers could be found for
+    # logger ..." TODO: this should be done before plugins discovery
+    for directory in config.config_dir, config.work_dir:
+        le_util.make_or_verify_dir(
+            directory, constants.CONFIG_DIRS_MODE, os.geteuid())
+    # TODO: logs might contain sensitive data such as contents of the
+    # private key! #525
+    le_util.make_or_verify_dir(args.logs_dir, 0o700, os.geteuid())
+    _setup_logging(args)
+
+    # do not log `args`, as it contains sensitive data (e.g. revoke --key)!
+    logger.debug("Arguments: %r", cli_args)
+    logger.debug("Discovered plugins: %r", plugins)
+
+    sys.excepthook = functools.partial(_handle_exception, args=args)
 
     # Displayer
     if args.text_mode:
@@ -654,10 +719,6 @@ def main2(cli_args, args, config, plugins):
         displayer = display_util.NcursesDisplay()
     zope.component.provideUtility(displayer)
 
-    # do not log `args`, as it contains sensitive data (e.g. revoke --key)!
-    logger.debug("Arguments: %r", cli_args)
-    logger.debug("Discovered plugins: %r", plugins)
-
     # Reporter
     report = reporter.Reporter()
     zope.component.provideUtility(report)
@@ -677,39 +738,5 @@ def main2(cli_args, args, config, plugins):
     return args.func(args, config, plugins)
 
 
-def main(cli_args=sys.argv[1:]):
-    """Command line argument parsing and main script execution."""
-    # note: arg parser internally handles --help (and exits afterwards)
-    plugins = plugins_disco.PluginsRegistry.find_all()
-    args = create_parser(plugins, cli_args).parse_args(cli_args)
-    config = configuration.NamespaceConfig(args)
-
-    # Setup logging ASAP, otherwise "No handlers could be found for
-    # logger ..." TODO: this should be done before plugins discovery
-    for directory in config.config_dir, config.work_dir:
-        le_util.make_or_verify_dir(
-            directory, constants.CONFIG_DIRS_MODE, os.geteuid())
-    # TODO: logs might contain sensitive data such as contents of the
-    # private key! #525
-    le_util.make_or_verify_dir(args.logs_dir, 0o700, os.geteuid())
-    _setup_logging(args)
-
-    def handle_exception_common():
-        """Logs the exception and reraises it if in debug mode."""
-        logger.debug("Exiting abnormally", exc_info=True)
-        if args.debug:
-            raise
-
-    try:
-        return main2(cli_args, args, config, plugins)
-    except errors.Error as error:
-        handle_exception_common()
-        return error
-    except Exception: # pylint: disable=broad-except
-        handle_exception_common()
-        return ("An unexpected error occured. Please see the logfiles in {0} "
-                "for more details.".format(args.logs_dir))
-
-
 if __name__ == "__main__":
     sys.exit(main())  # pragma: no cover

letsencrypt/tests/cli_test.py

@@ -2,11 +2,14 @@
 import itertools
 import os
 import shutil
+import traceback
 import tempfile
 import unittest
 
 import mock
 
+from letsencrypt import errors
+
 
 class CLITest(unittest.TestCase):
     """Tests for different commands."""
@@ -20,16 +23,13 @@ class CLITest(unittest.TestCase):
     def tearDown(self):
         shutil.rmtree(self.tmp_dir)
 
-    def _call(self, args, client_mock_attrs=None):
+    def _call(self, args):
         from letsencrypt import cli
         args = ['--text', '--config-dir', self.config_dir,
                 '--work-dir', self.work_dir, '--logs-dir', self.logs_dir] + args
         with mock.patch('letsencrypt.cli.sys.stdout') as stdout:
             with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
                 with mock.patch('letsencrypt.cli.client') as client:
-                    if client_mock_attrs:
-                        # pylint: disable=star-args
-                        client.configure_mock(**client_mock_attrs)
                     ret = cli.main(args)
         return ret, stdout, stderr, client
 
@@ -59,19 +59,42 @@ class CLITest(unittest.TestCase):
                   for r in xrange(len(flags)))):
             self._call(['plugins',] + list(args))
 
-    def test_exceptions(self):
+    @mock.patch("letsencrypt.cli.sys")
-        from letsencrypt import errors
+    def test_handle_exception(self, mock_sys):
-        cmd_arg = ['config_changes']
+        # pylint: disable=protected-access
-        error = [errors.Error('problem')]
+        from letsencrypt import cli
-        attrs = {'view_config_changes.side_effect' : error}
+
-        self.assertRaises(
+        mock_open = mock.mock_open()
-            errors.Error, self._call, ['--debug'] + cmd_arg, attrs)
+        with mock.patch("letsencrypt.cli.open", mock_open, create=True):
-        self._call(cmd_arg, attrs)
+            exception = Exception("detail")
+            cli._handle_exception(
+                Exception, exc_value=exception, trace=None, args=None)
+            mock_open().write.assert_called_once_with("".join(
+                traceback.format_exception_only(Exception, exception)))
+            error_msg = mock_sys.exit.call_args_list[0][0][0]
+            self.assertTrue("unexpected error" in error_msg)
+
+        with mock.patch("letsencrypt.cli.open", mock_open, create=True):
+            mock_open.side_effect = [KeyboardInterrupt]
+            error = errors.Error("detail")
+            cli._handle_exception(
+                errors.Error, exc_value=error, trace=None, args=None)
+            # assert_any_call used because sys.exit doesn't exit in cli.py
+            mock_sys.exit.assert_any_call("".join(
+                traceback.format_exception_only(errors.Error, error)))
+
+        args = mock.MagicMock(debug=False)
+        cli._handle_exception(
+            Exception, exc_value=Exception("detail"), trace=None, args=args)
+        error_msg = mock_sys.exit.call_args_list[-1][0][0]
+        self.assertTrue("unexpected error" in error_msg)
+
+        interrupt = KeyboardInterrupt("detail")
+        cli._handle_exception(
+            KeyboardInterrupt, exc_value=interrupt, trace=None, args=None)
+        mock_sys.exit.assert_called_with("".join(
+            traceback.format_exception_only(KeyboardInterrupt, interrupt)))
 
-        attrs['view_config_changes.side_effect'] = [ValueError]
-        self.assertRaises(
-            ValueError, self._call, ['--debug'] + cmd_arg, attrs)
-        self._call(cmd_arg, attrs)
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover