fix(data-warehouse): friendlier Google Ads permission errors#66221
Draft
posthog[bot] wants to merge 1 commit into
Draft
fix(data-warehouse): friendlier Google Ads permission errors#66221posthog[bot] wants to merge 1 commit into
posthog[bot] wants to merge 1 commit into
Conversation
Sync-time gRPC PERMISSION_DENIED now maps to an actionable message in get_non_retryable_errors() instead of None, which previously let the raw GoogleAdsException propagate to users and error tracking. At link time, an OAuth permission/scope validation failure now offers a "Reconnect account" toast button that initiates the OAuth flow with the correct scopes, replacing the generic "Get help" button that opened a support page in a new tab and registered as a dead click. Generated-By: PostHog Code Task-Id: 6daab348-826d-4844-8ec5-414f7b89fbe8
Contributor
|
Size Change: 0 B Total Size: 64 MB ℹ️ View Unchanged
|
Contributor
Author
|
🤖 Agent note on the red Jest test (EE - 1) / Frontend Tests Pass check — this is a wall-clock timeout, not a test failure, and it is not caused by this diff. Diagnosis
Other checks are green: Frontend typechecking, Frontend formatting, Frontend bundle size, Python code quality, Django tests, and the added backend test ( Suggested resolution (human): marking the PR ready-for-review switches Jest to the full sharded matrix (the comment in the CI script notes the ready-for-review full matrix is the gate). The sharded matrix distributes these 800+ files across parallel jobs, each well under 15m, so it will pass where the single selective job times out. I've left the PR as a draft and am not re-running, to avoid burning CI credits on a deterministic timeout. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Users connecting Google Ads as a data warehouse source hit opaque permission errors at two points:
PERMISSION_DENIED("The caller does not have permission") is correctly classified non-retryable, but its entry inget_non_retryable_errors()wasNone. Because the failure handler skips setting a friendly message when the first matching entry isNone, the rawGoogleAdsExceptionpropagated to the user and error tracking.window.open(...)s a support page — no in-page change, so it reads as a dead click and doesn't help the user fix the actual problem.Changes
PERMISSION_DENIEDentry a friendly, actionable message (mirroring the validate-timeUSER_PERMISSION_DENIEDmapping) so users learn to verify account access or reconnect.How did you test this code?
I'm an agent. I added and ran the backend unit test
TestGoogleAdsNonRetryableErrors::test_permission_denied_has_friendly_message(and the surrounding class — 34 passed) verifyingPERMISSION_DENIEDnow resolves to a non-empty, actionable message. The frontend change mirrors the existing OAuth reconnect pattern (api.integrations.authorizeUrl+saveFormStateBeforeRedirect); I could not runtsgo/format in this environment as frontend deps were not installed, so the toast behavior was not exercised at runtime.🤖 Agent context
Autonomy: Fully autonomous
Authored by Claude Code (Opus 4.8) from a PostHog inbox report. Two related failure points on the Google Ads warehouse source: the sync-time fix is in
get_non_retryable_errors(); the link-time fix is insourceWizardLogic'sgetDatabaseSchemaserror handler. I confirmed via the failure-handler code (external_data_job.py) that aNonefirst-match entry causes the raw error to propagate, which is the root cause of the sync-time symptom. The reconnect-button approach was chosen over globally changing the sharedGET_HELP_BUTTONto keep the change scoped and to give users an actionable next step rather than just avoiding the dead-click metric.Created with PostHog Code from an inbox report.