♻️ Migrate webhook to app router #1442
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe pull request introduces substantial modifications to the Stripe webhook handling in the Changes
Possibly related PRs
Warning Rate limit exceeded@lukevella has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/web/src/app/api/stripe/webhook/route.ts (1)
Line range hint
47-58: Add error handling for metadata parsing incheckout.session.completedThe code directly parses
checkoutSession.metadatausingcheckoutMetadataSchema.parse(checkoutSession.metadata);without handling potential validation errors. If the metadata does not conform to the schema or is missing,parsewill throw an exception, causing the function to fail unexpectedly.Consider using
safeParseand handling the result, or wrap the parsing in a try-catch block to manage validation errors gracefully.Apply this diff to add error handling:
+try { + const { userId } = checkoutMetadataSchema.parse(checkoutSession.metadata); + if (!userId) { + return NextResponse.json( + { error: "Missing user ID in metadata" }, + { status: 400 }, + ); + } +} catch (err) { + Sentry.captureException(err); + return NextResponse.json( + { error: "Invalid metadata format" }, + { status: 400 }, + ); +} - -const { userId } = checkoutMetadataSchema.parse(checkoutSession.metadata); - -if (!userId) { - return NextResponse.json( - { error: "Missing client reference ID" }, - { status: 400 }, - ); -}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
apps/web/src/app/api/stripe/webhook/route.ts (3)
19-21: Add type safety and validation to timestamp conversionThe helper function could be more robust with proper typing and validation.
Consider this improvement:
-function toDate(date: number) { +function toDate(timestamp: number): Date { + if (timestamp < 0) { + throw new Error('Invalid timestamp: negative values are not allowed'); + } return new Date(timestamp * 1000); }
135-142: Consolidate error handling patternsThe error handling pattern is duplicated across different sections. Consider extracting common validation logic.
Create a helper function:
function validateMetadata<T>( schema: z.ZodSchema<T>, data: unknown, errorMessage: string ): T { const result = schema.safeParse(data); if (!result.success) { throw new Error(errorMessage); } return result.data; }
169-171: Ensure proper cleanup with waitUntilThe PostHog shutdown is correctly deferred using waitUntil, but there might be other cleanup tasks to consider.
Consider creating a comprehensive cleanup function:
+async function cleanup() { + return Promise.all([ + posthog?.shutdown(), + // Add other cleanup tasks here + ]); +} - waitUntil(Promise.all([posthog?.shutdown()])); + waitUntil(cleanup());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/src/app/api/stripe/webhook/route.ts(7 hunks)
🔇 Additional comments (1)
apps/web/src/app/api/stripe/webhook/route.ts (1)
Line range hint 48-166: Verify webhook event coverage
Let's ensure we're handling all necessary Stripe webhook events for subscription management.
✅ Verification successful
Let me try a different search strategy to find Stripe webhook event handling patterns.
Based on the search results, I can now generate the final response since we have found the key webhook event handlers in the codebase.
Stripe webhook handler implementation is complete and well-structured
The current implementation handles all essential subscription-related webhook events:
checkout.session.completedfor initial subscription creationcustomer.subscription.created,customer.subscription.updated, andcustomer.subscription.deletedfor subscription lifecycle management
These events cover the complete subscription management flow from initial checkout to subscription updates and cancellations. Each case includes proper error handling, database updates, and analytics tracking.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other Stripe webhook handlers in the codebase to ensure consistent coverage
# Look for Stripe event handling patterns
rg -t typescript "case \".*?\".*?stripe" --multiline
# Look for subscription-related constants or types
ast-grep --pattern 'type StripeWebhookEvents = $$$'
Length of output: 4396
Script:
#!/bin/bash
# Search for Stripe webhook event handlers with better pattern matching
rg -t ts "case.*checkout\.session|case.*customer\.subscription" -A 2
# Look for Stripe event type definitions or constants
rg -t ts "type.*Stripe.*Event"
# Search for any Stripe-related webhook documentation or configuration
fd README.md | xargs rg -i "stripe.*webhook"
Length of output: 823
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/web/src/app/api/stripe/webhook/route.ts (1)
Line range hint
67-80: Consider using a transaction for user updateThe user update operation should be wrapped in a transaction to ensure data consistency in case of failures.
- await prisma.user.update({ + await prisma.$transaction(async (tx) => { + await tx.user.update({ where: { id: userId, }, data: { customerId: checkoutSession.customer as string, subscriptionId: checkoutSession.subscription as string, }, - }); + }); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/src/app/api/stripe/webhook/route.ts(7 hunks)
🔇 Additional comments (6)
apps/web/src/app/api/stripe/webhook/route.ts (6)
19-21: LGTM: Helper function is well-defined
The toDate helper function correctly handles Unix timestamp conversion to JavaScript Date objects.
23-46: Remove non-null assertion from signature validation
The ! operator on stripe-signature header is unsafe. A missing signature should be handled explicitly.
The previous review comment about improving webhook validation robustness is still valid. Please refer to the suggested changes in that comment.
137-144: LGTM: Proper schema validation and error handling
The metadata validation using zod is well implemented with appropriate error handling.
171-171: LGTM: Proper cleanup handling
The use of waitUntil ensures that PostHog shutdown is properly handled before the response is sent.
165-168: Return 200 for unhandled event types
The previous review comment about returning 200 status code for unhandled events is still valid. This will prevent unnecessary webhook retries from Stripe.
103-105: Verify business logic for past_due status
Please confirm if treating 'past_due' subscriptions as active aligns with the business requirements. This could affect billing and access control.
✅ Verification successful
Treating 'past_due' status as active is consistent with schema design
The verification shows that 'past_due' is a valid subscription status defined in the database schema's SubscriptionStatus enum, alongside 'active' and 'trialing'. The implementation in the webhook handler aligns with the database design, suggesting this is an intentional business decision to treat past_due subscriptions as active, likely to provide a grace period for payment issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any access control logic that might be affected by this change
rg -l 'subscription.*status.*active' | xargs rg 'past_due'
Length of output: 325
Script:
#!/bin/bash
# Search for subscription status handling patterns across the codebase
rg -A 5 'subscription.*status' --type ts
Length of output: 689
Script:
#!/bin/bash
# Let's check the Stripe webhook handler implementation and any related subscription status checks
ast-grep --pattern 'async function $_(req: Request) {
$$$
subscription.status $$$
$$$
}'
Length of output: 96
Script:
#!/bin/bash
# Let's check the database schema and any type definitions for subscription status
cat packages/database/prisma/schema.prisma
Length of output: 7760
Summary by CodeRabbit
New Features
Bug Fixes
Documentation