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.

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

Handle asynchronous cancellations for HTTPConnection instances #802

Closed
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## Unreleased

- Handle asynchronous cancellations for the HTTPConnection instances. (#802)
- Fix pool timeout to account for the total time spent retrying. (#823)

## 1.0.0 (October 6th, 2023)
Expand Down
14 changes: 11 additions & 3 deletions httpcore/_async/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __init__(
self._connect_failed: bool = False
self._request_lock = AsyncLock()
self._socket_options = socket_options
self._is_new = True

async def handle_async_request(self, request: Request) -> Response:
if not self.can_handle_request(request.url.origin):
Expand All @@ -73,6 +74,7 @@ async def handle_async_request(self, request: Request) -> Response:
async with self._request_lock:
if self._connection is None:
try:
self._is_new = False
stream = await self._connect(request)

ssl_object = stream.get_extra_info("ssl_object")
Expand All @@ -94,7 +96,7 @@ async def handle_async_request(self, request: Request) -> Response:
stream=stream,
keepalive_expiry=self._keepalive_expiry,
)
except Exception as exc:
except BaseException as exc:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to be sure that it works when we cancel the asynchronous task.

self._connect_failed = True
raise exc
elif not self._connection.is_available():
Expand Down Expand Up @@ -174,10 +176,12 @@ async def aclose(self) -> None:

def is_available(self) -> bool:
if self._connection is None:
if self._is_new:
return True
# If HTTP/2 support is enabled, and the resulting connection could
# end up as HTTP/2 then we should indicate the connection as being
# available to service multiple requests.
return (
return ( # pragma: no cover
self._http2
and (self._origin.scheme == b"https" or not self._http1)
and not self._connect_failed
Expand All @@ -201,7 +205,11 @@ def is_closed(self) -> bool:

def info(self) -> str:
if self._connection is None:
return "CONNECTION FAILED" if self._connect_failed else "CONNECTING"
if self._is_new:
return "NEW CONNECTION"
return (
"CONNECTION FAILED" if self._connect_failed else "CONNECTING"
) # pragma: no cover
return self._connection.info()

def __repr__(self) -> str:
Expand Down
14 changes: 11 additions & 3 deletions httpcore/_sync/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __init__(
self._connect_failed: bool = False
self._request_lock = Lock()
self._socket_options = socket_options
self._is_new = True

def handle_request(self, request: Request) -> Response:
if not self.can_handle_request(request.url.origin):
Expand All @@ -73,6 +74,7 @@ def handle_request(self, request: Request) -> Response:
with self._request_lock:
if self._connection is None:
try:
self._is_new = False
stream = self._connect(request)

ssl_object = stream.get_extra_info("ssl_object")
Expand All @@ -94,7 +96,7 @@ def handle_request(self, request: Request) -> Response:
stream=stream,
keepalive_expiry=self._keepalive_expiry,
)
except Exception as exc:
except BaseException as exc:
self._connect_failed = True
raise exc
elif not self._connection.is_available():
Expand Down Expand Up @@ -174,10 +176,12 @@ def close(self) -> None:

def is_available(self) -> bool:
if self._connection is None:
if self._is_new:
return True
# If HTTP/2 support is enabled, and the resulting connection could
# end up as HTTP/2 then we should indicate the connection as being
# available to service multiple requests.
return (
return ( # pragma: no cover
self._http2
and (self._origin.scheme == b"https" or not self._http1)
and not self._connect_failed
Expand All @@ -201,7 +205,11 @@ def is_closed(self) -> bool:

def info(self) -> str:
if self._connection is None:
return "CONNECTION FAILED" if self._connect_failed else "CONNECTING"
if self._is_new:
return "NEW CONNECTION"
return (
"CONNECTION FAILED" if self._connect_failed else "CONNECTING"
) # pragma: no cover
return self._connection.info()

def __repr__(self) -> str:
Expand Down
4 changes: 2 additions & 2 deletions tests/_async/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ async def test_http_connection():
) as conn:
assert not conn.is_idle()
assert not conn.is_closed()
assert not conn.is_available()
assert conn.is_available()
assert not conn.has_expired()
assert repr(conn) == "<AsyncHTTPConnection [CONNECTING]>"
assert repr(conn) == "<AsyncHTTPConnection [NEW CONNECTION]>"

async with conn.stream("GET", "https://example.com/") as response:
assert (
Expand Down
4 changes: 2 additions & 2 deletions tests/_sync/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ def test_http_connection():
) as conn:
assert not conn.is_idle()
assert not conn.is_closed()
assert not conn.is_available()
assert conn.is_available()
assert not conn.has_expired()
assert repr(conn) == "<HTTPConnection [CONNECTING]>"
assert repr(conn) == "<HTTPConnection [NEW CONNECTION]>"

with conn.stream("GET", "https://example.com/") as response:
assert (
Expand Down