Skip to content

Commit ab4e523

Browse files
authored
fix(profiles): handle Prisma P2002 on concurrent username claims (#271)
* fix(profiles): handle P2002 on concurrent username claims 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. * 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.
1 parent fe788d7 commit ab4e523

2 files changed

Lines changed: 98 additions & 21 deletions

File tree

apps/backend/src/__tests__/profiles.test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe('PUT /api/profiles/me', () => {
9090
expect(res.json().error).toBe('Validation failed');
9191
});
9292

93-
it('should return 409 if username is already taken', async () => {
93+
it('should return 409 if username is already taken (pre-check)', async () => {
9494
mockPrisma.user.findFirst.mockResolvedValue({ id: 'other-user' });
9595
const app = await buildApp();
9696
const res = await app.inject({
@@ -101,4 +101,50 @@ describe('PUT /api/profiles/me', () => {
101101
expect(res.statusCode).toBe(409);
102102
expect(res.json().error).toBe('Username already taken');
103103
});
104+
105+
it('should return 409 when a concurrent request wins the unique constraint race (P2002)', async () => {
106+
// Both requests pass the findFirst check; the DB unique constraint fires on
107+
// the losing write — Prisma raises P2002.
108+
mockPrisma.user.findFirst.mockResolvedValue(null);
109+
const p2002 = Object.assign(new Error('Unique constraint failed'), { code: 'P2002' });
110+
mockPrisma.user.update.mockRejectedValue(p2002);
111+
112+
const app = await buildApp();
113+
const res = await app.inject({
114+
method: 'PUT',
115+
url: '/api/profiles/me',
116+
payload: { username: 'raced-username' },
117+
});
118+
119+
expect(res.statusCode).toBe(409);
120+
expect(res.json().error).toBe('Username already taken');
121+
});
122+
123+
it('should return 500 for unexpected database errors during update', async () => {
124+
mockPrisma.user.findFirst.mockResolvedValue(null);
125+
mockPrisma.user.update.mockRejectedValue(new Error('Connection refused'));
126+
127+
const app = await buildApp();
128+
const res = await app.inject({
129+
method: 'PUT',
130+
url: '/api/profiles/me',
131+
payload: { username: 'anyuser' },
132+
});
133+
134+
expect(res.statusCode).toBe(500);
135+
expect(res.json().error).toBe('Internal server error');
136+
});
137+
138+
it('should not call findFirst when no username is provided in the update', async () => {
139+
mockPrisma.user.update.mockResolvedValue({ ...mockUser, displayName: 'New Name' });
140+
const app = await buildApp();
141+
const res = await app.inject({
142+
method: 'PUT',
143+
url: '/api/profiles/me',
144+
payload: { displayName: 'New Name' },
145+
});
146+
147+
expect(res.statusCode).toBe(200);
148+
expect(mockPrisma.user.findFirst).not.toHaveBeenCalled();
149+
});
104150
});

apps/backend/src/routes/profiles.ts

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,23 @@ import {
66
reorderLinksSchema,
77
} from '../utils/validators.js';
88

9+
// ── Response types ────────────────────────────────────────────────────────────
10+
// Declared explicitly so the API contract is visible without tracing through
11+
// Prisma's generic return types. Follows the convention in public.ts.
12+
13+
type ProfileUpdateResponse = {
14+
id: string;
15+
email: string;
16+
username: string;
17+
displayName: string;
18+
bio: string | null;
19+
pronouns: string | null;
20+
role: string | null;
21+
company: string | null;
22+
avatarUrl: string | null;
23+
accentColor: string;
24+
};
25+
926
export async function profileRoutes(app: FastifyInstance) {
1027
// All profile routes require auth
1128
app.addHook('preHandler', app.authenticate);
@@ -50,9 +67,11 @@ export async function profileRoutes(app: FastifyInstance) {
5067
return reply.status(400).send({ error: 'Validation failed', details: parsed.error.flatten() });
5168
}
5269

53-
// Check username uniqueness if changing
54-
// Note: For production, consider adding a timestamp/version field to handle
55-
// race conditions where two users might try to claim the same username simultaneously.
70+
// Fast-path uniqueness check. This read-before-write eliminates the common
71+
// case (clearly taken username) without touching the write path, but it
72+
// cannot prevent the race window between two concurrent requests that both
73+
// pass this check simultaneously. The unique constraint on the DB is the
74+
// authoritative guard — P2002 below is the definitive conflict signal.
5675
if (parsed.data.username) {
5776
const existing = await app.prisma.user.findFirst({
5877
where: {
@@ -65,24 +84,36 @@ export async function profileRoutes(app: FastifyInstance) {
6584
}
6685
}
6786

68-
const updated = await app.prisma.user.update({
69-
where: { id: userId },
70-
data: parsed.data,
71-
select: {
72-
id: true,
73-
email: true,
74-
username: true,
75-
displayName: true,
76-
bio: true,
77-
pronouns: true,
78-
role: true,
79-
company: true,
80-
avatarUrl: true,
81-
accentColor: true,
82-
},
83-
});
87+
try {
88+
const response: ProfileUpdateResponse = await app.prisma.user.update({
89+
where: { id: userId },
90+
data: parsed.data,
91+
select: {
92+
id: true,
93+
email: true,
94+
username: true,
95+
displayName: true,
96+
bio: true,
97+
pronouns: true,
98+
role: true,
99+
company: true,
100+
avatarUrl: true,
101+
accentColor: true,
102+
},
103+
});
84104

85-
return updated;
105+
return response;
106+
} catch (err: any) {
107+
// Unique constraint violation — two concurrent requests raced through the
108+
// findFirst check above and both attempted the write. The DB constraint
109+
// fires on the losing request; surface it as a deterministic 409 rather
110+
// than leaking a raw Prisma error as a 500.
111+
if (err?.code === 'P2002') {
112+
return reply.status(409).send({ error: 'Username already taken' });
113+
}
114+
app.log.error({ err }, 'DB error in PUT /profiles/me');
115+
return reply.status(500).send({ error: 'Internal server error' });
116+
}
86117
});
87118

88119
// ─── Add Platform Link ───

0 commit comments

Comments
 (0)