Handle checkout session creation errors#59
Conversation
Co-authored-by: dogesman098 <dogesman098@gmail.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughRemoved explicit Stripe API version in two API endpoints so the Stripe client uses the library default. Switched package.json script runners from Bun-based wrappers to direct tsx/node commands. No control flow or public API signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @otdoges's task —— View job Code Review for PR: Handle checkout session creation errorsReview Checklist:
📋 SummaryThis PR addresses API 404 errors and JSON parsing issues by:
✅ Positive ChangesStripe API Version Fix
Package Script Reliability
🔍 Code Quality AssessmentSecurity ⭐⭐⭐⭐⭐
Error Handling ⭐⭐⭐⭐⭐
Code Structure ⭐⭐⭐⭐⭐
🚨 Areas for ImprovementMinor Issues
Performance Considerations
🧪 Testing RecommendationsMissing Test Coverage
Integration Testing
🔧 Technical DebtScript Migration Follow-upThe package.json changes suggest a migration from Bun to Node.js. Consider:
📊 Overall Assessment
✅ RecommendationAPPROVE - This PR successfully addresses the stated issues and maintains high code quality standards. The fixes are critical for preventing runtime errors, and the script changes improve development reliability. Suggested Follow-ups (non-blocking):
Branch: |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (9)
api/create-checkout-session.ts (5)
6-12: Fix CORS: wildcard origin with credentials is invalidAccess-Control-Allow-Credentials cannot be true when Access-Control-Allow-Origin is "*". Also add Vary: Origin for proper caching.
Apply:
- const origin = allowOrigin ?? process.env.PUBLIC_ORIGIN ?? '*'; - res.setHeader('Access-Control-Allow-Origin', origin); - res.setHeader('Access-Control-Allow-Credentials', 'true'); + const origin = allowOrigin ?? process.env.PUBLIC_ORIGIN ?? '*'; + const allowCredentials = origin !== '*'; + res.setHeader('Access-Control-Allow-Origin', origin); + res.setHeader('Access-Control-Allow-Credentials', String(allowCredentials)); + res.setHeader('Vary', 'Origin');
58-61: Avoid logging PII (email) in server logsLogging raw emails is unnecessary and leaks PII. Log a redacted token or omit.
- console.log('Token verified for user:', authenticatedUserId, 'email:', userEmail); + console.log('Token verified for user:', authenticatedUserId);
74-80: Guard against missing email before calling StripeIf userEmail is absent, customers.list({ email: undefined }) may degrade to an unfiltered list, risking mismatched customers. Fail fast.
if (!authenticatedUserId) { return withCors(res, allowedOrigin).status(401).json({ error: 'Unauthorized', message: 'User ID not found in token' }); } + if (!userEmail) { + return withCors(res, allowedOrigin).status(400).json({ + error: 'Bad Request', + message: 'Authenticated user email is required for billing' + }); + }
81-89: Validate and sanitize request body (planId, period)Harden input using a schema (allowed plan IDs, allowed periods) instead of free-form destructuring.
- // Parse request body to get the plan information - const { planId, period = 'month' } = req.body; - - if (!planId) { - return withCors(res, allowedOrigin).status(400).json({ - error: 'Bad Request', - message: 'planId is required' - }); - } + // Parse and validate request body + const parseResult = CheckoutRequestSchema.safeParse(req.body); + if (!parseResult.success) { + return withCors(res, allowedOrigin).status(400).json({ + error: 'Bad Request', + message: 'Invalid request body', + issues: parseResult.error.issues.map(i => ({ path: i.path, message: i.message })) + }); + } + const { planId, period } = parseResult.data;Add the following outside this hunk (e.g., near imports or above handler):
import { z } from 'zod'; const CheckoutRequestSchema = z.object({ planId: z.enum(['starter', 'pro']), period: z.enum(['month', 'year']).default('month'), });
187-191: Rename response property and include Stripe customer ID for clarityThe API currently returns the application user’s ID under
customer_id, which is misleading. Update both endpoints to:
- Rename
customer_id→user_id- Add
stripe_customer_id→ the actualcustomer.idfrom Stripe- Preserve
session_idandurlfieldsLocations to update:
- api/create-checkout-session.ts (around lines 187–191)
- api/create-portal-session.ts (around lines 116–118)
Suggested diffs:
// api/create-checkout-session.ts return withCors(res, allowedOrigin).status(200).json({ url: session.url, - customer_id: authenticatedUserId, + user_id: authenticatedUserId, + stripe_customer_id: customer.id, session_id: session.id, });// api/create-portal-session.ts return withCors(res, allowedOrigin).status(200).json({ url: session.url, - customer_id: authenticatedUserId, + user_id: authenticatedUserId, + stripe_customer_id: customer.id, session_id: session.id, });Don’t forget to update any client code or TypeScript types that consume
customer_id.api/create-portal-session.ts (4)
6-12: Fix CORS: wildcard origin with credentials is invalidSame as the checkout endpoint: don’t send credentials with "*"; add Vary: Origin.
- const origin = allowOrigin ?? process.env.PUBLIC_ORIGIN ?? '*'; - res.setHeader('Access-Control-Allow-Origin', origin); - res.setHeader('Access-Control-Allow-Credentials', 'true'); + const origin = allowOrigin ?? process.env.PUBLIC_ORIGIN ?? '*'; + const allowCredentials = origin !== '*'; + res.setHeader('Access-Control-Allow-Origin', origin); + res.setHeader('Access-Control-Allow-Credentials', String(allowCredentials)); + res.setHeader('Vary', 'Origin');
58-61: Avoid logging PII (email) in server logsRemove the email from logs.
- console.log('Token verified for user:', authenticatedUserId, 'email:', userEmail); + console.log('Token verified for user:', authenticatedUserId);
74-80: Guard against missing email before listing customersFail fast if userEmail is missing; otherwise the list call can behave unexpectedly.
if (!authenticatedUserId) { return withCors(res, allowedOrigin).status(401).json({ error: 'Unauthorized', message: 'User ID not found in token' }); } + if (!userEmail) { + return withCors(res, allowedOrigin).status(400).json({ + error: 'Bad Request', + message: 'Authenticated user email is required for billing' + }); + }
114-118: Rename and clarify response fields: useuser_idandstripe_customer_idThe handler currently returns the authenticated user’s ID under
customer_id, which is misleading since we also have a Stripe customer ID. Update the JSON shape inapi/create-portal-session.ts(lines 114–118) as follows:- return withCors(res, allowedOrigin).status(200).json({ - url: session.url, - customer_id: authenticatedUserId, - session_id: session.id, - }); + return withCors(res, allowedOrigin).status(200).json({ + url: session.url, + user_id: authenticatedUserId, + stripe_customer_id: customer.id, + session_id: session.id, + });• A global
rg -n '\bcustomer_id\b'only finds matches in
api/create-portal-session.tsandapi/create-checkout-session.ts,
so the frontend does not rely on this field.
• For consistency, consider applying the same rename inapi/create-checkout-session.ts(lines 188–191).
🧹 Nitpick comments (3)
api/create-checkout-session.ts (2)
121-129: Remove unreachable ‘enterprise’ price map entryYou already block enterprise purchases earlier (Lines 98-104). Keeping it in the map is misleading.
- 'enterprise': { - 'month': process.env.STRIPE_ENTERPRISE_MONTHLY_PRICE_ID || 'price_enterprise_monthly', - 'year': process.env.STRIPE_ENTERPRISE_YEARLY_PRICE_ID || 'price_enterprise_yearly', - },
167-177: Prefer automatic payment methods over hardcoding ['card']Stripe recommends automatic_payment_methods for broader coverage (Apple Pay, wallets, etc.) without extra config.
- const session = await stripe.checkout.sessions.create({ + const session = await stripe.checkout.sessions.create({ customer: customer.id, - payment_method_types: ['card'], + automatic_payment_methods: { enabled: true }, line_items: [ { price: stripePriceId, quantity: 1, }, ], mode: 'subscription',package.json (1)
15-15: Align dev:full with chosen package managerGiven packageManager is Bun (and a prior learning prefers Bun for local dev), avoid mixing npm-run inside scripts. It can cause environment drift.
- "dev:full": "concurrently \"npm run dev:api\" \"npm run dev\"", + "dev:full": "concurrently \"bun run dev:api\" \"bun run dev\"",If you prefer pnpm/npm instead, I can generate a consistent variant across all scripts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
api/create-checkout-session.ts(1 hunks)api/create-portal-session.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/convex-security.mdc)
**/*.{ts,tsx}: All Convex queries and mutations MUST use proper authentication. Never accept user IDs from client parameters.
Always verify user owns the data before allowing access.
Use the authenticated user's identity.subject for user references.
Implement proper error messages that don't leak information.
Authentication verification in every function.
Authorization checks for data ownership.
Input validation and sanitization.
Error handling without information leakage.
**/*.{ts,tsx}: Use Sonner for toast notifications to provide consistent user feedback, including success, error, and loading states.
Always handle errors gracefully using try-catch blocks in asynchronous functions, providing user feedback and logging errors.
Provide specific, actionable error messages for form validation errors using toast notifications.
Handle common network error scenarios in catch blocks, providing appropriate toast messages for network errors, authentication errors, and unexpected errors.If using TypeScript, use an enum to store flag names.
Strict TypeScript must be used with no 'any' types allowed
**/*.{ts,tsx}: NEVER useanytype - use proper TypeScript types
Useunknownfor truly unknown data types
Implement proper interface definitions
Do not use empty interfaces; use a type alias instead (e.g.,type InputProps = ...instead ofinterface InputProps {})
All function parameters must be typed
All return types should be explicit for public APIs
Use proper generic constraints
Implement discriminated unions for state management
Use proper interface definitions for error handling types (e.g.,interface ValidationResult { isValid: boolean; error?: string; })
**/*.{ts,tsx}: Always sanitize user input before storing or displaying using a sanitization function likesanitizeText.
Implement comprehensive input validation, including length checks and detection of malicious patterns, as shown in thevalidateInputfunction.
Define and use security constants suc...
Files:
api/create-portal-session.tsapi/create-checkout-session.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/posthog-integration.mdc)
**/*.{js,jsx,ts,tsx}: Use a consistent naming convention for this storage. enum/const object members should be written UPPERCASE_WITH_UNDERSCORE.
If a custom property for a person or event is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object, as above in feature flags.
Files:
api/create-portal-session.tsapi/create-checkout-session.ts
🧠 Learnings (1)
📚 Learning: 2025-08-12T03:04:41.035Z
Learnt from: CR
PR: otdoges/zapdev#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T03:04:41.035Z
Learning: Prefer Bun as the package manager (fallback to pnpm/npm) for local development
Applied to files:
package.json
🔇 Additional comments (2)
package.json (1)
13-13: dev:api via tsx: good changeDirectly using tsx improves local reliability over bunx wrappers while keeping fast reloads.
api/create-portal-session.ts (1)
88-88: Stripe client init without apiVersion: good and consistentMatches the checkout endpoint change and avoids invalid version errors.
- Resolved package-lock.json merge conflict by regenerating lock file - All other changes merged automatically from main branch
|
Claude finished @otdoges's task —— View job PR Review: Handle checkout session creation errorsReview Checklist:
📋 SummaryThis PR addresses critical API 404 errors and JSON parsing issues through two main changes:
✅ Positive ChangesCritical Stripe API Fix ⭐⭐⭐⭐⭐
Package Script Reliability ⭐⭐⭐⭐
🚨 Critical Issues Found🔴 Incomplete Stripe API Version RemovalThe invalid
Impact: These files will continue to cause runtime errors until fixed. 🔴 Package.json Script IssuesLines 22, 24: Scripts attempt to run TypeScript files with "test-ai": "node src/test-ai-implementation.ts",
"validate": "node src/validate-ai.ts"Issue: Node.js cannot execute TypeScript files directly 🔍 Security Analysis ⭐⭐⭐⭐⭐Authentication & Authorization
Input Validation
CORS Implementation
Error Handling
🔧 Code Quality AssessmentTypeScript Usage ⭐⭐⭐⭐⭐
Code Structure ⭐⭐⭐⭐⭐
Error Handling ⭐⭐⭐⭐⭐
⚡ Performance ConsiderationsStripe Customer Lookup ⭐⭐⭐
API Response Times ⭐⭐⭐⭐
🔄 Logical IssuesEnterprise Plan Inconsistency
Customer Creation vs Portal Access
📊 Test Coverage RecommendationsMissing Test Scenarios
Integration Testing
📈 Overall Assessment
🎯 RecommendationsImmediate (Blocking)
High Priority
Medium Priority
✅ VerdictCONDITIONAL APPROVAL - The core fixes are excellent and address critical runtime issues. However, incomplete Stripe API version removal and package.json script issues need to be resolved before merge. Must-fix before merge:
Follow-up tasks:
| Branch: |
Fixes API 404 errors and frontend JSON parsing issues by correcting API server startup scripts and removing an invalid Stripe API version.
The 404s and JSON parse errors were caused by the API dev server failing to start due to incorrect
buninvocations inpackage.jsonscripts. This PR updates the scripts to usetsxandnodefor reliability. Additionally, an invalidapiVersionwas removed from Stripe API calls to prevent runtime errors.Summary by CodeRabbit