fix(payment): harden Stripe webhook handler with idempotency and safe state updates (#22)#29
Open
1337Xcode wants to merge 1 commit into
Open
fix(payment): harden Stripe webhook handler with idempotency and safe state updates (#22)#291337Xcode wants to merge 1 commit into
1337Xcode wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR implements idempotent Stripe webhook processing with signature verification and safe payment state updates. New repositories and services handle webhook event deduplication and route payment events to guarded order state handlers that prevent duplicate orders and inconsistent transitions. ChangesStripe webhook idempotency and safe payment state transitions
Sequence Diagram(s)sequenceDiagram
participant Stripe
participant Controller
participant StripeConfig
participant WebhookEventRepo
participant WebhookService
participant OrderRepo
participant Firestore
Stripe->>Controller: POST /api/payment/webhook with signature
Controller->>Controller: Check stripe-signature header
Controller->>StripeConfig: createWebhook(body, signature)
StripeConfig->>StripeConfig: Verify using STRIPE_WEBHOOK_SECRET
StripeConfig-->>Controller: Stripe.Event or throw
Controller->>WebhookEventRepo: hasBeenProcessed(event.id)
WebhookEventRepo->>Firestore: Query stripe_webhook_events collection
Firestore-->>WebhookEventRepo: Document exists?
WebhookEventRepo-->>Controller: boolean
alt Duplicate event
Controller-->>Stripe: 200 OK (idempotent)
else New event
Controller->>WebhookService: handleStripeEvent(event)
WebhookService->>OrderRepo: getOrderByPaymentIntentId
OrderRepo->>Firestore: Query orders by paymentIntentId
Firestore-->>OrderRepo: Order or null
OrderRepo-->>WebhookService: Order
WebhookService->>Firestore: Update order paymentStatus
Firestore-->>WebhookService: OK
WebhookService-->>Controller: void
Controller->>WebhookEventRepo: markAsProcessed(event.id, event.type)
WebhookEventRepo->>Firestore: Write WebhookEventRecord
Firestore-->>WebhookEventRepo: OK
Controller-->>Stripe: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
Hey @CherryCIC, would you mind taking a look when you get a chance? Happy to walk through anything if needed. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the Stripe webhook endpoint (POST /api/payment/webhook) by fixing signature verification, adding durable idempotency, and routing events through safer order state-transition handlers.
Changes:
- Fixes Stripe webhook signature verification to use
STRIPE_WEBHOOK_SECRET(and adds a boot-time env guard). - Adds a Firestore-backed idempotency store (
stripe_webhook_events) and introduces aWebhookServicefor event-type routing. - Expands
PaymentStatuswith'processing'and adds an order lookup bypaymentIntentId, plus Jest/TS config updates and new webhook controller tests.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds Jest/Node type resolution to unblock ts-jest typings. |
| src/shared/config/stripeConfig.ts | Uses STRIPE_WEBHOOK_SECRET for webhook verification and guards env vars at boot. |
| src/modules/payment/WebhookEventRepository.ts | Introduces Firestore-backed webhook idempotency record store. |
| src/modules/payment/services/WebhookService.ts | Adds per-event-type handlers for safe order/payment state transitions. |
| src/modules/payment/controllers/paymentController.ts | Rewrites webhook controller with signature verification + idempotency + routing. |
| src/modules/payment/tests/stripeWebhook.test.ts | Adds controller-level tests for signature validation, duplicates, and key event types. |
| src/modules/order/repositories/OrderRepository.ts | Adds getOrderByPaymentIntentId() and documents legacy paymentStatus fallback. |
| src/modules/order/model/Order.ts | Expands PaymentStatus to include 'processing' with a legacy-data note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,237 @@ | |||
| import Stripe from 'stripe'; | |||
| import { OrderRepository } from '../order/repositories/OrderRepository'; | |||
Comment on lines
+14
to
+30
| * Each processed event is stored as a document whose ID is the Stripe event ID | ||
| * (e.g. "evt_1AbCdEfG..."). Because the document ID equals the event ID: | ||
| * - Lookups are O(1) key reads — no collection scans required. | ||
| * - Concurrent duplicate deliveries from Stripe are safely serialised by | ||
| * Firestore; at most one `set` will win and the other will see the document | ||
| * already present on its subsequent `hasBeenProcessed` check. | ||
| * | ||
| * Firestore collection: `stripe_webhook_events` | ||
| */ | ||
| export class WebhookEventRepository { | ||
| /** | ||
| * Returns true if the given Stripe event ID has already been processed. | ||
| * This must be called before executing any side effects for a webhook event. | ||
| */ | ||
| async hasBeenProcessed(eventId: string): Promise<boolean> { | ||
| const doc = await firestore.collection(COLLECTION).doc(eventId).get(); | ||
| return doc.exists; |
Comment on lines
+133
to
+145
| // ── Step 2: Idempotency check ───────────────────────────────────────────────── | ||
| const webhookEventRepo = new WebhookEventRepository(); | ||
|
|
||
| try { | ||
| const alreadyProcessed = await webhookEventRepo.hasBeenProcessed(event.id); | ||
| if (alreadyProcessed) { | ||
| console.log( | ||
| `[Webhook] Duplicate event ignored: ${event.type} (id: ${event.id})` | ||
| ); | ||
| // Return 200 so Stripe knows we received the event and stops retrying. | ||
| ResponseHandler.success(res, { received: true }, 'Duplicate event ignored'); | ||
| return; | ||
| } |
Comment on lines
+160
to
+164
|
|
||
| const doc = snapshot.docs[0]; | ||
| const data = doc.data() as Omit<Order, 'id'>; | ||
| return { id: doc.id, ...data }; | ||
| } |
Comment on lines
+105
to
+146
| /** | ||
| * payment_intent.processing | ||
| * | ||
| * Stripe is processing the payment (e.g. bank transfer, delayed settlement). | ||
| * The outcome is not yet known. This MUST NOT mark the order as complete. | ||
| * | ||
| * Safe behaviour: | ||
| * - No-op if no order exists for this PaymentIntent. | ||
| * - Updates paymentStatus to 'processing' only if the order is currently | ||
| * 'pending' — prevents rolling back a 'succeeded' or 'failed' order. | ||
| * - Does NOT create or fulfil any order. | ||
| */ | ||
| private async handlePaymentProcessing(paymentIntent: Stripe.PaymentIntent): Promise<void> { | ||
| console.log( | ||
| `[WebhookService] payment_intent.processing — paymentIntentId: ${paymentIntent.id}` | ||
| ); | ||
|
|
||
| const order = await this.orderRepo.getOrderByPaymentIntentId(paymentIntent.id); | ||
|
|
||
| if (!order) { | ||
| console.log( | ||
| `[WebhookService] payment_intent.processing — no order found for paymentIntentId: ${paymentIntent.id}. No action taken.` | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| if (order.paymentStatus !== 'pending') { | ||
| // Do not overwrite a terminal or already-advanced state with 'processing'. | ||
| console.log( | ||
| `[WebhookService] payment_intent.processing — order ${order.id} has paymentStatus '${order.paymentStatus}', not overwriting with 'processing'.` | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| await this.orderRepo.updateOrder(order.id, { | ||
| paymentStatus: 'processing', | ||
| }); | ||
|
|
||
| console.log( | ||
| `[WebhookService] payment_intent.processing — order ${order.id} updated to paymentStatus: processing.` | ||
| ); | ||
| } |
Comment on lines
+118
to
+123
| // constructEvent throws when the signature is invalid or the payload is | ||
| // malformed. This is not a server error — return 400 so Stripe stops retrying. | ||
| console.warn( | ||
| `[Webhook] Rejected: invalid signature. Error: ${err instanceof Error ? err.message : String(err)}` | ||
| ); | ||
| ResponseHandler.badRequest( |
Comment on lines
+9
to
+11
| // TODO (future engineer): Once you are confident no documents with unrecognised | ||
| // paymentStatus values exist in Firestore (e.g. after a data migration or sufficient | ||
| // run-time), you may narrow this type back and remove the || 'pending' fallback. |
Comment on lines
+113
to
+116
| let event; | ||
| try { | ||
| // req.body is the raw Buffer provided by express.raw() in paymentRoutes.ts. | ||
| event = createWebhook(req.body as Buffer, sig); |
Comment on lines
+1
to
+22
| import { firestore } from '../../shared/config/firebaseConfig'; | ||
|
|
||
| const COLLECTION = 'stripe_webhook_events'; | ||
|
|
||
| export interface WebhookEventRecord { | ||
| eventId: string; | ||
| eventType: string; | ||
| processedAt: Date; | ||
| } | ||
|
|
||
| /** | ||
| * Provides durable idempotency for Stripe webhook events using Firestore. | ||
| * | ||
| * Each processed event is stored as a document whose ID is the Stripe event ID | ||
| * (e.g. "evt_1AbCdEfG..."). Because the document ID equals the event ID: | ||
| * - Lookups are O(1) key reads — no collection scans required. | ||
| * - Concurrent duplicate deliveries from Stripe are safely serialised by | ||
| * Firestore; at most one `set` will win and the other will see the document | ||
| * already present on its subsequent `hasBeenProcessed` check. | ||
| * | ||
| * Firestore collection: `stripe_webhook_events` | ||
| */ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/modules/payment/tests/stripeWebhook.test.ts (1)
135-188: ⚡ Quick winAdd a controller test for
payment_intent.processing.This suite covers succeeded/failed/canceled but skips the subscribed
payment_intent.processingpath. Add a parity test to ensure it is dispatched and marked processed like other accepted events.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/payment/tests/stripeWebhook.test.ts` around lines 135 - 188, Add a new test in stripeWebhook.test.ts that mirrors the "payment_intent.succeeded" happy-path for the "payment_intent.processing" event: use buildStripeEvent('payment_intent.processing') and have mockCreateWebhook return it, mockHasBeenProcessed resolve to false, mockHandleStripeEvent resolve successfully, and mockMarkAsProcessed resolve; call stripeWebhook with a req (headers: 'stripe-signature': 'valid_sig', body: Buffer.from('{}')) and a res from createResponse(), then assert mockHandleStripeEvent was called with the event, mockMarkAsProcessed was called with event.id and event.type, and res.status was called with 200 so the processing path is dispatched and persisted like other accepted events.tsconfig.json (1)
11-12: 🏗️ Heavy liftSplit Jest typings out of the base TypeScript config
tsconfig.jsoncurrently includes Jest typings in the shared compiler options ("types": ["jest", "node"]), and there’s only this onetsconfig.jsonwhilenpm run buildruns plaintsc, so Jest globals can leak into non-test compilation."typeRoots": ["./node_modules/@types", "./src/types"], "types": ["jest", "node"]Prefer a
tsconfig.test.json(extends the base) that sets"types": ["jest", "node"], and keep the base config for non-test compilation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tsconfig.json` around lines 11 - 12, Move Jest typings out of the shared compiler options by removing the "types": ["jest","node"] entry from tsconfig.json and create a new tsconfig.test.json that extends the base and sets "types": ["jest","node"]; update test scripts (e.g., the jest or test npm script) to use tsconfig.test.json for test type-checking where needed so normal tsc builds use the base tsconfig.json without Jest globals.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/payment/controllers/paymentController.ts`:
- Around line 136-169: The current get-then-set idempotency
(webhookEventRepo.hasBeenProcessed + markAsProcessed) is race-prone; replace it
with an atomic "claim" before running side effects: add a new repository method
(e.g., webhookEventRepo.claimProcessing(event.id, { type: event.type, receivedAt
})) that uses Firestore doc.create(...) or a conditional transaction to create
the marker only if it doesn't exist, then in the controller call claimProcessing
and proceed to new WebhookService().handleStripeEvent(event) only when the claim
succeeds; if claim fails because the doc already exists, treat it as a duplicate
and return the same 200 duplicate response, and keep existing error handling for
real Firestore errors. Ensure markAsProcessed is no longer relied on for the
primary idempotency guard (it can still update the claim after successful
processing if you want to store result metadata).
In `@src/modules/payment/tests/stripeWebhook.test.ts`:
- Around line 241-261: The test "safely accepts and marks an unsupported event
type without error" documents that unsupported Stripe events should be marked
processed but never asserts it; update the test in stripeWebhook.test.ts to
assert that mockMarkAsProcessed was called (with the event id or the built
event) after stripeWebhook runs so idempotency is enforced; locate the test
using the it(...) block and add an expectation like
expect(mockMarkAsProcessed).toHaveBeenCalledWith(event) (or toHaveBeenCalled()
if id not available) after the existing mockHandleStripeEvent and res.status
assertions.
---
Nitpick comments:
In `@src/modules/payment/tests/stripeWebhook.test.ts`:
- Around line 135-188: Add a new test in stripeWebhook.test.ts that mirrors the
"payment_intent.succeeded" happy-path for the "payment_intent.processing" event:
use buildStripeEvent('payment_intent.processing') and have mockCreateWebhook
return it, mockHasBeenProcessed resolve to false, mockHandleStripeEvent resolve
successfully, and mockMarkAsProcessed resolve; call stripeWebhook with a req
(headers: 'stripe-signature': 'valid_sig', body: Buffer.from('{}')) and a res
from createResponse(), then assert mockHandleStripeEvent was called with the
event, mockMarkAsProcessed was called with event.id and event.type, and
res.status was called with 200 so the processing path is dispatched and
persisted like other accepted events.
In `@tsconfig.json`:
- Around line 11-12: Move Jest typings out of the shared compiler options by
removing the "types": ["jest","node"] entry from tsconfig.json and create a new
tsconfig.test.json that extends the base and sets "types": ["jest","node"];
update test scripts (e.g., the jest or test npm script) to use
tsconfig.test.json for test type-checking where needed so normal tsc builds use
the base tsconfig.json without Jest globals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f14e5c8-2351-4ea0-805f-03912465f6e4
📒 Files selected for processing (8)
src/modules/order/model/Order.tssrc/modules/order/repositories/OrderRepository.tssrc/modules/payment/WebhookEventRepository.tssrc/modules/payment/controllers/paymentController.tssrc/modules/payment/services/WebhookService.tssrc/modules/payment/tests/stripeWebhook.test.tssrc/shared/config/stripeConfig.tstsconfig.json
Comment on lines
+136
to
+169
| try { | ||
| const alreadyProcessed = await webhookEventRepo.hasBeenProcessed(event.id); | ||
| if (alreadyProcessed) { | ||
| console.log( | ||
| `[Webhook] Duplicate event ignored: ${event.type} (id: ${event.id})` | ||
| ); | ||
| // Return 200 so Stripe knows we received the event and stops retrying. | ||
| ResponseHandler.success(res, { received: true }, 'Duplicate event ignored'); | ||
| return; | ||
| } | ||
| } catch (err) { | ||
| // Firestore read failure. Return 500 so Stripe retries — we would rather | ||
| // retry than risk processing a duplicate without the idempotency guard. | ||
| console.error( | ||
| `[Webhook] Failed to check idempotency for event ${event.id}:`, | ||
| err | ||
| ); | ||
| ResponseHandler.internalServerError( | ||
| res, | ||
| 'Webhook idempotency check failed', | ||
| err instanceof Error ? err.message : 'Unknown error' | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // ── Step 3: Process the event ───────────────────────────────────────────────── | ||
| try { | ||
| const webhookService = new WebhookService(); | ||
| await webhookService.handleStripeEvent(event); | ||
|
|
||
| // Mark as processed ONLY after successful side effects. If handleStripeEvent | ||
| // throws, we intentionally skip this call so Stripe will retry. | ||
| await webhookEventRepo.markAsProcessed(event.id, event.type); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify non-atomic check-then-act flow in controller and write semantics in repository.
rg -n "hasBeenProcessed\\(|handleStripeEvent\\(|markAsProcessed\\(" src/modules/payment/controllers/paymentController.ts -C 3
rg -n "markAsProcessed|\\.set\\(" src/modules/payment/WebhookEventRepository.ts -C 3Repository: Cherry-CIC/cherry-Backend
Length of output: 1471
🏁 Script executed:
#!/bin/bash
# Inspect hasBeenProcessed implementation and collection/doc usage.
rg -n "hasBeenProcessed\\(" src/modules/payment/WebhookEventRepository.ts -n
rg -n "async hasBeenProcessed|hasBeenProcessed" src/modules/payment/WebhookEventRepository.ts -n -C 5Repository: Cherry-CIC/cherry-Backend
Length of output: 1070
🏁 Script executed:
#!/bin/bash
# Show relevant portion around hasBeenProcessed and the COLLECTION/doc key.
sed -n '1,140p' src/modules/payment/WebhookEventRepository.tsRepository: Cherry-CIC/cherry-Backend
Length of output: 1819
Fix race-prone webhook idempotency by using an atomic “claim” instead of get-then-set
hasBeenProcessed()usesdoc(eventId).get()andmarkAsProcessed()usesdoc(eventId).set(...)afterhandleStripeEvent; concurrent duplicate deliveries can both seedoc.exists === falseand both run side effects before either write completes, so strict idempotency under concurrency isn’t guaranteed.- The repository’s comment about Firestore “safely serialised” duplicates doesn’t hold for this current read-then-write flow with plain
.set().
Use an atomic claim before side effects (e.g., doc(eventId).create(record) and proceed only on success; otherwise treat as duplicate) or a transaction/conditional write.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/payment/controllers/paymentController.ts` around lines 136 - 169,
The current get-then-set idempotency (webhookEventRepo.hasBeenProcessed +
markAsProcessed) is race-prone; replace it with an atomic "claim" before running
side effects: add a new repository method (e.g.,
webhookEventRepo.claimProcessing(event.id, { type: event.type, receivedAt }))
that uses Firestore doc.create(...) or a conditional transaction to create the
marker only if it doesn't exist, then in the controller call claimProcessing and
proceed to new WebhookService().handleStripeEvent(event) only when the claim
succeeds; if claim fails because the doc already exists, treat it as a duplicate
and return the same 200 duplicate response, and keep existing error handling for
real Firestore errors. Ensure markAsProcessed is no longer relied on for the
primary idempotency guard (it can still update the claim after successful
processing if you want to store result metadata).
Comment on lines
+241
to
+261
| it('safely accepts and marks an unsupported event type without error', async () => { | ||
| // WebhookService.handleStripeEvent logs unsupported types and returns normally. | ||
| // The controller should not error out — Stripe must receive 200. | ||
| const event = buildStripeEvent('customer.subscription.created'); | ||
| mockCreateWebhook.mockReturnValue(event); | ||
| mockHasBeenProcessed.mockResolvedValue(false); | ||
| // The real WebhookService would just log and return void for unknown types. | ||
| mockHandleStripeEvent.mockResolvedValue(undefined); | ||
| mockMarkAsProcessed.mockResolvedValue(undefined); | ||
|
|
||
| const req: any = { | ||
| headers: { 'stripe-signature': 'valid_sig' }, | ||
| body: Buffer.from('{}'), | ||
| }; | ||
| const res = createResponse(); | ||
|
|
||
| await stripeWebhook(req, res); | ||
|
|
||
| expect(mockHandleStripeEvent).toHaveBeenCalledWith(event); | ||
| expect(res.status).toHaveBeenCalledWith(200); | ||
| }); |
There was a problem hiding this comment.
Assert markAsProcessed for unsupported event types.
The test description says unsupported events are marked, but it never verifies mockMarkAsProcessed was called. Add that assertion to lock idempotency behavior for ignored events.
Proposed test assertion
await stripeWebhook(req, res);
expect(mockHandleStripeEvent).toHaveBeenCalledWith(event);
+ expect(mockMarkAsProcessed).toHaveBeenCalledWith(event.id, event.type);
expect(res.status).toHaveBeenCalledWith(200);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/payment/tests/stripeWebhook.test.ts` around lines 241 - 261, The
test "safely accepts and marks an unsupported event type without error"
documents that unsupported Stripe events should be marked processed but never
asserts it; update the test in stripeWebhook.test.ts to assert that
mockMarkAsProcessed was called (with the event id or the built event) after
stripeWebhook runs so idempotency is enforced; locate the test using the it(...)
block and add an expectation like
expect(mockMarkAsProcessed).toHaveBeenCalledWith(event) (or toHaveBeenCalled()
if id not available) after the existing mockHandleStripeEvent and res.status
assertions.
Contributor
|
Hey @1337Xcode thank you for your contribution! I certainly will. Please bear with but I appreciate your work and will review this soon. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #22
Summary
Hardens
POST /api/payment/webhookagainst duplicate event delivery, signature forgery, and incorrect order state transitions.The critical fix here is a wrong webhook secret bug that left all webhook calls effectively unverified. Everything else builds on top of that.
Why
Stripe retries webhook events on failure. Without idempotency, duplicate deliveries could produce duplicate orders, shipments, or donation records. Without signature verification using the correct secret, the endpoint accepted any POST request.
What changed
Bug fix: wrong webhook secret (critical)
createWebhookwas verifying signatures againstSTRIPE_SECRET_KEYinstead ofSTRIPE_WEBHOOK_SECRET. These are different credentials. Fixed. The server now refuses to start ifSTRIPE_WEBHOOK_SECRETis missing from the environment.Idempotency via WebhookEventRepository
Processed event IDs are written to Firestore (
stripe_webhook_events) using the event ID as the document key (O(1) read, no query). Duplicate deliveries return200immediately with no side effects.markAsProcessedonly runs after successful handling, so if business logic throws, the event stays unprocessed and Stripe retries correctly.Event handling via WebhookService
Each subscribed event type has a dedicated handler:
payment_intent.succeededpaymentStatus: succeeded. No-op if already succeeded or no matching order.payment_intent.processingpaymentStatus: processingonly when currentlypending. Never overwrites a terminal state, never touches order fulfilment.payment_intent.payment_failedpaymentStatus/status: failed. Guards against overwriting a succeeded order.payment_intent.canceledpayment_failed.200so Stripe stops retrying.No orders are created from webhook events. This handler is reconciliation only.
Backwards-compatible PaymentStatus expansion
Added
'processing'to thePaymentStatusunion inOrder.ts. Existing Firestore documents only containpending | succeeded | failed, so the|| 'pending'fallback inOrderRepository.getOrdersByDateRangehandles them safely. Both files have a TODO noting when the fallback can be removed.Raw body
express.raw()was already applied on the webhook route. Confirmed correct, no change needed.Tests
23/23 passing. New cases added:
payment_intent.succeededaccepted400)400200, no side effects)payment_intent.payment_failedhandled safelypayment_intent.canceledhandled safely500and does not mark event as processed (retry safety)Also fixes a pre-existing issue: added
"types": ["jest", "node"]totsconfig.jsonso@types/jestresolves correctly under ts-jest, which was blocking the whole test suite.Files changed
src/shared/config/stripeConfig.tssrc/modules/order/model/Order.tssrc/modules/order/repositories/OrderRepository.tsgetOrderByPaymentIntentId()src/modules/payment/WebhookEventRepository.tssrc/modules/payment/services/WebhookService.tssrc/modules/payment/controllers/paymentController.tsstripeWebhooksrc/modules/payment/tests/stripeWebhook.test.tstsconfig.jsonSummary by CodeRabbit
New Features
Bug Fixes