From 61aa119363c67ff492e6f30087da3b6188c51e24 Mon Sep 17 00:00:00 2001 From: Denys Davydov Date: Wed, 11 Oct 2023 19:52:47 +0300 Subject: [PATCH] :bug: Source Facebook Marketing: do not request funding_source_details if api raises an error (#31284) Co-authored-by: davydov-d --- .../source-facebook-marketing/Dockerfile | 2 +- .../source-facebook-marketing/metadata.yaml | 2 +- .../streams/streams.py | 8 +++- .../unit_tests/test_errors.py | 42 ++++++++++++------- .../sources/facebook-marketing.md | 1 + 5 files changed, 38 insertions(+), 17 deletions(-) diff --git a/airbyte-integrations/connectors/source-facebook-marketing/Dockerfile b/airbyte-integrations/connectors/source-facebook-marketing/Dockerfile index 05e0e6d5f780..f1918a7ae667 100644 --- a/airbyte-integrations/connectors/source-facebook-marketing/Dockerfile +++ b/airbyte-integrations/connectors/source-facebook-marketing/Dockerfile @@ -14,5 +14,5 @@ ENV AIRBYTE_ENTRYPOINT "python /airbyte/integration_code/main.py" ENTRYPOINT ["python", "/airbyte/integration_code/main.py"] -LABEL io.airbyte.version=1.1.15 +LABEL io.airbyte.version=1.1.16 LABEL io.airbyte.name=airbyte/source-facebook-marketing diff --git a/airbyte-integrations/connectors/source-facebook-marketing/metadata.yaml b/airbyte-integrations/connectors/source-facebook-marketing/metadata.yaml index 6d46b5d1996c..20a02664bc26 100644 --- a/airbyte-integrations/connectors/source-facebook-marketing/metadata.yaml +++ b/airbyte-integrations/connectors/source-facebook-marketing/metadata.yaml @@ -5,7 +5,7 @@ data: connectorSubtype: api connectorType: source definitionId: e7778cfc-e97c-4458-9ecb-b4f2bba8946c - dockerImageTag: 1.1.15 + dockerImageTag: 1.1.16 dockerRepository: airbyte/source-facebook-marketing githubIssueLabel: source-facebook-marketing icon: facebook.svg diff --git a/airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/streams/streams.py b/airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/streams/streams.py index f77c28586032..23fd4b565bdf 100644 --- a/airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/streams/streams.py +++ b/airbyte-integrations/connectors/source-facebook-marketing/source_facebook_marketing/streams/streams.py @@ -201,11 +201,17 @@ def list_objects(self, params: Mapping[str, Any]) -> Iterable: try: return [FBAdAccount(self._api.account.get_id()).api_get(fields=fields)] except FacebookRequestError as e: - # This is a workaround for cases when account seem have all the required permissions + # This is a workaround for cases when account seem to have all the required permissions # but despite of that is not allowed to get `owner` field. See (https://github.com/airbytehq/oncall/issues/3167) if e.api_error_code() == 200 and e.api_error_message() == "(#200) Requires business_management permission to manage the object": fields.remove("owner") return [FBAdAccount(self._api.account.get_id()).api_get(fields=fields)] + # FB api returns a non-obvious error when accessing the `funding_source_details` field + # even though user is granted all the required permissions (`MANAGE`) + # https://github.com/airbytehq/oncall/issues/3031 + if e.api_error_code() == 100 and e.api_error_message() == "Unsupported request - method type: get": + fields.remove("funding_source_details") + return [FBAdAccount(self._api.account.get_id()).api_get(fields=fields)] raise e diff --git a/airbyte-integrations/connectors/source-facebook-marketing/unit_tests/test_errors.py b/airbyte-integrations/connectors/source-facebook-marketing/unit_tests/test_errors.py index ef151f109316..372ca7c5cdd2 100644 --- a/airbyte-integrations/connectors/source-facebook-marketing/unit_tests/test_errors.py +++ b/airbyte-integrations/connectors/source-facebook-marketing/unit_tests/test_errors.py @@ -212,6 +212,8 @@ # # # https://developers.facebook.com/community/threads/1232870724022634/ # # I observed that if I remove preview_shareable_link field from the request, the code is working properly. + # # Update (Denys Davydov): same for me, but removing the `funding_source_details` field helps, so + # # we do remove it and do not raise errors; this is tested by a different unit test - see `test_adaccount_list_objects_retry`. # # ), ] @@ -375,7 +377,30 @@ def test_config_error_insights_during_actual_nodes_read(self, requests_mock, nam assert error.failure_type == FailureType.config_error assert friendly_msg in error.message - def test_adaccount_list_objects_retry(self, requests_mock): + @pytest.mark.parametrize( + "failure_response", + ( + { + "status_code": 403, + "json": { + "message": "(#200) Requires business_management permission to manage the object", + "type": "OAuthException", + "code": 200, + "fbtrace_id": "AOm48i-YaiRlzqnNEnECcW8", + }, + }, + { + "status_code": 400, + "json": { + "message": "Unsupported request - method type: get", + "type": "GraphMethodException", + "code": 100, + "fbtrace_id": "AOm48i-YaiRlzqnNEnECcW8", + }, + }, + ), + ) + def test_adaccount_list_objects_retry(self, requests_mock, failure_response): """ Sometimes we get an error: "Requires business_management permission to manage the object" when account has all the required permissions: [ @@ -395,19 +420,8 @@ def test_adaccount_list_objects_retry(self, requests_mock): assigend_users = {"account_id": account_id, "tasks": ["TASK"]} requests_mock.register_uri("GET", f"{act_url}assigned_users", status_code=200, json=assigend_users) - responses = [ - { - "status_code": 403, - "json": { - "message": "(#200) Requires business_management permission to manage the object", - "type": "OAuthException", - "code": 200, - "fbtrace_id": "AOm48i-YaiRlzqnNEnECcW8", - }, - }, - {"status_code": 200, "json": {"account_id": account_id}}, - ] - requests_mock.register_uri("GET", f"{act_url}", responses) + success_response = {"status_code": 200, "json": {"account_id": account_id}} + requests_mock.register_uri("GET", f"{act_url}", [failure_response, success_response]) record_gen = stream.read_records(sync_mode=SyncMode.full_refresh, stream_slice=None, stream_state={}) assert list(record_gen) == [{"account_id": "unknown_account", "id": "act_unknown_account"}] diff --git a/docs/integrations/sources/facebook-marketing.md b/docs/integrations/sources/facebook-marketing.md index 50d41cf459a3..818b8996f07d 100644 --- a/docs/integrations/sources/facebook-marketing.md +++ b/docs/integrations/sources/facebook-marketing.md @@ -178,6 +178,7 @@ The Facebook Marketing connector uses the `lookback_window` parameter to repeate | Version | Date | Pull Request | Subject | |:--------|:-----------|:---------------------------------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| 1.1.16 | 2023-10-11 | [31284](https://github.com/airbytehq/airbyte/pull/31284) | Fix error occurring when trying to access the `funding_source_details` field of the `AdAccount` stream | | 1.1.15 | 2023-10-06 | [31132](https://github.com/airbytehq/airbyte/pull/31132) | Fix permission error for `AdAccount` stream | | 1.1.14 | 2023-09-26 | [30758](https://github.com/airbytehq/airbyte/pull/30758) | Exception should not be raises if a stream is not found | | 1.1.13 | 2023-09-22 | [30706](https://github.com/airbytehq/airbyte/pull/30706) | Performance testing - include socat binary in docker image |