diff --git a/CHANGES/5403.bugfix b/CHANGES/5403.bugfix new file mode 100644 index 00000000000..40cc5a22294 --- /dev/null +++ b/CHANGES/5403.bugfix @@ -0,0 +1 @@ +Stop automatically releasing the ``ClientResponse`` object on calls to the ``ok`` property for the failed requests. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index b1278f52325..48c122174c4 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -3,6 +3,7 @@ A. Jesse Jiryu Davis Adam Bannister Adam Cooper +Adam Horacek Adam Mills Adrian Krupa Adrián Chaves diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 97b2ce25f00..801bc6dcda7 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -923,14 +923,10 @@ def ok(self) -> bool: This is **not** a check for ``200 OK`` but a check that the response status is under 400. """ - try: - self.raise_for_status() - except ClientResponseError: - return False - return True + return 400 > self.status def raise_for_status(self) -> None: - if 400 <= self.status: + if not self.ok: # reason should always be not None for a started response assert self.reason is not None self.release() diff --git a/tests/test_client_response.py b/tests/test_client_response.py index 9b81e3ea6f2..9f3c4990085 100644 --- a/tests/test_client_response.py +++ b/tests/test_client_response.py @@ -1243,3 +1243,24 @@ def test_response_links_empty(loop: Any, session: Any) -> None: ) response._headers = CIMultiDict() assert response.links == {} + + +def test_response_not_closed_after_get_ok(mocker) -> None: + response = ClientResponse( + "get", + URL("http://del-cl-resp.org"), + request_info=mock.Mock(), + writer=mock.Mock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=mock.Mock(), + session=mock.Mock(), + ) + response.status = 400 + response.reason = "Bad Request" + response._closed = False + spy = mocker.spy(response, "raise_for_status") + assert not response.ok + assert not response.closed + assert spy.call_count == 0