From: cdeler Date: Wed, 2 Sep 2020 12:02:59 +0000 (+0300) Subject: Issue warning on unclosed `AsyncClient`. (#1197) X-Git-Tag: 0.15.0~37 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=def9f1c320fd416c8ab8a5359afa36467ad831ad;p=thirdparty%2Fhttpx.git Issue warning on unclosed `AsyncClient`. (#1197) * Made Client and AsyncClient checking for being closed in __del__ (#871) * Changed the AsyncClient closing warning type from ResourceWarning (which is too quiet) to UserWarning (#871) * Fixed tests and client's __exit__ and __aexit__ after the difficult merge (#871) * Update test_proxies.py * Update test_proxies.py Co-authored-by: Tom Christie --- diff --git a/httpx/_client.py b/httpx/_client.py index 8d323f26..b78c92ea 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -1,5 +1,6 @@ import functools import typing +import warnings from types import TracebackType import httpcore @@ -79,6 +80,14 @@ class BaseClient: self.max_redirects = max_redirects self._trust_env = trust_env self._netrc = NetRCInfo() + self._is_closed = True + + @property + def is_closed(self) -> bool: + """ + Check if the client being closed + """ + return self._is_closed @property def trust_env(self) -> bool: @@ -696,6 +705,8 @@ class Client(BaseClient): [0]: /advanced/#request-instances """ + self._is_closed = False + timeout = self.timeout if isinstance(timeout, UnsetType) else Timeout(timeout) auth = self._build_request_auth(request, auth) @@ -1029,16 +1040,20 @@ class Client(BaseClient): """ Close transport and proxies. """ - self._transport.close() - for proxy in self._proxies.values(): - if proxy is not None: - proxy.close() + if not self.is_closed: + self._is_closed = True + + self._transport.close() + for proxy in self._proxies.values(): + if proxy is not None: + proxy.close() def __enter__(self) -> "Client": self._transport.__enter__() for proxy in self._proxies.values(): if proxy is not None: proxy.__enter__() + self._is_closed = False return self def __exit__( @@ -1047,10 +1062,16 @@ class Client(BaseClient): exc_value: BaseException = None, traceback: TracebackType = None, ) -> None: - self._transport.__exit__(exc_type, exc_value, traceback) - for proxy in self._proxies.values(): - if proxy is not None: - proxy.__exit__(exc_type, exc_value, traceback) + if not self.is_closed: + self._is_closed = True + + self._transport.__exit__(exc_type, exc_value, traceback) + for proxy in self._proxies.values(): + if proxy is not None: + proxy.__exit__(exc_type, exc_value, traceback) + + def __del__(self) -> None: + self.close() class AsyncClient(BaseClient): @@ -1305,6 +1326,8 @@ class AsyncClient(BaseClient): [0]: /advanced/#request-instances """ + self._is_closed = False + timeout = self.timeout if isinstance(timeout, UnsetType) else Timeout(timeout) auth = self._build_request_auth(request, auth) @@ -1640,16 +1663,20 @@ class AsyncClient(BaseClient): """ Close transport and proxies. """ - await self._transport.aclose() - for proxy in self._proxies.values(): - if proxy is not None: - await proxy.aclose() + if not self.is_closed: + self._is_closed = True + + await self._transport.aclose() + for proxy in self._proxies.values(): + if proxy is not None: + await proxy.aclose() async def __aenter__(self) -> "AsyncClient": await self._transport.__aenter__() for proxy in self._proxies.values(): if proxy is not None: await proxy.__aenter__() + self._is_closed = False return self async def __aexit__( @@ -1658,10 +1685,20 @@ class AsyncClient(BaseClient): exc_value: BaseException = None, traceback: TracebackType = None, ) -> None: - await self._transport.__aexit__(exc_type, exc_value, traceback) - for proxy in self._proxies.values(): - if proxy is not None: - await proxy.__aexit__(exc_type, exc_value, traceback) + if not self.is_closed: + self._is_closed = True + await self._transport.__aexit__(exc_type, exc_value, traceback) + for proxy in self._proxies.values(): + if proxy is not None: + await proxy.__aexit__(exc_type, exc_value, traceback) + + def __del__(self) -> None: + if not self.is_closed: + warnings.warn( + f"Unclosed {self!r}. " + "See https://www.python-httpx.org/async/#opening-and-closing-clients " + "for details." + ) class StreamContextManager: diff --git a/tests/client/test_async_client.py b/tests/client/test_async_client.py index 126c50e9..4272301f 100644 --- a/tests/client/test_async_client.py +++ b/tests/client/test_async_client.py @@ -206,3 +206,45 @@ async def test_context_managed_transport(): "transport.aclose", "transport.__aexit__", ] + + +@pytest.mark.usefixtures("async_environment") +async def test_that_async_client_is_closed_by_default(): + client = httpx.AsyncClient() + + assert client.is_closed + + +@pytest.mark.usefixtures("async_environment") +async def test_that_send_cause_async_client_to_be_not_closed(): + client = httpx.AsyncClient() + + await client.get("http://example.com") + + assert not client.is_closed + + await client.aclose() + + +@pytest.mark.usefixtures("async_environment") +async def test_that_async_client_is_not_closed_in_with_block(): + async with httpx.AsyncClient() as client: + assert not client.is_closed + + +@pytest.mark.usefixtures("async_environment") +async def test_that_async_client_is_closed_after_with_block(): + async with httpx.AsyncClient() as client: + pass + + assert client.is_closed + + +@pytest.mark.usefixtures("async_environment") +async def test_that_async_client_caused_warning_when_being_deleted(): + async_client = httpx.AsyncClient() + + await async_client.get("http://example.com") + + with pytest.warns(UserWarning): + del async_client diff --git a/tests/client/test_client.py b/tests/client/test_client.py index fe427ec6..d5a8c0e0 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -247,3 +247,29 @@ def test_context_managed_transport(): "transport.close", "transport.__exit__", ] + + +def test_that_client_is_closed_by_default(): + client = httpx.Client() + + assert client.is_closed + + +def test_that_send_cause_client_to_be_not_closed(): + client = httpx.Client() + + client.get("http://example.com") + + assert not client.is_closed + + +def test_that_client_is_not_closed_in_with_block(): + with httpx.Client() as client: + assert not client.is_closed + + +def test_that_client_is_closed_after_with_block(): + with httpx.Client() as client: + pass + + assert client.is_closed diff --git a/tests/client/test_proxies.py b/tests/client/test_proxies.py index c4a64e1a..6d304383 100644 --- a/tests/client/test_proxies.py +++ b/tests/client/test_proxies.py @@ -123,14 +123,16 @@ def test_transport_for_request(url, proxies, expected): @pytest.mark.asyncio async def test_async_proxy_close(): try: - client = httpx.AsyncClient(proxies={"all://": PROXY_URL}) + client = httpx.AsyncClient(proxies={"https://": PROXY_URL}) + await client.get("http://example.com") finally: await client.aclose() def test_sync_proxy_close(): try: - client = httpx.Client(proxies={"all://": PROXY_URL}) + client = httpx.Client(proxies={"https://": PROXY_URL}) + client.get("http://example.com") finally: client.close()