fix(cards): add Serializable isolation + P2034 retry to setDefaultCard

[Srejoye]

Summary

Closes #566.

setDefaultCard ran its clear-then-set pair under Postgres's default Read Committed isolation. Two concurrent calls for the same user could interleave so that the updateMany from Tx2 (clear all defaults) committed after Tx1's update (set card A as default), wiping the flag without any replacement — leaving the user with zero isDefault: true cards.

This silently breaks any UI or API logic that assumes exactly one default card exists per user (the invariant that createCard and deleteCard both uphold carefully).

Root cause

// Before — no isolationLevel; runs under Read Committed
await app.prisma.$transaction(async (tx) => {
  await tx.card.updateMany({ where: { userId }, data: { isDefault: false } });
  await tx.card.update({ where: { id }, data: { isDefault: true } });
});

Under Read Committed, the two writes are not serialized against concurrent transactions touching the same rows, so lost-update interleavings are possible.

Fix

Mirror the pattern already used in createCard:

1. Pass { isolationLevel: 'Serializable' } to $transaction — Postgres will detect write-write conflicts and abort one of the concurrent transactions with P2034.
2. Wrap in a retry loop (max 3 attempts), continuing on P2034 and re-throwing immediately on any other error.

// After
const maxRetries = 3;
for (let attempt = 1; attempt <= maxRetries; attempt++) {
  try {
    await app.prisma.$transaction(
      async (tx) => {
        await tx.card.updateMany({ where: { userId }, data: { isDefault: false } });
        await tx.card.update({ where: { id }, data: { isDefault: true } });
      },
      { isolationLevel: 'Serializable' },
    );
    return { message: 'Default card updated' };
  } catch (error: unknown) {
    if (isP2034(error) && attempt < maxRetries) continue;
    app.log.error(error);
    throw error;
  }
}
throw new Error('Failed to set default card after retrying serialization conflicts');

Files changed

File	Change
apps/backend/src/services/cardService.ts	Add isolationLevel: 'Serializable' + P2034 retry loop to setDefaultCard
apps/backend/src/__tests__/cards.test.ts	5 new tests covering isolation option forwarding, retry on P2034, retry exhaustion, no-retry on non-P2034 errors, concurrent-call race simulation

Tests

New test cases in describe('PUT /api/cards/:id/default — serialization isolation & retry'):

• ✅ isolationLevel: 'Serializable' is passed to $transaction
• ✅ Retries on P2034 and succeeds on second attempt
• ✅ Returns 500 after exhausting all 3 attempts on persistent P2034
• ✅ Does not retry non-P2034 errors (single attempt → 500)
• ✅ Concurrent calls: serialization abort → retry → both callers receive 200

Checklist

• Mirrors the retry pattern already established in createCard
• No behaviour change for the happy path
• No new dependencies
• Existing tests still pass

[vercel]

@Srejoye is attempting to deploy a commit to the Prashantkumar Khatri's projects Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

Hi @Srejoye,

Thanks for opening this pull request.

This PR has been automatically classified based on the files modified.

Applied Labels

• gssoc:approved
• backend

Primary Review Area

• backend

Reviewer

@Harxhit has been identified as the primary reviewer for this pull request.

If you have any questions regarding the affected area or implementation details, feel free to reach out to the assigned reviewer.

Thank you for your contribution!

[github-actions]

CI — All Checks Passed

Backend — PASS

Check	Result
Lint	PASS
Test	PASS
Typecheck	PASS

Mobile — SKIP

Check	Result
Lint	-
Test	-

Web — SKIP

Check	Result
Build	-

---

Last updated: Tue, 16 Jun 2026 18:04:54 GMT

[Harxhit]

Checked this out and tested locally. The fix faithfully mirrors the established createCard retry pattern, and the lost-update analysis in the description is accurate — under Read Committed the updateMany/update pair could interleave and leave a user with zero defaults.

Verified ✅

• npm run typecheck → clean (tsc --noEmit, exit 0).
• npx vitest run src/__tests__/cards.test.ts → 26/26 pass, including all 5 new cases (isolation-option forwarding, retry-then-succeed, retry exhaustion, no-retry on non-P2034, concurrent race).
• npx eslint on both changed files → clean (exit 0).
• Confirmed the route handler (src/routes/cards.ts:137) wraps the call in try/catch → handleDbError, so a thrown error after exhaustion does surface as an error response, and the existence findFirst correctly sits outside the retry loop.
• Diff is tightly scoped: only setDefaultCard + its tests, no unrelated reformatting.

Suggestions

1. (Medium) Real P2034 exhaustion returns 400, not 500. On the final attempt attempt < maxRetries is false, so the original error is re-thrown — and a genuine Prisma serialization failure is a PrismaClientKnownRequestError with code: 'P2034'. In handleDbError that lands in the PrismaClientKnownRequestError branch and falls through to status(400) (Database error: ...), whereas the tests assert 500 (they throw a plain object, which skips that branch). Real and tested behaviour diverge here. Consider re-throwing the trailing Error('Failed to set default card...') for the exhausted case (so it maps to 500), or handling P2034 explicitly in handleDbError — a persistent serialization conflict is arguably a 503/500, not a 400 client error.
2. (Low) throw new Error('Failed to set default card...') is effectively unreachable for P2034. Because exhaustion re-throws the original error from inside the loop, the post-loop throw only fires in theory. Pairing it with suggestion #1 would make it the actual exhaustion path.
3. (Nit) The P2034 check is now duplicated verbatim in createCard and setDefaultCard. The PR description references an isP2034(error) helper that isn't in the diff (the check is inlined). Extracting a small isP2034(error: unknown): boolean would DRY both call sites and match the description.
4. (Nit) Concurrent-call test is a loose assertion. toBeGreaterThanOrEqual(2) with shared mutable mock state verifies "both eventually 200" rather than true serialization semantics. Fine as a regression guard, but the comment overstates what it proves.

Overall: correct, well-tested, and cleanly scoped — just worth aligning the exhaustion status code with the tests before merge.

_{Reviewed locally on branch fix/566-set-default-card-serializable-isolation: typecheck + 26/26 vitest + eslint all green.}