Cpappas/rev 3810 (#236)

* fix: catching RequestExceptions in fulfillment task

* chore: bump version

ecommerce_worker/__init__.py

@@ -1,3 +1,3 @@
 """init"""
 
-__version__ = '3.3.4'
+__version__ = '3.3.5'

ecommerce_worker/fulfillment/v1/tasks.py

@@ -6,7 +6,7 @@
 from celery import shared_task
 from celery.exceptions import Ignore
 from celery.utils.log import get_task_logger
-from requests.exceptions import HTTPError, Timeout
+from requests.exceptions import HTTPError, Timeout, RequestException
 
 from ecommerce_worker.utils import get_configuration, get_access_token
 
@@ -72,6 +72,6 @@ def fulfill_order(self, order_number, site_code=None, email_opt_in=False):
             exc_info=True
         )
         _retry_order(self, exc, max_fulfillment_retries, order_number)
-    except (Timeout, SSLError) as exc:
+    except (Timeout, SSLError, RequestException) as exc:
         # Fulfillment failed, retry
         _retry_order(self, exc, max_fulfillment_retries, order_number)

ecommerce_worker/fulfillment/v1/tests/test_tasks.py

@@ -7,7 +7,7 @@
 import responses
 from celery.exceptions import Ignore
 from edx_rest_api_client import exceptions
-from requests.exceptions import HTTPError
+from requests.exceptions import HTTPError, RequestException
 
 # Ensures that a Celery app is initialized when tests are run.
 from ecommerce_worker import celery_app  # pylint: disable=unused-import
@@ -144,6 +144,28 @@ def test_fulfillment_retry_success(self):
         result = fulfill_order.delay(self.ORDER_NUMBER).get()
         self.assertIsNone(result)
 
+    @mock.patch('ecommerce_worker.fulfillment.v1.tasks.get_access_token')
+    @responses.activate
+    def test_fulfillment_retry_on_request_exception(self, mock_get_access_token):
+        """
+        Regression: In the case that the LMS is unavailable, the client that calls
+        the LMS for an authentication token may error with RequestException.
+        If this happens, we should retry like other HTTPErrors. Previously we were
+        not catching this exception type.
+        """
+        # First time should fail to get a token. Second time should succeed.
+        mock_get_access_token.side_effect = [RequestException, 'FAKE-access-token']
+        responses.add(responses.PUT, self.API_URL, status=200, body="{}")
+
+        result = fulfill_order.delay(self.ORDER_NUMBER).get()
+        self.assertIsNone(result)
+
+        # Validate the value of the HTTP Authorization header.
+        last_request = responses.calls[-1].request
+        token = last_request.headers.get('Authorization').split()[1]
+        self.assertEqual(token, self.ACCESS_TOKEN)
+        assert mock_get_access_token.call_count == 2
+
     @ddt.data(
         [True, 'True'],
         [False, 'False']