From 8f4fa2a233fa599bedd059517926e7a70cfd120c Mon Sep 17 00:00:00 2001 From: Lukas May Date: Fri, 6 Mar 2026 14:37:21 +0100 Subject: [PATCH] fix: disambiguate CONFLICT errors in registerProject by inspecting the SQLite UNIQUE constraint column Previously a single "that name or URL" message was thrown regardless of which column violated uniqueness. Now the catch block inspects the error string from SQLite to emit a name-specific or url-specific message, with a generic fallback when neither column can be identified. Adds vitest tests covering all four scenarios: name conflict, url conflict, unknown column conflict, and non-UNIQUE error passthrough. Co-Authored-By: Claude Sonnet 4.6 --- apps/server/trpc/routers/project.test.ts | 92 ++++++++++++++++++++++++ apps/server/trpc/routers/project.ts | 17 ++++- 2 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 apps/server/trpc/routers/project.test.ts diff --git a/apps/server/trpc/routers/project.test.ts b/apps/server/trpc/routers/project.test.ts new file mode 100644 index 0000000..4e0beb7 --- /dev/null +++ b/apps/server/trpc/routers/project.test.ts @@ -0,0 +1,92 @@ +/** + * Tests for registerProject CONFLICT error disambiguation. + * Verifies that UNIQUE constraint failures on specific columns produce + * column-specific error messages. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { TRPCError } from '@trpc/server'; +import { router, publicProcedure, createCallerFactory } from '../trpc.js'; +import { projectProcedures } from './project.js'; +import type { TRPCContext } from '../context.js'; +import type { ProjectRepository } from '../../db/repositories/project-repository.js'; + +const testRouter = router({ + ...projectProcedures(publicProcedure), +}); + +const createCaller = createCallerFactory(testRouter); + +function makeCtx(mockCreate: () => Promise): TRPCContext { + const projectRepository: ProjectRepository = { + create: mockCreate as unknown as ProjectRepository['create'], + findById: vi.fn().mockResolvedValue(null), + findByName: vi.fn().mockResolvedValue(null), + findAll: vi.fn().mockResolvedValue([]), + update: vi.fn(), + delete: vi.fn(), + addProjectToInitiative: vi.fn(), + removeProjectFromInitiative: vi.fn(), + findProjectsByInitiativeId: vi.fn().mockResolvedValue([]), + setInitiativeProjects: vi.fn(), + }; + + return { + eventBus: {} as TRPCContext['eventBus'], + serverStartedAt: null, + processCount: 0, + projectRepository, + // No workspaceRoot — prevents cloneProject from running + }; +} + +const INPUT = { name: 'my-project', url: 'https://github.com/example/repo' }; + +describe('registerProject — CONFLICT error disambiguation', () => { + it('throws CONFLICT with name-specific message on projects.name UNIQUE violation', async () => { + const caller = createCaller(makeCtx(() => { + throw new Error('UNIQUE constraint failed: projects.name'); + })); + + const err = await caller.registerProject(INPUT).catch(e => e); + expect(err).toBeInstanceOf(TRPCError); + expect(err.code).toBe('CONFLICT'); + expect(err.message).toBe('A project with this name already exists'); + }); + + it('throws CONFLICT with url-specific message on projects.url UNIQUE violation', async () => { + const caller = createCaller(makeCtx(() => { + throw new Error('UNIQUE constraint failed: projects.url'); + })); + + const err = await caller.registerProject(INPUT).catch(e => e); + expect(err).toBeInstanceOf(TRPCError); + expect(err.code).toBe('CONFLICT'); + expect(err.message).toBe('A project with this URL already exists'); + }); + + it('throws CONFLICT with fallback message on unknown UNIQUE constraint violation', async () => { + const caller = createCaller(makeCtx(() => { + throw new Error('UNIQUE constraint failed: projects.unknown_col'); + })); + + const err = await caller.registerProject(INPUT).catch(e => e); + expect(err).toBeInstanceOf(TRPCError); + expect(err.code).toBe('CONFLICT'); + expect(err.message).toBe('A project with this name or URL already exists'); + }); + + it('rethrows non-UNIQUE errors without wrapping in a CONFLICT', async () => { + const originalError = new Error('SQLITE_BUSY'); + const caller = createCaller(makeCtx(() => { + throw originalError; + })); + + const err = await caller.registerProject(INPUT).catch(e => e); + expect(err).toBeDefined(); + // Must not be surfaced as a CONFLICT — the catch block should re-throw as-is + expect(err).not.toMatchObject({ code: 'CONFLICT' }); + // The original error message must be preserved somewhere + expect(err.message).toContain('SQLITE_BUSY'); + }); +}); diff --git a/apps/server/trpc/routers/project.ts b/apps/server/trpc/routers/project.ts index 5d79400..cb0188e 100644 --- a/apps/server/trpc/routers/project.ts +++ b/apps/server/trpc/routers/project.ts @@ -30,11 +30,24 @@ export function projectProcedures(publicProcedure: ProcedureBuilder) { ...(input.defaultBranch && { defaultBranch: input.defaultBranch }), }); } catch (error) { - const msg = (error as Error).message; + const msg = (error as Error).message ?? ''; if (msg.includes('UNIQUE') || msg.includes('unique')) { + if (msg.includes('projects.name') || (msg.includes('name') && !msg.includes('url'))) { + throw new TRPCError({ + code: 'CONFLICT', + message: 'A project with this name already exists', + }); + } + if (msg.includes('projects.url') || msg.includes('url')) { + throw new TRPCError({ + code: 'CONFLICT', + message: 'A project with this URL already exists', + }); + } + // fallback: neither column identifiable throw new TRPCError({ code: 'CONFLICT', - message: `A project with that name or URL already exists`, + message: 'A project with this name or URL already exists', }); } throw error;