Fix check for parameters in case of connection error.

If any direct call to api_execute was made, and a connection error
occurred, this would result in an error because the params would be
None.

2 files changed, 39 insertions, 22 deletions

diff --git a/src/etcd/client.py b/src/etcd/client.py
index d01524a..afeabe8 100644
--- a/src/etcd/client.py
+++ b/src/etcd/client.py
@@ -804,9 +804,10 @@ class Client(object):
                     # don't wrap socket errors either.
                 except (urllib3.exceptions.HTTPError,
                         HTTPException, socket.error) as e:
-                    if (params.get("wait") == "true" and
-                            isinstance(e,
-                                       urllib3.exceptions.ReadTimeoutError)):
+                    if (isinstance(params, dict) and
+                        params.get("wait") == "true" and
+                        isinstance(e,
+                                   urllib3.exceptions.ReadTimeoutError)):
                         _log.debug("Watch timed out.")
                         raise etcd.EtcdWatchTimedOut(
                             "Watch timed out: %r" % e,
diff --git a/src/etcd/tests/unit/test_request.py b/src/etcd/tests/unit/test_request.py
index 2456ae1..0942523 100644
--- a/src/etcd/tests/unit/test_request.py
+++ b/src/etcd/tests/unit/test_request.py
@@ -1,3 +1,4 @@
+import socket
 import urllib3
 
 import etcd
@@ -68,7 +69,8 @@ class TestClientApiInternals(TestClientApiBase):
         self._mock_api(200, d)
         self.client.write('/newdir', None, dir=True)
         self.assertEquals(self.client.api_execute.call_args,
-            (('/v2/keys/newdir', 'PUT'), dict(params={'dir': 'true'})))
+                          (('/v2/keys/newdir', 'PUT'),
+                           dict(params={'dir': 'true'})))
 
 
 class TestClientApiInterface(TestClientApiBase):
@@ -89,7 +91,9 @@ class TestClientApiInterface(TestClientApiBase):
     @mock.patch('etcd.Client.machines', new_callable=mock.PropertyMock)
     def test_use_proxies(self, mocker):
         """Do not overwrite the machines cache when using proxies"""
-        mocker.return_value = ['https://10.0.0.2:4001', 'https://10.0.0.3:4001', 'https://10.0.0.4:4001']
+        mocker.return_value = ['https://10.0.0.2:4001',
+                               'https://10.0.0.3:4001',
+                               'https://10.0.0.4:4001']
         c = etcd.Client(
             host=(('localhost', 4001), ('localproxy', 4001)),
             protocol='https',
@@ -99,17 +103,16 @@ class TestClientApiInterface(TestClientApiBase):
 
         self.assertEquals(c._machines_cache, ['https://localproxy:4001'])
         self.assertEquals(c._base_uri, 'https://localhost:4001')
-        self.assertNotIn(c.base_uri,c._machines_cache)
+        self.assertNotIn(c.base_uri, c._machines_cache)
 
         c = etcd.Client(
-            host=(('localhost', 4001), ('10.0.0.2',4001)),
+            host=(('localhost', 4001), ('10.0.0.2', 4001)),
             protocol='https',
             allow_reconnect=True,
             use_proxies=False
         )
         self.assertIn('https://10.0.0.3:4001', c._machines_cache)
-        self.assertNotIn(c.base_uri,c._machines_cache)
-
+        self.assertNotIn(c.base_uri, c._machines_cache)
 
     def test_members(self):
         """ Can request machines """
@@ -453,20 +456,32 @@ class TestClientRequest(TestClientApiInterface):
 
     def test_read_cluster_id_changed(self):
         """ Read timeout set to the default """
-        d = {u'action': u'set',
-             u'node': {
+        d = {
+            u'action': u'set',
+            u'node': {
                 u'expiration': u'2013-09-14T00:56:59.316195568+02:00',
                 u'modifiedIndex': 6,
                 u'key': u'/testkey',
                 u'ttl': 19,
-                u'value': u'test'
-                }
-             }
+                u'value': u'test',
+            }
+        }
         self._mock_api(200, d, cluster_id="notabcd1234")
         self.assertRaises(etcd.EtcdClusterIdChanged,
                           self.client.read, '/testkey')
         self.client.read("/testkey")
 
+    def test_read_connection_error(self):
+        self.client.http.request = mock.create_autospec(
+            self.client.http.request,
+            side_effect=socket.error()
+        )
+        self.assertRaises(etcd.EtcdConnectionFailed,
+                          self.client.read, '/something')
+        # Direct GET request
+        self.assertRaises(etcd.EtcdConnectionFailed,
+                          self.client.api_execute, '/a', 'GET')
+
     def test_not_in(self):
         pass
 
@@ -475,22 +490,23 @@ class TestClientRequest(TestClientApiInterface):
 
     def test_update_fails(self):
         """ Non-atomic updates fail """
-        d = {u'action': u'set',
-             u'node': {
+        d = {
+            u'action': u'set',
+            u'node': {
                 u'expiration': u'2013-09-14T00:56:59.316195568+02:00',
                 u'modifiedIndex': 6,
                 u'key': u'/testkey',
                 u'ttl': 19,
                 u'value': u'test'
-                }
-             }
+            }
+        }
         res = etcd.EtcdResult(**d)
 
         error = {
-            "errorCode":101,
-            "message":"Compare failed",
-            "cause":"[ != bar] [7 != 6]",
-            "index":6}
+            "errorCode": 101,
+            "message": "Compare failed",
+            "cause": "[ != bar] [7 != 6]",
+            "index": 6}
         self._mock_api(412, error)
         res.value = 'bar'
         self.assertRaises(ValueError, self.client.update, res)