Implement granular URL error hierarchy in the HTTP client

This patch introduces 5 granular user-facing exceptions that may occur
when HTTP requests are made:
* `InvalidUrlClientError`
* `RedirectClientError`
* `NonHttpUrlClientError`
* `InvalidUrlRedirectClientError`
* `NonHttpUrlRedirectClientError`

Previously `ValueError` or `InvalidURL` was raised and screening out was
complicated (a valid URL that redirects to invalid one raised the same error
as an invalid URL).

Ref: aio-libs#6722 (comment)

PR aio-libs#6722

Resolves aio-libs#2507
Resolves aio-libs#2630
Resolves aio-libs#3315

Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit fb465e1)

CHANGES/2507.feature.rst

@@ -0,0 +1 @@
+6722.feature

CHANGES/3315.feature.rst

@@ -0,0 +1 @@
+6722.feature

CHANGES/6722.feature

@@ -0,0 +1,12 @@
+Added 5 new exceptions: :py:exc:`~aiohttp.InvalidUrlClientError`, :py:exc:`~aiohttp.RedirectClientError`,
+:py:exc:`~aiohttp.NonHttpUrlClientError`, :py:exc:`~aiohttp.InvalidUrlRedirectClientError`,
+:py:exc:`~aiohttp.NonHttpUrlRedirectClientError`
+
+:py:exc:`~aiohttp.InvalidUrlRedirectClientError`, :py:exc:`~aiohttp.NonHttpUrlRedirectClientError`
+are raised instead of :py:exc:`ValueError` or :py:exc:`~aiohttp.InvalidURL` when the redirect URL is invalid. Classes
+:py:exc:`~aiohttp.InvalidUrlClientError`, :py:exc:`~aiohttp.RedirectClientError`,
+:py:exc:`~aiohttp.NonHttpUrlClientError` are base for them.
+
+The :py:exc:`~aiohttp.InvalidURL` now exposes a ``description`` property with the text explanation of the error details.
+
+-- by :user:`setla`

CONTRIBUTORS.txt

@@ -366,5 +366,6 @@ Yuvi Panda
 Zainab Lawal
 Zeal Wierslee
 Zlatan Sičanica
+Łukasz Setla
 Марк Коренберг
 Семён Марьясин

aiohttp/__init__.py

@@ -25,7 +25,12 @@
     ContentTypeError,
     Fingerprint,
     InvalidURL,
+    InvalidUrlClientError,
+    InvalidUrlRedirectClientError,
     NamedPipeConnector,
+    NonHttpUrlClientError,
+    NonHttpUrlRedirectClientError,
+    RedirectClientError,
     RequestInfo,
     ServerConnectionError,
     ServerDisconnectedError,
@@ -137,6 +142,11 @@
     "ContentTypeError",
     "Fingerprint",
     "InvalidURL",
+    "InvalidUrlClientError",
+    "InvalidUrlRedirectClientError",
+    "NonHttpUrlClientError",
+    "NonHttpUrlRedirectClientError",
+    "RedirectClientError",
     "RequestInfo",
     "ServerConnectionError",
     "ServerDisconnectedError",

aiohttp/client.py

@@ -52,6 +52,11 @@
     ConnectionTimeoutError,
     ContentTypeError,
     InvalidURL,
+    InvalidUrlClientError,
+    InvalidUrlRedirectClientError,
+    NonHttpUrlClientError,
+    NonHttpUrlRedirectClientError,
+    RedirectClientError,
     ServerConnectionError,
     ServerDisconnectedError,
     ServerFingerprintMismatch,
@@ -109,6 +114,11 @@
     "ConnectionTimeoutError",
     "ContentTypeError",
     "InvalidURL",
+    "InvalidUrlClientError",
+    "RedirectClientError",
+    "NonHttpUrlClientError",
+    "InvalidUrlRedirectClientError",
+    "NonHttpUrlRedirectClientError",
     "ServerConnectionError",
     "ServerDisconnectedError",
     "ServerFingerprintMismatch",
@@ -168,6 +178,7 @@ class ClientTimeout:
 
 # https://www.rfc-editor.org/rfc/rfc9110#section-9.2.2
 IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "TRACE", "PUT", "DELETE"})
+HTTP_SCHEMA_SET = frozenset({"http", "https", ""})
 
 _RetType = TypeVar("_RetType")
 _CharsetResolver = Callable[[ClientResponse, bytes], str]
