pal-e-docs

Review: Fix Stripe SDK v15 webhook crash + add webhook Prometheus metrics

review-856-2026-04-04 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Bug
  • [x] Lineage -- references #343, #346
  • [x] Repo -- forgejo_admin/basketball-api
  • [x] What Broke -- detailed root cause with error message and SDK version mismatch
  • [x] Repro Steps -- 5 clear steps
  • [x] Expected Behavior -- clear
  • [x] Environment -- cluster, SDK versions, pin info
  • [x] Acceptance Criteria -- 5 items
  • [x] Related -- present with issue links and project reference
All required bug template sections are present and well-written.

Traceability

  • [x] story:WS-S18 -- "As a parent, I want to receive a branded email with jersey ordering link so that I can order without bringing cash"
  • [x] story note verified -- found in project-westside-basketball user-stories section (Parent stories list)
  • [x] arch:basketball-api label present
  • [ ] arch note MISSING -- [SCOPE] Create architecture note arch-basketball-api for component basketball-api
  • [x] Forgejo issue -- forgejo_admin/basketball-api#350, open

File Targets

  • [x] src/basketball_api/routes/webhooks.py -- verified: file exists, contains 23 Stripe object .get() calls (issue claims 25, minor discrepancy -- 2 are db.get/dict.get, not Stripe objects). Line 301 confirmed as .get("metadata") call.
  • [ ] routes/webhooks.py -- [BODY] Issue uses package-relative path. Repo-root path is src/basketball_api/routes/webhooks.py. Update for agent clarity.
  • [x] pyproject.toml -- verified: line 16 shows "stripe>=11.0" with no upper bound, confirming root cause.
  • [x] src/basketball_api/routes/health.py -- verified: existing /metrics endpoint at line 34 exposes only a basic up gauge. Webhook counters would be added here or in a new metrics module. No prometheus_client dependency exists yet in pyproject.toml.

Repo Placement

OK. Issue filed on forgejo_admin/basketball-api, fix is in basketball-api. Single repo scope confirmed.

Dependencies

  • #719 (qa column): "Stripe webhook not syncing payment to order status" -- significant scope overlap. Both address webhook handler failures on Stripe data. #719 may be a symptom of the same SDK v15 crash. If so, this ticket supersedes #719 and #719 should be closed as duplicate or marked as resolved-by this ticket.
  • #720 (backlog): "Legacy jersey webhook handler missing payment_status guard" -- same arch, same story. Independent bug but related code area. Not a blocker.
  • #731/#733 (backlog/in_progress): "Fix 9 jersey/checkout test failures blocking CI deploy" -- AC #5 ("Existing tests pass") depends on these test fixes landing first. If tests are already broken, this ticket's agent cannot verify AC #5.
  • Downstream: Issue mentions "pal-e-platform webhook alerting ticket (depends on these metrics)" -- downstream dependency noted but not tracked as a board item link.

Acceptance Criteria

  • AC 1 (pin stripe <15): verifiable via grep on pyproject.toml. Clear.
  • AC 2 (replace .get() calls): verifiable via grep. Clear.
  • AC 3 (live webhook test): requires deployed pod + Stripe test payment. Not automatable by agent in CI -- manual validation needed. Acceptable for a bug fix.
  • AC 4 (Prometheus metrics): this is a feature, not a bug fix. Mixing concerns, but scope is small enough. Verifiable via curl /metrics. However, no prometheus_client dependency exists -- AC should mention adding it to pyproject.toml.
  • AC 5 (existing tests pass): depends on #731/#733 test fixes landing first. May be unverifiable at dispatch time.

Blast Radius

Low. Stripe .get() usage is isolated to webhooks.py (23 calls). No other files in src/ use .get() on Stripe objects. The /metrics endpoint change is additive. Pin change affects all Stripe usage but <15 preserves current working behavior.
One concern: AC 2 says replace .get() with bracket notation or dict() wrapping for "v15 compat." If the intent is to eventually upgrade to v15, dict() wrapping is the forward-compatible fix. If the intent is to stay on <15, the pin alone suffices and AC 2 is defense-in-depth. The ticket should clarify intent: pin-and-stay vs pin-now-migrate-later.

Decomposition Assessment

2 file targets in 1 repo, 5 acceptance criteria. Borderline on the 5-minute rule. The bug fix (pin + .get() replacement) is mechanical and fast. The metrics addition is a small feature bolted on. A single agent can handle both in one pass. No decomposition needed.

Recommendation

  • [BODY] Fix file path: routes/webhooks.py to src/basketball_api/routes/webhooks.py. Add src/basketball_api/routes/health.py as a file target for metrics work.
  • [BODY] Fix .get() count: 25 to 23 (2 are db.get/dict.get, not Stripe objects).
  • [BODY] Add note that prometheus_client dependency needs adding to pyproject.toml for AC 4.
  • [BODY] Clarify relationship with #719 -- if #719 is a symptom of the same SDK crash, mark as duplicate or add "supersedes #719" to Related.
  • [BODY] Reword AC 5 to "No new test failures introduced" (since #731/#733 indicate pre-existing test failures).
  • [SCOPE] Create architecture note arch-basketball-api for component basketball-api.