From 4f85b5fb6a93a70c3258e5b5541951f8cdc83607 Mon Sep 17 00:00:00 2001 From: Shantanu Dhiman Date: Thu, 23 Mar 2023 19:44:23 +0530 Subject: [PATCH] [TDL-22316] : Handle ChunkEncodingError & JSONDecodeError (#36) * Changes: 1) Added logic to handle ChunkedEncodingError and JSONDecodeError. 2) Added necessary unit test case. * Changes: 1) Improved exception throwing and handling logic. 2) Added backoff for ConnectionResetError. 3) Added unit test case for ConnectionResetError. --- tap_activecampaign/client.py | 8 ++++- tests/unittests/test_error_handling.py | 42 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/tap_activecampaign/client.py b/tap_activecampaign/client.py index 5cd1687..b5cf274 100644 --- a/tap_activecampaign/client.py +++ b/tap_activecampaign/client.py @@ -87,6 +87,8 @@ def should_retry_error(exception): # Tap raises Exception: ConnectionResetError(104, 'Connection reset by peer'). That's why we are retrying this error also. # Reference: https://app.circleci.com/pipelines/github/singer-io/tap-activecampaign/554/workflows/d448258e-20df-4e66-b2aa-bc8bd1f08912/jobs/558 return True + elif type(exception) == ConnectionResetError: + return True else: return False @@ -235,7 +237,11 @@ def request(self, method, path=None, url=None, api_version=None, **kwargs): except Exception as err: LOGGER.error('{}'.format(err)) LOGGER.error('response content: {}'.format(response.content)) - raise Exception(err) + if response.content != b"": + raise err + + # Handling empty response b'' given by ActiveCampaign APIs + response_json = {} return response_json diff --git a/tests/unittests/test_error_handling.py b/tests/unittests/test_error_handling.py index 59a8be3..c582dae 100644 --- a/tests/unittests/test_error_handling.py +++ b/tests/unittests/test_error_handling.py @@ -187,6 +187,21 @@ def test_request_with_handling_for_500_exception_handling(self, mocked_request, self.assertEqual(str(e.exception), expected_error_message) + @patch("time.sleep") + @patch("tap_activecampaign.client.ActiveCampaignClient.check_api_token") + @patch("requests.Session.request", return_value=Mockresponse("", 200, content=b"")) + def test_request_with_handling_for_empty_content(self, mocked_request, mock_api_token, mock_sleep): + """ + Test that `request` method gives empty json `{}` response when content is empty for a 200 response. + """ + _client = client.ActiveCampaignClient('dummy_url', 'dummy_token') + + response = _client.request("base_url") + + # Verifying the empty response + self.assertEqual({}, response) + + class TestActiveCampaignErrorhandlingForCheckApiTokenMethod(unittest.TestCase): @patch("requests.Session.get", side_effect=mock_send_400) @@ -398,6 +413,19 @@ def test_enter_method_handle_connection_reset_exception(self, mocked_request, mo # Verify that requests.Session.get called 5 times self.assertEqual(mocked_request.call_count, 5) + @patch("requests.Session.get", side_effect=ConnectionResetError(104, 'Connection reset by peer')) + def test_enter_method_handle_connection_reset_error(self, mocked_request, mock_sleep): + """ + Test that `__enter__` method retry ConnectionResetError(104) with Exception 5 times. + """ + _client = client.ActiveCampaignClient('dummy_url', 'dummy_token') + + with self.assertRaises(ConnectionResetError) as e: + _client.__enter__() + + # Verify that requests.Session.get called 5 times + self.assertEqual(mocked_request.call_count, 5) + @patch("tap_activecampaign.client.ActiveCampaignClient.check_api_token") @patch("requests.Session.request", side_effect=mock_send_429) def test_request_method_handle_429_exception(self, mocked_request, mock_api_token, mock_sleep): @@ -495,3 +523,17 @@ def test_request_method_handle_connection_reset_exception(self, mocked_request, # Verify that requests.Session.request called 5 times self.assertEqual(mocked_request.call_count, 5) + + @patch("tap_activecampaign.client.ActiveCampaignClient.check_api_token") + @patch("requests.Session.request", side_effect=ConnectionResetError(104, 'Connection reset by peer')) + def test_request_method_handle_connection_reset_error(self, mocked_request, mock_api_token, mock_sleep): + """ + Test that `request` method retry ConnectionResetError(104) with Exception 5 times. + """ + _client = client.ActiveCampaignClient('dummy_url', 'dummy_token') + + with self.assertRaises(ConnectionResetError) as e: + _client.request("base_url") + + # Verify that requests.Session.request called 5 times + self.assertEqual(mocked_request.call_count, 5)