fix(types,rest): one named unique-violation predicate — MySQL conflicts return 409 UNIQUE_VIOLATION, not 500 (#6250)

.changeset/unique-violation-shared-predicate.md

@@ -0,0 +1,58 @@
+---
+"@objectstack/types": patch
+"@objectstack/rest": patch
+---
+
+fix(types,rest): one named unique-violation predicate — a MySQL conflict is 409 UNIQUE_VIOLATION, not 500 (#6250)
+
+**On MySQL, every unique-constraint conflict came back as `500 INTERNAL_ERROR`.**
+The API contract registers `UNIQUE_VIOLATION` as a 409 code
+(`packages/spec/src/api/error-code-ledger.zod.ts`), so a front end had no way to
+tell "this email is already taken" from "the server fell over" — no retry advice,
+no field to point at, and a 5xx in the operator's dashboards for what is an
+ordinary client outcome. SQLite and Postgres deployments never saw it, which is
+why it survived: their conflict prose happens to contain the words the mapping
+looked for.
+
+**Cause: the conflict verdict was nested inside a leak heuristic.** REST's 409
+branch lived inside the true-branch of `looksLikeInternalErrorLeak()`, keyed on
+the substrings `unique constraint` / `unique violation`. MySQL says
+`ER_DUP_ENTRY: Duplicate entry '…' for key '…'`, which matches no limb of that
+heuristic, so the conflict never reached the `if` at all and fell out of the
+terminal `UNCLASSIFIED_FAULT`. Two unrelated questions — "is this a conflict?"
+and "would echoing this text leak internals?" — had been fused into one, and
+MySQL is where they disagree.
+
+Measured on the previous release, through the real error mapper:
+
+```
+mysql,    bare message       500 INTERNAL_ERROR  →  409 UNIQUE_VIOLATION
+mysql,    knex-wrapped SQL   500 DATABASE_ERROR  →  409 UNIQUE_VIOLATION
+postgres, SQLSTATE only      500 INTERNAL_ERROR  →  409 UNIQUE_VIOLATION
+sqlite,   message            409 UNIQUE_VIOLATION   (unchanged)
+postgres, message            409 UNIQUE_VIOLATION   (unchanged)
+```
+
+So the hole was never MySQL-only: the mapping read one of the two channels
+drivers use. A Postgres error carrying SQLSTATE `23505` with unremarkable prose
+was a 500 as well.
+
+**New: `isUniqueViolationError(error)`, exported from `@objectstack/types`.** One
+named predicate replaces the substring test, reading every channel a driver
+uses — `code` (`23505` / `ER_DUP_ENTRY` / `SQLITE_CONSTRAINT_UNIQUE`), `errno`
+(`1062`), the message, and one step down the `cause` chain that pool and
+query-builder layers wrap with. Its vocabulary is the union of the four
+hand-written copies the repo already carried, so routing REST through it cannot
+narrow any verdict clients rely on today; an unrecognised error is never a
+conflict, because a false 409 tells an SDK not to retry and points the user at a
+value that is fine.
+
+**The internal-leak classifier is byte-identical.** The fix hoists the conflict
+question out of it rather than widening its criteria, so nothing else it guards
+is reclassified as safe-to-expose. And the 409 body is fixed text: MySQL embeds
+the offending user data in its message (`Duplicate entry 'a@b.com' …`) and
+Postgres the index and column names, none of which reaches the client. The full
+driver text still reaches the server log.
+
+No action needed. Clients that already handled `409 UNIQUE_VIOLATION` on SQLite
+and Postgres now receive it on MySQL too.

packages/rest/src/rest-server.ts

@@ -7,6 +7,7 @@ import {
 import {
     isMcpServerEnabled,
     looksLikeInternalErrorLeak,
+    isUniqueViolationError,
     declaresServerFault,
     INTERNAL_ERROR_MESSAGE,
 } from '@objectstack/types';
@@ -743,6 +744,62 @@ export function mapDataError(error: any, object?: string): { status: number; bod
         };
     }
 
+    // [#6250] Unique-constraint conflict → 409 `UNIQUE_VIOLATION`.
+    //
+    // The verdict is the shared `isUniqueViolationError` predicate
+    // (`@objectstack/types`), and BOTH halves of that sentence are the fix.
+    //
+    // **Why it moved up here.** This branch used to live *inside* the
+    // `looksLikeInternalErrorLeak(raw)` true-branch below, so a conflict was
+    // recognised only if the message first looked like a server-internals leak
+    // — two unrelated questions, one nested inside the other. MySQL is where
+    // they disagree. `ER_DUP_ENTRY: Duplicate entry 'a@b.com' for key
+    // 'idx_email_unique'` matches not one of the leak heuristic's limbs
+    // (`sqlite_` / `sqlstate` / `constraint failed` / `unique constraint` /
+    // `foreign key` / a leading `insert into `/`update `/`select `/`delete
+    // from `), so it never reached the `if` at all and fell out of
+    // `UNCLASSIFIED_FAULT` as `500 INTERNAL_ERROR` — on EVERY unique conflict
+    // in a MySQL deployment, against an API contract that registers
+    // `UNIQUE_VIOLATION` (`error-code-ledger.zod.ts`). The front end could not
+    // tell "this email is taken" from "the server fell over". SQLite and
+    // Postgres hid it: their prose happens to contain `unique constraint`.
+    //
+    // The fix is deliberately NOT to teach the leak heuristic about MySQL.
+    // That heuristic decides what text is unsafe to echo; widening it to reach
+    // a status mapping would make an information-disclosure rule depend on a
+    // conflict vocabulary, and every future dialect would have to be taught to
+    // both. Asking the conflict question by name, first and independently, is
+    // the #5841 `isMissingTableError` move — and it leaves the leak classifier
+    // byte-identical, so nothing else it guards is reclassified.
+    //
+    // **Why the predicate rather than more substrings.** The message is only
+    // one of the two channels drivers use. Postgres surfaces SQLSTATE `23505`
+    // and mysql2 an `ER_DUP_ENTRY` / `errno 1062` — measured, a Postgres error
+    // carrying the code but a plain message was also a 500 here. The predicate
+    // reads code, errno, message and one step of `cause`; a substring added to
+    // this file would have been the fifth private vocabulary, which is the
+    // defect #6250 is named for.
+    //
+    // **The body says nothing the driver said.** The message is a fixed
+    // sentence and the only interpolated value is the object name the ROUTE
+    // supplied. That is load-bearing, not incidental: MySQL's text embeds the
+    // offending USER DATA (`Duplicate entry 'acme@example.com' …`) and
+    // Postgres' embeds the index and column names, so echoing the driver here
+    // would trade a status-code bug for an information-disclosure one. Pinned
+    // in `rest-unique-violation-dialects.test.ts`. The full text still reaches
+    // the operator: `handleRouteError` / `logWithheldServerFault` log the
+    // original error untouched.
+    if (isUniqueViolationError(error)) {
+        return {
+            status: 409,
+            body: {
+                error: 'A record with this value already exists',
+                code: 'UNIQUE_VIOLATION',
+                ...(object ? { object } : {}),
+            },
+        };
+    }
+
     const raw = String(error?.message ?? error ?? '');
     const lower = raw.toLowerCase();
 
@@ -945,18 +1002,13 @@ export function mapDataError(error: any, object?: string): { status: number; bod
     // returned raw SQL to clients. Behaviour here is unchanged; only the
     // predicate's home moved.
     if (looksLikeInternalErrorLeak(raw)) {
-        // Surface unique-constraint violations as a structured 409 so
-        // the UI can map them to "this value already exists".
-        if (lower.includes('unique constraint') || lower.includes('unique violation')) {
-            return {
-                status: 409,
-                body: {
-                    error: 'A record with this value already exists',
-                    code: 'UNIQUE_VIOLATION',
-                    ...(object ? { object } : {}),
-                },
-            };
-        }
+        // [#6250] The unique-constraint 409 used to be nested HERE, keyed on
+        // `unique constraint` / `unique violation`. Both substrings are now
+        // limbs of the shared `isUniqueViolationError` predicate, which runs
+        // far above this line and unconditionally — so this branch cannot
+        // narrow the verdict, and a conflict no longer has to look like a leak
+        // to be recognised as one. What is left here is the original job:
+        // withhold text that would ship driver internals.
         return DATA_STORE_FAULT();
     }
     return UNCLASSIFIED_FAULT();