Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception hierarchy #1095

Merged
merged 9 commits into from
Jul 31, 2020
Merged

Exception hierarchy #1095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9b4a00e
Exception heirachy
tomchristie Jul 27, 2020
f022092
Exception heirarchy
tomchristie Jul 27, 2020
d57f7c8
Formatting tweaks
tomchristie Jul 27, 2020
6f83adb
Merge branch 'master' into exception-heirarchy
florimondmanca Jul 30, 2020
d1cece3
Update httpx/_exceptions.py
tomchristie Jul 31, 2020
124bd13
Update httpx/_exceptions.py
tomchristie Jul 31, 2020
4886574
Update httpx/_exceptions.py
tomchristie Jul 31, 2020
63aa028
Update httpx/_exceptions.py
tomchristie Jul 31, 2020
639be5d
Merge branch 'master' into exception-heirarchy
tomchristie Jul 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions httpx/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,16 @@
ProxyError,
ReadError,
ReadTimeout,
RedirectError,
RequestBodyUnavailable,
RequestError,
RequestNotRead,
ResponseClosed,
ResponseNotRead,
StreamConsumed,
StreamError,
TimeoutException,
TooManyRedirects,
TransportError,
WriteError,
WriteTimeout,
)
Expand Down Expand Up @@ -76,7 +77,7 @@
"ProtocolError",
"ReadError",
"ReadTimeout",
"RedirectError",
"RequestError",
"RequestBodyUnavailable",
"ResponseClosed",
"ResponseNotRead",
Expand All @@ -87,6 +88,7 @@
"ProxyError",
"TimeoutException",
"TooManyRedirects",
"TransportError",
"WriteError",
"WriteTimeout",
"URL",
Expand Down
24 changes: 16 additions & 8 deletions httpx/_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,24 @@ def auth_flow(self, request: Request) -> typing.Generator[Request, Response, Non
# need to build an authenticated request.
return

header = response.headers["www-authenticate"]
challenge = self._parse_challenge(header)
challenge = self._parse_challenge(request, response)
request.headers["Authorization"] = self._build_auth_header(request, challenge)
yield request

def _parse_challenge(self, header: str) -> "_DigestAuthChallenge":
def _parse_challenge(
self, request: Request, response: Response
) -> "_DigestAuthChallenge":
"""
Returns a challenge from a Digest WWW-Authenticate header.
These take the form of:
`Digest realm="[email protected]",qop="auth,auth-int",nonce="abc",opaque="xyz"`
"""
header = response.headers["www-authenticate"]

scheme, _, fields = header.partition(" ")
if scheme.lower() != "digest":
raise ProtocolError("Header does not start with 'Digest'")
message = "Header does not start with 'Digest'"
raise ProtocolError(message, request=request)

header_dict: typing.Dict[str, str] = {}
for field in parse_http_list(fields):
Expand All @@ -146,7 +150,8 @@ def _parse_challenge(self, header: str) -> "_DigestAuthChallenge":
realm=realm, nonce=nonce, qop=qop, opaque=opaque, algorithm=algorithm
)
except KeyError as exc:
raise ProtocolError("Malformed Digest WWW-Authenticate header") from exc
message = "Malformed Digest WWW-Authenticate header"
raise ProtocolError(message, request=request) from exc

