Gracefully handle LMS connection errors

ecommerce/health/constants.py

+"""Health check constants."""
+
+
+class Status(object):
+    """Health statuses."""
+    OK = u"OK"
+    UNAVAILABLE = u"UNAVAILABLE"
+
+
+class UnavailabilityMessage(object):
+    """Messages to be logged when services are unavailable."""
+    DATABASE = u"Unable to connect to database"
+    LMS = u"Unable to connect to LMS"

ecommerce/health/tests/test_views.py

@@ -4,74 +4,81 @@ import logging
 
 import mock
 from requests import Response
+from requests.exceptions import RequestException
 from rest_framework import status
 from django.test import TestCase
 from django.db import DatabaseError
 from django.core.urlresolvers import reverse
 
-from ecommerce.health.views import OK, UNAVAILABLE
+from ecommerce.health.constants import Status
 
 
 @mock.patch('requests.get')
-class HealthViewTests(TestCase):
+class HealthTests(TestCase):
+    """Tests of the health endpoint."""
     def setUp(self):
         self.fake_lms_response = Response()
 
         # Override all loggers, suppressing logging calls of severity CRITICAL and below
         logging.disable(logging.CRITICAL)
 
-    def tearDown(self):
         # Remove logger override
-        logging.disable(logging.NOTSET)
+        self.addCleanup(logging.disable, logging.NOTSET)
 
-    def test_healthy(self, mock_lms_request):
+    def test_all_services_available(self, mock_lms_request):
+        """Test that the endpoint reports when all services are healthy."""
         self.fake_lms_response.status_code = status.HTTP_200_OK
         mock_lms_request.return_value = self.fake_lms_response
 
-        response = self.client.get(reverse('health'))
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response['content-type'], 'application/json')
-
-        expected_data = {
-            'overall_status': OK,
-            'detailed_status': {
-                'database_status': OK,
-                'lms_status': OK
-            }
-        }
-        self.assertDictEqual(json.loads(response.content), expected_data)
+        self._assert_health(status.HTTP_200_OK, Status.OK, Status.OK, Status.OK)
 
     @mock.patch('django.db.backends.BaseDatabaseWrapper.cursor', mock.Mock(side_effect=DatabaseError))
     def test_database_outage(self, mock_lms_request):
+        """Test that the endpoint reports when the database is unavailable."""
         self.fake_lms_response.status_code = status.HTTP_200_OK
         mock_lms_request.return_value = self.fake_lms_response
 
-        response = self.client.get(reverse('health'))
-        self.assertEqual(response.status_code, status.HTTP_503_SERVICE_UNAVAILABLE)
-        self.assertEqual(response['content-type'], 'application/json')
+        self._assert_health(
+            status.HTTP_503_SERVICE_UNAVAILABLE,
+            Status.UNAVAILABLE,
+            Status.UNAVAILABLE,
+            Status.OK
+        )
 
-        expected_data = {
-            'overall_status': UNAVAILABLE,
-            'detailed_status': {
-                'database_status': UNAVAILABLE,
-                'lms_status': OK
-            }
-        }
-        self.assertDictEqual(json.loads(response.content), expected_data)
-
-    def test_health_lms_outage(self, mock_lms_request):
+    def test_lms_outage(self, mock_lms_request):
+        """Test that the endpoint reports when the LMS is experiencing difficulties."""
         self.fake_lms_response.status_code = status.HTTP_503_SERVICE_UNAVAILABLE
         mock_lms_request.return_value = self.fake_lms_response
 
+        self._assert_health(
+            status.HTTP_503_SERVICE_UNAVAILABLE,
+            Status.UNAVAILABLE,
+            Status.OK,
+            Status.UNAVAILABLE
+        )
+
+    def test_lms_connection_failure(self, mock_lms_request):
+        """Test that the endpoint reports when it cannot contact the LMS."""
+        mock_lms_request.side_effect = RequestException
+
+        self._assert_health(
+            status.HTTP_503_SERVICE_UNAVAILABLE,
+            Status.UNAVAILABLE,
+            Status.OK,
+            Status.UNAVAILABLE
+        )
+
+    def _assert_health(self, status_code, overall_status, database_status, lms_status):
+        """Verify that the response matches expectations."""
         response = self.client.get(reverse('health'))
-        self.assertEqual(response.status_code, status.HTTP_503_SERVICE_UNAVAILABLE)
+        self.assertEqual(response.status_code, status_code)
         self.assertEqual(response['content-type'], 'application/json')
 
         expected_data = {
-            'overall_status': UNAVAILABLE,
+            'overall_status': overall_status,
             'detailed_status': {
-                'database_status': OK,
-                'lms_status': UNAVAILABLE
+                'database_status': database_status,
+                'lms_status': lms_status
             }
         }
         self.assertDictEqual(json.loads(response.content), expected_data)

ecommerce/health/views.py

@@ -2,16 +2,17 @@
 import logging
 
 import requests
+from requests.exceptions import RequestException
 from rest_framework import status
 from django.conf import settings
 from django.db import connection, DatabaseError
 from django.http import JsonResponse
 
+from ecommerce.health.constants import Status, UnavailabilityMessage
+
 
 logger = logging.getLogger(__name__)
 
-OK = u'OK'
-UNAVAILABLE = u'UNAVAILABLE'
 LMS_HEALTH_PAGE = getattr(settings, 'LMS_HEARTBEAT_URL')
 
 
@@ -34,26 +35,31 @@ def health(_):
         >>> response.content
         '{"overall_status": "OK", "detailed_status": {"database_status": "OK", "lms_status": "OK"}}'
     """
-    overall_status = database_status = lms_status = UNAVAILABLE
+    overall_status = database_status = lms_status = Status.UNAVAILABLE
 
     try:
         cursor = connection.cursor()
         cursor.execute("SELECT 1")
         cursor.fetchone()
         cursor.close()
-        database_status = OK
+        database_status = Status.OK
     except DatabaseError:
-        logger.critical('Unable to connect to database')
-        database_status = UNAVAILABLE
 
-    response = requests.get(LMS_HEALTH_PAGE)
-    if response.status_code == status.HTTP_200_OK:
-        lms_status = OK
-    else:
-        logger.critical('Unable to connect to LMS')
-        lms_status = UNAVAILABLE
+        database_status = Status.UNAVAILABLE
+
+    try:
+        response = requests.get(LMS_HEALTH_PAGE)
+
+        if response.status_code == status.HTTP_200_OK:
+            lms_status = Status.OK
+        else:
+            logger.critical(UnavailabilityMessage.LMS)
+            lms_status = Status.UNAVAILABLE
+    except RequestException:
+        logger.critical(UnavailabilityMessage.LMS)
+        lms_status = Status.UNAVAILABLE
 
-    overall_status = OK if (database_status == lms_status == OK) else UNAVAILABLE
+    overall_status = Status.OK if (database_status == lms_status == Status.OK) else Status.UNAVAILABLE
 
     data = {
         'overall_status': overall_status,
@@ -63,7 +69,7 @@ def health(_):
         },
     }
 
-    if overall_status == OK:
+    if overall_status == Status.OK:
         return JsonResponse(data)
     else:
         return JsonResponse(data, status=status.HTTP_503_SERVICE_UNAVAILABLE)