Catch + report BadZipFile errors in get_wheel_distribution

news/10535.feature.rst

@@ -0,0 +1 @@
+Present a better error message when an invalid wheel file is encountered, providing more context where the invalid wheel file is.

src/pip/_internal/exceptions.py

@@ -132,6 +132,17 @@ class UnsupportedWheel(InstallationError):
     """Unsupported wheel."""
 
 
+class InvalidWheel(InstallationError):
+    """Invalid (e.g. corrupt) wheel."""
+
+    def __init__(self, location: str, name: str):
+        self.location = location
+        self.name = name
+
+    def __str__(self) -> str:
+        return f"Wheel '{self.name}' located at {self.location} is invalid."
+
+
 class MetadataInconsistent(InstallationError):
     """Built metadata contains inconsistent information.
 

src/pip/_internal/metadata/pkg_resources.py

@@ -1,12 +1,14 @@
 import email.message
 import logging
 from typing import Collection, Iterable, Iterator, List, NamedTuple, Optional
+from zipfile import BadZipFile
 
 from pip._vendor import pkg_resources
 from pip._vendor.packaging.requirements import Requirement
 from pip._vendor.packaging.utils import NormalizedName, canonicalize_name
 from pip._vendor.packaging.version import parse as parse_version
 
+from pip._internal.exceptions import InvalidWheel
 from pip._internal.utils import misc  # TODO: Move definition here.
 from pip._internal.utils.packaging import get_installer, get_metadata
 from pip._internal.utils.wheel import pkg_resources_distribution_for_wheel
@@ -34,8 +36,16 @@ def __init__(self, dist: pkg_resources.Distribution) -> None:
 
     @classmethod
     def from_wheel(cls, wheel: Wheel, name: str) -> "Distribution":
-        with wheel.as_zipfile() as zf:
-            dist = pkg_resources_distribution_for_wheel(zf, name, wheel.location)
+        """Load the distribution from a given wheel.
+
+        :raises InvalidWheel: Whenever loading of the wheel causes a
+            :py:exc:`zipfile.BadZipFile` exception to be thrown.
+        """
+        try:
+            with wheel.as_zipfile() as zf:
+                dist = pkg_resources_distribution_for_wheel(zf, name, wheel.location)
+        except BadZipFile as e:
+            raise InvalidWheel(wheel.location, name) from e
         return cls(dist)
 
     @property

tests/data/packages/corruptwheel-1.0-py2.py3-none-any.whl

@@ -0,0 +1 @@
+This is a corrupt wheel which _clearly_ is not a zip file.

tests/functional/test_install_wheel.py

@@ -45,13 +45,20 @@ def test_install_from_future_wheel_version(script, tmpdir):
     result.assert_installed("futurewheel", without_egg_link=True, editable=False)
 
 
-def test_install_from_broken_wheel(script, data):
+@pytest.mark.parametrize(
+    "wheel_name",
+    [
+        "brokenwheel-1.0-py2.py3-none-any.whl",
+        "corruptwheel-1.0-py2.py3-none-any.whl",
+    ],
+)
+def test_install_from_broken_wheel(script, data, wheel_name):
     """
     Test that installing a broken wheel fails properly
     """
     from tests.lib import TestFailure
 
-    package = data.packages.joinpath("brokenwheel-1.0-py2.py3-none-any.whl")
+    package = data.packages.joinpath(wheel_name)
     result = script.pip("install", package, "--no-index", expect_error=True)
     with pytest.raises(TestFailure):
         result.assert_installed("futurewheel", without_egg_link=True, editable=False)

tests/unit/test_network_lazy_wheel.py

@@ -1,9 +1,9 @@
 from typing import Iterator
-from zipfile import BadZipfile
 
 from pip._vendor.packaging.version import Version
 from pytest import fixture, mark, raises
 
+from pip._internal.exceptions import InvalidWheel
 from pip._internal.network.lazy_wheel import (
     HTTPRangeRequestUnsupported,
     dist_from_wheel_url,
@@ -62,5 +62,5 @@ def test_dist_from_wheel_url_no_range(
 @mark.network
 def test_dist_from_wheel_url_not_zip(session: PipSession) -> None:
     """Test handling with the given URL does not point to a ZIP."""
-    with raises(BadZipfile):
+    with raises(InvalidWheel):
         dist_from_wheel_url("python", "https://www.python.org/", session)

tests/unit/test_wheel.py

@@ -252,6 +252,15 @@ def test_wheel_root_is_purelib(text: str, expected: bool) -> None:
     assert wheel.wheel_root_is_purelib(message_from_string(text)) == expected
 
 
+def test_dist_from_broken_wheel_fails(data: TestData) -> None:
+    from pip._internal.exceptions import InvalidWheel
+    from pip._internal.metadata import FilesystemWheel, get_wheel_distribution
+
+    package = data.packages.joinpath("corruptwheel-1.0-py2.py3-none-any.whl")
+    with pytest.raises(InvalidWheel):
+        get_wheel_distribution(FilesystemWheel(package), "brokenwheel")
+
+
 class TestWheelFile:
     def test_unpack_wheel_no_flatten(self, tmpdir: Path) -> None:
         filepath = os.path.join(DATA_DIR, "packages", "meta-1.0-py2.py3-none-any.whl")