Re-raise unrecognized exceptions on retry.

.travis.yml

@@ -11,7 +11,7 @@ matrix:
   # Disabled until gRPC supports PyPy.
   # - python: pypy
   #   env: TOXENV=pypy
-  - python: 3.6
+  - python: 3.5
     env: TOXENV=lint
   - python: 3.6
     env: TOXENV=docs

google/gax/retry.py

@@ -32,9 +32,13 @@
 from __future__ import absolute_import, division
 
 import random
+import sys
 import time
 
-from google.gax import config, errors
+import six
+
+from google.gax import config
+from google.gax import errors
 
 _MILLIS_PER_SECOND = 1000
 
@@ -101,8 +105,6 @@ def inner(*args):
         by the options in ``retry``.
         """
         delay = retry_options.backoff_settings.initial_retry_delay_millis
-        exc = errors.RetryError('Retry total timeout exceeded before any'
-                                'response was received')
         if has_timeout_settings:
             timeout = (
                 retry_options.backoff_settings.initial_rpc_timeout_millis /
@@ -118,16 +120,14 @@ def inner(*args):
             try:
                 to_call = add_timeout_arg(a_func, timeout, **kwargs)
                 return to_call(*args)
-            except Exception as exception:  # pylint: disable=broad-except
+            except tuple(config.API_ERRORS) as exception:
                 code = config.exc_to_code(exception)
-                if code not in retry_options.retry_codes:
-                    raise errors.RetryError(
-                        'Exception occurred in retry method that was not'
-                        ' classified as transient', exception)
 
-                # pylint: disable=redefined-variable-type
-                exc = errors.RetryError(
-                    'Retry total timeout exceeded with exception', exception)
+                # If this is an exception that we do not know for sure should
+                # be retried, wrap it in the way that we would any standard
+                # error.
+                if code not in retry_options.retry_codes:
+                    raise errors.create_error(str(exception), cause=exception)
 
                 # Sleep a random number which will, on average, equal the
                 # expected delay.
@@ -140,6 +140,16 @@ def inner(*args):
                     timeout = min(
                         timeout * timeout_mult, max_timeout, deadline - now)
 
-        raise exc
+                # If we have passed our deadline, raise the exception.
+                if deadline is not None and now >= deadline:
+                    exc = errors.RetryError(
+                        'Retry timeout exceeded; exception: %s' % exception,
+                        cause=exception,
+                    )
+                    six.reraise(errors.RetryError, exc, sys.exc_info()[2])
+
+        # If we got here, then we timed out and never got a response at all.
+        raise errors.RetryError('Retry total timeout exceeded before any'
+                                'response was received.')
 
     return inner

setup.py

@@ -41,6 +41,7 @@
     'oauth2client>=2.0.0, <4.0dev',
     'ply==3.8',
     'protobuf>=3.0.0, <4.0dev',
+    'six>=1.9.0',
 ]
 
 with open('README.rst', 'r') as readme:

tests/test_api_callable.py

@@ -43,7 +43,8 @@
     __version__ as GAX_VERSION, _CallSettings, api_callable, BackoffSettings,
     BundleDescriptor, BundleOptions, bundling, CallOptions, INITIAL_PAGE,
     PageDescriptor, RetryOptions)
-from google.gax.errors import GaxError
+
+from tests.utils import errors
 
 # pylint: disable=no-member
 GRPC_VERSION = pkg_resources.get_distribution('grpcio').version
@@ -106,9 +107,6 @@
                'code_c': Exception}
 
 
-_FAKE_STATUS_CODE_1 = object()
-
-
 class CustomException(Exception):
     def __init__(self, msg, code):
         super(CustomException, self).__init__(msg)
@@ -142,24 +140,26 @@ def test_call_kwargs(self):
                          'updated')
 
     @mock.patch('time.time')
-    @mock.patch('google.gax.config.exc_to_code')
-    def test_retry(self, mock_exc_to_code, mock_time):
-        mock_exc_to_code.side_effect = lambda e: e.code
+    def test_retry(self, mock_time):
         to_attempt = 3
         retry = RetryOptions(
-            [_FAKE_STATUS_CODE_1],
-            BackoffSettings(0, 0, 0, 0, 0, 0, 1))
-
-        # Succeeds on the to_attempt'th call, and never again afterward
-        mock_call = mock.Mock()
-        mock_call.side_effect = ([CustomException('', _FAKE_STATUS_CODE_1)] *
-                                 (to_attempt - 1) + [mock.DEFAULT])
-        mock_call.return_value = 1729
+            ['BOGUS_STATUS_CODE'],
+            BackoffSettings(0, 0, 0, 0, 0, 0, 1),
+        )
+
+        # Define the exception to be raised on every attempt before the
+        # last one, and the result for the last attempt.
+        to_attempt = 3
+        exc = errors.MockGrpcException(code='BOGUS_STATUS_CODE')
+        mock_func = mock.Mock()
+        mock_func.side_effect = [exc] * (to_attempt - 1) + [mock.DEFAULT]
+        mock_func.return_value = 1729
+
         mock_time.return_value = 0
-        settings = _CallSettings(timeout=0, retry=retry)
-        my_callable = api_callable.create_api_call(mock_call, settings)
+        settings = _CallSettings(timeout=None, retry=retry)
+        my_callable = api_callable.create_api_call(mock_func, settings)
         self.assertEqual(my_callable(None), 1729)
-        self.assertEqual(mock_call.call_count, to_attempt)
+        self.assertEqual(mock_func.call_count, to_attempt)
 
     def test_page_streaming(self):
         # A mock grpc function that page streams a list of consecutive
@@ -388,7 +388,7 @@ def other_error_func(*dummy_args, **dummy_kwargs):
 
         gax_error_callable = api_callable.create_api_call(
             abortion_error_func, _CallSettings())
-        self.assertRaises(GaxError, gax_error_callable, None)
+        self.assertRaises(errors.GaxError, gax_error_callable, None)
 
         other_error_callable = api_callable.create_api_call(
             other_error_func, _CallSettings())

tests/test_retry.py

@@ -32,158 +32,156 @@
 
 from __future__ import absolute_import, division
 
+import unittest
+
 import mock
-import unittest2
 
-from google.gax import BackoffSettings, errors, retry, RetryOptions
+from google.gax import BackoffSettings
+from google.gax import retry
+from google.gax import RetryOptions
 
-_MILLIS_PER_SEC = 1000
+from tests.utils import errors
 
 
-_FAKE_STATUS_CODE_1 = object()
-
+_MILLIS_PER_SEC = 1000
 
+_FAKE_STATUS_CODE_1 = object()
 _FAKE_STATUS_CODE_2 = object()
 
 
-class CustomException(Exception):
-    def __init__(self, msg, code):
-        super(CustomException, self).__init__(msg)
-        self.code = code
-
-
-class TestRetry(unittest2.TestCase):
-
-    @mock.patch('google.gax.config.exc_to_code')
+class TestRetry(unittest.TestCase):
     @mock.patch('time.time')
-    def test_retryable_without_timeout(self, mock_time, mock_exc_to_code):
+    def test_retryable_without_timeout(self, mock_time):
         mock_time.return_value = 0
-        mock_exc_to_code.side_effect = lambda e: e.code
 
+        # Define the exception to be raised on every attempt before the
+        # last one, and the result for the last attempt.
         to_attempt = 3
-        mock_call = mock.Mock()
-        mock_call.side_effect = ([CustomException('', _FAKE_STATUS_CODE_1)] *
-                                 (to_attempt - 1) + [mock.DEFAULT])
-        mock_call.return_value = 1729
+        exc = errors.MockGrpcException(code=_FAKE_STATUS_CODE_1)
+        mock_func = mock.Mock()
+        mock_func.side_effect = [exc] * (to_attempt - 1) + [mock.DEFAULT]
+        mock_func.return_value = 1729
 
         retry_options = RetryOptions(
             [_FAKE_STATUS_CODE_1],
-            BackoffSettings(0, 0, 0, None, None, None, None))
+            BackoffSettings(0, 0, 0, None, None, None, None),
+        )
 
-        my_callable = retry.retryable(mock_call, retry_options)
+        my_callable = retry.retryable(mock_func, retry_options)
+        result = my_callable(None)
 
-        self.assertEqual(my_callable(None), 1729)
-        self.assertEqual(to_attempt, mock_call.call_count)
+        self.assertEqual(result, 1729)
+        self.assertEqual(to_attempt, mock_func.call_count)
 
-    @mock.patch('google.gax.config.exc_to_code')
     @mock.patch('time.time')
-    def test_retryable_with_timeout(self, mock_time, mock_exc_to_code):
+    def test_retryable_with_timeout(self, mock_time):
         mock_time.return_value = 1
-        mock_exc_to_code.side_effect = lambda e: e.code
 
-        mock_call = mock.Mock()
-        mock_call.side_effect = [CustomException('', _FAKE_STATUS_CODE_1),
-                                 mock.DEFAULT]
-        mock_call.return_value = 1729
+        mock_func = mock.Mock()
+        mock_func.side_effect = [
+            errors.MockGrpcException(code=_FAKE_STATUS_CODE_1),
+            mock.DEFAULT,
+        ]
+        mock_func.return_value = 1729
 
         retry_options = RetryOptions(
             [_FAKE_STATUS_CODE_1],
-            BackoffSettings(0, 0, 0, 0, 0, 0, 0))
+            BackoffSettings(0, 0, 0, 0, 0, 0, 0),
+        )
 
-        my_callable = retry.retryable(mock_call, retry_options)
+        my_callable = retry.retryable(mock_func, retry_options)
 
         self.assertRaises(errors.RetryError, my_callable)
-        self.assertEqual(0, mock_call.call_count)
+        self.assertEqual(0, mock_func.call_count)
 
-    @mock.patch('google.gax.config.exc_to_code')
     @mock.patch('time.time')
-    def test_retryable_when_no_codes(self, mock_time, mock_exc_to_code):
+    def test_retryable_when_no_codes(self, mock_time):
         mock_time.return_value = 0
-        mock_exc_to_code.side_effect = lambda e: e.code
-
-        mock_call = mock.Mock()
-        mock_call.side_effect = [CustomException('', _FAKE_STATUS_CODE_1),
-                                 mock.DEFAULT]
-        mock_call.return_value = 1729
 
+        # Set up the mock function to raise an exception that is *not*
+        # an expected code.
+        mock_func = mock.Mock()
+        mock_func.side_effect = [
+            errors.MockGrpcException(code=_FAKE_STATUS_CODE_2),
+            mock.DEFAULT,
+        ]
+        mock_func.return_value = 1729
+
+        # Set the retry options not to actually honor any codes
+        # (thus, our code is not in the list).
         retry_options = RetryOptions(
             [],
-            BackoffSettings(0, 0, 0, 0, 0, 0, 1))
+            BackoffSettings(0, 0, 0, 0, 0, 0, 1),
+        )
 
-        my_callable = retry.retryable(mock_call, retry_options)
-
-        try:
+        # Create the callable and establish that we get a GaxError.
+        my_callable = retry.retryable(mock_func, retry_options)
+        with self.assertRaises(errors.GaxError):
             my_callable(None)
-            self.fail('Should not have been reached')
-        except errors.RetryError as exc:
-            self.assertIsInstance(exc.cause, CustomException)
 
-        self.assertEqual(1, mock_call.call_count)
+        # The actual retryable function should have been called exactly once.
+        mock_func.assert_called_once()
 
-    @mock.patch('google.gax.config.exc_to_code')
     @mock.patch('time.time')
-    def test_retryable_aborts_on_unexpected_exception(
-            self, mock_time, mock_exc_to_code):
+    def test_retryable_aborts_on_unexpected_exception(self, mock_time):
         mock_time.return_value = 0
-        mock_exc_to_code.side_effect = lambda e: e.code
 
-        mock_call = mock.Mock()
-        mock_call.side_effect = [CustomException('', _FAKE_STATUS_CODE_2),
-                                 mock.DEFAULT]
-        mock_call.return_value = 1729
+        # Set up the mock function to raise an exception that should be
+        # bubbled up (because it is not recognized).
+        mock_func = mock.Mock()
+        mock_func.side_effect = [
+            ValueError('bogus'),
+            mock.DEFAULT,
+        ]
+        mock_func.return_value = 1729
 
         retry_options = RetryOptions(
             [_FAKE_STATUS_CODE_1],
-            BackoffSettings(0, 0, 0, 0, 0, 0, 1))
-
-        my_callable = retry.retryable(mock_call, retry_options)
+            BackoffSettings(0, 0, 0, 0, 0, 0, 1),
+        )
+        my_callable = retry.retryable(mock_func, retry_options)
 
-        try:
+        # Establish that the custom exception is bubbled up (not wrapped), and
+        # that the retryable function was called only once, not twice.
+        with self.assertRaises(ValueError):
             my_callable(None)
-            self.fail('Should not have been reached')
-        except errors.RetryError as exc:
-            self.assertIsInstance(exc.cause, CustomException)
-
-        self.assertEqual(1, mock_call.call_count)
+        mock_func.assert_called_once()
 
-    @mock.patch('google.gax.config.exc_to_code')
     @mock.patch('time.sleep')
     @mock.patch('time.time')
-    def test_retryable_exponential_backoff(
-            self, mock_time, mock_sleep, mock_exc_to_code):
+    def test_retryable_exponential_backoff(self, mock_time, mock_sleep):
         def incr_time(secs):
             mock_time.return_value += secs
 
         def api_call(timeout):
             incr_time(timeout)
-            raise CustomException(str(timeout), _FAKE_STATUS_CODE_1)
+            raise errors.MockGrpcException(str(timeout), _FAKE_STATUS_CODE_1)
 
         mock_time.return_value = 0
         mock_sleep.side_effect = incr_time
-        mock_exc_to_code.side_effect = lambda e: e.code
 
-        mock_call = mock.Mock()
-        mock_call.side_effect = api_call
+        mock_func = mock.Mock()
+        mock_func.side_effect = api_call
 
         params = BackoffSettings(3, 2, 24, 5, 2, 80, 2500)
         retry_options = RetryOptions([_FAKE_STATUS_CODE_1], params)
 
-        my_callable = retry.retryable(mock_call, retry_options)
+        my_callable = retry.retryable(mock_func, retry_options)
 
         try:
             my_callable()
             self.fail('Should not have been reached')
         except errors.RetryError as exc:
-            self.assertIsInstance(exc.cause, CustomException)
+            self.assertIsInstance(exc.cause, errors.MockGrpcException)
 
         self.assertGreaterEqual(mock_time(),
                                 params.total_timeout_millis / _MILLIS_PER_SEC)
 
         # Very rough bounds
         calls_lower_bound = params.total_timeout_millis / (
             params.max_retry_delay_millis + params.max_rpc_timeout_millis)
-        self.assertGreater(mock_call.call_count, calls_lower_bound)
+        self.assertGreater(mock_func.call_count, calls_lower_bound)
 
         calls_upper_bound = (params.total_timeout_millis /
                              params.initial_retry_delay_millis)
-        self.assertLess(mock_call.call_count, calls_upper_bound)
+        self.assertLess(mock_func.call_count, calls_upper_bound)