Merge pull request #9752 from edx/peter-fogg/handle-elasticsearch-errors

Add error handling for elastic search.

lms/djangoapps/teams/errors.py

@@ -16,6 +16,11 @@ class AlreadyOnTeamInCourse(TeamAPIRequestError):
     pass
 
 
+class ElasticSearchConnectionError(TeamAPIRequestError):
+    """The system was unable to connect to the configured elasticsearch instance."""
+    pass
+
+
 class ImmutableMembershipFieldException(Exception):
-    """An attempt was made to change an immutable field on a CourseTeamMembership model"""
+    """An attempt was made to change an immutable field on a CourseTeamMembership model."""
     pass

lms/djangoapps/teams/search_indexes.py

 """ Search index used to load data into elasticsearch"""
 
+import logging
+from elasticsearch.exceptions import ConnectionError
+
 from django.conf import settings
 from django.db.models.signals import post_delete, post_save
 from django.dispatch import receiver
@@ -8,6 +11,7 @@ from functools import wraps
 
 from search.search_engine_base import SearchEngine
 
+from .errors import ElasticSearchConnectionError
 from .serializers import CourseTeamSerializer, CourseTeam
 
 
@@ -103,8 +107,11 @@ class CourseTeamIndexer(object):
         """
         Return course team search engine (if feature is enabled).
         """
-        if cls.search_is_enabled():
+        try:
             return SearchEngine.get_search_engine(index=cls.INDEX_NAME)
+        except ConnectionError as err:
+            logging.error('Error connecting to elasticsearch: %s', err)
+            raise ElasticSearchConnectionError
 
     @classmethod
     def search_is_enabled(cls):
@@ -119,7 +126,10 @@ def course_team_post_save_callback(**kwargs):
     """
     Reindex object after save.
     """
-    CourseTeamIndexer.index(kwargs['instance'])
+    try:
+        CourseTeamIndexer.index(kwargs['instance'])
+    except ElasticSearchConnectionError:
+        pass
 
 
 @receiver(post_delete, sender=CourseTeam, dispatch_uid='teams.signals.course_team_post_delete_callback')
@@ -127,4 +137,7 @@ def course_team_post_delete_callback(**kwargs):  # pylint: disable=invalid-name
     """
     Reindex object after delete.
     """
-    CourseTeamIndexer.remove(kwargs['instance'])
+    try:
+        CourseTeamIndexer.remove(kwargs['instance'])
+    except ElasticSearchConnectionError:
+        pass

lms/djangoapps/teams/tests/test_views.py

@@ -5,6 +5,9 @@ import pytz
 from datetime import datetime
 from dateutil import parser
 import ddt
+from elasticsearch.exceptions import ConnectionError
+from mock import patch
+from search.search_engine_base import SearchEngine
 
 from django.core.urlresolvers import reverse
 from django.conf import settings
@@ -1397,3 +1400,49 @@ class TestDeleteMembershipAPI(EventTestMixin, TeamAPITestCase):
 
     def test_missing_membership(self):
         self.delete_membership(self.wind_team.team_id, self.users['student_enrolled'].username, 404)
+
+
+class TestElasticSearchErrors(TeamAPITestCase):
+    """Test that the Team API is robust to Elasticsearch connection errors."""
+
+    ES_ERROR = ConnectionError('N/A', 'connection error', {})
+
+    @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR)
+    def test_list_teams(self, __):
+        """Test that text searches return a 503 when Elasticsearch is down.
+
+        The endpoint should still return 200 when a search is not supplied."""
+        self.get_teams_list(
+            expected_status=503,
+            data={'course_id': self.test_course_1.id, 'text_search': 'zoinks'},
+            user='staff'
+        )
+        self.get_teams_list(
+            expected_status=200,
+            data={'course_id': self.test_course_1.id},
+            user='staff'
+        )
+
+    @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR)
+    def test_create_team(self, __):
+        """Test that team creation is robust to Elasticsearch errors."""
+        self.post_create_team(
+            expected_status=200,
+            data=self.build_team_data(name='zoinks'),
+            user='staff'
+        )
+
+    @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR)
+    def test_delete_team(self, __):
+        """Test that team deletion is robust to Elasticsearch errors."""
+        self.delete_team(self.wind_team.team_id, 204, user='staff')
+
+    @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR)
+    def test_patch_team(self, __):
+        """Test that team updates are robust to Elasticsearch errors."""
+        self.patch_team_detail(
+            self.wind_team.team_id,
+            200,
+            data={'description': 'new description'},
+            user='staff'
+        )

lms/djangoapps/teams/views.py

@@ -58,7 +58,7 @@ from .serializers import (
     add_team_count
 )
 from .search_indexes import CourseTeamIndexer
-from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam
+from .errors import AlreadyOnTeamInCourse, ElasticSearchConnectionError, NotEnrolledInCourseForTeam
 
 TEAM_MEMBERSHIPS_PER_PAGE = 2
 TOPICS_PER_PAGE = 12
@@ -293,6 +293,9 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
             example, the course_id may not reference a real course or the page
             number may be beyond the last page.
 
+            If the server is unable to connect to Elasticsearch, and
+            the text_search parameter is supplied, a 503 error is returned.
+
         **Response Values for POST**
 
             Any logged in user who has verified their email address can create
@@ -366,7 +369,14 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
                 return Response(error, status=status.HTTP_400_BAD_REQUEST)
             result_filter.update({'topic_id': topic_id})
         if text_search and CourseTeamIndexer.search_is_enabled():
-            search_engine = CourseTeamIndexer.engine()
+            try:
+                search_engine = CourseTeamIndexer.engine()
+            except ElasticSearchConnectionError:
+                return Response(
+                    build_api_error(ugettext_noop('Error connecting to elasticsearch')),
+                    status=status.HTTP_503_SERVICE_UNAVAILABLE
+                )
+
             result_filter.update({'course_id': course_id_string})
 
             search_results = search_engine.search(