Delay PayPal, Tremendous, and stablecoin payouts until charge funds settle - #4043
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces a stablecoin-specific payout scheduling flow with a generic settlement-timing mechanism. A new ChangesSettlement-Timing Payout Overhaul
Sequence DiagramsequenceDiagram
participant QStash
participant Route as POST /charge-succeeded
participant DB as Prisma DB
participant Stripe as Stripe API
participant Scheduler as scheduleDelayedPayouts
participant Queues as queueStripe / sendPaypal / queueTremendous
QStash->>Route: POST with Stripe invoice event
Route->>DB: Update processing payout methods where method IS NULL
Route->>Stripe: List balance transactions for charge
Stripe-->>Route: balance transaction with available_on
alt funds already available
Route->>Queues: allSettled with fundsAvailable true
else funds not yet settled
Route->>DB: Group processing payout methods
Route->>Scheduler: scheduleDelayedPayouts with scheduledAt
Route->>Queues: allSettled with fundsAvailable false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts (1)
101-104: 💤 Low valueMisleading comment: "15 minutes from now" should be "15 minutes after available_on".
The comment says "15 minutes from now" but the code calculates 15 minutes after
availableOnMs(the Stripeavailable_ontimestamp), not 15 minutes from the current time.📝 Suggested fix
return { fundsAvailable: false, - scheduledAt: new Date(availableOnMs + 15 * 60 * 1000), // 15 minutes from now + scheduledAt: new Date(availableOnMs + 15 * 60 * 1000), // 15 minutes after available_on };🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts around lines 101 - 104, The comment on the scheduledAt line is inaccurate. It currently states "15 minutes from now" but the code actually adds 15 minutes to availableOnMs (the Stripe available_on timestamp), not the current time. Update the comment to correctly describe that the scheduled time is 15 minutes after the available_on timestamp, not 15 minutes from the current moment.apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts (1)
93-96: 💤 Low valueRedundant check: query already guarantees method is in
postSettlementPayoutMethods.The
groupByquery on lines 78-91 filters bymethod: { in: postSettlementPayoutMethods }, so any result inpayoutsByMethodwill have a method in that list. The.some()check will always be true ifpayoutsByMethod.length > 0.♻️ Suggested simplification
- const shouldSchedulePayouts = payoutsByMethod.some( - (p) => - p.method !== null && postSettlementPayoutMethods.includes(p.method), - ); - - if (shouldSchedulePayouts) { + if (payoutsByMethod.length > 0) { await scheduleDelayedPayouts({ invoice, executeAt: fundSettlementTiming.scheduledAt, }); }🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/route.ts around lines 93 - 96, The .some() check in the shouldSchedulePayouts assignment is redundant because the groupBy query on lines 78-91 already filters results by method: { in: postSettlementPayoutMethods }, so every payout in payoutsByMethod is guaranteed to have a valid method. Replace the .some() check that iterates through payoutsByMethod and checks if p.method is in postSettlementPayoutMethods with a simple check for whether payoutsByMethod has any items (payoutsByMethod.length > 0), since the query has already guaranteed the filtering condition.
🤖 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.
Nitpick comments:
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/route.ts:
- Around line 93-96: The .some() check in the shouldSchedulePayouts assignment
is redundant because the groupBy query on lines 78-91 already filters results by
method: { in: postSettlementPayoutMethods }, so every payout in payoutsByMethod
is guaranteed to have a valid method. Replace the .some() check that iterates
through payoutsByMethod and checks if p.method is in postSettlementPayoutMethods
with a simple check for whether payoutsByMethod has any items
(payoutsByMethod.length > 0), since the query has already guaranteed the
filtering condition.
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts:
- Around line 101-104: The comment on the scheduledAt line is inaccurate. It
currently states "15 minutes from now" but the code actually adds 15 minutes to
availableOnMs (the Stripe available_on timestamp), not the current time. Update
the comment to correctly describe that the scheduled time is 15 minutes after
the available_on timestamp, not 15 minutes from the current moment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b953680e-f41d-4857-8fc6-07d39fea5d89
📒 Files selected for processing (5)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/send-paypal-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts
|
@coderabbitai full review please |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== route.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/route.ts
echo ""
echo "=== utils.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/utils.ts
echo ""
echo "=== queue-stripe-payouts.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts
echo ""
echo "=== queue-tremendous-payouts.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.ts
echo ""
echo "=== send-paypal-payouts.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/send-paypal-payouts.tsLength of output: 17632
PR SummaryThis PR changes the payout dispatch strategy for PayPal, Tremendous, and stablecoin payouts to wait until the Stripe charge's funds have actually settled ( Issues Found1. 🔴 Potential infinite scheduling loopFile: When the delayed cron job re-runs, if
There is no guard against re-scheduling if the invoice has already had a delayed job scheduled. This could loop indefinitely. Suggestion: Track a re-schedule count (e.g., pass it through the QStash body and cap at 3–5 retries), or mark payouts as // In the body of the delayed QStash job
body: {
invoiceId: invoice.id,
attempt: (attempt ?? 0) + 1,
},// At the top of POST handler, after fundSettlement check
const MAX_SETTLEMENT_RETRIES = 5;
if (!fundSettlement.fundsAvailable && attempt >= MAX_SETTLEMENT_RETRIES) {
return logAndRespond(`Exceeded max settlement retries for invoice ${invoiceId}.`);
}2. 🔴 Stablecoin aggregate missing
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts (1)
42-44:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winNo retry counter protection against infinite scheduling loops. The QStash job body contains only
invoiceId, so when jobs re-run after timing drift or API delays causefundsAvailable: falseagain, the system reschedules indefinitely with no upper bound.
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts#L42-L44: AddretryCountto the QStash body and enforce a maximum (e.g., 5) before scheduling; return early or log an error if exceeded.apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts#L68-L103: ExtendpayloadSchemato parseretryCount, then pass it through toscheduleDelayedPayouts.🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts around lines 42 - 44, Add retry counter protection to prevent infinite scheduling loops in the QStash job system. In apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts lines 42-44, include retryCount in the QStash job body and enforce a maximum retry limit (e.g., 5) before scheduling; return early or log an error if the limit is exceeded. In apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts lines 68-103, extend the payloadSchema to parse retryCount from the incoming payload and pass it through to the scheduleDelayedPayouts function call so that the retry counter is properly tracked across job re-executions.
🧹 Nitpick comments (6)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts (2)
103-103: 💤 Low valueMisleading comment: the offset is from
available_on, not "from now".The comment says "15 minutes from now" but the calculation is
availableOnMs + 15 * 60 * 1000, which is 15 minutes afteravailable_on.📝 Proposed fix
return { fundsAvailable: false, - scheduledAt: new Date(availableOnMs + 15 * 60 * 1000), // 15 minutes from now + scheduledAt: new Date(availableOnMs + 15 * 60 * 1000), // 15 minutes after available_on };🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts at line 103, The comment on the line with `scheduledAt: new Date(availableOnMs + 15 * 60 * 1000)` incorrectly states "15 minutes from now" when the code actually adds 15 minutes to `availableOnMs` (the available_on timestamp), not to the current time. Update the comment to accurately reflect that the scheduled time is 15 minutes after the available_on time, not 15 minutes from the current moment.
88-88: ⚡ Quick winExtract hardcoded 1-hour fallback delay into a named constant.
The magic number
60 * 60 * 1000reduces readability and maintainability.♻️ Proposed fix
+const FALLBACK_DELAY_MS = 60 * 60 * 1000; // 1 hour + // ... if (balanceTransactions.data.length === 0) { console.log(`No balance transaction found for charge ${chargeId}`); return { fundsAvailable: false, - scheduledAt: new Date(now + 60 * 60 * 1000), // 1 hour from now + scheduledAt: new Date(now + FALLBACK_DELAY_MS), }; }🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts at line 88, Extract the hardcoded magic number 60 * 60 * 1000 (representing 1 hour in milliseconds) into a named constant at the top of the file or module scope. Define this constant with a descriptive name like ONE_HOUR_IN_MS or CHARGE_RETRY_DELAY_MS, then replace the hardcoded value in the scheduledAt line to use this constant instead. This improves code readability and makes it easier to maintain or adjust the delay value in the future.apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts (2)
92-95: 💤 Low valueRedundant check — simplifies to
payoutsByMethod.length > 0.The query at lines 77-90 already filters by
method: { in: postSettlementPayoutMethods }, sopayoutsByMethodwill only contain those methods. The.some()check that re-validates against the same list is unnecessary.♻️ Proposed fix
- const shouldSchedulePayouts = payoutsByMethod.some( - (p) => - p.method !== null && postSettlementPayoutMethods.includes(p.method), - ); + const shouldSchedulePayouts = payoutsByMethod.length > 0;🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/route.ts around lines 92 - 95, The `shouldSchedulePayouts` variable uses a redundant `.some()` check because the query that populates `payoutsByMethod` already filters results by `method: { in: postSettlementPayoutMethods }`, meaning all items in `payoutsByMethod` are guaranteed to have a method within the allowed list. Remove the `.some()` iteration and the redundant null check, and simplify `shouldSchedulePayouts` to just check if `payoutsByMethod.length > 0`.
8-12: ExtractstripeChargeMetadataSchemato a shared module.The schema is defined identically in both
queue-stripe-payouts.ts(lines 12–14) andutils.ts(lines 16–18). Changes to the schema require updates in both files, creating unnecessary maintenance overhead.🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/route.ts around lines 8 - 12, The stripeChargeMetadataSchema is defined identically in both queue-stripe-payouts.ts and utils.ts, creating duplicate code and maintenance overhead. Extract this schema definition to a dedicated shared module, then update both queue-stripe-payouts.ts and utils.ts to import stripeChargeMetadataSchema from the new shared location instead of defining it locally. This eliminates duplication and ensures any future schema changes only need to be made in one place.apps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts (2)
70-76: 💤 Low valueNarrow error type before accessing
.message.In strict TypeScript,
errorin a catch block isunknown. Accessing.messagedirectly may cause type errors or runtime issues if a non-Error is thrown.♻️ Suggested type-safe error handling
} catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); await log({ - message: `Failed to fund Dub's financial account for stablecoin payouts: ${error.message}`, + message: `Failed to fund Dub's financial account for stablecoin payouts: ${errorMessage}`, type: "errors", }); throw error; }🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts around lines 70 - 76, The catch block in the error handling code is accessing error.message on an unknown type without narrowing it first. Add a type guard to check if the error is an instance of Error before accessing the .message property. If it is an Error, use its message; otherwise, convert the error to a string representation (using String(error) or JSON.stringify). Apply this same type-safe pattern to the log call where error.message is accessed.
12-14: Consider consolidating the duplicate schema definition.
stripeChargeMetadataSchemais defined identically in bothutils.ts(line 16) andqueue-stripe-payouts.ts(line 12). The schema should be exported fromutils.tsand imported inqueue-stripe-payouts.tsto avoid drift risk if the schema shape changes.🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts around lines 12 - 14, The schema definition for stripeChargeMetadataSchema is duplicated in two files. Remove the stripeChargeMetadataSchema definition from queue-stripe-payouts.ts (the z.object block defining id as z.string()), then export this same schema from utils.ts where it is already defined, and finally import stripeChargeMetadataSchema from utils.ts into queue-stripe-payouts.ts to use the single source of truth.
🤖 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/cron/payouts/charge-succeeded/queue-stripe-payouts.ts:
- Around line 46-55: The where clause in the stablecoin payout query is missing
a status filter, which causes the aggregate to include payouts with different
statuses on job re-runs, leading to mismatched amounts and Stripe idempotency
failures. Add a status filter with value "processing" to the where object
(alongside the existing invoiceId, method, and amount filters) to ensure only
payouts in the processing state are aggregated, maintaining consistent
calculations across re-runs with the same idempotencyKey.
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/route.ts:
- Around line 68-103: Extend the payload schema in the charge-succeeded route
handler to parse the retryCount parameter from the incoming QStash request body.
Then update the call to scheduleDelayedPayouts (where the invoice and
fundSettlement.scheduledAt are passed) to include the parsed retryCount value as
an additional property in the parameters object, ensuring the value is
propagated through to support the infinite-loop protection logic in the delayed
payout scheduling.
---
Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts:
- Around line 42-44: Add retry counter protection to prevent infinite scheduling
loops in the QStash job system. In
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts lines 42-44,
include retryCount in the QStash job body and enforce a maximum retry limit
(e.g., 5) before scheduling; return early or log an error if the limit is
exceeded. In apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts lines
68-103, extend the payloadSchema to parse retryCount from the incoming payload
and pass it through to the scheduleDelayedPayouts function call so that the
retry counter is properly tracked across job re-executions.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts:
- Around line 70-76: The catch block in the error handling code is accessing
error.message on an unknown type without narrowing it first. Add a type guard to
check if the error is an instance of Error before accessing the .message
property. If it is an Error, use its message; otherwise, convert the error to a
string representation (using String(error) or JSON.stringify). Apply this same
type-safe pattern to the log call where error.message is accessed.
- Around line 12-14: The schema definition for stripeChargeMetadataSchema is
duplicated in two files. Remove the stripeChargeMetadataSchema definition from
queue-stripe-payouts.ts (the z.object block defining id as z.string()), then
export this same schema from utils.ts where it is already defined, and finally
import stripeChargeMetadataSchema from utils.ts into queue-stripe-payouts.ts to
use the single source of truth.
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/route.ts:
- Around line 92-95: The `shouldSchedulePayouts` variable uses a redundant
`.some()` check because the query that populates `payoutsByMethod` already
filters results by `method: { in: postSettlementPayoutMethods }`, meaning all
items in `payoutsByMethod` are guaranteed to have a method within the allowed
list. Remove the `.some()` iteration and the redundant null check, and simplify
`shouldSchedulePayouts` to just check if `payoutsByMethod.length > 0`.
- Around line 8-12: The stripeChargeMetadataSchema is defined identically in
both queue-stripe-payouts.ts and utils.ts, creating duplicate code and
maintenance overhead. Extract this schema definition to a dedicated shared
module, then update both queue-stripe-payouts.ts and utils.ts to import
stripeChargeMetadataSchema from the new shared location instead of defining it
locally. This eliminates duplication and ensures any future schema changes only
need to be made in one place.
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts:
- Line 103: The comment on the line with `scheduledAt: new Date(availableOnMs +
15 * 60 * 1000)` incorrectly states "15 minutes from now" when the code actually
adds 15 minutes to `availableOnMs` (the available_on timestamp), not to the
current time. Update the comment to accurately reflect that the scheduled time
is 15 minutes after the available_on time, not 15 minutes from the current
moment.
- Line 88: Extract the hardcoded magic number 60 * 60 * 1000 (representing 1
hour in milliseconds) into a named constant at the top of the file or module
scope. Define this constant with a descriptive name like ONE_HOUR_IN_MS or
CHARGE_RETRY_DELAY_MS, then replace the hardcoded value in the scheduledAt line
to use this constant instead. This improves code readability and makes it easier
to maintain or adjust the delay value in the future.
🪄 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
Run ID: a2f6ac28-d090-421d-a995-7fc32a4032f6
📒 Files selected for processing (5)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/send-paypal-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts (1)
66-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winType-unsafe access to
error.message.In TypeScript,
errorin catch blocks has typeunknown. Accessing.messagedirectly will cause a type error in strict mode and could logundefinedif the thrown value isn't anErrorinstance.🛡️ Proposed fix
} catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); await log({ - message: `Failed to fund Dub's financial account for stablecoin payouts: ${error.message}`, + message: `Failed to fund Dub's financial account for stablecoin payouts: ${errorMessage}`, type: "errors", }); throw error; }🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts around lines 66 - 73, The catch block accesses error.message directly, but the error variable has type unknown in TypeScript, making this type-unsafe and potentially logging undefined. Replace the direct error.message access with a type-safe approach: check if error is an instance of Error using instanceof, and if so access message; otherwise convert the error to a string using String(error) or similar fallback. Update the log call in the catch block to use this safe message extraction, ensuring the logged message always contains meaningful error information regardless of what type of value was thrown.
🤖 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.
Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts:
- Around line 66-73: The catch block accesses error.message directly, but the
error variable has type unknown in TypeScript, making this type-unsafe and
potentially logging undefined. Replace the direct error.message access with a
type-safe approach: check if error is an instance of Error using instanceof, and
if so access message; otherwise convert the error to a string using
String(error) or similar fallback. Update the log call in the catch block to use
this safe message extraction, ensuring the logged message always contains
meaningful error information regardless of what type of value was thrown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 43aba95b-fc46-4abd-9e38-6eb29da34b5e
📒 Files selected for processing (3)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts
- apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts
|
@coderabbitai final review please, any breaking changes, potential bugs, security issues? |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== route.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/route.ts
echo ""
echo "=== utils.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/utils.ts
echo ""
echo "=== queue-stripe-payouts.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.ts
echo ""
echo "=== queue-tremendous-payouts.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.ts
echo ""
echo "=== send-paypal-payouts.ts ==="
cat apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/send-paypal-payouts.tsLength of output: 18601 🏁 Script executed: #!/bin/bash
echo "=== Other callers of queueStripePayouts ==="
rg "queueStripePayouts" --type=ts -n -B2 -A2
echo ""
echo "=== Other callers of queueTremendousPayouts ==="
rg "queueTremendousPayouts" --type=ts -n -B2 -A2
echo ""
echo "=== Other callers of sendPaypalPayouts ==="
rg "sendPaypalPayouts" --type=ts -n -B2 -A2
echo ""
echo "=== Other callers of scheduleDelayedPayouts ==="
rg "scheduleDelayedPayouts" --type=ts -n -B2 -A2
echo ""
echo "=== payloadSchema / invoiceId parsing in route.ts (check if attempt field exists) ==="
rg "attempt" apps/web/app/\(ee\)/api/cron/payouts/charge-succeeded/ -nLength of output: 7306
Final Review: Breaking Changes, Bugs & Security✅ What's been fixed since the initial full review
🚨 Breaking ChangesNo external breaking changes. All three functions whose signatures changed ( 🔴 Critical Bug — Infinite Scheduling Loop (still not fixed)File: There is still no retry counter anywhere in the // route.ts — payloadSchema has no attempt field
const payloadSchema = z.object({
invoiceId: z.string(),
// ⚠️ No attempt/retryCount — loop has no exit condition
});// utils.ts — body has no attempt counter
body: {
invoiceId: invoice.id,
// ⚠️ No attempt counter passed through
},Suggested fix: // payloadSchema in route.ts
const payloadSchema = z.object({
invoiceId: z.string(),
attempt: z.number().int().min(0).default(0),
});
// After fundSettlement check in route.ts
const MAX_SETTLEMENT_RETRIES = 5;
if (!fundSettlement.fundsAvailable && attempt >= MAX_SETTLEMENT_RETRIES) {
return logAndRespond(
`Exceeded max settlement retries (${MAX_SETTLEMENT_RETRIES}) for invoice ${invoiceId}.`,
);
}
// In scheduleDelayedPayouts call
await scheduleDelayedPayouts({
invoice,
executeAt: fundSettlementTiming.scheduledAt,
attempt: attempt + 1,
});
// In utils.ts scheduleDelayedPayouts body
body: {
invoiceId: invoice.id,
attempt,
},🟡 Bug — QStash Dedup Window Shorter Than Scheduled DelayFile: The comment in deduplicationId: `retry-delayed-payouts-${invoice.id}`, // The deduplication window is 10 minutes
// ⚠️ But delay can be 15+ minutes — dedup expires before the job firesThis is partially mitigated by the 🟡 Bug — Balance Transaction Selection Without Type FilterFile:
// ⚠️ data[0] might be a refund fee or dispute fee, not the original charge
const balanceTransaction = balanceTransactions.data[0];Suggested fix: const balanceTransaction = balanceTransactions.data.find(
(bt) => bt.type === "charge",
);
if (!balanceTransaction) {
return {
fundsAvailable: false,
scheduledAt: new Date(Date.now() + 60 * 60 * 1000),
};
}🟢 Minor — Unused
|
| Check | Status |
|---|---|
verifyQstashSignature guards the endpoint |
✅ Present — only QStash can invoke this route |
Raw SQL parameterization (${invoice.id}) |
✅ Safe — Prisma's tagged template escapes values |
External data validated before use (stripeChargeMetadataSchema.parse) |
✅ Present |
Stablecoin aggregate scoped to status: "processing" |
✅ Fixed — prevents cross-status amount mismatch on re-runs |
Idempotency key reuse risk on fundFinancialAccount |
✅ Safe — same invoiceId key with same status: "processing" filter guarantees same amount |
The one operationally significant security risk is the infinite scheduling loop: runaway QStash jobs consume quota and incur cost without a bound. This is the single most important fix needed before merging.
✅ Action performed
Full review finished.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/lib/partners/create-stripe-transfer.ts (1)
302-309:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUndefined payout access when
currentInvoicePayoutsis empty.When
invoiceIdis omitted (optional parameter),currentInvoicePayoutsis set toPromise.resolve([])(line 81–94). IfpreviouslyProcessedPayoutshas items, the function proceeds to line 303 whereconst payout = currentInvoicePayouts[0]evaluates toundefined. In the non-forceWithdrawalbranch (line 309), accessingpayout.amountandpayout.program.namecauses a runtime crash.Proposed fix: Use the first available payout (from
currentInvoicePayoutsif present, otherwisepreviouslyProcessedPayouts) to ensure the email subject and body reference a valid payout.💥 Proposed fix to safely select payout for email
if (partner.email) { - const payout = currentInvoicePayouts[0]; + const payout = currentInvoicePayouts[0] || previouslyProcessedPayouts[0]; + if (!payout) { + // Should never happen as allPayouts is non-empty at line 101, but guard against unforeseen edge cases + console.warn(`[createStripeTransfer] No payout available for email, skipping email send`); + } else { const emailRes = await sendEmail({ variant: "notifications", to: partner.email, subject: forceWithdrawal ? `A withdrawal of ${currencyFormatter(totalTransferableAmount)} has been initiated from your Dub account` : `You've received a ${currencyFormatter(payout.amount)} payout from ${payout.program.name}`, react: forceWithdrawal ? PartnerPayoutForceWithdrawal({ email: partner.email, payout: { amount: totalTransferableAmount, method: "connect", }, }) : PartnerPayoutProcessed({ email: partner.email, program: payout.program, payout, }), }); console.log(`Resend email sent: ${JSON.stringify(emailRes, null, 2)}`); + } }🤖 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/partners/create-stripe-transfer.ts` around lines 302 - 309, The code attempts to access the first element of currentInvoicePayouts without checking if the array is empty, which can result in undefined when invoiceId is omitted. In the sendEmail call around line 302-309, replace the direct assignment of payout with logic that safely selects the first available payout from either currentInvoicePayouts (if it contains items) or previouslyProcessedPayouts as a fallback. This ensures that the payout object used in the email subject line (specifically when accessing payout.amount and payout.program.name in the non-forceWithdrawal branch) is always defined and prevents the runtime crash.apps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.ts (1)
21-45:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCap delayed payout reschedules.
The delayed job body only carries
invoiceId, so every re-run starts fresh. If Stripe still returns no usable settlement transaction, this path can keep scheduling itself indefinitely. Carry an attempt counter through QStash and stop/alert after a small cap.🛠️ Proposed fix
-import { APP_DOMAIN_WITH_NGROK } from "`@dub/utils`"; +import { APP_DOMAIN_WITH_NGROK, log } from "`@dub/utils`"; import * as z from "zod/v4"; +const MAX_DELAYED_PAYOUT_ATTEMPTS = 5; + // Schedule the QStash job export async function scheduleDelayedPayouts({ invoice, executeAt, + retryCount = 0, }: { invoice: Pick<Invoice, "id">; executeAt: Date; + retryCount?: number; }) { + if (retryCount >= MAX_DELAYED_PAYOUT_ATTEMPTS) { + await log({ + message: `Exceeded delayed payout retry limit for invoice ${invoice.id}; not scheduling another delayed payout job.`, + type: "cron", + }); + return; + } + const scheduleTimeMs = executeAt.getTime(); @@ body: { invoiceId: invoice.id, + retryCount: retryCount + 1, }, });Also parse and pass
retryCountin the route payload before calling this helper.Also applies to: 84-90, 104-113
🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts around lines 21 - 45, The scheduleDelayedPayouts function needs to track retry attempts to prevent indefinite rescheduling loops. Modify the function signature to accept a retryCount parameter, increment it, and include it in the QStash message body alongside invoiceId. Add validation logic to check if retryCount has exceeded a reasonable cap (such as 3-5 attempts) and either stop scheduling or log an alert when the limit is reached. Apply these changes throughout the file where scheduleDelayedPayouts is called, ensuring the retryCount is passed as part of the initial call and incremented appropriately in subsequent retry paths.
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.ts (1)
17-21: ⚡ Quick winKeep the funds-available guard at the Tremendous boundary.
The route currently skips this call when funds are unavailable, but this exported function can still enqueue Tremendous payouts if reused directly. Accept
fundsAvailableand return early here too.🛠️ Proposed fix
export async function queueTremendousPayouts({ invoice, + fundsAvailable, }: { invoice: Pick<Invoice, "id" | "paymentMethod" | "payoutMode" | "programId">; + fundsAvailable: boolean; }) { + if (!fundsAvailable) { + console.log( + `Funds are not available for invoice ${invoice.id}, skipping Tremendous payouts...`, + ); + return; + } + if (invoice.payoutMode === "external") {Update the route call to pass
fundsAvailableas well.🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.ts around lines 17 - 21, The exported function queueTremendousPayouts lacks a guard to prevent enqueueing Tremendous payouts when funds are unavailable, even though its caller (the route) currently checks this condition. Add a fundsAvailable parameter to the function's destructured object parameter alongside invoice, then add an early return guard at the start of the function body to return immediately if fundsAvailable is false. Update the route's call to queueTremendousPayouts to pass the fundsAvailable value along with the invoice parameter.apps/web/app/(ee)/api/cron/payouts/charge-succeeded/send-paypal-payouts.ts (1)
10-14: ⚡ Quick winKeep the funds-available guard at the PayPal boundary.
The route currently skips this call when funds are unavailable, but this exported function can still dispatch PayPal payouts if reused directly. Accept
fundsAvailableand return early here too.🛠️ Proposed fix
export async function sendPaypalPayouts({ invoice, + fundsAvailable, }: { invoice: Pick<Invoice, "id" | "payoutMode">; + fundsAvailable: boolean; }) { + if (!fundsAvailable) { + console.log( + `Funds are not available for invoice ${invoice.id}, skipping PayPal payouts...`, + ); + return; + } + if (invoice.payoutMode === "external") {Update the route call to pass
fundsAvailableas well.🤖 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/app/`(ee)/api/cron/payouts/charge-succeeded/send-paypal-payouts.ts around lines 10 - 14, The sendPaypalPayouts function lacks a funds-availability guard at the function level, so it can still dispatch PayPal payouts if called directly even when funds are unavailable. Add fundsAvailable as a parameter to the function signature (alongside the existing invoice parameter in the destructured object), then add an early return guard at the beginning of the function body that exits if funds are not available. Finally, update the route caller to pass the fundsAvailable value when invoking sendPaypalPayouts to ensure the guard is properly checked.
🤖 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/cron/payouts/charge-succeeded/utils.ts:
- Around line 78-94: The current code uses balanceTransactions.list() and
selects data[0], which depends on unpredictable list response order. Instead,
replace the stripe.balanceTransactions.list() call with
stripe.charges.retrieve(chargeId, { expand: ["balance_transaction"] }) to
directly retrieve the specific balance transaction associated with the charge.
Access the balance transaction from the returned charge object's
balance_transaction property instead of selecting from a list, and update the
condition that checks if balanceTransactions.data.length === 0 to check if the
balance_transaction property exists or is null.
---
Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/utils.ts:
- Around line 21-45: The scheduleDelayedPayouts function needs to track retry
attempts to prevent indefinite rescheduling loops. Modify the function signature
to accept a retryCount parameter, increment it, and include it in the QStash
message body alongside invoiceId. Add validation logic to check if retryCount
has exceeded a reasonable cap (such as 3-5 attempts) and either stop scheduling
or log an alert when the limit is reached. Apply these changes throughout the
file where scheduleDelayedPayouts is called, ensuring the retryCount is passed
as part of the initial call and incremented appropriately in subsequent retry
paths.
In `@apps/web/lib/partners/create-stripe-transfer.ts`:
- Around line 302-309: The code attempts to access the first element of
currentInvoicePayouts without checking if the array is empty, which can result
in undefined when invoiceId is omitted. In the sendEmail call around line
302-309, replace the direct assignment of payout with logic that safely selects
the first available payout from either currentInvoicePayouts (if it contains
items) or previouslyProcessedPayouts as a fallback. This ensures that the payout
object used in the email subject line (specifically when accessing payout.amount
and payout.program.name in the non-forceWithdrawal branch) is always defined and
prevents the runtime crash.
---
Nitpick comments:
In
`@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.ts:
- Around line 17-21: The exported function queueTremendousPayouts lacks a guard
to prevent enqueueing Tremendous payouts when funds are unavailable, even though
its caller (the route) currently checks this condition. Add a fundsAvailable
parameter to the function's destructured object parameter alongside invoice,
then add an early return guard at the start of the function body to return
immediately if fundsAvailable is false. Update the route's call to
queueTremendousPayouts to pass the fundsAvailable value along with the invoice
parameter.
In `@apps/web/app/`(ee)/api/cron/payouts/charge-succeeded/send-paypal-payouts.ts:
- Around line 10-14: The sendPaypalPayouts function lacks a funds-availability
guard at the function level, so it can still dispatch PayPal payouts if called
directly even when funds are unavailable. Add fundsAvailable as a parameter to
the function signature (alongside the existing invoice parameter in the
destructured object), then add an early return guard at the beginning of the
function body that exits if funds are not available. Finally, update the route
caller to pass the fundsAvailable value when invoking sendPaypalPayouts to
ensure the guard is properly checked.
🪄 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
Run ID: d92d2442-8cc7-42d6-909c-bda65dacdc62
📒 Files selected for processing (6)
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-stripe-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/queue-tremendous-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/send-paypal-payouts.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/utils.tsapps/web/lib/partners/create-stripe-transfer.ts
2 participants
Summary by CodeRabbit
charge-succeededcron payout flow to use settlement-availability timing with a unified delayed-scheduling helper.fundsAvailableflag; conditional PayPal/Tremendous enqueuing and stablecoin funding/partner selection now depend on funds availability.processingpayouts with unset methods.fundsAvailablegating behavior across payout types.