Merge pull request #124 from globusonline/develop

Version v0.3.16

CHANGELOG.md

@@ -4,6 +4,11 @@
 Below are major changes for each version Release. For detailed information,
 see the list of commits from the last version or use `git log`.
 
+## 0.3.16 - 2020-10-20
+
+- Added broad exception handling on facet_modifiers
+    - Catch all exceptions except import exceptions (developer errors)
+
 ## 0.3.15 - 2020-10-09
 
 - Added 'facet_modifiers' field to index config

globus_portal_framework/gsearch.py

@@ -845,9 +845,18 @@ def get_facets(search_result, portal_defined_facets, filters,
             else:
                 bucket['checked'] = bucket['value'] in active_filter_vals
                 bucket['datetime'] = None
-    # Apply user modifications to all finished facets
+    # Apply user modifications to all finished facets. Catch ALL facet mod
+    # exceptions. Typically, this happens due to an edge case in the modifier,
+    # and should not result in the search page failing to load.
     facet_modifiers = (facet_modifiers if facet_modifiers is not None
                        else DEFAULT_FACET_MODIFIERS)
     for fmodder in facet_modifiers:
-        facets = import_string(fmodder)(facets)
+        try:
+            facets = import_string(fmodder)(facets)
+        except ModuleNotFoundError:
+            # Don't catch these. Developer error.
+            raise
+        except Exception as e:
+            log.exception(e)
+            log.error('Facet modifier raised exception {}'.format(fmodder))
     return facets

globus_portal_framework/modifiers/facets.py

@@ -22,7 +22,7 @@ def sort_terms(facets):
     """Sort terms lexicographically instead of by the number of results
     returned. This will only sort 'terms' type facets."""
     for facet in facets:
-        if facet['type'] == 'terms':
+        if facet['type'] == 'terms' and facet.get('buckets'):
             facet['buckets'].sort(key=lambda x: x['value'])
     return facets
 
@@ -32,7 +32,7 @@ def sort_terms_numerically(facets):
     applies to 'terms' type facets, and facets that contain only numbers
     in their buckets."""
     for facet in facets:
-        if facet['type'] == 'terms':
+        if facet['type'] == 'terms' and facet.get('buckets'):
             try:
                 facet['buckets'].sort(key=lambda x: float(x['value']))
             except ValueError:

globus_portal_framework/tests/test_gsearch.py

@@ -14,6 +14,7 @@
     prepare_search_facets, serialize_gsearch_range, deserialize_gsearch_range,
     get_facet_filter_type,
 )
+import globus_portal_framework.modifiers.facets
 from globus_portal_framework.exc import (
     IndexNotFound, GlobusPortalException, InvalidRangeFilter,
 )
@@ -138,7 +139,7 @@ class TestGSearch(TestCase):
     def setUp(self):
         self.mock_gs_facets = deepcopy(MOCK_GS_FACETS)
         self.factory = RequestFactory()
-        self.MOCK_FACET_MODIFIER = mock.Mock()
+        self.mock_facet_modifier = mock.Mock()
 
     @override_settings(SEARCH_INDEXES={'foo': {}})
     def test_get_index(self):
@@ -384,10 +385,9 @@ def test_get_facet_with_modifiers(self):
         search_response = MockGlobusResponse()
         search_response.data = self.mock_gs_facets
         mock_mod = mock.Mock()
-        import globus_portal_framework.modifiers.facets
-        setattr(globus_portal_framework.modifiers.facets, 'mock_mod', mock_mod)
-
-        mock_mods = ['globus_portal_framework.modifiers.facets.mock_mod']
+        setattr(globus_portal_framework.tests.test_gsearch, 'mock_mod',
+                mock_mod)
+        mock_mods = ['globus_portal_framework.tests.test_gsearch.mock_mod']
         get_facets(search_response, MOCK_PORTAL_DEFINED_FACETS, [],
                    filter_match=None, facet_modifiers=mock_mods)
         self.assertTrue(mock_mod.called)
@@ -410,6 +410,24 @@ def test_default_get_facet_with_no_modifiers(self, default_modifier):
                    filter_match=None, facet_modifiers=[])
         self.assertFalse(default_modifier.called)
 
+    def test_facet_modifiers_raise_import_errors(self):
+        search_response = MockGlobusResponse()
+        search_response.data = self.mock_gs_facets
+
+        with self.assertRaises(ImportError):
+            get_facets(search_response, MOCK_PORTAL_DEFINED_FACETS, [],
+                       filter_match=None, facet_modifiers=['does.not.exist'])
+
+    def test_facet_modifiers_do_not_raise_non_import_exceptions(self):
+        search_response = MockGlobusResponse()
+        search_response.data = self.mock_gs_facets
+        exc_mod = mock.Mock(side_effect=Exception())
+        setattr(globus_portal_framework.tests.test_gsearch, 'exc_mod', exc_mod)
+        mock_mods = ['globus_portal_framework.tests.test_gsearch.exc_mod']
+        get_facets(search_response, MOCK_PORTAL_DEFINED_FACETS, [],
+                   filter_match=None, facet_modifiers=mock_mods)
+        self.asserTrue(exc_mod.called)
+
     def test_get_invalid_search_range_raises_error(self):
         with self.assertRaises(GlobusPortalException):
             get_date_range_for_date('2018-02-02', 'fortnight')

globus_portal_framework/version.py

@@ -1 +1 @@
-__version__ = '0.3.15'
+__version__ = '0.3.16'