fix(stripe): retry, don't reject, when Stripe is down during validation - #76406
fix(stripe): retry, don't reject, when Stripe is down during validation#76406Gilbert09 wants to merge 2 commits into
Conversation
…t invalid credentials The source-creation credential probe caught every non-auth Stripe exception in _probe_endpoint's bare `except Exception` and folded it into a StripeValidationError, which source.py rendered as "Stripe validation failed - <resource>: <raw Stripe text>". A transient Stripe 5xx (e.g. "Error while communicating with one of our backends") therefore surfaced as a permanent validation failure with Stripe's internal wording glued on, even though the key was fine. Classify APIError / APIConnectionError / RateLimitError from the probe as a new StripeTransientError and surface a retry hint instead. Auth (401) and permission (403) handling is unchanged. Generated-By: PostHog Code Task-Id: fea44adb-4a45-4c0f-8a2a-dc20f01a3137
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
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 |
There was a problem hiding this comment.
Pull request overview
This PR improves the Stripe warehouse source setup-time credential validation flow by distinguishing transient Stripe-side failures (5xx/connection/rate limiting) from real credential/permission problems, so the UI can prompt a retry instead of reporting a permanent validation failure with Stripe’s internal error text.
Changes:
- Introduces
StripeTransientErrorand raises it from_probe_endpointforstripe.APIError,stripe.APIConnectionError, andstripe.RateLimitError. - Updates
StripeSource.validate_credentialsto mapStripeTransientErrorto a retry-oriented message. - Adds tests to ensure transient Stripe errors are classified correctly and surfaced with a retry hint.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| products/warehouse_sources/backend/temporal/data_imports/sources/stripe/stripe.py | Adds StripeTransientError and updates probe logic to raise it for transient Stripe failures. |
| products/warehouse_sources/backend/temporal/data_imports/sources/stripe/source.py | Catches StripeTransientError during source creation validation and returns a retry hint message. |
| products/warehouse_sources/backend/temporal/data_imports/sources/stripe/tests/test_stripe_source.py | Adds regression tests for transient error classification and user-facing messaging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tage The transient-error classification is raised from the shared _probe_endpoint, which check_endpoint_permissions also uses. That function promises to return a per-endpoint map and never raise (401 aside), so tolerate StripeTransientError there and record it as the endpoint's reason instead of letting it propagate into the schema-selection UI. Also correct the now-stale docstring and comment that still listed rate limits under StripeValidationError. Generated-By: PostHog Code Task-Id: fea44adb-4a45-4c0f-8a2a-dc20f01a3137
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained error-classification fix in the Stripe source validation path, all three Copilot review concerns were addressed with follow-up commits and regression tests, and the author is on the owning team.
- Author wrote 0% of the modified lines and has 10 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 | ✓ | 53L, 2F substantive, 108L/3F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (108L, 3F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 96e2437 · reviewed head 891b0f3 |
Problem
When you add a Stripe source, we validate the key by probing a cheap endpoint (
customers.list, limit 1)._probe_endpointhandled 401 (bad key) and 403 (missing scope) explicitly, but everything else fell into a bareexcept Exceptionand got folded into aStripeValidationError.validate_credentialsinsource.pythen rendered that as:That text is Stripe's own message for a transient 5xx on their side. The key is fine. But we reported it as a permanent validation failure and glued Stripe's internal wording onto the toast, so the user has nothing to act on and no reason to just try again.
Changes
_probe_endpointnow classifiesstripe.APIError(Stripe's generic 5xx),stripe.APIConnectionError, andstripe.RateLimitErroras a newStripeTransientErrorinstead of collecting them as validation failures.source.py'svalidate_credentialsmaps that to a retry hint:Genuine 401 (
StripeAuthenticationError) and 403 (StripePermissionError) handling is untouched, as is theStripeValidationErrorpath for real non-transient probe failures (schema, unexpected 4xx). A transient error now also fails the multi-endpoint probe loop fast rather than probing every remaining resource just to hit the same outage.Note
This is the source-creation validation path only. It's separate from the several open PRs that classify the same transient Stripe errors as retryable on the Temporal sync path (
get_retryable_errors()/_call_stripe) — those keep transient failures out of error tracking during ongoing imports; this one fixes what the user sees in the setup wizard.How did you test this code?
Automated, in
test_stripe_source.py:TestValidateCredentialsTransientClassification— parameterized overAPIError/APIConnectionError/RateLimitError, asserts the probe raisesStripeTransientErrorrather than swallowing it into a validation error. Guards the classification.test_validate_credentials_transient_error_returns_retry_message— asserts the source mapsStripeTransientErrorto a retry message and does not echo Stripe's internal text or the "validation failed" wording. Guards the leak/mislabel regression.Full Stripe suite passes locally (166 passed), plus
ruff. I (Claude) did not exercise this against the live Stripe API.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Fully autonomous
Written by Claude while triaging data-warehouse source-creation failures. Stripe showed a transient backend 5xx surfaced to the user as a permanent "validation failed" with Stripe's raw text attached. I confirmed via the SDK that these are distinct exception classes (
APIError/APIConnectionError/RateLimitErrorall derive directly fromStripeError, so type-matching cleanly separates them from the auth/permission classes handled first), which is why the fix keys on type rather than message text.Considered whether the existing open Stripe PRs already covered this — they classify the same errors as retryable but only on the sync path (
get_retryable_errors), which never runs during wizard validation, so this is a distinct fix. Invoked/writing-testsbefore adding tests.Created with PostHog Code