From 4a3385be5e2f145cec5d28ff500e83428091a731 Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Mon, 19 Jun 2017 16:47:33 +0200 Subject: [PATCH 1/4] Reset self.sock if connection fails If we don't reset `self.sock` on connection error, then `my_socket.is_open()` will still return `True` after `TTransportException` is thrown. --- thriftpy/transport/socket.py | 1 + 1 file changed, 1 insertion(+) diff --git a/thriftpy/transport/socket.py b/thriftpy/transport/socket.py index 51a5283..a7dbd9f 100644 --- a/thriftpy/transport/socket.py +++ b/thriftpy/transport/socket.py @@ -99,6 +99,7 @@ def open(self): self.sock.settimeout(self.socket_timeout) except (socket.error, OSError): + self.sock = None raise TTransportException( type=TTransportException.NOT_OPEN, message="Could not connect to %s" % str(addr)) From c85cb5db44218e5c86c1d2911fb9d21bce93203b Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Tue, 20 Jun 2017 08:36:53 +0200 Subject: [PATCH 2/4] Always set self.sock=None on close() Otherwise, `self.is_open()` may still return `True` after `close()` --- thriftpy/transport/socket.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/thriftpy/transport/socket.py b/thriftpy/transport/socket.py index a7dbd9f..a40a2c7 100644 --- a/thriftpy/transport/socket.py +++ b/thriftpy/transport/socket.py @@ -141,7 +141,7 @@ def close(self): self.sock.close() self.sock = None except (socket.error, OSError): - pass + self.sock = None class TServerSocket(object): From ec73667b30acbd35fd53d6985754ab0d8df86aea Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Mon, 31 Jul 2017 15:47:39 +0200 Subject: [PATCH 3/4] Add tests for is_open() on failure --- tests/test_socket.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_socket.py b/tests/test_socket.py index f0015b8..18a8f3c 100644 --- a/tests/test_socket.py +++ b/tests/test_socket.py @@ -146,3 +146,24 @@ def test_client_socket_set_timeout(): conn.close() client_socket.close() server_socket.close() + + +def test_client_socket_not_open_after_failed_open(): + client_socket = TSocket(host="localhost", port=12345, socket_timeout=100) + assert not client_socket.is_open() + + with pytest.raises(TTransportException): + client_socket.open() + + assert not client_socket.is_open() + + +def test_client_socket_not_open_after_close(): + client_socket = TSocket(host="localhost", port=12345, socket_timeout=100) + assert not client_socket.is_open() + + with pytest.raises(TTransportException): + client_socket.open() + client_socket.close() + + assert not client_socket.is_open() From 8e047cc390e8583d4aeca8fef51e730ff6d43e32 Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Tue, 1 Aug 2017 09:49:25 +0200 Subject: [PATCH 4/4] Remove timeout args as suggested by @wbolster --- tests/test_socket.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_socket.py b/tests/test_socket.py index 18a8f3c..3657069 100644 --- a/tests/test_socket.py +++ b/tests/test_socket.py @@ -149,7 +149,7 @@ def test_client_socket_set_timeout(): def test_client_socket_not_open_after_failed_open(): - client_socket = TSocket(host="localhost", port=12345, socket_timeout=100) + client_socket = TSocket(host="localhost", port=12345) assert not client_socket.is_open() with pytest.raises(TTransportException): @@ -159,7 +159,7 @@ def test_client_socket_not_open_after_failed_open(): def test_client_socket_not_open_after_close(): - client_socket = TSocket(host="localhost", port=12345, socket_timeout=100) + client_socket = TSocket(host="localhost", port=12345) assert not client_socket.is_open() with pytest.raises(TTransportException):