Repository: pypa/twine

Issue:
Consider reverting #248
While I understand the motivation for the changes introduced in #248 ("Rework handling and display of exceptions"), I think they should probably reverted, and a different approach to solving #228 be taken instead (like specifically handling this edge case rather than applying it to all exceptions).

Suppressing the entire stack trace causes two problems:

#### 1. Difficult to debug user problems
For example, in #383, as a maintainer, I have no idea where this exception is getting raised from because the stack trace has been suppressed. The user has even less of an idea where to look to solve the problem themselves.

#### 2. Difficult to work on `twine`
As a contributor, I need to uncomment these changes in order to do development on twine, when I may occasionally make a mistake and raise an exception from somewhere. Not having the stacktrace available slows me down as a contributor.

I'm happy to do the work as proposed, but wanted to raise this for discussion first before forging ahead. (cc @dstufft @sigmavirus24 @brainwane).

Discussion:
theacodes: @sigmavirus24 you were more involved in the discussion, do you wanna take a look?

Patch:
diff --git a/setup.cfg b/setup.cfg
index 66d2ceb..925c448 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -16,6 +16,5 @@ requires-dist =
     requests-toolbelt >= 0.8.0
     pkginfo >= 1.4.2
     setuptools >= 0.7.0
-    argparse; python_version == '2.6'
     pyblake2; extra == 'with-blake2' and python_version < '3.6'
     keyring; extra == 'keyring'
diff --git a/setup.py b/setup.py
index d90b6b6..b96202a 100644
--- a/setup.py
+++ b/setup.py
@@ -18,21 +18,6 @@ import sys
 import twine
 
 
-install_requires = [
-    "tqdm >= 4.14",
-    "pkginfo >= 1.4.2",
-    "readme_renderer >= 21.0",
-    "requests >= 2.5.0, != 2.15, != 2.16",
-    "requests-toolbelt >= 0.8.0",
-    "setuptools >= 0.7.0",
-]
-
-if sys.version_info[:2] < (2, 7):
-    install_requires += [
-        "argparse",
-    ]
-
-
 blake2_requires = []
 
 if sys.version_info[:2] < (3, 6):
@@ -91,7 +76,14 @@ setup(
         ],
     },
 
