Add regression test for #360

tests/test_asyncio/test_connection.py

@@ -112,3 +112,37 @@ async def test_connect_timeout_error_without_retry():
         await conn.connect()
     assert conn._connect.call_count == 1
     assert str(e.value) == "Timeout connecting to server"
+
+
+@pytest.mark.parametrize(
+    "exc_type",
+    [
+        Exception,
+        pytest.param(
+            BaseException,
+            marks=pytest.mark.xfail(
+                reason="https://github.com/redis/redis-py/issues/360",
+            ),
+        ),
+    ],
+)
+async def test_read_response__interrupt_does_not_corrupt(exc_type):
+    conn = Connection()
+
+    await conn.send_command("GET non_existent_key")
+    resp = await conn.read_response()
+    assert resp is None
+
+    with pytest.raises(exc_type):
+        await conn.send_command("EXISTS non_existent_key")
+        # due to the interrupt, the integer '0' result of EXISTS will remain
+        # on the socket's buffer
+        with patch.object(socket.socket, "recv", side_effect=exc_type) as mock_recv:
+            await conn.read_response()
+        mock_recv.assert_called_once()
+
+    await conn.send_command("GET non_existent_key")
+    resp = await conn.read_response()
+    # If working properly, this will get a None.
+    # If not, it will get a zero (the integer result of the previous EXISTS command).
+    assert resp is None

tests/test_connection.py

@@ -122,3 +122,48 @@ def test_connect_timeout_error_without_retry(self):
         assert conn._connect.call_count == 1
         assert str(e.value) == "Timeout connecting to server"
         self.clear(conn)
+
+    @pytest.mark.parametrize(
+        "exc_type",
+        [
+            Exception,
+            pytest.param(
+                BaseException,
+                marks=pytest.mark.xfail(
+                    reason="https://github.com/redis/redis-py/issues/360",
+                ),
+            ),
+        ],
+    )
+    def test_read_response__interrupt_does_not_corrupt(self, exc_type):
+        conn = Connection()
+
+        # A note on BaseException:
+        # While socket.recv is not supposed to raise BaseException, gevent's version
+        # of socket (which, when using gevent + redis-py, one would monkey-patch in)
+        # can raise BaseException on a timer elapse, since `gevent.Timeout` derives
+        # from BaseException. This design suggests that a timeout should
+        # not be suppressed but rather allowed to propagate.
+        # asyncio.exceptions.CancelledError also derives from BaseException
+        # for same reason.
+        #
+        # What we should ensure, one way or another, is that the connection is
+        # not left in a corrupted state.
+
+        conn.send_command("GET non_existent_key")
+        resp = conn.read_response()
+        assert resp is None
+
+        with pytest.raises(exc_type):
+            conn.send_command("EXISTS non_existent_key")
+            # due to the interrupt, the integer '0' result of EXISTS will remain
+            # on the socket's buffer
+            with patch.object(socket.socket, "recv", side_effect=exc_type) as mock_recv:
+                _ = conn.read_response()
+            mock_recv.assert_called_once()
+
+        conn.send_command("GET non_existent_key")
+        resp = conn.read_response()
+        # If working properly, this will get a None.
+        # If not, it will get a zero (the integer result of the previous command).
+        assert resp is None