fix: Include ConnectionError and urllib3 exception as retriable (#282)

* fix: Include ConnectionError and urllib3 exception as retriable

* fix: include Py3 ConnectionError class as retriable, and refactor

Author: andrewsg

packages/google-resumable-media/google/resumable_media/_helpers.py

@@ -20,7 +20,6 @@
 import hashlib
 import logging
 import random
-import time
 import warnings
 
 from google.resumable_media import common
@@ -135,78 +134,6 @@ def calculate_retry_wait(base_wait, max_sleep, multiplier=2.0):
     return new_base_wait, new_base_wait + 0.001 * jitter_ms
 
 
-def wait_and_retry(func, get_status_code, retry_strategy):
-    """Attempts to retry a call to ``func`` until success.
-
-    Expects ``func`` to return an HTTP response and uses ``get_status_code``
-    to check if the response is retry-able.
-
-    ``func`` is expected to raise a failure status code as a
-    common.InvalidResponse, at which point this method will check the code
-    against the common.RETRIABLE list of retriable status codes.
-
-    Will retry until :meth:`~.RetryStrategy.retry_allowed` (on the current
-    ``retry_strategy``) returns :data:`False`. Uses
-    :func:`calculate_retry_wait` to double the wait time (with jitter) after
-    each attempt.
-
-    Args:
-        func (Callable): A callable that takes no arguments and produces
-            an HTTP response which will be checked as retry-able.
-        get_status_code (Callable[Any, int]): Helper to get a status code
-            from a response.
-        retry_strategy (~google.resumable_media.common.RetryStrategy): The
-            strategy to use if the request fails and must be retried.
-
-    Returns:
-        object: The return value of ``func``.
-    """
-    total_sleep = 0.0
-    num_retries = 0
-    # base_wait will be multiplied by the multiplier on the first retry.
-    base_wait = float(retry_strategy.initial_delay) / retry_strategy.multiplier
-
-    # Set the retriable_exception_type if possible. We expect requests to be
-    # present here and the transport to be using requests.exceptions errors,
-    # but due to loose coupling with the transport layer we can't guarantee it.
-    try:
-        connection_error_exceptions = _get_connection_error_classes()
-    except ImportError:
-        # We don't know the correct classes to use to catch connection errors,
-        # so an empty tuple here communicates "catch no exceptions".
-        connection_error_exceptions = ()
-
-    while True:  # return on success or when retries exhausted.
-        error = None
-        try:
-            response = func()
-        except connection_error_exceptions as e:
-            error = e  # Fall through to retry, if there are retries left.
-        except common.InvalidResponse as e:
-            # An InvalidResponse is only retriable if its status code matches.
-            # The `process_response()` method on a Download or Upload method
-            # will convert the status code into an exception.
-            if get_status_code(e.response) in common.RETRYABLE:
-                error = e  # Fall through to retry, if there are retries left.
-            else:
-                raise  # If the status code is not retriable, raise w/o retry.
-        else:
-            return response
-
-        base_wait, wait_time = calculate_retry_wait(
-            base_wait, retry_strategy.max_sleep, retry_strategy.multiplier
-        )
-        num_retries += 1
-        total_sleep += wait_time
-
-        # Check if (another) retry is allowed. If retries are exhausted and
-        # no acceptable response was received, raise the retriable error.
-        if not retry_strategy.retry_allowed(total_sleep, num_retries):
-            raise error
-
-        time.sleep(wait_time)
-
-
 def _get_crc32c_object():
     """Get crc32c object
     Attempt to use the Google-CRC32c package. If it isn't available, try
@@ -375,22 +302,6 @@ def _get_checksum_object(checksum_type):
         raise ValueError("checksum must be ``'md5'``, ``'crc32c'`` or ``None``")
 
 
-def _get_connection_error_classes():
-    """Get the exception error classes.
-
-    Requests is a soft dependency here so that multiple transport layers can be
-    added in the future. This code is in a separate function here so that the
-    test framework can override its behavior to simulate requests being
-    unavailable."""
-
-    import requests.exceptions
-
-    return (
-        requests.exceptions.ConnectionError,
-        requests.exceptions.ChunkedEncodingError,
-    )
-
-
 class _DoNothingHash(object):
     """Do-nothing hash object.
 

packages/google-resumable-media/google/resumable_media/requests/_request_helpers.py

@@ -17,9 +17,13 @@
 This utilities are explicitly catered to ``requests``-like transports.
 """
 
+import requests.exceptions
+import urllib3.exceptions
 
-from google.resumable_media import common
+import time
 
+from google.resumable_media import common
+from google.resumable_media import _helpers
 
 _DEFAULT_RETRY_STRATEGY = common.RetryStrategy()
 _SINGLE_GET_CHUNK_SIZE = 8192
@@ -30,6 +34,13 @@
 # The number of seconds to wait between bytes sent from the server.
 _DEFAULT_READ_TIMEOUT = 60
 
+_CONNECTION_ERROR_CLASSES = (
+    requests.exceptions.ConnectionError,
+    requests.exceptions.ChunkedEncodingError,
+    urllib3.exceptions.ProtocolError,
+    ConnectionError,  # Python 3.x only, superclass of ConnectionResetError.
+)
+
 
 class RequestsMixin(object):
     """Mix-in class implementing ``requests``-specific behavior.
@@ -93,3 +104,69 @@ def _get_body(response):
             )
             response._content_consumed = True
         return response._content
