fix(google_ads): retry transient timeouts listing accessible accounts

posthog/models/integration.py

@@ -40,6 +40,7 @@
 from rest_framework.request import Request
 from slack_sdk import WebClient
 from slack_sdk.errors import SlackApiError
+from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed
 
 from posthog.cache_utils import cache_for
 from posthog.egress.github.transport import github_request
@@ -1931,6 +1932,21 @@ def google_ads_hierarchy_level(account: dict) -> int:
     return int(account.get("level") or 0)
 
 
+@retry(
+    stop=stop_after_attempt(3),
+    wait=wait_fixed(2),
+    retry=retry_if_exception_type((requests.exceptions.Timeout, requests.exceptions.ConnectionError)),
+    reraise=True,
+)
+def _google_ads_request(method: str, url: str, **kwargs) -> requests.Response:
+    """`requests.request` with retries for a transient timeout/connection blip.
+
+    `list_google_ads_accessible_accounts` walks the account hierarchy with a chain of sequential
+    requests, so a single transient blip on any one of them would otherwise fail the whole walk.
+    """
+    return requests.request(method, url, **kwargs)
+
+
 class GoogleAdsIntegration:
     integration: Integration
 
@@ -1990,7 +2006,7 @@ def list_google_ads_conversion_actions(self, customer_id, parent_id=None) -> lis
     # Google Ads manager accounts can have access to other accounts (including other manager accounts).
     # Filter out duplicates where a user has direct access and access through a manager account, while prioritizing direct access.
     def list_google_ads_accessible_accounts(self) -> list[dict[str, Any]]:
-        response = requests.request(
+        response = _google_ads_request(
             "GET",
             "https://googleads.googleapis.com/v24/customers:listAccessibleCustomers",
             headers={
@@ -2030,7 +2046,7 @@ def list_google_ads_accessible_accounts(self) -> list[dict[str, Any]]:
         def dfs(account_id, accounts=None, parent_id=None) -> list[dict]:
             if accounts is None:
                 accounts = []
-            response = requests.request(
+            response = _google_ads_request(
                 "POST",
                 f"https://googleads.googleapis.com/v24/customers/{account_id}/googleAds:searchStream",
                 json={

posthog/models/test/test_integration_model.py

@@ -3335,6 +3335,35 @@ def test_accessible_accounts_empty_when_login_has_no_accessible_customers(self,
 
         assert accounts == []
 
+    @override_settings(GOOGLE_ADS_DEVELOPER_TOKEN="dev_token")
+    @patch("tenacity.nap.time.sleep")
+    @patch("posthog.models.integration.requests.request")
+    def test_accessible_accounts_rides_out_one_transient_read_timeout(self, mock_request, mock_sleep):
+        # The walk is a chain of sequential requests; a single timed-out request used to fail the whole
+        # walk. It must instead be retried once and succeed.
+        accessible = MagicMock(status_code=200)
+        accessible.json.return_value = {"resourceNames": ["customers/6501924158"]}
+        stream = MagicMock(status_code=200)
+        stream.json.return_value = [{"results": [self._customer_client("1234567890", "Client One", level="1")]}]
+        mock_request.side_effect = [accessible, requests.exceptions.ReadTimeout("read timed out"), stream]
+
+        accounts = GoogleAdsIntegration(self._integration()).list_google_ads_accessible_accounts()
+
+        assert [account["id"] for account in accounts] == ["1234567890"]
+
+    @override_settings(GOOGLE_ADS_DEVELOPER_TOKEN="dev_token")
+    @patch("tenacity.nap.time.sleep")
+    @patch("posthog.models.integration.requests.request")
+    def test_accessible_accounts_raises_after_repeated_read_timeouts(self, mock_request, mock_sleep):
+        # Retries must be bounded: a persistently unreachable endpoint should still fail rather than
+        # retry forever or get silently swallowed.
+        mock_request.side_effect = requests.exceptions.ReadTimeout("read timed out")
+
+        with pytest.raises(requests.exceptions.ReadTimeout):
+            GoogleAdsIntegration(self._integration()).list_google_ads_accessible_accounts()
+
+        assert mock_request.call_count == 3
+
 
 class TestPinterestAdsIntegrationDisplayName(BaseTest):
     @parameterized.expand(

products/warehouse_sources/backend/temporal/data_imports/sources/google_ads/source.py

@@ -3,6 +3,7 @@
 
 from django.core.cache import cache
 
+import requests
 from rest_framework.exceptions import ValidationError
 
 from posthog.schema import (
@@ -318,6 +319,13 @@ def get_oauth_accounts(
                 "Google rejected the credentials for this integration. Please reconnect your Google Ads "
                 "integration and make sure the connected account can access your Google Ads accounts."
             ) from e
+        except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as e:
+            # The walk retries a transient blip internally; this means every attempt on some request
+            # timed out or failed to connect. Actionable and retryable from the user's side, so surface
+            # a clean message instead of the raw connection error.
+            raise IntegrationAccountListingError(
+                "Google Ads did not respond in time while listing your accounts. Please try again."
+            ) from e
 
         names_by_id = {account["id"]: account["name"] for account in accounts}
         integration_accounts = [

products/warehouse_sources/backend/temporal/data_imports/sources/google_ads/tests/test_google_ads_source.py

@@ -13,6 +13,7 @@
 
 import grpc
 import pyarrow as pa
+import requests
 from google.ads.googleads.errors import GoogleAdsException
 from google.ads.googleads.v23.enums import types as ga_enums
 from google.ads.googleads.v23.errors.types.errors import ErrorCode, GoogleAdsError, GoogleAdsFailure
@@ -24,6 +25,9 @@
 
 from posthog.models.integration import Integration
 
+from products.warehouse_sources.backend.temporal.data_imports.sources.common.integration_accounts import (
+    IntegrationAccountListingError,
+)
 from products.warehouse_sources.backend.temporal.data_imports.sources.generated_configs.googleads import (
     GoogleAdsIsMccAccountConfig,
     GoogleAdsSourceConfig,
@@ -1204,6 +1208,34 @@ def test_distinct_search_terms_reuse_one_hierarchy_walk(self):
         assert [account.value for account in second] == ["987-654-3210"]
 
 
+class TestGetOAuthAccountsNetworkErrorHandling:
+    @pytest.mark.parametrize(
+        "network_error",
+        [
+            requests.exceptions.ReadTimeout("read timed out"),
+            requests.exceptions.ConnectionError("connection reset"),
+        ],
+    )
+    def test_transient_network_error_becomes_actionable(self, network_error):
+        # list_google_ads_accessible_accounts retries a transient blip internally, so this exception means
+        # every attempt failed. Previously nothing here caught it, so it propagated as an unhandled 500
+        # instead of the same actionable error the credential-rejection path already raises.
+        cache.clear()
+        source = GoogleAdsSource()
+        integration = mock.Mock(errors=None)
+
+        with (
+            mock.patch.object(GoogleAdsSource, "get_oauth_integration", return_value=integration),
+            mock.patch(f"{_SOURCE_MODULE}.OauthIntegration") as mock_oauth,
+            mock.patch(f"{_SOURCE_MODULE}.GoogleAdsIntegration") as mock_google_ads,
+        ):
+            mock_oauth.return_value.access_token_expired.return_value = False
+            mock_google_ads.return_value.list_google_ads_accessible_accounts.side_effect = network_error
+
+            with pytest.raises(IntegrationAccountListingError):
+                source.get_oauth_accounts(1, 2)
+
+
 class TestGoogleAdsQueryConstruction:
     _MODULE = "products.warehouse_sources.backend.temporal.data_imports.sources.google_ads.google_ads"