fix: Adding ConnectionError to retry mechanism (#822)

This commit fixes issue #558

Co-authored-by: Bu Sun Kim <[email protected]>

Author: damgad

googleapiclient/http.py

@@ -81,6 +81,11 @@
 
 _LEGACY_BATCH_URI = "https://www.googleapis.com/batch"
 
+if six.PY2:
+    # That's a builtin python3 exception, nonexistent in python2.
+    # Defined to None to avoid NameError while trying to catch it
+    ConnectionError = None
+
 
 def _should_retry_response(resp_status, content):
     """Determines whether a response should be retried.
@@ -177,6 +182,10 @@ def _retry_request(
             # It's important that this be before socket.error as it's a subclass
             # socket.timeout has no errorcode
             exception = socket_timeout
+        except ConnectionError as connection_error:
+            # Needs to be before socket.error as it's a subclass of
+            # OSError (socket.error)
+            exception = connection_error
         except socket.error as socket_error:
             # errno's contents differ by platform, so we have to match by name.
             if socket.errno.errorcode.get(socket_error.errno) not in {

tests/test_http.py

@@ -132,32 +132,31 @@ def __init__(self, num_errors, success_json, success_data):
     def request(self, *args, **kwargs):
         if not self.num_errors:
             return httplib2.Response(self.success_json), self.success_data
+        elif self.num_errors == 5 and PY3:
+            ex = ConnectionResetError  # noqa: F821
+        elif self.num_errors == 4:
+            ex = httplib2.ServerNotFoundError()
+        elif self.num_errors == 3:
+            ex = socket.error()
+            ex.errno = socket.errno.EPIPE
+        elif self.num_errors == 2:
+            ex = ssl.SSLError()
         else:
-            self.num_errors -= 1
-            if self.num_errors == 1:  # initial == 2
-                raise ssl.SSLError()
-            if self.num_errors == 3:  # initial == 4
-                raise httplib2.ServerNotFoundError()
-            else:  # initial != 2,4
-                if self.num_errors == 2:
-                    # first try a broken pipe error (#218)
-                    ex = socket.error()
-                    ex.errno = socket.errno.EPIPE
+            # Initialize the timeout error code to the platform's error code.
+            try:
+                # For Windows:
+                ex = socket.error()
+                ex.errno = socket.errno.WSAETIMEDOUT
+            except AttributeError:
+                # For Linux/Mac:
+                if PY3:
+                    ex = socket.timeout()
                 else:
-                    # Initialize the timeout error code to the platform's error code.
-                    try:
-                        # For Windows:
-                        ex = socket.error()
-                        ex.errno = socket.errno.WSAETIMEDOUT
-                    except AttributeError:
-                        # For Linux/Mac:
-                        if PY3:
-                            ex = socket.timeout()
-                        else:
-                            ex = socket.error()
-                            ex.errno = socket.errno.ETIMEDOUT
-                # Now raise the correct error.
-                raise ex
+                    ex = socket.error()
+                    ex.errno = socket.errno.ETIMEDOUT
+
+        self.num_errors -= 1
+        raise ex
 
 
 class HttpMockWithNonRetriableErrors(object):
@@ -562,14 +561,14 @@ def test_media_io_base_download_handle_4xx(self):
 
     def test_media_io_base_download_retries_connection_errors(self):
         self.request.http = HttpMockWithErrors(
-            4, {"status": "200", "content-range": "0-2/3"}, b"123"
+            5, {"status": "200", "content-range": "0-2/3"}, b"123"
         )
 
         download = MediaIoBaseDownload(fd=self.fd, request=self.request, chunksize=3)
         download._sleep = lambda _x: 0  # do nothing
         download._rand = lambda: 10
 
-        status, done = download.next_chunk(num_retries=4)
+        status, done = download.next_chunk(num_retries=5)
 
         self.assertEqual(self.fd.getvalue(), b"123")
         self.assertEqual(True, done)
@@ -899,13 +898,13 @@ def test_no_retry_connection_errors(self):
     def test_retry_connection_errors_non_resumable(self):
         model = JsonModel()
         request = HttpRequest(
-            HttpMockWithErrors(4, {"status": "200"}, '{"foo": "bar"}'),
+            HttpMockWithErrors(5, {"status": "200"}, '{"foo": "bar"}'),
             model.response,
             u"https://www.example.com/json_api_endpoint",
         )
         request._sleep = lambda _x: 0  # do nothing
         request._rand = lambda: 10
-        response = request.execute(num_retries=4)
+        response = request.execute(num_retries=5)
         self.assertEqual({u"foo": u"bar"}, response)
 
     def test_retry_connection_errors_resumable(self):
@@ -918,7 +917,7 @@ def test_retry_connection_errors_resumable(self):
 
         request = HttpRequest(
             HttpMockWithErrors(
-                4, {"status": "200", "location": "location"}, '{"foo": "bar"}'
+                5, {"status": "200", "location": "location"}, '{"foo": "bar"}'
             ),
             model.response,
             u"https://www.example.com/file_upload",
@@ -927,7 +926,7 @@ def test_retry_connection_errors_resumable(self):
         )
         request._sleep = lambda _x: 0  # do nothing
         request._rand = lambda: 10
-        response = request.execute(num_retries=4)
+        response = request.execute(num_retries=5)
         self.assertEqual({u"foo": u"bar"}, response)
 
     def test_retry(self):