+
+
+def wait_and_retry(func, get_status_code, retry_strategy):
+    """Attempts to retry a call to ``func`` until success.
+
+    Expects ``func`` to return an HTTP response and uses ``get_status_code``
+    to check if the response is retry-able.
+
+    ``func`` is expected to raise a failure status code as a
+    common.InvalidResponse, at which point this method will check the code
+    against the common.RETRIABLE list of retriable status codes.
+
+    Will retry until :meth:`~.RetryStrategy.retry_allowed` (on the current
+    ``retry_strategy``) returns :data:`False`. Uses
+    :func:`_helpers.calculate_retry_wait` to double the wait time (with jitter)
+    after each attempt.
+
+    Args:
+        func (Callable): A callable that takes no arguments and produces
+            an HTTP response which will be checked as retry-able.
+        get_status_code (Callable[Any, int]): Helper to get a status code
+            from a response.
+        retry_strategy (~google.resumable_media.common.RetryStrategy): The
+            strategy to use if the request fails and must be retried.
+
+    Returns:
+        object: The return value of ``func``.
+    """
+    total_sleep = 0.0
+    num_retries = 0
+    # base_wait will be multiplied by the multiplier on the first retry.
+    base_wait = float(retry_strategy.initial_delay) / retry_strategy.multiplier
+
+    # Set the retriable_exception_type if possible. We expect requests to be
+    # present here and the transport to be using requests.exceptions errors,
+    # but due to loose coupling with the transport layer we can't guarantee it.
+
+    while True:  # return on success or when retries exhausted.
+        error = None
+        try:
+            response = func()
+        except _CONNECTION_ERROR_CLASSES as e:
+            error = e  # Fall through to retry, if there are retries left.
+        except common.InvalidResponse as e:
+            # An InvalidResponse is only retriable if its status code matches.
+            # The `process_response()` method on a Download or Upload method
+            # will convert the status code into an exception.
+            if get_status_code(e.response) in common.RETRYABLE:
+                error = e  # Fall through to retry, if there are retries left.
+            else:
+                raise  # If the status code is not retriable, raise w/o retry.
+        else:
+            return response
+
+        base_wait, wait_time = _helpers.calculate_retry_wait(
+            base_wait, retry_strategy.max_sleep, retry_strategy.multiplier
+        )
+        num_retries += 1
+        total_sleep += wait_time
+
+        # Check if (another) retry is allowed. If retries are exhausted and
+        # no acceptable response was received, raise the retriable error.
+        if not retry_strategy.retry_allowed(total_sleep, num_retries):
+            raise error
+
+        time.sleep(wait_time)

packages/google-resumable-media/google/resumable_media/requests/download.py

@@ -171,7 +171,7 @@ def retriable_request():
 
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 
@@ -306,7 +306,7 @@ def retriable_request():
 
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 
@@ -381,7 +381,7 @@ def retriable_request():
             self._process_response(result)
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 
@@ -457,7 +457,7 @@ def retriable_request():
             self._process_response(result)
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 

packages/google-resumable-media/google/resumable_media/requests/upload.py

@@ -20,7 +20,6 @@
 
 
 from google.resumable_media import _upload
-from google.resumable_media import _helpers
 from google.resumable_media.requests import _request_helpers
 
 
@@ -80,7 +79,7 @@ def retriable_request():
 
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 
@@ -151,7 +150,7 @@ def retriable_request():
 
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 
@@ -418,7 +417,7 @@ def retriable_request():
 
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 
@@ -513,7 +512,7 @@ def retriable_request():
 
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )
 
@@ -552,6 +551,6 @@ def retriable_request():
 
             return result
 
-        return _helpers.wait_and_retry(
+        return _request_helpers.wait_and_retry(
             retriable_request, self._get_status_code, self._retry_strategy
         )