fix(google_ads): retry transient timeouts listing accessible accounts - #75902
Conversation
The Google Ads account picker walks the customer hierarchy with a chain of sequential REST calls. Any single one of them hitting a read timeout or connection error failed the whole walk with an unhandled exception, surfacing as a 500 in error tracking instead of a clean, retryable message. Retry each request on a transient timeout/connection error, and translate an exhausted retry into the same actionable error the credential-rejection path already raises. Generated-By: PostHog Code Task-Id: 0b7c1fd5-757e-430a-a97d-dbc99ad8d5c5
|
😎 This pull request was merged. |
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🤖 CI report
|
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained resiliency fix adding bounded retries and a clean error path for transient Google Ads network errors, owned by the author's own team, using an already-vendored dependency, with matching test coverage and no risky-territory surface touched.
- Author wrote 0% of the modified lines and has 15 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 28L, 2F substantive, 89L/4F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (89L, 4F, two-areas, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 74f316a · reviewed head 6e21437 |
1 participant
Problem
The Google Ads account picker (
get_oauth_accounts) walks the full customer hierarchy through a chain of sequential REST calls to list accessible accounts. A single transient read timeout or connection error anywhere in that walk had no error handling and propagated as an unhandled exception, surfacing as a 500 and error-tracking noise instead of a clean, retryable message.Changes
GoogleAdsIntegration.list_google_ads_accessible_accounts(used by both the warehouse source account picker and another Google Ads endpoint) now retries a transientTimeout/ConnectionErroron each request, up to 3 attempts, before giving up.get_oauth_accounts) now catches an exhausted retry and raises the same actionableIntegrationAccountListingErrorthe credential-rejection path already uses, instead of leaking a raw connection error.How did you test this code?
test_accessible_accounts_rides_out_one_transient_read_timeoutandtest_accessible_accounts_raises_after_repeated_read_timeoutstoTestGoogleAdsIntegrationModel: covers a single transient timeout succeeding on retry, and bounded retries still failing when the timeout persists.TestGetOAuthAccountsNetworkErrorHandling::test_transient_network_error_becomes_actionable(parameterized overReadTimeout/ConnectionError): covers the picker no longer leaking a raw exception.pytest posthog/models/test/test_integration_model.py -k GoogleAdsIntegrationModel(6 passed) andpytest products/warehouse_sources/backend/temporal/data_imports/sources/google_ads/tests/test_google_ads_source.py -k OAuthAccounts(3 passed).ruff check/ruff formatclean,uv run mypy --cache-fine-grained .clean,hogli ci:preflight --fixreports 0 failures.Automatic notifications
Docs update
None needed. This is an internal resiliency fix with no user-facing config or documented workflow change.
🤖 Agent context
Autonomy: Fully autonomous
Written by Claude Code from a PostHog error tracking alert. Skills invoked:
/writing-user-facing-copy,/writing-tests.The exception (a
ReadTimeoutfromgoogleads.googleapis.com) traced toGoogleAdsIntegration.list_google_ads_accessible_accountsinposthog/models/integration.py, called from the warehouse source'sget_oauth_accountsinproducts/warehouse_sources/.../google_ads/source.py. That method walks the customer hierarchy with a chain of sequential HTTP requests and had no timeout resilience, unlike the source's gRPC sync path which already retries transient errors. I treated this as fixable fragility rather than a non-retryable user error: a single slow response anywhere in a long hierarchy walk shouldn't kill the whole account listing, and the failure isn't part of the sync pipeline's retry-classification path at all, so it wouldn't have been reachable viaNonRetryableErrors.Error tracking issue
Created with PostHog Code