From afcef3287f7af2977dbb5cc9d6b11fc2ff38b30b Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Fri, 10 Jul 2020 09:54:36 -0700 Subject: [PATCH 1/2] fix(exception-handling): Catch errors when requesting datafile --- optimizely/config_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 8761fb38..46a7dac3 100644 --- a/optimizely/config_manager.py +++ b/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 From b2ef8f16753adc636ba9c7e39fbd89ecef191bfd Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Fri, 10 Jul 2020 10:37:29 -0700 Subject: [PATCH 2/2] Adding unit test --- tests/test_config_manager.py | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 9381b431..42f5f76d 100644 --- a/tests/test_config_manager.py +++ b/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. """