invoiceId dedupe key between stripe and /track/sale - #4225
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughInvoice idempotency now uses workspace-scoped Redis keys across track-sale processing and Stripe webhooks, while checking legacy Stripe-only keys during the transition. ChangesInvoice idempotency migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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
`@apps/web/app/`(ee)/api/stripe/integration/webhook/checkout-session-completed.ts:
- Around line 340-354: Move the legacy deduplication checks to immediately after
extracting invoiceId, before any customer, lead, promotion, or other mutable
work. Apply this in
apps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.ts
at lines 340-354 and
apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts at lines
142-152, preserving the existing duplicate response behavior.
In `@apps/web/lib/api/conversions/track-sale.ts`:
- Around line 69-81: Update the cachedResponse branch in the track-sale flow to
validate or discriminate the value as a trackSaleResponse before returning it;
never return Stripe webhook metadata markers from invoiceDedupeKey. When
validation fails, return the existing inert idempotent result, while preserving
the legacyRecord handling.
- Around line 716-720: Update the _trackSale flow so invoiceDedupeKey is not
written as completed before all waitUntil-scheduled sale side effects and
persistence finish successfully. Separate in-progress from completed state, or
move the completed-key write into a success path that runs only after durable
work succeeds, with retry-safe handling that leaves failed sales eligible for
reprocessing.
- Around line 716-720: The shared invoice deduplication key must distinguish an
in-progress claim from a completed sale: in
apps/web/lib/api/conversions/track-sale.ts:716-720, move publishing the
completed response until the scheduled sale work succeeds; in
apps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.ts:356-385
and apps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.ts:154-182,
release or expire the processing claim whenever sale or invoice processing fails
so retries remain possible.
🪄 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: bbd9389c-5612-4e22-8a00-e53a13eb4768
📒 Files selected for processing (4)
apps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.tsapps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.tsapps/web/lib/api/conversions/invoice-idempotency.tsapps/web/lib/api/conversions/track-sale.ts
|
@coderabbitai full review. any breaking changes? |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/web/lib/api/conversions/track-sale.ts`:
- Around line 69-88: Update the deduplication handling in the track-sale flow to
schema-validate legacyRecord with trackSaleResponseSchema.safeParse before
returning the inert placeholder. Return the parsed legacy response when valid,
while preserving the existing cachedResponse handling and fallback for webhook
marker records or invalid legacy data.
🪄 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: 03e4905a-727f-48fc-b744-e4562dcf3544
📒 Files selected for processing (4)
apps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.tsapps/web/app/(ee)/api/stripe/integration/webhook/invoice-paid.tsapps/web/lib/api/conversions/invoice-idempotency.tsapps/web/lib/api/conversions/track-sale.ts
| const [cachedResponse, legacyRecord] = await redis.mget([ | ||
| invoiceDedupeKey(workspace.id, invoiceId), | ||
| legacyStripeInvoiceDedupeKey(invoiceId), | ||
| ]); | ||
|
|
||
| if (cachedResponse) { | ||
| return cachedResponse; | ||
| const parsedCachedResponse = | ||
| trackSaleResponseSchema.safeParse(cachedResponse); | ||
|
|
||
| if (parsedCachedResponse.success) { | ||
| return parsedCachedResponse.data; | ||
| } | ||
| } | ||
|
|
||
| if (cachedResponse || legacyRecord) { | ||
| return { | ||
| eventName, | ||
| customer: null, | ||
| sale: null, | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve valid track-sale responses stored under the legacy key.
The legacy key may contain a serialized trackSaleResponse from pre-migration track-sale requests, not only Stripe webhook markers. Because only cachedResponse is schema-validated, retries with a valid legacyRecord incorrectly return the empty placeholder response.
Parse legacyRecord with trackSaleResponseSchema.safeParse before falling back to the inert result; webhook marker objects will still fail validation.
Proposed fix
if (cachedResponse) {
const parsedCachedResponse =
trackSaleResponseSchema.safeParse(cachedResponse);
if (parsedCachedResponse.success) {
return parsedCachedResponse.data;
}
}
+ const parsedLegacyResponse =
+ trackSaleResponseSchema.safeParse(legacyRecord);
+
+ if (parsedLegacyResponse.success) {
+ return parsedLegacyResponse.data;
+ }
+
if (cachedResponse || legacyRecord) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [cachedResponse, legacyRecord] = await redis.mget([ | |
| invoiceDedupeKey(workspace.id, invoiceId), | |
| legacyStripeInvoiceDedupeKey(invoiceId), | |
| ]); | |
| if (cachedResponse) { | |
| return cachedResponse; | |
| const parsedCachedResponse = | |
| trackSaleResponseSchema.safeParse(cachedResponse); | |
| if (parsedCachedResponse.success) { | |
| return parsedCachedResponse.data; | |
| } | |
| } | |
| if (cachedResponse || legacyRecord) { | |
| return { | |
| eventName, | |
| customer: null, | |
| sale: null, | |
| }; | |
| const [cachedResponse, legacyRecord] = await redis.mget([ | |
| invoiceDedupeKey(workspace.id, invoiceId), | |
| legacyStripeInvoiceDedupeKey(invoiceId), | |
| ]); | |
| if (cachedResponse) { | |
| const parsedCachedResponse = | |
| trackSaleResponseSchema.safeParse(cachedResponse); | |
| if (parsedCachedResponse.success) { | |
| return parsedCachedResponse.data; | |
| } | |
| } | |
| const parsedLegacyResponse = | |
| trackSaleResponseSchema.safeParse(legacyRecord); | |
| if (parsedLegacyResponse.success) { | |
| return parsedLegacyResponse.data; | |
| } | |
| if (cachedResponse || legacyRecord) { | |
| return { | |
| eventName, | |
| customer: null, | |
| sale: null, | |
| }; |
🤖 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 `@apps/web/lib/api/conversions/track-sale.ts` around lines 69 - 88, Update the
deduplication handling in the track-sale flow to schema-validate legacyRecord
with trackSaleResponseSchema.safeParse before returning the inert placeholder.
Return the parsed legacy response when valid, while preserving the existing
cachedResponse handling and fallback for webhook marker records or invalid
legacy data.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/web/lib/api/conversions/track-sale.ts`:
- Around line 65-70: Update the cached-response branch in the track-sale flow to
validate cachedResponse with trackSaleResponseSchema.safeParse before returning
it. Return the parsed track-sale response only when validation succeeds;
otherwise continue through the inert idempotent fallback rather than returning
shared invoice-key marker objects.
- Around line 65-70: Update the deduplication lookup around invoiceDedupeKey to
read both the workspace-scoped key and legacyStripeInvoiceDedupeKey, preserving
the legacy key during migration. Schema-validate each stored response and return
the first valid cached response; continue processing only when neither lookup
yields a valid result.
🪄 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: 0a57171a-0dc2-4a57-8ce6-86f0d893f597
📒 Files selected for processing (1)
apps/web/lib/api/conversions/track-sale.ts
| const cachedResponse = await redis.get( | ||
| `trackSale:${workspace.id}:invoiceId:${invoiceId}`, | ||
| invoiceDedupeKey(workspace.id, invoiceId), | ||
| ); | ||
|
|
||
| if (cachedResponse) { | ||
| return cachedResponse; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
Restore cached-response validation before returning.
Line 69 returns any value stored under the shared invoice key, including Stripe webhook marker objects. Restore trackSaleResponseSchema.safeParse(cachedResponse) and return only validated track-sale responses; otherwise preserve the inert idempotent fallback.
🤖 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 `@apps/web/lib/api/conversions/track-sale.ts` around lines 65 - 70, Update the
cached-response branch in the track-sale flow to validate cachedResponse with
trackSaleResponseSchema.safeParse before returning it. Return the parsed
track-sale response only when validation succeeds; otherwise continue through
the inert idempotent fallback rather than returning shared invoice-key marker
objects.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the legacy Stripe-key read during migration.
This lookup only checks invoiceDedupeKey(workspace.id, invoiceId), but legacyStripeInvoiceDedupeKey(invoiceId) remains part of the migration contract. Existing invoices stored under the legacy key will miss deduplication and may be processed again. Read both keys and schema-validate either stored response before continuing.
🤖 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 `@apps/web/lib/api/conversions/track-sale.ts` around lines 65 - 70, Update the
deduplication lookup around invoiceDedupeKey to read both the workspace-scoped
key and legacyStripeInvoiceDedupeKey, preserving the legacy key during
migration. Schema-validate each stored response and return the first valid
cached response; continue processing only when neither lookup yields a valid
result.
2 participants
Summary by CodeRabbit