Fixes credits and submissions race condition#1411
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds ChangesIdempotent Submission and Credit Transaction
Sequence Diagram(s)sequenceDiagram
participant Handler as API Handler
participant TX as prisma.$transaction
participant DB as Database
participant canUserSubmit
participant createSubmission
participant consumeCredit
Handler->>TX: begin transaction
TX->>DB: SELECT user WHERE id=userId FOR UPDATE
TX->>createSubmission: createSubmission(userId, listingId, data, listing, {client: txClient})
createSubmission->>canUserSubmit: canUserSubmit(userId, txClient)
canUserSubmit->>DB: creditLedger.aggregate + user.findUnique (via txClient)
DB-->>canUserSubmit: balance + user plan
canUserSubmit-->>createSubmission: hasCredits (boolean)
createSubmission->>DB: client.submission.create(dedupeKey, ...)
DB-->>createSubmission: submission | P2002 → "Submission already exists"
createSubmission->>consumeCredit: consumeCredit(userId, submissionId, txClient)
consumeCredit->>DB: creditLedger.create(idempotencyKey, ...)
DB-->>consumeCredit: ledger entry | P2002 (swallowed)
TX-->>Handler: commit → submission
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 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: 7
🤖 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/features/credits/utils/allocateCredits.ts`:
- Around line 11-15: The consumeCredit function is missing an explicit return
type annotation. Add a return type annotation of Promise<void> to the function
signature after the closing parenthesis of the parameters list and before the
opening curly brace. This ensures the function complies with the requirement
that top-level module functions in TypeScript have explicit return types.
- Around line 6-7: The currentMonth and nextMonth variables are computed at
module load time on lines 6-7, which causes them to become stale if the module
remains loaded across a month boundary. Move these variable declarations from
the module scope into the consumeCredit function itself, so they are
recalculated on each invocation to ensure they always represent the current and
next month at call time. Additionally, add an explicit return type annotation of
Promise<void> to the consumeCredit function signature on line 11 to comply with
the explicit return type requirement for top-level module functions.
- Around line 29-32: The P2002 error suppression in the allocateCredits function
is too broad and masks all unique constraint violations, including unrelated
foreign key constraint failures. Instead of returning whenever error.code ===
'P2002', narrow the condition to check the error's metadata field to verify it's
specifically the idempotency constraint violation before suppressing it. Only
return early if the constraint name matches the idempotency constraint fields
(typically a combination of userId, submissionId, and applicationId), allowing
other P2002 errors to propagate and be handled appropriately.
In `@src/features/credits/utils/canUserSubmit.ts`:
- Around line 7-10: The function `canUserSubmit` is missing an explicit return
type annotation. Add `Promise<boolean>` as the return type annotation to the
function signature to comply with coding guidelines that require top-level
module functions to declare their return types explicitly rather than relying on
TypeScript's type inference.
In `@src/features/credits/utils/creditAggregate.ts`:
- Around line 6-9: The creditAggregate function is missing an explicit return
type annotation. Add the return type Promise<number> to the function signature
of creditAggregate since it returns a numeric value representing the effective
balance. This aligns with the requirement that top-level exported functions in
TypeScript must declare their return types explicitly.
In `@src/pages/api/submission/create.ts`:
- Around line 162-188: The generic Error thrown for insufficient credits in the
prisma transaction (the throw new Error statement within the canUserSubmit
check) gets caught by the existing catch block and misclassified as a 403 error.
Instead of throwing a generic Error, create a custom error class or use an error
with a specific type/code that can be distinguished from unexpected database
errors in the outer catch handler. This way, the insufficient credits scenario
can remain a 403, while genuine database failures and other unexpected errors in
the transaction flow are properly returned as 500 errors to maintain correct API
error semantics.
- Around line 24-34: The functions `submissionDedupeKey` and `createSubmission`
are top-level module functions that lack explicit return type annotations, which
violates the coding guidelines. Add an explicit return type annotation of `:
string` to the `submissionDedupeKey` function signature, and add the appropriate
return type annotation to the `createSubmission` function signature based on
what it actually returns. These annotations should be placed between the closing
parenthesis of the parameter list and the opening curly brace of each function.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6f867a8-3f3c-4650-b0b1-e1206bc96ab3
📒 Files selected for processing (5)
prisma/schema.prismasrc/features/credits/utils/allocateCredits.tssrc/features/credits/utils/canUserSubmit.tssrc/features/credits/utils/creditAggregate.tssrc/pages/api/submission/create.ts
| const currentMonth = dayjs.utc().startOf('month').toDate(); | ||
| const nextMonth = dayjs.utc().add(1, 'month').startOf('month').toDate(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/features/credits/utils/allocateCredits.tsRepository: SuperteamDAO/earn
Length of output: 9732
Compute currentMonth at call time, not module load time.
Line 6 freezes currentMonth when the process starts. If the module stays loaded across a month boundary, subsequent calls to consumeCredit will write debits to the previous month's effectiveMonth, corrupting monthly balance accounting.
🛠️ Suggested fix
-const currentMonth = dayjs.utc().startOf('month').toDate();
-const nextMonth = dayjs.utc().add(1, 'month').startOf('month').toDate();
+function getCurrentMonth(): Date {
+ return dayjs.utc().startOf('month').toDate();
+}
+
+function getNextMonth(): Date {
+ return dayjs.utc().add(1, 'month').startOf('month').toDate();
+}
@@
- effectiveMonth: currentMonth,
+ effectiveMonth: getCurrentMonth(),Also add explicit return type Promise<void> to consumeCredit (line 11) to comply with the guideline requiring explicit return types for top-level module functions.
🤖 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/features/credits/utils/allocateCredits.ts` around lines 6 - 7, The
currentMonth and nextMonth variables are computed at module load time on lines
6-7, which causes them to become stale if the module remains loaded across a
month boundary. Move these variable declarations from the module scope into the
consumeCredit function itself, so they are recalculated on each invocation to
ensure they always represent the current and next month at call time.
Additionally, add an explicit return type annotation of Promise<void> to the
consumeCredit function signature on line 11 to comply with the explicit return
type requirement for top-level module functions.
| export async function consumeCredit( | ||
| userId: string, | ||
| submissionId: string, | ||
| client: PrismaLike = prisma, | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "allocateCredits.ts" -type fRepository: SuperteamDAO/earn
Length of output: 109
🏁 Script executed:
cat -n ./src/features/credits/utils/allocateCredits.ts | head -30Repository: SuperteamDAO/earn
Length of output: 1198
🏁 Script executed:
cat -n ./src/features/credits/utils/allocateCredits.ts | sed -n '11,50p'Repository: SuperteamDAO/earn
Length of output: 1247
Add an explicit return type on consumeCredit.
The function lacks an explicit return annotation. Declare Promise<void> to comply with the requirement that top-level module functions in TypeScript have explicit return types.
🤖 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/features/credits/utils/allocateCredits.ts` around lines 11 - 15, The
consumeCredit function is missing an explicit return type annotation. Add a
return type annotation of Promise<void> to the function signature after the
closing parenthesis of the parameters list and before the opening curly brace.
This ensures the function complies with the requirement that top-level module
functions in TypeScript have explicit return types.
Source: Coding guidelines
| error instanceof PrismaClientKnownRequestError && | ||
| error.code === 'P2002' | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/features/credits/utils/allocateCredits.tsRepository: SuperteamDAO/earn
Length of output: 9732
🏁 Script executed:
fd -t f schema.prismaRepository: SuperteamDAO/earn
Length of output: 82
🏁 Script executed:
cat prisma/schema.prisma | grep -A 50 "model CreditLedger"Repository: SuperteamDAO/earn
Length of output: 1569
🏁 Script executed:
rg "P2002|meta\.target" --type ts --type tsx -B 3 -A 3Repository: SuperteamDAO/earn
Length of output: 89
🏁 Script executed:
rg "P2002|meta\.target" -B 3 -A 3Repository: SuperteamDAO/earn
Length of output: 2512
🏁 Script executed:
cat prisma/schema.prisma | grep -B 5 -A 30 "model CreditLedger"Repository: SuperteamDAO/earn
Length of output: 1207
Narrow P2002 suppression to the idempotency constraint only.
The current code suppresses all P2002 errors, which masks unrelated unique constraint violations. This can silently skip a credit debit if a foreign key constraint fails (e.g., invalid userId, submissionId, or applicationId), causing data integrity issues.
🛠️ Suggested fix
- if (
- error instanceof PrismaClientKnownRequestError &&
- error.code === 'P2002'
- ) {
+ if (
+ error instanceof PrismaClientKnownRequestError &&
+ error.code === 'P2002' &&
+ (
+ (Array.isArray(error.meta?.target) &&
+ error.meta.target.includes('idempotencyKey')) ||
+ error.meta?.target === 'idempotencyKey'
+ )
+ ) {
return;
}📝 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.
| error instanceof PrismaClientKnownRequestError && | |
| error.code === 'P2002' | |
| ) { | |
| return; | |
| if ( | |
| error instanceof PrismaClientKnownRequestError && | |
| error.code === 'P2002' && | |
| ( | |
| (Array.isArray(error.meta?.target) && | |
| error.meta.target.includes('idempotencyKey')) || | |
| error.meta?.target === 'idempotencyKey' | |
| ) | |
| ) { | |
| return; | |
| } |
🤖 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/features/credits/utils/allocateCredits.ts` around lines 29 - 32, The
P2002 error suppression in the allocateCredits function is too broad and masks
all unique constraint violations, including unrelated foreign key constraint
failures. Instead of returning whenever error.code === 'P2002', narrow the
condition to check the error's metadata field to verify it's specifically the
idempotency constraint violation before suppressing it. Only return early if the
constraint name matches the idempotency constraint fields (typically a
combination of userId, submissionId, and applicationId), allowing other P2002
errors to propagate and be handled appropriately.
| export async function canUserSubmit( | ||
| userId: string, | ||
| client: PrismaLike = prisma, | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/features/credits/utils/canUserSubmit.tsRepository: SuperteamDAO/earn
Length of output: 494
Add return type annotation Promise<boolean> to canUserSubmit.
The function currently relies on TypeScript's type inference. Top-level module functions must declare their return types explicitly per coding guidelines.
🤖 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/features/credits/utils/canUserSubmit.ts` around lines 7 - 10, The
function `canUserSubmit` is missing an explicit return type annotation. Add
`Promise<boolean>` as the return type annotation to the function signature to
comply with coding guidelines that require top-level module functions to declare
their return types explicitly rather than relying on TypeScript's type
inference.
Source: Coding guidelines
| export async function creditAggregate( | ||
| userId: string, | ||
| client: PrismaLike = prisma, | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/features/credits/utils/creditAggregate.tsRepository: SuperteamDAO/earn
Length of output: 1006
Add explicit return type annotation to the exported creditAggregate function.
The function is missing a return type declaration. Since it returns a numeric value (effectiveBalance), add Promise<number> to the function signature:
export async function creditAggregate(
userId: string,
client: PrismaLike = prisma,
): Promise<number> {Per coding guidelines, top-level module functions in TypeScript must declare return types.
🤖 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/features/credits/utils/creditAggregate.ts` around lines 6 - 9, The
creditAggregate function is missing an explicit return type annotation. Add the
return type Promise<number> to the function signature of creditAggregate since
it returns a numeric value representing the effective balance. This aligns with
the requirement that top-level exported functions in TypeScript must declare
their return types explicitly.
Source: Coding guidelines
| function submissionDedupeKey( | ||
| userId: string, | ||
| listingId: string, | ||
| options?: { isAgent?: boolean; agentId?: string }, | ||
| ) { | ||
| if (options?.isAgent && options.agentId) { | ||
| return `agent:${options.agentId}:listing:${listingId}`; | ||
| } | ||
|
|
||
| return `user:${userId}:listing:${listingId}`; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/pages/api/submission/create.ts | head -50Repository: SuperteamDAO/earn
Length of output: 2197
🏁 Script executed:
cat -n src/pages/api/submission/create.ts | sed -n '36,150p'Repository: SuperteamDAO/earn
Length of output: 3868
Add explicit return type annotations to top-level module functions.
submissionDedupeKey (line 24) and createSubmission (line 36) are top-level module functions without explicit return type annotations. Annotate submissionDedupeKey with : string and createSubmission with its appropriate return type.
Per the coding guidelines: "Declare return types for top-level module functions in TypeScript (exception: React components returning JSX)".
🤖 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/pages/api/submission/create.ts` around lines 24 - 34, The functions
`submissionDedupeKey` and `createSubmission` are top-level module functions that
lack explicit return type annotations, which violates the coding guidelines. Add
an explicit return type annotation of `: string` to the `submissionDedupeKey`
function signature, and add the appropriate return type annotation to the
`createSubmission` function signature based on what it actually returns. These
annotations should be placed between the closing parenthesis of the parameter
list and the opening curly brace of each function.
Source: Coding guidelines
| const result = | ||
| !isHackathon && !isPro | ||
| ? await prisma.$transaction(async (tx) => { | ||
| await tx.$queryRaw` | ||
| SELECT id FROM \`User\` | ||
| WHERE id = ${userId as string} | ||
| FOR UPDATE | ||
| `; | ||
|
|
||
| const hasCredits = await canUserSubmit(userId as string, tx); | ||
| if (!hasCredits) { | ||
| throw new Error('Insufficient credits'); | ||
| } | ||
|
|
||
| const submission = await createSubmission( | ||
| userId as string, | ||
| listingId, | ||
| { link, tweet, otherInfo, eligibilityAnswers, ask, telegram }, | ||
| listing, | ||
| { client: tx }, | ||
| ); | ||
|
|
||
| await consumeCredit(userId, submission.id, tx); | ||
| logger.info(`Consumed 1 credit from user ${userId} for submission`); | ||
|
|
||
| return submission; | ||
| }) |
There was a problem hiding this comment.
Avoid generic throws in the transaction flow; they get misclassified as 403.
Line 173 introduces a generic Error, and this new transactional path can also surface unexpected DB errors. With the existing catch behavior, those failures are returned as 403 instead of 500, which breaks API error semantics and incident visibility.
As per coding guidelines, "Use HTTP status codes: ... 500 for internal server 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 `@src/pages/api/submission/create.ts` around lines 162 - 188, The generic Error
thrown for insufficient credits in the prisma transaction (the throw new Error
statement within the canUserSubmit check) gets caught by the existing catch
block and misclassified as a 403 error. Instead of throwing a generic Error,
create a custom error class or use an error with a specific type/code that can
be distinguished from unexpected database errors in the outer catch handler.
This way, the insufficient credits scenario can remain a 403, while genuine
database failures and other unexpected errors in the transaction flow are
properly returned as 500 errors to maintain correct API error semantics.
Source: Coding guidelines
1 participant
What does this PR do?
idempotencyKeyanddedupeKeyto properly identify credits and submissionsWhere should the reviewer start?
How should this be manually tested?
Any background context you want to provide?
What are the relevant issues?
Screenshots (if appropriate)
Summary by CodeRabbit
New Features
Bug Fixes