Improve redirect error message

docs/changelog.rst

@@ -3,6 +3,7 @@
 =========
 Changelog
 =========
+* :feature:`310` Now provide a more meaningful error on redirect during upload.
 * :release:`1.13.0 <2019-02-13>`
 * :bug:`452` Restore prompts while retaining support for suppressing prompts.
 * :bug:`447` Avoid requests-toolbelt to 0.9.0 to prevent attempting to use

tests/conftest.py

@@ -0,0 +1,31 @@
+import textwrap
+
+import pytest
+
+from twine import settings
+
+
+@pytest.fixture()
+def pypirc(tmpdir):
+    return tmpdir / ".pypirc"
+
+
+@pytest.fixture()
+def make_settings(pypirc):
+    """Returns a factory function for settings.Settings with defaults."""
+
+    default_pypirc = """
+        [pypi]
+        username:foo
+        password:bar
+    """
+
+    def _settings(pypirc_text=default_pypirc, **settings_kwargs):
+        pypirc.write(textwrap.dedent(pypirc_text))
+
+        settings_kwargs.setdefault('sign_with', None)
+        settings_kwargs.setdefault('config_file', str(pypirc))
+
+        return settings.Settings(**settings_kwargs)
+
+    return _settings

tests/test_register.py

@@ -0,0 +1,39 @@
+from __future__ import unicode_literals
+
+import pytest
+import pretend
+
+from twine.commands import register
+from twine import exceptions
+
+
+# TODO: Copied from test_upload.py. Extract to helpers?
+
+WHEEL_FIXTURE = 'tests/fixtures/twine-1.5.0-py2.py3-none-any.whl'
+
+
+def test_exception_for_redirect(make_settings):
+    register_settings = make_settings("""
+        [pypi]
+        repository: https://test.pypi.org/legacy
+        username:foo
+        password:bar
+    """)
+
+    stub_response = pretend.stub(
+        is_redirect=True,
+        status_code=301,
+        headers={'location': 'https://test.pypi.org/legacy/'}
+    )
+
+    stub_repository = pretend.stub(
+        register=lambda package: stub_response,
+        close=lambda: None
+    )
+
+    register_settings.create_repository = lambda: stub_repository
+
+    with pytest.raises(exceptions.RedirectDetected) as err:
+        register.register(register_settings, WHEEL_FIXTURE)
+
+    assert "https://test.pypi.org/legacy/" in err.value.args[0]

tests/test_upload.py

@@ -13,46 +13,18 @@
 # limitations under the License.
 from __future__ import unicode_literals
 
-import textwrap
-
 import pretend
 import pytest
 
 from twine.commands import upload
-from twine import package, cli, exceptions, settings
+from twine import package, cli, exceptions
 import twine
 
 import helpers
 
 WHEEL_FIXTURE = 'tests/fixtures/twine-1.5.0-py2.py3-none-any.whl'
 
 
-@pytest.fixture()
-def pypirc(tmpdir):
-    return tmpdir / ".pypirc"
-
-
-@pytest.fixture()
-def make_settings(pypirc):
-    """Returns a factory function for settings.Settings with defaults."""
-
-    default_pypirc = """
-        [pypi]
-        username:foo
-        password:bar
-    """
-
-    def _settings(pypirc_text=default_pypirc, **settings_kwargs):
-        pypirc.write(textwrap.dedent(pypirc_text))
-
-        settings_kwargs.setdefault('sign_with', None)
-        settings_kwargs.setdefault('config_file', str(pypirc))
-
-        return settings.Settings(**settings_kwargs)
-
-    return _settings
-
-
 def test_successful_upload(make_settings):
     upload_settings = make_settings()
 
@@ -75,21 +47,20 @@ def test_successful_upload(make_settings):
     assert result is None
 
 
-def test_get_config_old_format(make_settings):
+def test_get_config_old_format(make_settings, pypirc):
     try:
         make_settings("""
             [server-login]
             username:foo
             password:bar
         """)
     except KeyError as err:
