Skip to content

Commit

Permalink
Ensure pool timeout is applied, even after attempts leading to Connec…
Browse files Browse the repository at this point in the history
…tionNotAvailable (encode#823)

* add failing tests

* fix pooltimeout

* Revert "add failing tests"

This reverts commit cacc248.

* mark `if timeout < 0` as not covered, since the test is reverted

* Update CHANGELOG.md
  • Loading branch information
valsteen committed Oct 12, 2023
1 parent 8780c9c commit f8ff1c4
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## Unreleased

- Fix pool timeout to account for the total time spent retrying. (#823)

## 1.0.0 (November 6th, 2023)

From version 1.0 our async support is now optional, as the package has minimal dependencies by default.
Expand Down
16 changes: 13 additions & 3 deletions httpcore/_async/connection_pool.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import ssl
import sys
import time
from types import TracebackType
from typing import AsyncIterable, AsyncIterator, Iterable, List, Optional, Type

from .._backends.auto import AutoBackend
from .._backends.base import SOCKET_OPTION, AsyncNetworkBackend
from .._exceptions import ConnectionNotAvailable, UnsupportedProtocol
from .._exceptions import ConnectionNotAvailable, PoolTimeout, UnsupportedProtocol
from .._models import Origin, Request, Response
from .._synchronization import AsyncEvent, AsyncLock, AsyncShieldCancellation
from .connection import AsyncHTTPConnection
Expand Down Expand Up @@ -220,15 +221,20 @@ async def handle_async_request(self, request: Request) -> Response:
)

status = RequestStatus(request)
timeouts = request.extensions.get("timeout", {})
timeout = timeouts.get("pool", None)

if timeout is not None:
deadline = time.monotonic() + timeout
else:
deadline = float("inf")

async with self._pool_lock:
self._requests.append(status)
await self._close_expired_connections()
await self._attempt_to_acquire_connection(status)

while True:
timeouts = request.extensions.get("timeout", {})
timeout = timeouts.get("pool", None)
try:
connection = await status.wait_for_connection(timeout=timeout)
except BaseException as exc:
Expand Down Expand Up @@ -263,6 +269,10 @@ async def handle_async_request(self, request: Request) -> Response:
else:
break

timeout = deadline - time.monotonic()
if timeout < 0:
raise PoolTimeout # pragma: nocover

# When we return the response, we wrap the stream in a special class
# that handles notifying the connection pool once the response
# has been released.
Expand Down
16 changes: 13 additions & 3 deletions httpcore/_sync/connection_pool.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import ssl
import sys
import time
from types import TracebackType
from typing import Iterable, Iterator, Iterable, List, Optional, Type

from .._backends.sync import SyncBackend
from .._backends.base import SOCKET_OPTION, NetworkBackend
from .._exceptions import ConnectionNotAvailable, UnsupportedProtocol
from .._exceptions import ConnectionNotAvailable, PoolTimeout, UnsupportedProtocol
from .._models import Origin, Request, Response
from .._synchronization import Event, Lock, ShieldCancellation
from .connection import HTTPConnection
Expand Down Expand Up @@ -220,15 +221,20 @@ def handle_request(self, request: Request) -> Response:
)

status = RequestStatus(request)
timeouts = request.extensions.get("timeout", {})
timeout = timeouts.get("pool", None)

if timeout is not None:
deadline = time.monotonic() + timeout
else:
deadline = float("inf")

with self._pool_lock:
self._requests.append(status)
self._close_expired_connections()
self._attempt_to_acquire_connection(status)

while True:
timeouts = request.extensions.get("timeout", {})
timeout = timeouts.get("pool", None)
try:
connection = status.wait_for_connection(timeout=timeout)
except BaseException as exc:
Expand Down Expand Up @@ -263,6 +269,10 @@ def handle_request(self, request: Request) -> Response:
else:
break

timeout = deadline - time.monotonic()
if timeout < 0:
raise PoolTimeout # pragma: nocover

# When we return the response, we wrap the stream in a special class
# that handles notifying the connection pool once the response
# has been released.
Expand Down

0 comments on commit f8ff1c4

Please sign in to comment.