Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source Facebook Marketing: Retry pattern not working for unknown errors #25383

Closed
NahidOulmi opened this issue Apr 21, 2023 · 0 comments · Fixed by #27979
Closed

Source Facebook Marketing: Retry pattern not working for unknown errors #25383

NahidOulmi opened this issue Apr 21, 2023 · 0 comments · Fixed by #27979

Comments

@NahidOulmi
Copy link
Contributor

Environment

  • OS Version / Instance: Debian
  • Memory / Disk: 32GB memory / 50GB disk
  • Deployment: docker-compose
  • Airbyte Version: 0.40.26
  • Source name/version: Facebook Marketing Source connector
  • Destination name/version: bigquery

Current Behavior

The reduce_request_record_limit function that is in the streams.common.py package is not working as expected.

When the connector requests too much data and the call failed, this function should divide the limit param by 2 and try again with the decreased parameter.

The retry pattern has this condition in common.py :
exc.api_error_message() == "Please reduce the amount of data you're asking for, then retry your request" for dividing the limit by 2.

However, sometimes the API call returns the following error : "An unknown error occurred" and I noticed that decreasing the limit also fixed the API error in that case. So I added the case in the reduce_request_record_limit function :

exc.api_error_message() == "Please reduce the amount of data you're asking for, then retry your request" or exc.api_error_message() == "An unknown error occurred"

But if I do that, the limit parameter gets decreased for all next API calls. And I want it to reset the limit to the default value (usually 100) whenever the API call worked and not stay at the value it was set during the retry process because the next calls will take far too long with too small size.

For some reason, the limit parameter is rightly reset in the condition exc.api_error_message() == "Please reduce the amount of data you're asking for, then retry your request"

but not if in the condition exc.api_error_message() == "An unknown error occurred"

Expected Behavior

The reduce_request_record_limit function should also decrease the limit for "An unknown error occurred" errors and reset it to the default limit after we go out of the retry process.

Logs

{"type": "LOG", "log": {"level": "INFO", "message": "Starting syncing SourceFacebookMarketing"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: activities "}}
{"type": "LOG", "log": {"level": "INFO", "message": "Limit parameter is set at 100"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Backing off call(...) for 5.0s (facebook_business.exceptions.FacebookRequestError: \n\n  Message: Call was not successful\n  Method:  GET\n  Path:    ```
https://graph.facebook.com/v15.0/act_xxxx/activities?access_token=****&fields=actor_id%2Cactor_name%2Capplication_id%2Capplication_name%2Cdate_time_in_timezone%2Cevent_time%2Cevent_type%2Cextra_data%2Cobject_id%2Cobject_name%2Cobject_type%2Ctranslated_event_type&summary=true&since=1677628800&after=cHVtYV9hbmNob3I6TVRZAM09EZA3hNRGt6TnpRNE5TTTBOV1UxWlRreE1UVTRaV1V5TW1SbU5XTmlNelEwTURBNE5XUTFOelZArTW1WbE16azBNVFpqTXpjME9XRmtOak00WTJSaU1UZA3pObUpqTURNMk5UQTQjNjYZD&limit=100\n  Params:  {'summary': 'true'}\n\n  Status:  500\n  Response:\n    {\n      \"error\": {\n        \"code\": 1,\n        \"message\": \"An unknown error occurred\",\n        \"error_subcode\": 99\n      }\n    })"}}
{"type": "LOG", "log": {"level": "INFO", "message": "\n\n  Message: Call was not successful\n  Method:  GET\n  Path:    https://graph.facebook.com/v15.0/act_892520001410300/activities?access_token=****&fields=actor_id%2Cactor_name%2Capplication_id%2Capplication_name%2Cdate_time_in_timezone%2Cevent_time%2Cevent_type%2Cextra_data%2Cobject_id%2Cobject_name%2Cobject_type%2Ctranslated_event_type&summary=true&since=1677628800&after=cHVtYV9hbmNob3I6TVRZAM09EZA3hNRGt6TnpRNE5TTTBOV1UxWlRreE1UVTRaV1V5TW1SbU5XTmlNelEwTURBNE5XUTFOelZArTW1WbE16azBNVFpqTXpjME9XRmtOak00WTJSaU1UZA3pObUpqTURNMk5UQTQjNjYZD&limit=100\n  Params:  {'summary': 'true'}\n\n  Status:  500\n  Response:\n    {\n      \"error\": {\n        \"code\": 1,\n        \"message\": \"An unknown error occurred\",\n        \"error_subcode\": 99\n      }\n    }\n"}}

{"type": "LOG", "log": {"level": "INFO", "message": "API call succeeded"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Limit parameter is set at 50"}} # Should be reset at 100 here after it succeeded

Steps to Reproduce

  1. Add a exc.api_error_message() == "An unknown error occurred" condition in the reduce_request_record_limit function or create a Test case
  2. Observe that the limit parameter never gets reset back at its default value afterwards

Are you willing to submit a PR?

I hardcoded the reset in the load_next_page function in the patches.py file :

self.params["limit"] = 100

but it will not be accepted in a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
4 participants