[3.8] Eliminate side-effects from the ClientResponse.ok property (#…

…5407)

This change makes it so accessing `ClientResponse.ok` only does the status
code check.

Prior to this commit, it'd call `ClientResponse.raise_for_status()` which
in turn, closed the underlying TCP session whenever the status was 400 or
higher making it effectively impossible to keep working with the response,
including reading the HTTP response payload.

PR #5404 by @adamko147

Fixes #5403

Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit 3250c5d)

Co-authored-by: Adam Horacek <[email protected]>

CHANGES/5403.bugfix

@@ -0,0 +1 @@
+Stop automatically releasing the ``ClientResponse`` object on calls to the ``ok`` property for the failed requests.

CONTRIBUTORS.txt

@@ -3,6 +3,7 @@
 A. Jesse Jiryu Davis
 Adam Bannister
 Adam Cooper
+Adam Horacek
 Adam Mills
 Adrian Krupa
 Adrián Chaves

aiohttp/client_reqrep.py

@@ -996,14 +996,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()

tests/test_client_response.py

@@ -1256,3 +1256,24 @@ def test_response_links_empty(loop, session) -> 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