fix(stripe): surface Stripe error codes and fail loudly when the webhook endpoint listing errors #42
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/DUNITE-10-stripe-http-errors"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
DUNITE-10. Two defects that both end the same way: a Stripe failure the consumer cannot act on.
list_webhook_endpointsswallowed HTTP errorsThe method sent the request, ignored
resp.status(), parsed the body as JSON, found nodataarray and returnedOk(vec![]). A 401 (revoked key), 403 (restricted key) or 500 was therefore indistinguishable from "this Stripe account has no webhook endpoints" - which the admin screen renders as "not configured", inviting the operator to create a duplicate endpoint against an account that already has one.create_webhook_endpointanddelete_webhook_endpointalready had the status check; the list path now has the same one.Stripe error codes never reached the caller
Every failed call collapsed into
StripeServiceError::internal("Failed to ...").error.codeanderror.typewere logged and then discarded, so the consumer'sFrom<StripeServiceError> for AppErrorhad nothing to branch on and a declined card, a rate limit and a genuine Stripe fault all became a 500.New variant:
StripeErrorDetailscarrieshttp_status,error_type,code,decline_code,request_id, withis_resource_missing()/is_rate_limited()/is_card_error()predicates, plusstripe_details()/stripe_code()/stripe_http_status()accessors on the error itself. All 23async-stripecall sites and all three raw-REST sites build it. The details are boxed so the enum does not grow past clippy'sresult_large_errthreshold.No PII in the details
StripeErrorDetailshas no field for Stripe'serror.messageon purpose. Those strings embed customer email, subscription ids and card last-4 (BUNYIP-265), and unlike the old log-only handling this struct is expected to reach response bodies.stripe_error_envelopekeeps the machine-readable fields, drops the message, and now also reads theRequest-Idheader - the one identifier Stripe support asks for.Two limits of the SDK path, documented at the conversion:
async-stripedoes not surface the response header, sorequest_idisNonethere; and it modelscodeas a closed enum, so a code Stripe added after 0.37 arrives asNoneand the failure is classified byerror_typealone.get_invoiceno longer lies about 404sIt mapped every retrieve failure to
NotFound("Invoice"), so a Stripe outage or an expired API key was served to the user as "invoice not found". Now onlyresource_missing/ 404 is a missing invoice; anything else keeps its Stripe classification.Consumer impact
Additive:
StripeServiceErrorgains a variant, so a consumer'sFromimpl needs an arm for it (matching is exhaustive). The simplest migration is to treat it as the existing 500 and then opt into finer mapping -details.is_card_error()to 402,is_resource_missing()to 404,is_rate_limited()to 429. Messages already returned are unchanged, so no user-facing string moves.Testing
just fmt,just check,just lint,just testall clean. This adds the crate's first tests (12):StripeErrorDetails: SDK classification is preserved, the Stripe message never survives intoDisplay,ErrorType::Unknownis reported as absent rather than as a bogusunknown_error, a transport failure yields empty details instead of an invented status, and the card / rate-limit / resource-missing predicates.wiremocktests onstripe_error_envelopeagainst realreqwest::Responses: full envelope, message dropped, non-JSON 502 body still reporting status + request id, and the 401-listing regression.Not covered: the wiring inside
list_webhook_endpointsitself, because the Stripe base URL is hardcoded toapi.stripe.com. Making it injectable means adding a field to the publicStripeConfig, which every consumer constructs by struct literal, so it is called out as a follow-up rather than smuggled in here.Flagged, not changed
The
tracing::error!(error = %e, ...)lines on the SDK path still logStripeError'sDisplay, which includes Stripe'smessageand therefore the same PII that BUNYIP-265 removed from the raw-HTTP path. Sanitizing them would also cost operators debug detail the returned error deliberately does not carry, so that trade-off belongs in its own ticket.Two defects in `dunite-stripe`, both of which turn a Stripe failure into something the consumer cannot act on (DUNITE-10). `list_webhook_endpoints` never checked the HTTP status. A 401 from a revoked key, a 403 from a restricted key or a 500 all parse as JSON, find no `data` array, and return `Ok(vec![])` - indistinguishable from "this account has no webhook endpoints". The admin screen reads that as "not configured" and invites the operator to create a duplicate endpoint. It now takes the same status check `create_webhook_endpoint` and `delete_webhook_endpoint` already had. Stripe's own classification never left the service. Every failed call collapsed into `StripeServiceError::internal("Failed to ...")`, so `error.code` and `error.type` existed only in a log line and the consumer had nothing to map on: a declined card, a rate limit and a genuine server fault all became a 500. A new `StripeServiceError::Stripe { message, details }` variant carries `StripeErrorDetails` (http_status, error_type, code, decline_code, request_id) with `is_resource_missing()` / `is_rate_limited()` / `is_card_error()` predicates and `stripe_details()` / `stripe_code()` / `stripe_http_status()` accessors on the error. All 23 async-stripe call sites and all three raw-REST sites now build it; the details are boxed so the enum stays small (clippy's `result_large_err`). `StripeErrorDetails` deliberately has no field for Stripe's `error.message`. Those strings embed customer email, subscription ids and card last-4 (BUNYIP-265) and this struct is expected to reach response bodies; `stripe_error_envelope` drops the message while keeping the machine-readable fields, and now also reads the `Request-Id` header, which is the one identifier Stripe support asks for. The SDK path cannot supply a request id (async-stripe does not surface the header) and reports a code Stripe added after 0.37 as `None`, since the SDK models codes as a closed enum. `get_invoice` no longer reports every failure as `NotFound("Invoice")`. Only `resource_missing` / 404 is a missing invoice; a Stripe outage or an expired key used to be served to the user as "invoice not found". Adds the crate's first tests: 8 unit tests over the details type (classification, PII exclusion, `ErrorType::Unknown` reported as absent, transport failure yielding empty details rather than an invented status) and 4 wiremock tests over `stripe_error_envelope` against real `reqwest::Response`s, including the 401-listing regression. The wiring inside `list_webhook_endpoints` itself is still uncovered because the Stripe base URL is hardcoded; making it injectable would change the public `StripeConfig`, so it is left as a follow-up.