Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
StripeAuthenticationError,
StripePermissionError,
StripeResumeConfig,
StripeTransientError,
StripeValidationError,
_all_known_webhook_events,
check_endpoint_permissions as check_stripe_endpoint_permissions,
Expand Down Expand Up @@ -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,
Comment thread
Gilbert09 marked this conversation as resolved.
"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)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")."""
Comment thread
Gilbert09 marked this conversation as resolved.

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
Expand Down Expand Up @@ -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
Comment thread
Gilbert09 marked this conversation as resolved.
except Exception as e:
return None, _clean_stripe_error_message(str(e))

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}'
Expand Down Expand Up @@ -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",
[
Expand Down Expand Up @@ -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):
Expand Down
Loading