Merge pull request #5798 from pradyunsg/improve-configuration-handling

Improve configuration handling

news/5798.feature

@@ -0,0 +1 @@
+Malformed configuration files now show helpful error messages, instead of tracebacks.

src/pip/_internal/cli/parser.py

@@ -9,6 +9,7 @@ from distutils.util import strtobool
 
 from pip._vendor.six import string_types
 
+from pip._internal.cli.status_codes import UNKNOWN_ERROR
 from pip._internal.configuration import Configuration, ConfigurationError
 from pip._internal.utils.compat import get_terminal_size
 
@@ -232,7 +233,7 @@ class ConfigOptionParser(CustomOptionParser):
         try:
             self.config.load()
         except ConfigurationError as err:
-            self.exit(2, err.args[0])
+            self.exit(UNKNOWN_ERROR, str(err))
 
         defaults = self._update_defaults(self.defaults.copy())  # ours
         for option in self._get_all_options():
@@ -244,7 +245,7 @@ class ConfigOptionParser(CustomOptionParser):
 
     def error(self, msg):
         self.print_usage(sys.stderr)
-        self.exit(2, "%s\n" % msg)
+        self.exit(UNKNOWN_ERROR, "%s\n" % msg)
 
 
 def invalid_config_error_message(action, key, val):

src/pip/_internal/configuration.py

@@ -18,7 +18,9 @@ import os
 from pip._vendor import six
 from pip._vendor.six.moves import configparser
 
-from pip._internal.exceptions import ConfigurationError
+from pip._internal.exceptions import (
+    ConfigurationError, ConfigurationFileCouldNotBeLoaded,
+)
 from pip._internal.locations import (
     legacy_config_file, new_config_file, running_under_virtualenv,
     site_config_files, venv_config_file,
@@ -289,11 +291,16 @@ class Configuration(object):
             try:
                 parser.read(fname)
             except UnicodeDecodeError:
-                raise ConfigurationError((
-                    "ERROR: "
-                    "Configuration file contains invalid %s characters.\n"
-                    "Please fix your configuration, located at %s\n"
-                ) % (locale.getpreferredencoding(False), fname))
+                # See https://github.com/pypa/pip/issues/4963
+                raise ConfigurationFileCouldNotBeLoaded(
+                    reason="contains invalid {} characters".format(
+                        locale.getpreferredencoding(False)
+                    ),
+                    fname=fname,
+                )
+            except configparser.Error as error:
+                # See https://github.com/pypa/pip/issues/4893
+                raise ConfigurationFileCouldNotBeLoaded(error=error)
         return parser
 
     def _load_environment_vars(self):

src/pip/_internal/exceptions.py

@@ -247,3 +247,22 @@ class HashMismatch(HashError):
 class UnsupportedPythonVersion(InstallationError):
     """Unsupported python version according to Requires-Python package
     metadata."""
+
+
+class ConfigurationFileCouldNotBeLoaded(ConfigurationError):
+    """When there are errors while loading a configuration file
+    """
+
+    def __init__(self, reason="could not be loaded", fname=None, error=None):
+        super(ConfigurationFileCouldNotBeLoaded, self).__init__(error)
+        self.reason = reason
+        self.fname = fname
+        self.error = error
+
+    def __str__(self):
+        if self.fname is not None:
+            message_part = " in {}.".format(self.fname)
+        else:
+            assert self.error is not None
+            message_part = ".\n{}\n".format(self.error.message)
+        return "Configuration file {}{}".format(self.reason, message_part)

tests/unit/test_configuration.py

@@ -68,6 +68,23 @@ class TestConfigurationLoading(ConfigurationMixin):
         with pytest.raises(ConfigurationError):
             self.configuration.get_value(":env:.version")
 
+    def test_environment_config_errors_if_malformed(self):
+        contents = """
+            test]
+            hello = 4
+        """
+        with self.tmpfile(contents) as config_file:
+            os.environ["PIP_CONFIG_FILE"] = config_file
+            with pytest.raises(ConfigurationError) as err:
+                self.configuration.load()
+
+        assert "section header" in str(err.value)  # error kind
+        assert "1" in str(err.value)  # line number
+        assert (  # file name
+            config_file in str(err.value) or
+            repr(config_file) in str(err.value)
+        )
+
 
 class TestConfigurationPrecedence(ConfigurationMixin):
     # Tests for methods to that determine the order of precedence of