From 770bff3271b6f66d2e6ebe5bce5ccb3f9a2eb142 Mon Sep 17 00:00:00 2001 From: Sourabh Gandhi Date: Wed, 13 Sep 2023 04:27:31 +0000 Subject: [PATCH 1/4] add retry logic for status code - 503 --- tap_facebook/__init__.py | 2 +- tests/unittests/test_retry_logic.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/tap_facebook/__init__.py b/tap_facebook/__init__.py index 8eacc60b..53cd143a 100755 --- a/tap_facebook/__init__.py +++ b/tap_facebook/__init__.py @@ -154,7 +154,7 @@ def should_retry_api_error(exception): elif isinstance(exception, FacebookRequestError): return (exception.api_transient_error() or exception.api_error_subcode() == 99 - or exception.http_status() == 500 + or exception.http_status() in (500, 503) # This subcode corresponds to a race condition between AdsInsights job creation and polling or exception.api_error_subcode() == 33 ) diff --git a/tests/unittests/test_retry_logic.py b/tests/unittests/test_retry_logic.py index 5daefba0..710f41b5 100644 --- a/tests/unittests/test_retry_logic.py +++ b/tests/unittests/test_retry_logic.py @@ -42,6 +42,34 @@ def test_retries_on_500(self): # 5 is the max tries specified in the tap self.assertEquals(5, mocked_account.get_ad_creatives.call_count ) + def test_retries_on_503(self): + """`AdCreative.sync.do_request()` calls a `facebook_business` method, + `get_ad_creatives()`, to make a request to the API. We mock this + method to raise a `FacebookRequestError` with an `http_status` of + `503`. + + We expect the tap to retry this request up to 5 times, which is + the current hard coded `max_tries` value. + """ + + # Create the mock and force the function to throw an error + mocked_account = Mock() + mocked_account.get_ad_creatives = Mock() + mocked_account.get_ad_creatives.side_effect = FacebookRequestError( + message='', + request_context={"":Mock()}, + http_status=503, + http_headers=Mock(), + body={} + ) + + # Initialize the object and call `sync()` + ad_creative_object = AdCreative('', mocked_account, '', '') + with self.assertRaises(FacebookRequestError): + ad_creative_object.sync() + # 5 is the max tries specified in the tap + self.assertEquals(5, mocked_account.get_ad_creatives.call_count ) + def test_catch_a_type_error(self): """`AdCreative.sync.do_request()` calls a `facebook_business` method `get_ad_creatives()`. We want to mock this to throw a `TypeError("string indices must be integers")` and assert From cca1e390f9551e7a4456ad20d6142d07750ae1eb Mon Sep 17 00:00:00 2001 From: Sourabh Gandhi Date: Wed, 13 Sep 2023 06:22:03 +0000 Subject: [PATCH 2/4] handle the attribute error by checking the error body instance is dictionary --- tap_facebook/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tap_facebook/__init__.py b/tap_facebook/__init__.py index 53cd143a..1a9a4a69 100755 --- a/tap_facebook/__init__.py +++ b/tap_facebook/__init__.py @@ -130,7 +130,7 @@ def raise_from(singer_error, fb_error): error_message = '{}: {} Message: {}'.format( http_method, fb_error.http_status(), - fb_error.body().get('error', {}).get('message') + fb_error.body().get('error', {}).get('message') if isinstance(fb_error.body(), dict) else str(fb_error.body()) ) else: # All other facebook errors are `FacebookError`s and we handle From 72200c03d075243a83c0f3fc098aa65fcdeb33a7 Mon Sep 17 00:00:00 2001 From: Sourabh Gandhi Date: Wed, 13 Sep 2023 06:31:00 +0000 Subject: [PATCH 3/4] Update error body in test case and indent the code --- tap_facebook/__init__.py | 3 ++- tests/unittests/test_retry_logic.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tap_facebook/__init__.py b/tap_facebook/__init__.py index 1a9a4a69..6fbcebd9 100755 --- a/tap_facebook/__init__.py +++ b/tap_facebook/__init__.py @@ -130,7 +130,8 @@ def raise_from(singer_error, fb_error): error_message = '{}: {} Message: {}'.format( http_method, fb_error.http_status(), - fb_error.body().get('error', {}).get('message') if isinstance(fb_error.body(), dict) else str(fb_error.body()) + fb_error.body().get('error', {}).get('message') + if isinstance(fb_error.body(), dict) else str(fb_error.body()) ) else: # All other facebook errors are `FacebookError`s and we handle diff --git a/tests/unittests/test_retry_logic.py b/tests/unittests/test_retry_logic.py index 710f41b5..56758ece 100644 --- a/tests/unittests/test_retry_logic.py +++ b/tests/unittests/test_retry_logic.py @@ -60,7 +60,7 @@ def test_retries_on_503(self): request_context={"":Mock()}, http_status=503, http_headers=Mock(), - body={} + body="Service Uavailable" ) # Initialize the object and call `sync()` From f6a2cfa06135dfbda32842acb593fe88f42351fd Mon Sep 17 00:00:00 2001 From: Sourabh Gandhi Date: Wed, 13 Sep 2023 06:42:32 +0000 Subject: [PATCH 4/4] setup and changelog update --- CHANGELOG.md | 3 +++ setup.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 232cd238..cd276b95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 1.19.1 + * Add retry logic for status code - 503 [#226](https://github.com/singer-io/tap-facebook/pull/226) + ## 1.19.0 * Add conversions to insights streams [#204](https://github.com/singer-io/tap-facebook/pull/204) diff --git a/setup.py b/setup.py index 5ef109c0..1197dba7 100755 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ from setuptools import setup setup(name='tap-facebook', - version='1.19.0', + version='1.19.1', description='Singer.io tap for extracting data from the Facebook Ads API', author='Stitch', url='https://singer.io',