fixes bug 916886 - retry on ConnectionError on middleware, r=rhelmer

webapp-django/crashstats/crashstats/models.py

@@ -67,7 +67,14 @@ class SocorroCommon(object):
     cache_seconds = 60 * 60
 
     def fetch(self, url, headers=None, method='get', data=None,
-              expect_json=True, dont_cache=False):
+              expect_json=True, dont_cache=False,
+              retries=None,
+              retry_sleeptime=None):
+
+        if retries is None:
+            retries = settings.MIDDLEWARE_RETRIES
+        if retry_sleeptime is None:
+            retry_sleeptime = settings.MIDDLEWARE_RETRY_SLEEPTIME
 
         if url.startswith('/'):
             url = self._complete_url(url)
@@ -143,7 +150,28 @@ def fetch(self, url, headers=None, method='get', data=None,
         else:
             raise ValueError(method)
 
-        resp = request_method(url=url, auth=auth, headers=headers, data=data)
+        try:
+            resp = request_method(
+                url=url,
+                auth=auth,
+                headers=headers,
+                data=data
+            )
+        except requests.ConnectionError:
+            if not retries:
+                raise
+            # https://bugzilla.mozilla.org/show_bug.cgi?id=916886
+            time.sleep(retry_sleeptime)
+            return self.fetch(
+                url,
+                headers=headers,
+                method=method,
+                data=data,
+                expect_json=expect_json,
+                dont_cache=dont_cache,
+                retry_sleeptime=retry_sleeptime,
+                retries=retries - 1
+            )
 
         self._process_response(method, url, resp.status_code)
 

webapp-django/crashstats/crashstats/tests/test_models.py

@@ -7,6 +7,7 @@
 import urllib
 import random
 import mock
+import requests
 from nose.tools import eq_, ok_
 from django.test import TestCase
 from django.core.cache import cache
@@ -1370,3 +1371,62 @@ def mocked_get(**options):
         bugnumbers = [str(random.randint(10000, 100000)) for __ in range(100)]
         info = api.get(bugnumbers, 'product')
         ok_(info)
+
+    @mock.patch('crashstats.crashstats.models.time')
+    @mock.patch('requests.get')
+    def test_retry_on_connectionerror_success(self, rget, mocked_time):
+        sleeps = []
+
+        def mocked_sleeper(seconds):
+            sleeps.append(seconds)
+
+        mocked_time.sleep = mocked_sleeper
+
+        # doesn't really matter which api we use
+        model = models.BugzillaBugInfo
+        api = model()
+
+        calls = []
+
+        def mocked_get(url, **options):
+            calls.append(url)
+            if len(calls) < 3:
+                raise requests.ConnectionError('unable to connect')
+            return Response('{"bugs": [{"product": "mozilla.org"}]}')
+
+        rget.side_effect = mocked_get
+        info = api.get(['987654'], 'product')
+        ok_(info['bugs'])
+
+        eq_(len(calls), 3)  # had to attempt 3 times
+        eq_(len(sleeps), 2)  # had to sleep 2 times
+
+    @mock.patch('crashstats.crashstats.models.time')
+    @mock.patch('requests.get')
+    def test_retry_on_connectionerror_failing(self, rget, mocked_time):
+        sleeps = []
+
+        def mocked_sleeper(seconds):
+            sleeps.append(seconds)
+
+        mocked_time.sleep = mocked_sleeper
+
+        # doesn't really matter which api we use
+        model = models.BugzillaBugInfo
+        api = model()
+
+        calls = []
+
+        def mocked_get(url, **options):
+            calls.append(url)
+            raise requests.ConnectionError('unable to connect')
+
+        rget.side_effect = mocked_get
+        self.assertRaises(
+            requests.ConnectionError,
+            api.get,
+            ['987654'],
+            'product'
+        )
+        ok_(len(calls) > 3)  # had to attempt more than 3 times
+        ok_(len(sleeps) > 2)  # had to sleep more than 2 times

webapp-django/crashstats/settings/base.py

@@ -286,3 +286,9 @@
 
 # Default number of days to show in explosive crashes reports
 EXPLOSIVE_REPORT_DAYS = 10
+
+# how many seconds to sleep when getting a ConnectionError
+MIDDLEWARE_RETRY_SLEEPTIME = 3
+
+# how many times to re-attempt on ConnectionError after some sleep
+MIDDLEWARE_RETRIES = 10