From b53a4fe150309037df5ced235560002d09d88796 Mon Sep 17 00:00:00 2001 From: Ridanshi Date: Sat, 23 May 2026 14:31:38 +0530 Subject: [PATCH 1/2] fix(profiles): handle P2002 on concurrent username claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The username update handler performs a read-before-write uniqueness check (findFirst -> update). Under concurrent requests, both callers can pass the read, race to write, and have Prisma throw P2002 on the losing write — propagating as an unhandled 500. Wrap user.update in a targeted try/catch: P2002 maps to a deterministic 409 Conflict with the same "Username already taken" message the pre-check already returns. Other errors are logged and returned as 500 unchanged. The existing findFirst read is preserved as a fast-path that avoids hitting the write path for clearly taken usernames. The DB unique constraint remains the authoritative guard against the race. Add tests covering: - P2002 on user.update (concurrent race simulation) -> 409 - unexpected DB errors on user.update -> 500 - no findFirst call when no username is in the payload Fixes #227. --- apps/backend/src/__tests__/profiles.test.ts | 48 +++++++++++++++++- apps/backend/src/routes/profiles.ts | 54 +++++++++++++-------- 2 files changed, 81 insertions(+), 21 deletions(-) diff --git a/apps/backend/src/__tests__/profiles.test.ts b/apps/backend/src/__tests__/profiles.test.ts index b6630a7a..07d10f98 100644 --- a/apps/backend/src/__tests__/profiles.test.ts +++ b/apps/backend/src/__tests__/profiles.test.ts @@ -90,7 +90,7 @@ describe('PUT /api/profiles/me', () => { expect(res.json().error).toBe('Validation failed'); }); - it('should return 409 if username is already taken', async () => { + it('should return 409 if username is already taken (pre-check)', async () => { mockPrisma.user.findFirst.mockResolvedValue({ id: 'other-user' }); const app = await buildApp(); const res = await app.inject({ @@ -101,4 +101,50 @@ describe('PUT /api/profiles/me', () => { expect(res.statusCode).toBe(409); expect(res.json().error).toBe('Username already taken'); }); + + it('should return 409 when a concurrent request wins the unique constraint race (P2002)', async () => { + // Both requests pass the findFirst check; the DB unique constraint fires on + // the losing write — Prisma raises P2002. + mockPrisma.user.findFirst.mockResolvedValue(null); + const p2002 = Object.assign(new Error('Unique constraint failed'), { code: 'P2002' }); + mockPrisma.user.update.mockRejectedValue(p2002); + + const app = await buildApp(); + const res = await app.inject({ + method: 'PUT', + url: '/api/profiles/me', + payload: { username: 'raced-username' }, + }); + + expect(res.statusCode).toBe(409); + expect(res.json().error).toBe('Username already taken'); + }); + + it('should return 500 for unexpected database errors during update', async () => { + mockPrisma.user.findFirst.mockResolvedValue(null); + mockPrisma.user.update.mockRejectedValue(new Error('Connection refused')); + + const app = await buildApp(); + const res = await app.inject({ + method: 'PUT', + url: '/api/profiles/me', + payload: { username: 'anyuser' }, + }); + + expect(res.statusCode).toBe(500); + expect(res.json().error).toBe('Internal server error'); + }); + + it('should not call findFirst when no username is provided in the update', async () => { + mockPrisma.user.update.mockResolvedValue({ ...mockUser, displayName: 'New Name' }); + const app = await buildApp(); + const res = await app.inject({ + method: 'PUT', + url: '/api/profiles/me', + payload: { displayName: 'New Name' }, + }); + + expect(res.statusCode).toBe(200); + expect(mockPrisma.user.findFirst).not.toHaveBeenCalled(); + }); }); \ No newline at end of file diff --git a/apps/backend/src/routes/profiles.ts b/apps/backend/src/routes/profiles.ts index 99aacb8e..e0457e5c 100644 --- a/apps/backend/src/routes/profiles.ts +++ b/apps/backend/src/routes/profiles.ts @@ -50,9 +50,11 @@ export async function profileRoutes(app: FastifyInstance) { return reply.status(400).send({ error: 'Validation failed', details: parsed.error.flatten() }); } - // Check username uniqueness if changing - // Note: For production, consider adding a timestamp/version field to handle - // race conditions where two users might try to claim the same username simultaneously. + // Fast-path uniqueness check. This read-before-write eliminates the common + // case (clearly taken username) without touching the write path, but it + // cannot prevent the race window between two concurrent requests that both + // pass this check simultaneously. The unique constraint on the DB is the + // authoritative guard — P2002 below is the definitive conflict signal. if (parsed.data.username) { const existing = await app.prisma.user.findFirst({ where: { @@ -65,24 +67,36 @@ export async function profileRoutes(app: FastifyInstance) { } } - const updated = await app.prisma.user.update({ - where: { id: userId }, - data: parsed.data, - select: { - id: true, - email: true, - username: true, - displayName: true, - bio: true, - pronouns: true, - role: true, - company: true, - avatarUrl: true, - accentColor: true, - }, - }); + try { + const updated = await app.prisma.user.update({ + where: { id: userId }, + data: parsed.data, + select: { + id: true, + email: true, + username: true, + displayName: true, + bio: true, + pronouns: true, + role: true, + company: true, + avatarUrl: true, + accentColor: true, + }, + }); - return updated; + return updated; + } catch (err: any) { + // Unique constraint violation — two concurrent requests raced through the + // findFirst check above and both attempted the write. The DB constraint + // fires on the losing request; surface it as a deterministic 409 rather + // than leaking a raw Prisma error as a 500. + if (err?.code === 'P2002') { + return reply.status(409).send({ error: 'Username already taken' }); + } + app.log.error({ err }, 'DB error in PUT /profiles/me'); + return reply.status(500).send({ error: 'Internal server error' }); + } }); // ─── Add Platform Link ─── From aea3191abefaaf3ee51fbfd88fb7cbbd5d6a4186 Mon Sep 17 00:00:00 2001 From: Ridanshi Date: Sat, 23 May 2026 23:19:30 +0530 Subject: [PATCH 2/2] refactor(profiles): add explicit ProfileUpdateResponse type to PUT /me Addresses maintainer feedback requesting typed responses. Previously the PUT /me handler returned `updated` typed implicitly through Prisma's deep generic inference (Prisma.UserGetPayload<...>), making the response contract invisible without tracing through generated types. Changes: - Declare `ProfileUpdateResponse` at module scope, following the explicit response-type convention already used in public.ts - Type the `user.update` result variable as `ProfileUpdateResponse` so the ten-field response contract is visible at the call site - Return the named variable rather than the raw Prisma result No logic changes. All 8 tests continue to pass. --- apps/backend/src/routes/profiles.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/apps/backend/src/routes/profiles.ts b/apps/backend/src/routes/profiles.ts index e0457e5c..4c02c503 100644 --- a/apps/backend/src/routes/profiles.ts +++ b/apps/backend/src/routes/profiles.ts @@ -6,6 +6,23 @@ import { reorderLinksSchema, } from '../utils/validators.js'; +// ── Response types ──────────────────────────────────────────────────────────── +// Declared explicitly so the API contract is visible without tracing through +// Prisma's generic return types. Follows the convention in public.ts. + +type ProfileUpdateResponse = { + id: string; + email: string; + username: string; + displayName: string; + bio: string | null; + pronouns: string | null; + role: string | null; + company: string | null; + avatarUrl: string | null; + accentColor: string; +}; + export async function profileRoutes(app: FastifyInstance) { // All profile routes require auth app.addHook('preHandler', app.authenticate); @@ -68,7 +85,7 @@ export async function profileRoutes(app: FastifyInstance) { } try { - const updated = await app.prisma.user.update({ + const response: ProfileUpdateResponse = await app.prisma.user.update({ where: { id: userId }, data: parsed.data, select: { @@ -85,7 +102,7 @@ export async function profileRoutes(app: FastifyInstance) { }, }); - return updated; + return response; } catch (err: any) { // Unique constraint violation — two concurrent requests raced through the // findFirst check above and both attempted the write. The DB constraint