@@ -455,7 +466,10 @@ async def _request(
         try:
             url = self._build_url(str_or_url)
         except ValueError as e:
-            raise InvalidURL(str_or_url) from e
+            raise InvalidUrlClientError(str_or_url) from e
+
+        if url.scheme not in HTTP_SCHEMA_SET:
+            raise NonHttpUrlClientError(url)
 
         skip_headers = set(self._skip_auto_headers)
         if skip_auto_headers is not None:
@@ -513,6 +527,15 @@ async def _request(
                 retry_persistent_connection = method in IDEMPOTENT_METHODS
                 while True:
                     url, auth_from_url = strip_auth_from_url(url)
+                    if not url.raw_host:
+                        # NOTE: Bail early, otherwise, causes `InvalidURL` through
+                        # NOTE: `self._request_class()` below.
+                        err_exc_cls = (
+                            InvalidUrlRedirectClientError
+                            if redirects
+                            else InvalidUrlClientError
+                        )
+                        raise err_exc_cls(url)
                     if auth and auth_from_url:
                         raise ValueError(
                             "Cannot combine AUTH argument with "
@@ -670,25 +693,35 @@ async def _request(
                             resp.release()
 
                         try:
-                            parsed_url = URL(
+                            parsed_redirect_url = URL(
                                 r_url, encoded=not self._requote_redirect_url
                             )
-
                         except ValueError as e:
-                            raise InvalidURL(r_url) from e
+                            raise InvalidUrlRedirectClientError(
+                                r_url,
+                                "Server attempted redirecting to a location that does not look like a URL",
+                            ) from e
 
-                        scheme = parsed_url.scheme
-                        if scheme not in ("http", "https", ""):
+                        scheme = parsed_redirect_url.scheme
+                        if scheme not in HTTP_SCHEMA_SET:
                             resp.close()
-                            raise ValueError("Can redirect only to http or https")
+                            raise NonHttpUrlRedirectClientError(r_url)
                         elif not scheme:
-                            parsed_url = url.join(parsed_url)
+                            parsed_redirect_url = url.join(parsed_redirect_url)
 
-                        if url.origin() != parsed_url.origin():
+                        try:
+                            redirect_origin = parsed_redirect_url.origin()
+                        except ValueError as origin_val_err:
+                            raise InvalidUrlRedirectClientError(
+                                parsed_redirect_url,
+                                "Invalid redirect URL origin",
+                            ) from origin_val_err
+
+                        if url.origin() != redirect_origin:
                             auth = None
                             headers.pop(hdrs.AUTHORIZATION, None)
 
-                        url = parsed_url
+                        url = parsed_redirect_url
                         params = {}
                         resp.release()
                         continue

aiohttp/client_exceptions.py

@@ -2,10 +2,10 @@
 
 import asyncio
 import warnings
-from typing import TYPE_CHECKING, Any, Optional, Tuple, Union
+from typing import TYPE_CHECKING, Optional, Tuple, Union
 
 from .http_parser import RawResponseMessage
-from .typedefs import LooseHeaders
+from .typedefs import LooseHeaders, StrOrURL
 
 try:
     import ssl
@@ -41,6 +41,11 @@
     "ContentTypeError",
     "ClientPayloadError",
     "InvalidURL",
+    "InvalidUrlClientError",
+    "RedirectClientError",
+    "NonHttpUrlClientError",
+    "InvalidUrlRedirectClientError",
+    "NonHttpUrlRedirectClientError",
 )
 
 
@@ -281,17 +286,52 @@ class InvalidURL(ClientError, ValueError):
 
     # Derive from ValueError for backward compatibility
 
-    def __init__(self, url: Any) -> None:
+    def __init__(self, url: StrOrURL, description: Union[str, None] = None) -> None:
         # The type of url is not yarl.URL because the exception can be raised
         # on URL(url) call
-        super().__init__(url)
+        self._url = url
+        self._description = description
+
+        if description:
+            super().__init__(url, description)
+        else:
+            super().__init__(url)
+
+    @property
+    def url(self) -> StrOrURL:
+        return self._url
 
     @property
-    def url(self) -> Any:
-        return self.args[0]
+    def description(self) -> "str | None":
+        return self._description
 
     def __repr__(self) -> str:
-        return f"<{self.__class__.__name__} {self.url}>"
+        return f"<{self.__class__.__name__} {self}>"
+
+    def __str__(self) -> str:
+        if self._description:
+            return f"{self._url} - {self._description}"
+        return str(self._url)
+
+
+class InvalidUrlClientError(InvalidURL):
+    """Invalid URL client error."""
+
+
+class RedirectClientError(ClientError):
+    """Client redirect error."""
+
+
+class NonHttpUrlClientError(ClientError):
+    """Non http URL client error."""
+
+
+class InvalidUrlRedirectClientError(InvalidUrlClientError, RedirectClientError):
+    """Invalid URL redirect client error."""
+
+
+class NonHttpUrlRedirectClientError(NonHttpUrlClientError, RedirectClientError):
+    """Non http URL redirect client error."""
 
 
 class ClientSSLError(ClientConnectorError):

docs/client_reference.rst

@@ -2115,6 +2115,41 @@ All exceptions are available as members of *aiohttp* module.
 
       Invalid URL, :class:`yarl.URL` instance.
 
+    .. attribute:: description
+
+      Invalid URL description, :class:`str` instance or :data:`None`.
+
+.. exception:: InvalidUrlClientError
+
+   Base class for all errors related to client url.
+
+   Derived from :exc:`InvalidURL`
+
+.. exception:: RedirectClientError
+
+   Base class for all errors related to client redirects.
+
+   Derived from :exc:`ClientError`
+
+.. exception:: NonHttpUrlClientError
+
+   Base class for all errors related to non http client urls.
+
+   Derived from :exc:`ClientError`
+
+.. exception:: InvalidUrlRedirectClientError
+
+   Redirect URL is malformed, e.g. it does not contain host part.
+
+   Derived from :exc:`InvalidUrlClientError` and :exc:`RedirectClientError`
+
+.. exception:: NonHttpUrlRedirectClientError
+
+   Redirect URL does not contain http schema.
+
+   Derived from :exc:`RedirectClientError` and :exc:`NonHttpUrlClientError`
+
+
 .. class:: ContentDisposition
 
     Represent Content-Disposition header
@@ -2331,3 +2366,17 @@ Hierarchy of exceptions
     * :exc:`WSServerHandshakeError`
 
   * :exc:`InvalidURL`
+
+    * :exc:`InvalidUrlClientError`
+
+      * :exc:`InvalidUrlRedirectClientError`
+
+  * :exc:`NonHttpUrlClientError`
+
+    * :exc:`NonHttpUrlRedirectClientError`
+
+  * :exc:`RedirectClientError`
+
+    * :exc:`InvalidUrlRedirectClientError`
+
+    * :exc:`NonHttpUrlRedirectClientError`

tests/test_client_exceptions.py

@@ -5,6 +5,7 @@
 from unittest import mock
 
 import pytest
+from yarl import URL
 
 from aiohttp import client, client_reqrep
 
@@ -298,8 +299,9 @@ def test_repr(self) -> None:
 
 class TestInvalidURL:
     def test_ctor(self) -> None:
-        err = client.InvalidURL(url=":wrong:url:")
+        err = client.InvalidURL(url=":wrong:url:", description=":description:")
         assert err.url == ":wrong:url:"
+        assert err.description == ":description:"
 
     def test_pickle(self) -> None:
         err = client.InvalidURL(url=":wrong:url:")
@@ -310,10 +312,27 @@ def test_pickle(self) -> None:
             assert err2.url == ":wrong:url:"
             assert err2.foo == "bar"
 
-    def test_repr(self) -> None:
+    def test_repr_no_description(self) -> None:
         err = client.InvalidURL(url=":wrong:url:")
+        assert err.args == (":wrong:url:",)
         assert repr(err) == "<InvalidURL :wrong:url:>"
 
-    def test_str(self) -> None:
+    def test_repr_yarl_URL(self) -> None:
+        err = client.InvalidURL(url=URL(":wrong:url:"))
+        assert repr(err) == "<InvalidURL :wrong:url:>"
+
+    def test_repr_with_description(self) -> None:
+        err = client.InvalidURL(url=":wrong:url:", description=":description:")
+        assert repr(err) == "<InvalidURL :wrong:url: - :description:>"
+
+    def test_str_no_description(self) -> None:
         err = client.InvalidURL(url=":wrong:url:")
         assert str(err) == ":wrong:url:"
+
+    def test_none_description(self) -> None:
+        err = client.InvalidURL(":wrong:url:")
+        assert err.description is None
+
+    def test_str_with_description(self) -> None:
+        err = client.InvalidURL(url=":wrong:url:", description=":description:")
+        assert str(err) == ":wrong:url: - :description:"