From 48dda193e1d2776dc700d054c16db75091365574 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Fri, 12 Jun 2026 09:11:26 -0300 Subject: [PATCH] fix(backend): replayed named creates return existing row (#1320) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit custom-command and embed-template creates hit @@unique(guildId, name) on replay and surfaced a p2002-derived 500 even though the first request succeeded. catch p2002 in the two routes, fetch the existing row, and return it with 200 (idempotent success per the house pattern in decisions/2026-06-12-idempotency-posture.md). divergent payloads land on the same path — edit via patch. the 'created' server log is skipped on replay since nothing was created. --- packages/backend/src/routes/management.ts | 32 ++++++++-- .../backend/src/routes/managementEmbeds.ts | 36 ++++++++--- packages/backend/src/utils/prismaErrors.ts | 7 +++ .../integration/routes/management.test.ts | 60 +++++++++++++++++++ .../routes/managementEmbeds.test.ts | 48 ++++++++++++++- 5 files changed, 169 insertions(+), 14 deletions(-) create mode 100644 packages/backend/src/utils/prismaErrors.ts diff --git a/packages/backend/src/routes/management.ts b/packages/backend/src/routes/management.ts index e0a05da26..e824a86b1 100644 --- a/packages/backend/src/routes/management.ts +++ b/packages/backend/src/routes/management.ts @@ -19,6 +19,7 @@ import { } from '@lucky/shared/services' import { setupEmbedRoutes } from './managementEmbeds' import { setupAutoMessageRoutes } from './managementAutoMessages' +import { isUniqueViolation } from '../utils/prismaErrors' function p(val: string | string[]): string { return typeof val === 'string' ? val : val[0] @@ -149,12 +150,31 @@ export function setupManagementRoutes(app: Express): void { const userId = requireUserId(req) const body = s.createCommandBody.parse(req.body) const { name, response, description } = body - const command = await customCommandService.createCommand( - guildId, - name, - response, - { description, createdBy: userId }, - ) + let command + try { + command = await customCommandService.createCommand( + guildId, + name, + response, + { description, createdBy: userId }, + ) + } catch (error) { + if (!isUniqueViolation(error)) { + throw error + } + // P2002 on the (guildId, name) natural key is idempotent + // success, not a failure: return the existing command (#1320). + // Divergent payloads also land here — edit via PATCH. + const existing = await customCommandService.getCommand( + guildId, + name, + ) + if (!existing) { + throw error + } + res.json(existing) + return + } await serverLogService.logCustomCommandChange( guildId, 'created', diff --git a/packages/backend/src/routes/managementEmbeds.ts b/packages/backend/src/routes/managementEmbeds.ts index d4eb96ae2..9dd7cad91 100644 --- a/packages/backend/src/routes/managementEmbeds.ts +++ b/packages/backend/src/routes/managementEmbeds.ts @@ -11,6 +11,7 @@ import { serverLogService, type EmbedData, } from '@lucky/shared/services' +import { isUniqueViolation } from '../utils/prismaErrors' function p(val: string | string[]): string { return typeof val === 'string' ? val : val[0] @@ -58,13 +59,34 @@ export function setupEmbedRoutes(app: Express): void { validation.errors, ) } - const template = await embedBuilderService.createTemplate( - guildId, - name, - embedData, - description, - userId, - ) + let template + try { + template = await embedBuilderService.createTemplate( + guildId, + name, + embedData, + description, + userId, + ) + } catch (error) { + if (!isUniqueViolation(error)) { + throw error + } + // P2002 on the (guildId, name) natural key is idempotent + // success, not a failure: return the existing template + // (#1320). Divergent payloads also land here — edit via + // PATCH. createTemplate stores the name lowercased; + // getTemplate matches the stored value verbatim. + const existing = await embedBuilderService.getTemplate( + guildId, + name.toLowerCase(), + ) + if (!existing) { + throw error + } + res.json(existing) + return + } await serverLogService.logEmbedTemplateChange( guildId, 'created', diff --git a/packages/backend/src/utils/prismaErrors.ts b/packages/backend/src/utils/prismaErrors.ts new file mode 100644 index 000000000..6607a9bfa --- /dev/null +++ b/packages/backend/src/utils/prismaErrors.ts @@ -0,0 +1,7 @@ +/** + * Narrow check for Prisma's unique-constraint violation (P2002) without + * importing the generated client error class across package boundaries. + */ +export function isUniqueViolation(error: unknown): boolean { + return error instanceof Error && 'code' in error && error.code === 'P2002' +} diff --git a/packages/backend/tests/integration/routes/management.test.ts b/packages/backend/tests/integration/routes/management.test.ts index 9bdf89677..afa6e2312 100644 --- a/packages/backend/tests/integration/routes/management.test.ts +++ b/packages/backend/tests/integration/routes/management.test.ts @@ -39,6 +39,7 @@ jest.mock('@lucky/shared/services', () => ({ customCommandService: { listCommands: jest.fn(), createCommand: jest.fn(), + getCommand: jest.fn(), updateCommand: jest.fn(), deleteCommand: jest.fn(), }, @@ -430,6 +431,65 @@ describe('Management Routes Integration', () => { ).toHaveBeenCalled() }) + test('returns the existing command on a replayed create (P2002 → idempotent success, #1320)', async () => { + const mockSessionService = sessionService as jest.Mocked< + typeof sessionService + > + mockSessionService.getSession.mockResolvedValue(MOCK_SESSION_DATA) + + const existing = { + name: 'newcmd', + response: 'Hello there!', + description: 'A greeting command', + } + + const mockCustomCommandService = + customCommandService as jest.Mocked + mockCustomCommandService.createCommand.mockRejectedValue( + Object.assign(new Error('Unique constraint failed'), { + code: 'P2002', + }), + ) + mockCustomCommandService.getCommand.mockResolvedValue(existing) + + const mockServerLogService = serverLogService as jest.Mocked< + typeof serverLogService + > + + const response = await request(app) + .post('/api/guilds/111111111111111111/commands') + .set('Cookie', ['sessionId=valid_session_id']) + .send({ name: 'newcmd', response: 'Hello there!' }) + .expect(200) + + expect(response.body).toEqual(existing) + expect( + mockServerLogService.logCustomCommandChange, + ).not.toHaveBeenCalled() + }) + + test('rethrows P2002 when the existing command vanished (no silent 200)', async () => { + const mockSessionService = sessionService as jest.Mocked< + typeof sessionService + > + mockSessionService.getSession.mockResolvedValue(MOCK_SESSION_DATA) + + const mockCustomCommandService = + customCommandService as jest.Mocked + mockCustomCommandService.createCommand.mockRejectedValue( + Object.assign(new Error('Unique constraint failed'), { + code: 'P2002', + }), + ) + mockCustomCommandService.getCommand.mockResolvedValue(null) + + await request(app) + .post('/api/guilds/111111111111111111/commands') + .set('Cookie', ['sessionId=valid_session_id']) + .send({ name: 'newcmd', response: 'Hello there!' }) + .expect(500) + }) + test('should return 401 when not authenticated', async () => { const mockSessionService = sessionService as jest.Mocked< typeof sessionService diff --git a/packages/backend/tests/integration/routes/managementEmbeds.test.ts b/packages/backend/tests/integration/routes/managementEmbeds.test.ts index c9e16ec48..f123a8772 100644 --- a/packages/backend/tests/integration/routes/managementEmbeds.test.ts +++ b/packages/backend/tests/integration/routes/managementEmbeds.test.ts @@ -24,6 +24,7 @@ jest.mock('@lucky/shared/services', () => ({ embedBuilderService: { listTemplates: jest.fn(), createTemplate: jest.fn(), + getTemplate: jest.fn(), updateTemplate: jest.fn(), deleteTemplate: jest.fn(), validateEmbedData: jest.fn(), @@ -113,7 +114,9 @@ describe('Embed Management Routes Integration', () => { const mockGuildAccessServiceSvc = guildAccessService as jest.Mocked< typeof guildAccessService > - mockGuildAccessServiceSvc.resolveGuildContext.mockResolvedValue(null) + mockGuildAccessServiceSvc.resolveGuildContext.mockResolvedValue( + null, + ) const response = await request(app) .get('/api/guilds/111111111111111111/embeds') @@ -174,6 +177,49 @@ describe('Embed Management Routes Integration', () => { ).toHaveBeenCalled() }) + test('returns the existing template on a replayed create (P2002 → idempotent success, #1320)', async () => { + const existing = { + name: 'welcome', + description: 'Welcome template', + embedData: { title: 'Welcome Embed', color: '#341503' }, + } + + const mockEmbedService = embedBuilderService as jest.Mocked< + typeof embedBuilderService + > + mockEmbedService.validateEmbedData.mockReturnValue({ valid: true }) + mockEmbedService.createTemplate.mockRejectedValue( + Object.assign(new Error('Unique constraint failed'), { + code: 'P2002', + }), + ) + mockEmbedService.getTemplate.mockResolvedValue(existing) + + const mockServerLogService = serverLogService as jest.Mocked< + typeof serverLogService + > + + const response = await request(app) + .post('/api/guilds/111111111111111111/embeds') + .set('Cookie', ['sessionId=valid_session_id']) + .send({ + name: 'Welcome', + description: 'Welcome template', + embedData: { title: 'Welcome Embed', color: '#341503' }, + }) + .expect(200) + + expect(response.body).toEqual(existing) + // lookup uses the stored (lowercased) name + expect(mockEmbedService.getTemplate).toHaveBeenCalledWith( + '111111111111111111', + 'welcome', + ) + expect( + mockServerLogService.logEmbedTemplateChange, + ).not.toHaveBeenCalled() + }) + test('should return 401 when not authenticated', async () => { const mockSessionService = sessionService as jest.Mocked< typeof sessionService