fix(stripe): finalize webhook idempotency after handler runs (BUNYIP-210) #233

Merged

longjacksonle merged 1 commit from fix/bunyip-210-webhook-idempotency-ordering into main

2026-06-26 02:31:31 +02:00

longjacksonle commented

2026-06-26 02:28:25 +02:00

Owner

What

Fixes BUNYIP-210: a successful Stripe payment could leave the user with no application/entitlement access.

Root cause

The BUNYIP-89 idempotency fence in stripe_webhook recorded event.id before running the matched handler, on a connection separate from the handler's work. If a handler then failed, Stripe retried, the fence saw the recorded row, returned 200, and the handler never ran again. Membership activation (subscription_status) and per-product entitlement grants were silently lost.

The common trigger: customer.subscription.created often arrives before the user's stripe_customer_id is linked, so find_by_stripe_customer_id returns not_found on first delivery. First delivery errored, the fence swallowed every retry. Because per-application entitlements are granted only by the subscription handlers (sync_stripe_entitlements), the user ended up with active membership status but zero entitlements: "payment succeeded, no access to the application."

Fix

Replace record-before-process with a claim/finalize/release lifecycle, so an event is treated as "already handled" only once its handler actually succeeded:

A delivery claims the event by writing status = 'processing'.
On handler success it is promoted to done (finalize_webhook_event).
On handler failure the claim is released (row deleted) so Stripe's retry reprocesses (release_webhook_event); the original error still propagates as non-2xx.
A processing claim left by a crashed handler is reclaimable after a 15-minute lease. The replacement re-runs the handler, whose DB writes (status updates) and entitlement sync (revoke-all-then-grant) are idempotent.
Concurrent duplicate deliveries still dedupe: a second delivery sees the fresh claim and returns 409 so Stripe retries after backoff instead of double-running.

Migration

20260625000020_stripe_webhook_events_processing_status.sql adds a status column defaulting to done (legacy rows, recorded by the old inline-handler code, are treated as already-processed) plus a (status, received_at) index for the claim lookup. Queries are runtime sqlx, so no .sqlx/ offline cache regen is required.

Testing

just check-migrations passes (versions unique and strictly increasing).
cargo clippy -p bunyip-api --all-targets clean; cargo fmt --check clean.
No automated regression test added: bunyip-api has no DB-backed Rust handler test harness (the e2e suite is Playwright, browser-driven), and a faithful "handler error on first delivery stays reprocessable" test needs a live Postgres pool with the migration applied. Flagged as follow-up.

Out of scope (separate ticket suggested)

handle_checkout_completed reads the locked price from session["line_items"] (webhook.rs), which Stripe omits from the checkout.session.completed payload unless expanded, so price-locking stores placeholder data. Noted in BUNYIP-210; worth its own ticket.

## What Fixes **BUNYIP-210**: a successful Stripe payment could leave the user with no application/entitlement access. ## Root cause The BUNYIP-89 idempotency fence in `stripe_webhook` recorded `event.id` **before** running the matched handler, on a connection separate from the handler's work. If a handler then failed, Stripe retried, the fence saw the recorded row, returned `200`, and the handler never ran again. Membership activation (`subscription_status`) and per-product entitlement grants were silently lost. The common trigger: `customer.subscription.created` often arrives before the user's `stripe_customer_id` is linked, so `find_by_stripe_customer_id` returns `not_found` on first delivery. First delivery errored, the fence swallowed every retry. Because per-application entitlements are granted only by the subscription handlers (`sync_stripe_entitlements`), the user ended up with active membership status but zero entitlements: "payment succeeded, no access to the application." ## Fix Replace record-before-process with a claim/finalize/release lifecycle, so an event is treated as "already handled" only once its handler actually succeeded: - A delivery **claims** the event by writing `status = 'processing'`. - On handler success it is promoted to `done` (`finalize_webhook_event`). - On handler failure the claim is **released** (row deleted) so Stripe's retry reprocesses (`release_webhook_event`); the original error still propagates as non-2xx. - A `processing` claim left by a crashed handler is reclaimable after a 15-minute lease. The replacement re-runs the handler, whose DB writes (status updates) and entitlement sync (revoke-all-then-grant) are idempotent. - Concurrent duplicate deliveries still dedupe: a second delivery sees the fresh claim and returns `409` so Stripe retries after backoff instead of double-running. ## Migration `20260625000020_stripe_webhook_events_processing_status.sql` adds a `status` column defaulting to `done` (legacy rows, recorded by the old inline-handler code, are treated as already-processed) plus a `(status, received_at)` index for the claim lookup. Queries are runtime sqlx, so no `.sqlx/` offline cache regen is required. ## Testing - `just check-migrations` passes (versions unique and strictly increasing). - `cargo clippy -p bunyip-api --all-targets` clean; `cargo fmt --check` clean. - No automated regression test added: bunyip-api has no DB-backed Rust handler test harness (the e2e suite is Playwright, browser-driven), and a faithful "handler error on first delivery stays reprocessable" test needs a live Postgres pool with the migration applied. Flagged as follow-up. ## Out of scope (separate ticket suggested) `handle_checkout_completed` reads the locked price from `session["line_items"]` (webhook.rs), which Stripe omits from the `checkout.session.completed` payload unless expanded, so price-locking stores placeholder data. Noted in BUNYIP-210; worth its own ticket.

longjacksonle added 1 commit

2026-06-26 02:28:25 +02:00

fix(stripe): finalize webhook idempotency after handler runs (BUNYIP-210)

E2E / Playwright against deployment (pull_request) Successful in 21s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 2m18s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

711fc54149

The BUNYIP-89 idempotency fence recorded `event.id` BEFORE running the matched handler, on a connection separate from the handler's own work. A handler error therefore left the event permanently marked processed: Stripe's retry hit `ON CONFLICT`, the webhook returned 200, and the handler never ran again. Membership activation and per-product entitlement grants were silently lost, presenting as "Stripe payment succeeded but the user has no application access" (e.g. `customer.subscription.created` arriving before the user's `stripe_customer_id` was linked, so `find_by_stripe_customer_id` returned `not_found` on first delivery).

Replace the record-before-process fence with a claim/finalize/release lifecycle so an event counts as "already handled" only once its handler actually succeeded: a delivery claims the event as `processing`, is promoted to `done` on success, and releases the claim (row deleted) on failure so the retry reprocesses. A stale `processing` claim left by a crashed handler is reclaimable after a 15-minute lease; the replacement re-runs the handler, whose DB writes (status updates) and entitlement sync (revoke-all-then-grant) are idempotent. Concurrent duplicate deliveries still dedupe: a second delivery sees the fresh claim and gets a 409 so Stripe retries after backoff rather than double-running.

Migration adds a `status` column (default `done`, so legacy rows recorded by the old inline-handler code are treated as already-processed) plus a `(status, received_at)` index for the claim lookup. The queries are runtime sqlx (no `.sqlx/` offline cache regen needed; the macro path lives only in bunyip-oidc).

No automated regression test: bunyip-api has no DB-backed Rust handler test harness (e2e is Playwright, browser-driven) and a faithful "handler error on first delivery stays reprocessable" test needs a live Postgres pool with the migration applied. Flagged for follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>