-    install_requires=install_requires,
+    install_requires=[
+        "pkginfo >= 1.4.2",
+        "readme_renderer >= 21.0",
+        "requests >= 2.5.0, != 2.15, != 2.16",
+        "requests-toolbelt >= 0.8.0",
+        "setuptools >= 0.7.0",
+        "tqdm >= 4.14",
+    ],
     extras_require={
         'with-blake2': blake2_requires,
         'keyring': [
diff --git a/twine/__main__.py b/twine/__main__.py
index 9eed3aa..fbf276d 100644
--- a/twine/__main__.py
+++ b/twine/__main__.py
@@ -17,13 +17,16 @@ from __future__ import unicode_literals
 
 import sys
 
+import requests
+
+from twine import exceptions
 from twine.cli import dispatch
 
 
 def main():
     try:
         return dispatch(sys.argv[1:])
-    except Exception as exc:
+    except (exceptions.TwineException, requests.exceptions.HTTPError) as exc:
         return '{0}: {1}'.format(
             exc.__class__.__name__,
             exc.args[0],
diff --git a/twine/commands/__init__.py b/twine/commands/__init__.py
index 55bc1a5..1bb61ae 100644
--- a/twine/commands/__init__.py
+++ b/twine/commands/__init__.py
@@ -17,6 +17,8 @@ from __future__ import unicode_literals
 import glob
 import os.path
 
+from twine import exceptions
+
 __all__ = []
 
 
@@ -40,7 +42,7 @@ def _find_dists(dists):
         files = glob.glob(filename)
         # If nothing matches, files is []
         if not files:
-            raise ValueError(
+            raise exceptions.InvalidDistribution(
                 "Cannot find file (or expand pattern): '%s'" % filename
             )
         # Otherwise, files will be filenames that exist
diff --git a/twine/commands/register.py b/twine/commands/register.py
index 30400f8..11e551a 100644
--- a/twine/commands/register.py
+++ b/twine/commands/register.py
@@ -16,8 +16,8 @@ from __future__ import absolute_import, unicode_literals, print_function
 import argparse
 import os.path
 
-from twine import exceptions as exc
 from twine.package import PackageFile
+from twine import exceptions
 from twine import settings
 
 
@@ -28,7 +28,7 @@ def register(register_settings, package):
     repository = register_settings.create_repository()
 
     if not os.path.exists(package):
-        raise exc.PackageNotFound(
+        raise exceptions.PackageNotFound(
             '"{0}" does not exist on the file system.'.format(package)
         )
 
@@ -38,7 +38,7 @@ def register(register_settings, package):
     repository.close()
 
     if resp.is_redirect:
-        raise exc.RedirectDetected(
+        raise exceptions.RedirectDetected(
             ('"{0}" attempted to redirect to "{1}" during registration.'
              ' Aborting...').format(repository_url,
                                     resp.headers["location"]))
diff --git a/twine/commands/upload.py b/twine/commands/upload.py
index 53fba25..f4cc59f 100644
--- a/twine/commands/upload.py
+++ b/twine/commands/upload.py
@@ -17,9 +17,9 @@ from __future__ import unicode_literals
 import argparse
 import os.path
 
-import twine.exceptions as exc
 from twine.commands import _find_dists
 from twine.package import PackageFile
+from twine import exceptions
 from twine import settings
 from twine import utils
 
@@ -85,7 +85,7 @@ def upload(upload_settings, dists):
         # by PyPI should never happen in reality. This should catch malicious
         # redirects as well.
         if resp.is_redirect:
-            raise exc.RedirectDetected(
+            raise exceptions.RedirectDetected(
                 ('"{0}" attempted to redirect to "{1}" during upload.'
                  ' Aborting...').format(repository_url,
                                         resp.headers["location"]))
diff --git a/twine/exceptions.py b/twine/exceptions.py
index d227db6..9e35ea0 100644
--- a/twine/exceptions.py
+++ b/twine/exceptions.py
@@ -73,3 +73,15 @@ class InvalidSigningConfiguration(TwineException):
     """Both the sign and identity parameters must be present."""
 
     pass
+
+
+class InvalidConfiguration(TwineException):
+    """Raised when configuration is invalid."""
+
+    pass
+
+
+class InvalidDistribution(TwineException):
+    """Raised when a distribution is invalid."""
+
+    pass
diff --git a/twine/package.py b/twine/package.py
index 4f07361..c9e5197 100644
--- a/twine/package.py
+++ b/twine/package.py
@@ -31,6 +31,7 @@ except ImportError:
 
 from twine.wheel import Wheel
 from twine.wininst import WinInst
+from twine import exceptions
 
 DIST_TYPES = {
     "bdist_wheel": Wheel,
@@ -78,7 +79,7 @@ class PackageFile(object):
                 meta = DIST_TYPES[dtype](filename)
                 break
         else:
-            raise ValueError(
+            raise exceptions.InvalidDistribution(
                 "Unknown distribution format: '%s'" %
                 os.path.basename(filename)
             )
@@ -151,7 +152,9 @@ class PackageFile(object):
 
     def add_gpg_signature(self, signature_filepath, signature_filename):
         if self.gpg_signature is not None:
-            raise ValueError('GPG Signature can only be added once')
+            raise exceptions.InvalidDistribution(
+                'GPG Signature can only be added once'
+            )
 
         with open(signature_filepath, "rb") as gpg:
             self.gpg_signature = (signature_filename, gpg.read())
diff --git a/twine/utils.py b/twine/utils.py
index 1f36af2..83a1867 100644
--- a/twine/utils.py
+++ b/twine/utils.py
@@ -35,7 +35,7 @@ try:
 except ImportError:
     from urllib.parse import urlparse, urlunparse
 
-import twine.exceptions
+from twine import exceptions
 
 # Shim for raw_input in python3
 if sys.version_info > (3,):
@@ -109,7 +109,7 @@ def get_repository_from_config(config_file, repository, repository_url=None):
             "password": None,
         }
     if repository_url and "://" not in repository_url:
-        raise twine.exceptions.UnreachableRepositoryURLDetected(
+        raise exceptions.UnreachableRepositoryURLDetected(
             "Repository URL {0} has no protocol. Please add "
             "'https://'. \n".format(repository_url))
     try:
@@ -125,7 +125,7 @@ def get_repository_from_config(config_file, repository, repository_url=None):
             repo=repository,
             cfg=config_file
         )
-        raise KeyError(msg)
+        raise exceptions.InvalidConfiguration(msg)
 
 
 _HOSTNAMES = set(["pypi.python.org", "testpypi.python.org", "upload.pypi.org",
diff --git a/twine/wheel.py b/twine/wheel.py
index cf6155d..8a52d6e 100644
--- a/twine/wheel.py
+++ b/twine/wheel.py
@@ -26,6 +26,8 @@ except ImportError:
 from pkginfo import distribution
 from pkginfo.distribution import Distribution
 
+from twine import exceptions
+
 # Monkeypatch Metadata 2.0 support
 distribution.HEADER_ATTRS_2_0 = distribution.HEADER_ATTRS_1_2
 distribution.HEADER_ATTRS.update({"2.0": distribution.HEADER_ATTRS_2_0})
@@ -69,7 +71,9 @@ class Wheel(Distribution):
     def read(self):
         fqn = os.path.abspath(os.path.normpath(self.filename))
         if not os.path.exists(fqn):
-            raise ValueError('No such file: %s' % fqn)
+            raise exceptions.InvalidDistribution(
+                'No such file: %s' % fqn
+            )
 
         if fqn.endswith('.whl'):
             archive = zipfile.ZipFile(fqn)
@@ -78,7 +82,9 @@ class Wheel(Distribution):
             def read_file(name):
                 return archive.read(name)
         else:
-            raise ValueError('Not a known archive format: %s' % fqn)
+            raise exceptions.InvalidDistribution(
+                'Not a known archive format: %s' % fqn
+            )
 
         try:
             for path in self.find_candidate_metadata_files(names):
@@ -89,7 +95,9 @@ class Wheel(Distribution):
         finally:
             archive.close()
 
-        raise ValueError('No METADATA in archive: %s' % fqn)
+        raise exceptions.InvalidDistribution(
+            'No METADATA in archive: %s' % fqn
+        )
 
     def parse(self, data):
         super(Wheel, self).parse(data)
diff --git a/twine/wininst.py b/twine/wininst.py
index 6951070..5e8932a 100644
--- a/twine/wininst.py
+++ b/twine/wininst.py
@@ -7,6 +7,8 @@ import zipfile
 
 from pkginfo.distribution import Distribution
 
+from twine import exceptions
+
 wininst_file_re = re.compile(r".*py(?P<pyver>\d+\.\d+)\.exe$")
 
 
@@ -28,7 +30,9 @@ class WinInst(Distribution):
     def read(self):
         fqn = os.path.abspath(os.path.normpath(self.filename))
         if not os.path.exists(fqn):
-            raise ValueError('No such file: %s' % fqn)
+            raise exceptions.InvalidDistribution(
+                'No such file: %s' % fqn
+            )
 
         if fqn.endswith('.exe'):
             archive = zipfile.ZipFile(fqn)
@@ -37,7 +41,9 @@ class WinInst(Distribution):
             def read_file(name):
                 return archive.read(name)
         else:
-            raise ValueError('Not a known archive format: %s' % fqn)
+            raise exceptions.InvalidDistribution(
+                'Not a known archive format: %s' % fqn
+            )
 
         try:
             tuples = [x.split('/') for x in names
@@ -51,4 +57,6 @@ class WinInst(Distribution):
         finally:
             archive.close()
 
-        raise ValueError('No PKG-INFO/.egg-info in archive: %s' % fqn)
+        raise exceptions.InvalidDistribution(
+            'No PKG-INFO/.egg-info in archive: %s' % fqn
+        )


Test patch:
diff --git a/tests/test_commands.py b/tests/test_commands.py
index 44252d4..af556af 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -3,6 +3,7 @@ import os
 import pytest
 
 from twine.commands import _find_dists, _group_wheel_files_first
+from twine import exceptions
 
 
 def test_ensure_wheel_files_uploaded_first():
@@ -34,7 +35,7 @@ def test_find_dists_expands_globs():
 
 
 def test_find_dists_errors_on_invalid_globs():
-    with pytest.raises(ValueError):
+    with pytest.raises(exceptions.InvalidDistribution):
         _find_dists(["twine/*.rb"])
 
 
diff --git a/tests/test_main.py b/tests/test_main.py
index e7c6037..9c8d20c 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -11,11 +11,14 @@
 # limitations under the License.
 
 from twine import __main__ as dunder_main
+from twine import exceptions
 
 import pretend
 
 
 def test_exception_handling(monkeypatch):
-    replaced_dispatch = pretend.raiser(KeyError('foo'))
+    replaced_dispatch = pretend.raiser(
+        exceptions.InvalidConfiguration('foo')
+    )
     monkeypatch.setattr(dunder_main, 'dispatch', replaced_dispatch)
-    assert dunder_main.main() == 'KeyError: foo'
+    assert dunder_main.main() == 'InvalidConfiguration: foo'