-        assert err.args[0] == (
-            "Missing 'pypi' section from the configuration file\n"
-            "or not a complete URL in --repository-url.\n"
-            "Maybe you have a out-dated '{0}' format?\n"
-            "more info: "
-            "https://docs.python.org/distutils/packageindex.html#pypirc\n"
-        ).format(pypirc)
+        assert all(text in err.args[0] for text in [
+            "'pypi'",
+            "--repository-url",
+            pypirc,
+            "https://docs.python.org/",
+        ])
 
 
 def test_deprecated_repo(make_settings):
@@ -103,19 +74,40 @@ def test_deprecated_repo(make_settings):
 
         upload.upload(upload_settings, [WHEEL_FIXTURE])
 
-    assert err.value.args[0] == (
-        "You're trying to upload to the legacy PyPI site "
-        "'https://pypi.python.org/pypi/'. "
-        "Uploading to those sites is deprecated. \n "
-        "The new sites are pypi.org and test.pypi.org. Try using "
-        "https://upload.pypi.org/legacy/ "
-        "(or https://test.pypi.org/legacy/) "
-        "to upload your packages instead. "
-        "These are the default URLs for Twine now. \n "
-        "More at "
-        "https://packaging.python.org/guides/migrating-to-pypi-org/ ."
+    assert all(text in err.value.args[0] for text in [
+        "https://pypi.python.org/pypi/",
+        "https://upload.pypi.org/legacy/",
+        "https://test.pypi.org/legacy/",
+        "https://packaging.python.org/",
+    ])
+
+
+def test_exception_for_redirect(make_settings):
+    upload_settings = make_settings("""
+        [pypi]
+        repository: https://test.pypi.org/legacy
+        username:foo
+        password:bar
+    """)
+
+    stub_response = pretend.stub(
+        is_redirect=True,
+        status_code=301,
+        headers={'location': 'https://test.pypi.org/legacy/'}
     )
 
+    stub_repository = pretend.stub(
+        upload=lambda package: stub_response,
+        close=lambda: None
+    )
+
+    upload_settings.create_repository = lambda: stub_repository
+
+    with pytest.raises(exceptions.RedirectDetected) as err:
+        upload.upload(upload_settings, [WHEEL_FIXTURE])
+
+    assert "https://test.pypi.org/legacy/" in err.value.args[0]
+
 
 def test_prints_skip_message_for_uploaded_package(make_settings, capsys):
     upload_settings = make_settings(skip_existing=True)

twine/commands/register.py

@@ -38,10 +38,10 @@ def register(register_settings, package):
     repository.close()
 
     if resp.is_redirect:
-        raise exceptions.RedirectDetected(
-            ('"{0}" attempted to redirect to "{1}" during registration.'
-             ' Aborting...').format(repository_url,
-                                    resp.headers["location"]))
+        raise exceptions.RedirectDetected.from_args(
+            repository_url,
+            resp.headers["location"],
+        )
 
     resp.raise_for_status()
 

twine/commands/upload.py

@@ -92,10 +92,10 @@ def upload(upload_settings, dists):
         # by PyPI should never happen in reality. This should catch malicious
         # redirects as well.
         if resp.is_redirect:
-            raise exceptions.RedirectDetected(
-                ('"{0}" attempted to redirect to "{1}" during upload.'
-                 ' Aborting...').format(repository_url,
-                                        resp.headers["location"]))
+            raise exceptions.RedirectDetected.from_args(
+                repository_url,
+                resp.headers["location"],
+            )
 
         if skip_upload(resp, upload_settings.skip_existing, package):
             print(skip_message)

twine/exceptions.py

@@ -29,7 +29,17 @@ class RedirectDetected(TwineException):
     redirecting them.
     """
 
-    pass
+    @classmethod
+    def from_args(cls, repository_url, redirect_url):
+        msg = "\n".join([
+            "{} attempted to redirect to {}."
+            .format(repository_url, redirect_url),
+            "If you trust these URLs, set {} as your repository URL."
+            .format(redirect_url),
+            "Aborting."
+        ])
+
+        return cls(msg)
 
 
 class PackageNotFound(TwineException):