diff --git a/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/source.py b/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/source.py index 2b46c593ac16..b1914956db92 100644 --- a/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/source.py +++ b/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/source.py @@ -57,6 +57,7 @@ StripeAuthenticationError, StripePermissionError, StripeResumeConfig, + StripeTransientError, StripeValidationError, _all_known_webhook_events, check_endpoint_permissions as check_stripe_endpoint_permissions, @@ -387,12 +388,20 @@ def validate_credentials( False, f"Stripe credentials lack permissions for {', '.join(e.missing_permissions.keys())}", ) + except StripeTransientError: + # Stripe was unreachable or 5xx'd during the probe. The key may be fine, so don't echo + # Stripe's internal text as a validation failure — point the user at a retry. + return ( + False, + "Couldn't reach Stripe to validate your credentials. This is usually temporary. Please try again in a few minutes.", + ) except StripeValidationError as e: - # Non-403 failures (network, schema, rate limit, etc.) are not configuration issues, so - # surface the underlying Stripe message verbatim — the cause isn't obvious from the - # resource name. Fold any 403s collected before the unknown error into the same toast. - # Guard against empty / whitespace-only error strings so we never crash the response - # path while reporting a different error. + # Non-403, non-transient failures (e.g. an unexpected schema or response error) are not + # configuration issues, so surface the underlying Stripe message verbatim — the cause + # isn't obvious from the resource name. Transient 5xx/connection/rate-limit failures are + # handled by the StripeTransientError branch above. Fold any 403s collected before the + # unknown error into the same toast. Guard against empty / whitespace-only error strings + # so we never crash the response path while reporting a different error. def _first_line(msg: str) -> str: lines = (msg or "").splitlines() return lines[0][:200] if lines else "(no detail)" diff --git a/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/stripe.py b/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/stripe.py index ca7bb195f03c..91a7fc35f08f 100644 --- a/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/stripe.py +++ b/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/stripe.py @@ -887,11 +887,24 @@ def __init__(self, stripe_message: str): super().__init__(stripe_message) +class StripeTransientError(Exception): + """Raised when a credential probe fails because Stripe itself was unavailable (a 5xx APIError, + a connection failure, or a rate limit) rather than because the credentials are wrong. The key + may be perfectly valid, so callers surface a retry hint instead of Stripe's internal error text + (e.g. "Error while communicating with one of our backends").""" + + def __init__(self, stripe_message: str): + self.stripe_message = stripe_message + super().__init__(stripe_message) + + class StripeValidationError(Exception): - """Raised when one or more resources failed with a non-403 exception (network, schema, rate - limit, etc.) during credential validation. Distinct from StripePermissionError so callers can - decide whether to surface the verbose underlying message — permission errors are - self-explanatory from the resource name, but unknown errors need the raw detail.""" + """Raised when one or more resources failed with a non-403, non-transient exception (e.g. an + unexpected response or schema error) during credential validation. Transient Stripe-side + failures (5xx, connection, rate limit) raise StripeTransientError instead. Distinct from + StripePermissionError so callers can decide whether to surface the verbose underlying message — + permission errors are self-explanatory from the resource name, but unknown errors need the raw + detail.""" def __init__(self, errors: dict[str, str], missing_permissions: Optional[dict[str, str]] = None): self.errors = errors @@ -927,6 +940,11 @@ def _probe_endpoint(resource: StripeResource) -> tuple[str | None, str | None]: except stripe_lib.PermissionError as e: raw = getattr(e, "user_message", None) or str(e) return _clean_stripe_error_message(raw), None + except (stripe_lib.APIError, stripe_lib.APIConnectionError, stripe_lib.RateLimitError) as e: + # Stripe was unreachable or returned a 5xx/rate-limit — transient and unrelated to the + # credentials. Fail fast with a distinct error so the caller can tell the user to retry + # rather than reporting Stripe's internal text as a validation failure. + raise StripeTransientError(_clean_stripe_error_message(str(e))) from e except Exception as e: return None, _clean_stripe_error_message(str(e)) @@ -1015,7 +1033,13 @@ def check_endpoint_permissions( continue _, probe_resource = _resolve_to_flat(name, all_resources) - permission_msg, error_msg = _probe_endpoint(probe_resource) + try: + permission_msg, error_msg = _probe_endpoint(probe_resource) + except StripeTransientError as e: + # A transient Stripe outage isn't a per-endpoint verdict, but this function must return + # the full picture rather than raise (401 aside), so record it as this endpoint's reason. + results[name] = e.stripe_message + continue results[name] = permission_msg or error_msg return results diff --git a/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/tests/test_stripe_source.py b/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/tests/test_stripe_source.py index 6bfc94d526b6..57dc7a97423c 100644 --- a/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/tests/test_stripe_source.py +++ b/products/warehouse_sources/backend/temporal/data_imports/sources/stripe/tests/test_stripe_source.py @@ -66,13 +66,16 @@ StripeAuthenticationError, StripeNestedResource, StripeResource, + StripeTransientError, _all_known_webhook_events, _coerce_incremental_cursor, _is_non_list_stripe_response, _is_truncated_stripe_list_response, _RateLimitRetryingRequestsClient, _scrub_client_secrets, + check_endpoint_permissions, get_rows, + validate_credentials as validate_stripe_credentials, ) _COMPLETE_LIST_BODY = b'{\n "object": "list",\n "data": [],\n "has_more": false\n}' @@ -281,6 +284,25 @@ def test_validate_credentials_does_not_echo_rejected_key(self): assert pasted_secret not in message assert message.startswith("Stripe rejected the API key.") + def test_validate_credentials_transient_error_returns_retry_message(self): + # A Stripe-side 5xx during the probe is transient and unrelated to the key. The user must + # get a retry hint, not Stripe's internal text reported as a permanent validation failure. + config = StripeSourceConfig( + auth_method=StripeAuthMethodConfig(selection="api_key", stripe_secret_key="rk_live_x") + ) + + with mock.patch( + "products.warehouse_sources.backend.temporal.data_imports.sources.stripe.source.validate_stripe_credentials", + side_effect=StripeTransientError("Error while communicating with one of our backends. Sorry about that!"), + ): + ok, message = self.source.validate_credentials(config, team_id=1) + + assert ok is False + assert message is not None + assert "try again" in message.lower() + assert "one of our backends" not in message + assert "validation failed" not in message.lower() + @pytest.mark.parametrize( "body,expected", [ @@ -398,6 +420,39 @@ def _run_nested_get_rows(nested_method, parent_objects=None, parent_has_nested=N return rows +class TestValidateCredentialsTransientClassification: + @pytest.mark.parametrize( + "error", + [ + stripe_lib.APIError("Error while communicating with one of our backends. Sorry about that!"), + stripe_lib.APIConnectionError("Unexpected error communicating with Stripe."), + stripe_lib.RateLimitError("Too many requests."), + ], + ) + def test_probe_backend_failure_raises_transient_not_validation(self, error): + # These are Stripe-unavailable failures, not credential problems: the probe must raise + # StripeTransientError so the caller offers a retry rather than a validation failure. + def boom(params=None): + raise error + + resource = StripeResource(method=boom) + with patch.object(stripe_module, "_build_resources", return_value={CUSTOMER_RESOURCE_NAME: resource}): + with pytest.raises(StripeTransientError): + validate_stripe_credentials("rk_live_x", endpoints=None) + + def test_check_endpoint_permissions_records_transient_without_raising(self): + # The schema-selection permissions map must stay whole during a Stripe outage: a transient + # error is recorded as the endpoint's reason, not raised out of check_endpoint_permissions. + def boom(params=None): + raise stripe_lib.APIError("Error while communicating with one of our backends. Sorry about that!") + + resource = StripeResource(method=boom) + with patch.object(stripe_module, "_build_resources", return_value={CUSTOMER_RESOURCE_NAME: resource}): + results = check_endpoint_permissions("rk_live_x", [CUSTOMER_RESOURCE_NAME]) + + assert results[CUSTOMER_RESOURCE_NAME] is not None + + class TestStripeNestedResourceGetRows: def test_skips_parent_deleted_mid_sync(self): def nested_method(customer=None, params=None):