def _build_auth_header(
self, request: Request, challenge: "_DigestAuthChallenge"
Expand All @@ -171,7 +176,7 @@ def digest(data: bytes) -> bytes:
if challenge.algorithm.lower().endswith("-sess"):
HA1 = digest(b":".join((HA1, challenge.nonce, cnonce)))

qop = self._resolve_qop(challenge.qop)
qop = self._resolve_qop(challenge.qop, request=request)
if qop is None:
digest_data = [HA1, challenge.nonce, HA2]
else:
Expand Down Expand Up @@ -221,7 +226,9 @@ def _get_header_value(self, header_fields: typing.Dict[str, bytes]) -> str:

return header_value

def _resolve_qop(self, qop: typing.Optional[bytes]) -> typing.Optional[bytes]:
def _resolve_qop(
self, qop: typing.Optional[bytes], request: Request
) -> typing.Optional[bytes]:
if qop is None:
return None
qops = re.split(b", ?", qop)
Expand All @@ -231,7 +238,8 @@ def _resolve_qop(self, qop: typing.Optional[bytes]) -> typing.Optional[bytes]:
if qops == [b"auth-int"]:
raise NotImplementedError("Digest auth-int support is not yet implemented")

raise ProtocolError(f'Unexpected qop value "{qop!r}" in digest auth')
message = f'Unexpected qop value "{qop!r}" in digest auth'
raise ProtocolError(message, request=request)


class _DigestAuthChallenge:
Expand Down
20 changes: 12 additions & 8 deletions httpx/_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ def _redirect_url(self, request: Request, response: Response) -> URL:

# Check that we can handle the scheme
if url.scheme and url.scheme not in ("http", "https"):
raise InvalidURL(f'Scheme "{url.scheme}" not supported.')
message = f'Scheme "{url.scheme}" not supported.'
raise InvalidURL(message, request=request)

# Handle malformed 'Location' headers that are "absolute" form, have no host.
# See: https://github.com/encode/httpx/issues/771
Expand Down Expand Up @@ -537,12 +538,13 @@ def _init_proxy_transport(
http2=http2,
)

def _transport_for_url(self, url: URL) -> httpcore.SyncHTTPTransport:
def _transport_for_url(self, request: Request) -> httpcore.SyncHTTPTransport:
"""
Returns the transport instance that should be used for a given URL.
This will either be the standard connection pool, or a proxy.
"""
enforce_http_url(url)
url = request.url
enforce_http_url(request)

if self._proxies and not should_not_be_proxied(url):
for matcher, transport in self._proxies.items():
Expand Down Expand Up @@ -590,7 +592,8 @@ def send(
timeout: typing.Union[TimeoutTypes, UnsetType] = UNSET,
) -> Response:
if request.url.scheme not in ("http", "https"):
raise InvalidURL('URL scheme must be "http" or "https".')
message = 'URL scheme must be "http" or "https".'
raise InvalidURL(message, request=request)

timeout = self.timeout if isinstance(timeout, UnsetType) else Timeout(timeout)

Expand Down Expand Up @@ -682,7 +685,7 @@ def _send_single_request(self, request: Request, timeout: Timeout) -> Response:
"""
Sends a single request, without handling any redirections.
"""
transport = self._transport_for_url(request.url)
transport = self._transport_for_url(request)

with map_exceptions(HTTPCORE_EXC_MAP, request=request):
(
Expand Down Expand Up @@ -1059,12 +1062,13 @@ def _init_proxy_transport(
http2=http2,
)

def _transport_for_url(self, url: URL) -> httpcore.AsyncHTTPTransport:
def _transport_for_url(self, request: Request) -> httpcore.AsyncHTTPTransport:
"""
Returns the transport instance that should be used for a given URL.
This will either be the standard connection pool, or a proxy.
"""
enforce_http_url(url)
url = request.url
enforce_http_url(request)

if self._proxies and not should_not_be_proxied(url):
for matcher, transport in self._proxies.items():
Expand Down Expand Up @@ -1204,7 +1208,7 @@ async def _send_single_request(
"""
Sends a single request, without handling any redirections.
"""
transport = self._transport_for_url(request.url)
transport = self._transport_for_url(request)

with map_exceptions(HTTPCORE_EXC_MAP, request=request):
(
Expand Down
41 changes: 26 additions & 15 deletions httpx/_decoders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@
except ImportError: # pragma: nocover
brotli = None

if typing.TYPE_CHECKING: # pragma: no cover
from ._models import Request


class Decoder:
def __init__(self, request: "Request") -> None:
self.request = request

def decode(self, data: bytes) -> bytes:
raise NotImplementedError() # pragma: nocover

Expand All @@ -44,7 +50,8 @@ class DeflateDecoder(Decoder):
See: https://stackoverflow.com/questions/1838699
"""

def __init__(self) -> None:
def __init__(self, request: "Request") -> None:
self.request = request
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? (Applicable to all other decoders here)

Suggested change
self.request = request
super().__init__(request)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah a guess a lot of other Python developers would tend always just rely on "hey I'm supposed to call super right". Personally, I think explicit is preferable to indirect here. Calling into super isn't always necessary or inherently the "right" thing to do.

Copy link
Member

@florimondmanca florimondmanca Jul 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, well then I guess Decoder would really be better served by a Protocol, rather than a full-fledged base class… But we only have them on Py38+, which is unfortunate.

class Decoder(Protocol):
    request: Request

    def decode(self, data: bytes) -> bytes:
        ...

    def flush(self) -> bytes:
        ...

But maybe we could then also use the request: Request annotation on the Decoder interface, remove its constructor, and add an explicit constructor for IdentityDecoder (for consistency with other decoder classes)?

Copy link
Member

@cdeler cdeler Aug 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MultiDecoder inherits the Decoder interface, but do not have this property

From my point of view, MultiDecoder, TextDecoder and LineDecoder should be renamed, since they:

  1. on one hand these Decoders imitates the real decoders (having decode and flush)
  2. on the other hand these decoders violates Liskov's substitution principle (I don't want to be so nerd, mypy pointed it for me), e.g. as they accept different types as args of decode

self.first_attempt = True
self.decompressor = zlib.decompressobj()

Expand All @@ -57,13 +64,13 @@ def decode(self, data: bytes) -> bytes:
if was_first_attempt:
self.decompressor = zlib.decompressobj(-zlib.MAX_WBITS)
return self.decode(data)
raise DecodingError from exc
raise DecodingError(message=str(exc), request=self.request)

def flush(self) -> bytes:
try:
return self.decompressor.flush()
except zlib.error as exc: # pragma: nocover
raise DecodingError from exc
raise DecodingError(message=str(exc), request=self.request)


class GZipDecoder(Decoder):
Expand All @@ -73,20 +80,21 @@ class GZipDecoder(Decoder):
See: https://stackoverflow.com/questions/1838699
"""

def __init__(self) -> None:
def __init__(self, request: "Request") -> None:
self.request = request
self.decompressor = zlib.decompressobj(zlib.MAX_WBITS | 16)

def decode(self, data: bytes) -> bytes:
try:
return self.decompressor.decompress(data)
except zlib.error as exc:
raise DecodingError from exc
raise DecodingError(message=str(exc), request=self.request)

def flush(self) -> bytes:
try:
return self.decompressor.flush()
except zlib.error as exc: # pragma: nocover
raise DecodingError from exc
raise DecodingError(message=str(exc), request=self.request)


class BrotliDecoder(Decoder):
Expand All @@ -99,10 +107,11 @@ class BrotliDecoder(Decoder):
name. The top branches are for 'brotlipy' and bottom branches for 'Brotli'
"""

def __init__(self) -> None:
def __init__(self, request: "Request") -> None:
assert (
brotli is not None
), "The 'brotlipy' or 'brotli' library must be installed to use 'BrotliDecoder'"
self.request = request
self.decompressor = brotli.Decompressor()
self.seen_data = False
if hasattr(self.decompressor, "decompress"):
Expand All @@ -117,7 +126,7 @@ def decode(self, data: bytes) -> bytes:
try:
return self._decompress(data)
except brotli.error as exc:
raise DecodingError from exc
raise DecodingError(message=str(exc), request=self.request)

def flush(self) -> bytes:
if not self.seen_data:
Expand All @@ -127,7 +136,7 @@ def flush(self) -> bytes:
self.decompressor.finish()
return b""
except brotli.error as exc: # pragma: nocover
raise DecodingError from exc
raise DecodingError(message=str(exc), request=self.request)


class MultiDecoder(Decoder):
Expand Down Expand Up @@ -160,7 +169,8 @@ class TextDecoder:
Handles incrementally decoding bytes into text
"""

def __init__(self, encoding: typing.Optional[str] = None):
def __init__(self, request: "Request", encoding: typing.Optional[str] = None):
self.request = request
self.decoder: typing.Optional[codecs.IncrementalDecoder] = (
None if encoding is None else codecs.getincrementaldecoder(encoding)()
)
Expand Down Expand Up @@ -194,8 +204,8 @@ def decode(self, data: bytes) -> str:
self.buffer = None

return text
except UnicodeDecodeError: # pragma: nocover
raise DecodingError() from None
except UnicodeDecodeError as exc: # pragma: nocover
raise DecodingError(message=str(exc), request=self.request)

def flush(self) -> str:
try:
Expand All @@ -207,14 +217,15 @@ def flush(self) -> str:
return bytes(self.buffer).decode(self._detector_result())

return self.decoder.decode(b"", True)
except UnicodeDecodeError: # pragma: nocover
raise DecodingError() from None
except UnicodeDecodeError as exc: # pragma: nocover
raise DecodingError(message=str(exc), request=self.request)

def _detector_result(self) -> str:
self.detector.close()
result = self.detector.result["encoding"]
if not result: # pragma: nocover
raise DecodingError("Unable to determine encoding of content")
message = "Unable to determine encoding of content"
raise DecodingError(message, request=self.request)

return result

Expand Down
Loading