fix(exception-handling): Catch errors when requesting datafile (#285)

Author: aliabbasrizvi

optimizely/config_manager.py

@@ -325,7 +325,7 @@ def _handle_response(self, response):
         """
         try:
             response.raise_for_status()
-        except requests_exceptions.HTTPError as err:
+        except requests_exceptions.RequestException as err:
             self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err)))
             return
 

tests/test_config_manager.py

@@ -392,6 +392,47 @@ def test_fetch_datafile(self, _):
         )
         self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified)
         self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig)
+        self.assertTrue(project_config_manager.is_running)
+
+    def test_fetch_datafile__exception_raised(self, _):
+        """ Test that config_manager keeps running if exception is raised when fetching datafile. """
+        class MockExceptionResponse(object):
+            def raise_for_status(self):
+                raise requests.exceptions.RequestException('Error Error !!')
+
+        sdk_key = 'some_key'
+        mock_logger = mock.Mock()
+        with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile'):
+            project_config_manager = config_manager.PollingConfigManager(sdk_key=sdk_key, logger=mock_logger)
+        expected_datafile_url = enums.ConfigManager.DATAFILE_URL_TEMPLATE.format(sdk_key=sdk_key)
+        test_headers = {'Last-Modified': 'New Time'}
+        test_datafile = json.dumps(self.config_dict_with_features)
+        test_response = requests.Response()
+        test_response.status_code = 200
+        test_response.headers = test_headers
+        test_response._content = test_datafile
+        with mock.patch('requests.get', return_value=test_response):
+            project_config_manager.fetch_datafile()
+
+        self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified)
+        self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig)
+
+        # Call fetch_datafile again, but raise exception this time
+        with mock.patch('requests.get', return_value=MockExceptionResponse()) as mock_requests:
+            project_config_manager.fetch_datafile()
+
+        mock_requests.assert_called_once_with(
+            expected_datafile_url,
+            headers={'If-Modified-Since': test_headers['Last-Modified']},
+            timeout=enums.ConfigManager.REQUEST_TIMEOUT,
+        )
+        mock_logger.error.assert_called_once_with('Fetching datafile from {} failed. Error: Error Error !!'.format(
+            expected_datafile_url
+        ))
+        self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified)
+        self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig)
+        # Confirm that config manager keeps running
+        self.assertTrue(project_config_manager.is_running)
 
     def test_is_running(self, _):
         """ Test that polling thread is running after instance of PollingConfigManager is created. """