From: Itamar Turner-Trauring <itamar@futurefoundries.com>
Date: Tue, 26 Mar 2013 19:02:11 +0000 (-0400)
Subject: Retry cloud HTTP requests on *any* exception (the list is long, and hard to make... 
X-Git-Url: https://git.rkrishnan.org/reliability?a=commitdiff_plain;h=5f954cf8327eaf054a47c91f0148513408f0faf5;p=tahoe-lafs%2Ftahoe-lafs.git

Retry cloud HTTP requests on *any* exception (the list is long, and hard to make complete, so easier to just handle all exceptions).
---

diff --git a/src/allmydata/storage/backends/cloud/cloud_common.py b/src/allmydata/storage/backends/cloud/cloud_common.py
index ce3d2156..657d9662 100644
--- a/src/allmydata/storage/backends/cloud/cloud_common.py
+++ b/src/allmydata/storage/backends/cloud/cloud_common.py
@@ -340,7 +340,9 @@ class ContainerRetryMixin:
 
     Subclasses should define:
       ServiceError:
-          The error class to trap (CloudServiceError or similar).
+          An exceptions class with meaningful status codes that can be
+          filtered using _react_to_error. Other exceptions will cause
+          unconditional retries.
 
     and can override:
       _react_to_error(self, response_code):
@@ -366,8 +368,6 @@ class ContainerRetryMixin:
         return d
 
     def _handle_error(self, f, trynum, first_err_and_tb, description, operation, *args, **kwargs):
-        f.trap(self.ServiceError, TimeoutError)
-
         # Don't use f.getTracebackObject() since a fake traceback will not do for the 3-arg form of 'raise'.
         # tb can be None (which is acceptable for 3-arg raise) if we don't have a traceback.
         tb = getattr(f, 'tb', None)
diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index df7615f8..c27aabb0 100644
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -839,10 +839,12 @@ class ContainerRetryTests(unittest.TestCase, CloudStorageBackendMixin):
         second.callback(done)
         self.assertEqual(result, [done])
 
-    def test_retry_timeout(self):
+    def test_retry_random_exception(self):
         """
-        If an HTTP connection fails with a timeout, retry.
+        If a HTTP request fails with any exception at all, retry.
         """
+        class NewException(Exception):
+            pass
         first, second = defer.Deferred(), defer.Deferred()
         self.container._http_request = mock.create_autospec(
             self.container._http_request, side_effect=[first, second])
@@ -859,8 +861,7 @@ class ContainerRetryTests(unittest.TestCase, CloudStorageBackendMixin):
             body=None, need_response_body=True)
 
         # First response fails:
-        from twisted.internet.error import TimeoutError
-        first.errback(TimeoutError())
+        first.errback(NewException())
         self.assertFalse(result, result)
         self.assertEqual(self.container._http_request.call_count, 1)
         self.reactor.advance(0.1)