From 3adc00652846eb2b83a37136f39e748f20774f9c Mon Sep 17 00:00:00 2001 From: sagarwal Date: Wed, 2 Oct 2024 23:36:59 +0530 Subject: [PATCH 01/14] Solving #9141 as suggested by Sam --- aiohttp/client.py | 6 +++++- aiohttp/test_utils.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 1cdb046c460..fd5c571de3d 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -286,6 +286,7 @@ def __init__( max_line_size: int = 8190, max_field_size: int = 8190, fallback_charset_resolver: _CharsetResolver = lambda r, b: "utf-8", + test: bool = False, ) -> None: # We initialise _connector to None immediately, as it's referenced in __del__() # and could cause issues if an exception occurs during initialisation. @@ -367,6 +368,7 @@ def __init__( self._default_proxy = proxy self._default_proxy_auth = proxy_auth + self._test = test def __init_subclass__(cls: Type["ClientSession"]) -> None: raise TypeError( @@ -539,7 +541,9 @@ async def _request( try: with timer: # https://www.rfc-editor.org/rfc/rfc9112.html#name-retrying-requests - retry_persistent_connection = method in IDEMPOTENT_METHODS + retry_persistent_connection = ( + False if self._test else method in IDEMPOTENT_METHODS + ) while True: url, auth_from_url = strip_auth_from_url(url) if not url.raw_host: diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py index 88dcf8ebf1b..2c076f39ed9 100644 --- a/aiohttp/test_utils.py +++ b/aiohttp/test_utils.py @@ -293,7 +293,7 @@ def __init__( # type: ignore[misc] self._server = server if cookie_jar is None: cookie_jar = aiohttp.CookieJar(unsafe=True) - self._session = ClientSession(cookie_jar=cookie_jar, **kwargs) + self._session = ClientSession(cookie_jar=cookie_jar, test=True, **kwargs) self._closed = False self._responses: List[ClientResponse] = [] self._websockets: List[ClientWebSocketResponse] = [] From 765bc8e7428a7c570e5d63818b577d5941a29fb3 Mon Sep 17 00:00:00 2001 From: sagarwal Date: Wed, 2 Oct 2024 23:41:30 +0530 Subject: [PATCH 02/14] added name to contributions.txt --- CONTRIBUTORS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index fbfef32f398..673f65d5cee 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -308,6 +308,7 @@ Sergey Skripnick Serhii Charykov Serhii Kostel Serhiy Storchaka +Shubh Agarwal Simon Kennedy Sin-Woo Bang Stanislas Plum From e82bd0791c70ed28c74739a3750fba82df11c624 Mon Sep 17 00:00:00 2001 From: sagarwal Date: Wed, 2 Oct 2024 23:55:20 +0530 Subject: [PATCH 03/14] added file to Changes --- CHANGES/9141.misc.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 CHANGES/9141.misc.rst diff --git a/CHANGES/9141.misc.rst b/CHANGES/9141.misc.rst new file mode 100644 index 00000000000..6ee9d726d39 --- /dev/null +++ b/CHANGES/9141.misc.rst @@ -0,0 +1,3 @@ +- Modified the `ClientSession` class by adding a private attribute `_test` of type `bool`, with a default value of `False` to the class. +- Modified the default value of `retry_persistent_connection` in `ClientSession` to `False` if `_test` was `True`. +- Also, `TestClient` initialized `ClientSession` with `test=True`. From bdf609b6652860fcaec704141131a8c2b367f205 Mon Sep 17 00:00:00 2001 From: sagarwal Date: Thu, 3 Oct 2024 00:07:59 +0530 Subject: [PATCH 04/14] added _test for mypy --- aiohttp/client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/aiohttp/client.py b/aiohttp/client.py index 400d2edeb56..1910a6c5b0b 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -254,6 +254,7 @@ class ClientSession: "_resolve_charset", "_default_proxy", "_default_proxy_auth", + "_test", ) def __init__( From 256621d858892f5a7e9c1e7b97c157e470cbcfc4 Mon Sep 17 00:00:00 2001 From: sagarwal Date: Thu, 3 Oct 2024 22:18:09 +0530 Subject: [PATCH 05/14] done the suggested changes --- CHANGES/9141.misc.rst | 6 +++--- aiohttp/client.py | 7 +++---- aiohttp/test_utils.py | 3 ++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGES/9141.misc.rst b/CHANGES/9141.misc.rst index 6ee9d726d39..4250207e5ca 100644 --- a/CHANGES/9141.misc.rst +++ b/CHANGES/9141.misc.rst @@ -1,3 +1,3 @@ -- Modified the `ClientSession` class by adding a private attribute `_test` of type `bool`, with a default value of `False` to the class. -- Modified the default value of `retry_persistent_connection` in `ClientSession` to `False` if `_test` was `True`. -- Also, `TestClient` initialized `ClientSession` with `test=True`. +Modified the `ClientSession` class by adding a private attribute `_test` of type `bool`, with a default value of `False` to the class. +Modified the default value of `retry_persistent_connection` in `ClientSession` to `False` if `_test` was `True`. +Also, `TestClient` initialized `ClientSession` with `test=True`. -- by :user:`ShubhAgarwal-dev`. diff --git a/aiohttp/client.py b/aiohttp/client.py index bbc75e99d3a..8fce18b1a18 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -254,7 +254,7 @@ class ClientSession: "_resolve_charset", "_default_proxy", "_default_proxy_auth", - "_test", + "_retry_connection", ) def __init__( @@ -287,7 +287,6 @@ def __init__( max_line_size: int = 8190, max_field_size: int = 8190, fallback_charset_resolver: _CharsetResolver = lambda r, b: "utf-8", - test: bool = False, ) -> None: # We initialise _connector to None immediately, as it's referenced in __del__() # and could cause issues if an exception occurs during initialisation. @@ -369,7 +368,7 @@ def __init__( self._default_proxy = proxy self._default_proxy_auth = proxy_auth - self._test = test + self._retry_connection: bool = True def __init_subclass__(cls: Type["ClientSession"]) -> None: raise TypeError( @@ -543,7 +542,7 @@ async def _request( with timer: # https://www.rfc-editor.org/rfc/rfc9112.html#name-retrying-requests retry_persistent_connection = ( - False if self._test else method in IDEMPOTENT_METHODS + self._retry_connection and method in IDEMPOTENT_METHODS ) while True: url, auth_from_url = strip_auth_from_url(url) diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py index 2c076f39ed9..eb40ab2f4d7 100644 --- a/aiohttp/test_utils.py +++ b/aiohttp/test_utils.py @@ -293,7 +293,8 @@ def __init__( # type: ignore[misc] self._server = server if cookie_jar is None: cookie_jar = aiohttp.CookieJar(unsafe=True) - self._session = ClientSession(cookie_jar=cookie_jar, test=True, **kwargs) + self._session = ClientSession(cookie_jar=cookie_jar, **kwargs) + self._session._retry_connection = False self._closed = False self._responses: List[ClientResponse] = [] self._websockets: List[ClientWebSocketResponse] = [] From 8e27bff11625c655ab3ebe1d30e4baa031b5a0a3 Mon Sep 17 00:00:00 2001 From: sagarwal Date: Thu, 3 Oct 2024 22:27:26 +0530 Subject: [PATCH 06/14] Corrected the listed changes --- CHANGES/9141.misc.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES/9141.misc.rst b/CHANGES/9141.misc.rst index 4250207e5ca..0f740445426 100644 --- a/CHANGES/9141.misc.rst +++ b/CHANGES/9141.misc.rst @@ -1,3 +1,3 @@ -Modified the `ClientSession` class by adding a private attribute `_test` of type `bool`, with a default value of `False` to the class. -Modified the default value of `retry_persistent_connection` in `ClientSession` to `False` if `_test` was `True`. -Also, `TestClient` initialized `ClientSession` with `test=True`. -- by :user:`ShubhAgarwal-dev`. +Modified the `ClientSession` class by adding a private attribute `_retry_connection` of type `bool`, with a default value of `True` to the class. +`retry_persistent_connection` in `ClientSession` is set to `False` if `_retry_connection` was `False`. +In the session of `TestClient`, `ClientSession` attribute `_retry_connection` is changed to `False` -- by :user:`ShubhAgarwal-dev`. From 78722e26dbba648a56fc5115ce170548f7b6a22f Mon Sep 17 00:00:00 2001 From: sagarwal Date: Sat, 5 Oct 2024 21:49:52 +0530 Subject: [PATCH 07/14] added test --- tests/test_test_utils.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_test_utils.py b/tests/test_test_utils.py index 718b95b6512..fc166d42785 100644 --- a/tests/test_test_utils.py +++ b/tests/test_test_utils.py @@ -11,6 +11,7 @@ import aiohttp from aiohttp import web +from aiohttp.pytest_plugin import AiohttpClient from aiohttp.test_utils import ( AioHTTPTestCase, RawTestServer, @@ -316,6 +317,18 @@ def test_noop(self) -> None: result.stdout.fnmatch_lines(["*TypeError*"]) +async def test_disable_retry_persistent_connection(aiohttp_client: AiohttpClient): + async def handler(request: web.Request) -> web.Response: + request.close() + return web.Response(200) + + app = web.Application() + app.router.add_get("/", handler) + client = await aiohttp_client(app) + async with client.get("/") as _: + assert client._session.closed is False + + async def test_server_context_manager( app: web.Application, loop: asyncio.AbstractEventLoop ) -> None: From d629f5c8cb48c63d2987008fc18f35846d806cda Mon Sep 17 00:00:00 2001 From: sagarwal Date: Sat, 5 Oct 2024 21:56:11 +0530 Subject: [PATCH 08/14] removed on my error --- tests/test_test_utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_test_utils.py b/tests/test_test_utils.py index fc166d42785..9550abae030 100644 --- a/tests/test_test_utils.py +++ b/tests/test_test_utils.py @@ -317,10 +317,12 @@ def test_noop(self) -> None: result.stdout.fnmatch_lines(["*TypeError*"]) -async def test_disable_retry_persistent_connection(aiohttp_client: AiohttpClient): +async def test_disable_retry_persistent_connection( + aiohttp_client: AiohttpClient, +) -> None: async def handler(request: web.Request) -> web.Response: request.close() - return web.Response(200) + return web.Response() app = web.Application() app.router.add_get("/", handler) From b2da3073480e7da595f68427767de23c16a2f5e3 Mon Sep 17 00:00:00 2001 From: sagarwal Date: Sun, 6 Oct 2024 08:53:28 +0530 Subject: [PATCH 09/14] Corrected CHANGES fine --- CHANGES/9141.misc.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES/9141.misc.rst b/CHANGES/9141.misc.rst index 0f740445426..8e566e3c597 100644 --- a/CHANGES/9141.misc.rst +++ b/CHANGES/9141.misc.rst @@ -1,3 +1,3 @@ -Modified the `ClientSession` class by adding a private attribute `_retry_connection` of type `bool`, with a default value of `True` to the class. -`retry_persistent_connection` in `ClientSession` is set to `False` if `_retry_connection` was `False`. -In the session of `TestClient`, `ClientSession` attribute `_retry_connection` is changed to `False` -- by :user:`ShubhAgarwal-dev`. +Modified the :class:`aiohttp.ClientSession` class by adding a private attribute ``_retry_connection`` of type :class:`bool`, with a default value of :data:`True` to the class. +``retry_persistent_connection`` in :class:`aiohttp.ClientSession` is set to :data:`False` if `_retry_connection` was :data:`False`. +In the session of :class:`aiohttp.test_utils.TestClient`, :class:`aiohttp.ClientSession` attribute ``_retry_connection`` is changed to :data:`False` -- by :user:`ShubhAgarwal-dev`. From 00153614c4fde329f7092594e6d0eba5e609d283 Mon Sep 17 00:00:00 2001 From: sagarwal Date: Sun, 6 Oct 2024 08:53:28 +0530 Subject: [PATCH 10/14] Corrected CHANGES file --- CHANGES/9141.misc.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES/9141.misc.rst b/CHANGES/9141.misc.rst index 0f740445426..8e566e3c597 100644 --- a/CHANGES/9141.misc.rst +++ b/CHANGES/9141.misc.rst @@ -1,3 +1,3 @@ -Modified the `ClientSession` class by adding a private attribute `_retry_connection` of type `bool`, with a default value of `True` to the class. -`retry_persistent_connection` in `ClientSession` is set to `False` if `_retry_connection` was `False`. -In the session of `TestClient`, `ClientSession` attribute `_retry_connection` is changed to `False` -- by :user:`ShubhAgarwal-dev`. +Modified the :class:`aiohttp.ClientSession` class by adding a private attribute ``_retry_connection`` of type :class:`bool`, with a default value of :data:`True` to the class. +``retry_persistent_connection`` in :class:`aiohttp.ClientSession` is set to :data:`False` if `_retry_connection` was :data:`False`. +In the session of :class:`aiohttp.test_utils.TestClient`, :class:`aiohttp.ClientSession` attribute ``_retry_connection`` is changed to :data:`False` -- by :user:`ShubhAgarwal-dev`. From 721b1b7dc68b71e0ec3d4dc00d65f08a4ffa5809 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 6 Oct 2024 20:45:24 +0100 Subject: [PATCH 11/14] Fixup test --- tests/test_test_utils.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/test_test_utils.py b/tests/test_test_utils.py index 9550abae030..ece5092de51 100644 --- a/tests/test_test_utils.py +++ b/tests/test_test_utils.py @@ -10,7 +10,7 @@ from yarl import URL import aiohttp -from aiohttp import web +from aiohttp import web, ServerDisconnectedError from aiohttp.pytest_plugin import AiohttpClient from aiohttp.test_utils import ( AioHTTPTestCase, @@ -320,15 +320,22 @@ def test_noop(self) -> None: async def test_disable_retry_persistent_connection( aiohttp_client: AiohttpClient, ) -> None: + num_requests = 0 + async def handler(request: web.Request) -> web.Response: - request.close() + nonlocal num_requests + + num_requests += 1 + request.protocol.force_close() return web.Response() app = web.Application() app.router.add_get("/", handler) client = await aiohttp_client(app) - async with client.get("/") as _: - assert client._session.closed is False + with pytest.raises(aiohttp.ServerDisconnectedError): + await client.get("/") + + assert num_requests == 1 async def test_server_context_manager( From 30bb81f802d42656d34a414a263fdcbd7ccff672 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 6 Oct 2024 19:46:02 +0000 Subject: [PATCH 12/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_test_utils.py b/tests/test_test_utils.py index ece5092de51..10aab5c4d4c 100644 --- a/tests/test_test_utils.py +++ b/tests/test_test_utils.py @@ -10,7 +10,7 @@ from yarl import URL import aiohttp -from aiohttp import web, ServerDisconnectedError +from aiohttp import ServerDisconnectedError, web from aiohttp.pytest_plugin import AiohttpClient from aiohttp.test_utils import ( AioHTTPTestCase, From edb8ab169e74dbd0b76e5d8c914e4cb3aa6b4141 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 6 Oct 2024 20:50:08 +0100 Subject: [PATCH 13/14] Update 9141.misc.rst --- CHANGES/9141.misc.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGES/9141.misc.rst b/CHANGES/9141.misc.rst index 8e566e3c597..d23439fa742 100644 --- a/CHANGES/9141.misc.rst +++ b/CHANGES/9141.misc.rst @@ -1,3 +1,2 @@ -Modified the :class:`aiohttp.ClientSession` class by adding a private attribute ``_retry_connection`` of type :class:`bool`, with a default value of :data:`True` to the class. -``retry_persistent_connection`` in :class:`aiohttp.ClientSession` is set to :data:`False` if `_retry_connection` was :data:`False`. -In the session of :class:`aiohttp.test_utils.TestClient`, :class:`aiohttp.ClientSession` attribute ``_retry_connection`` is changed to :data:`False` -- by :user:`ShubhAgarwal-dev`. +Disabled automatic retries of failed requests in :class:`aiohttp.test_utils.TestClient`'s client session +(which could potentially hide errors in tests) -- by :user:`ShubhAgarwal-dev`. From 15c20026ebb6d516c54158a51720f6517ecc61bf Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 6 Oct 2024 20:51:45 +0100 Subject: [PATCH 14/14] Update tests/test_test_utils.py --- tests/test_test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_test_utils.py b/tests/test_test_utils.py index 10aab5c4d4c..e1145f81d53 100644 --- a/tests/test_test_utils.py +++ b/tests/test_test_utils.py @@ -10,7 +10,7 @@ from yarl import URL import aiohttp -from aiohttp import ServerDisconnectedError, web +from aiohttp import web from aiohttp.pytest_plugin import AiohttpClient from aiohttp.test_utils import ( AioHTTPTestCase,