Skip to content

REST: the UNIQUE_VIOLATION 409 message is hard-coded English and carries no field — and it is less informative than the bulk path's own message for the same constraint #7821

Closed

Description

@baozhoutao

Version: @objectstack/*@17.0.0-rc.6 (message originates in @objectstack/rest, packages/rest)

What happens

Save a record that violates a unique: true field and the REST layer answers with a hard-coded English sentence that no application can localize or override:

POST /api/v1/data/<object>            → 409 Conflict
{"error":"A record with this value already exists","code":"UNIQUE_VIOLATION","object":"<object>"}

Console renders body.error verbatim — in the form's error banner and in the toast. So an application whose every label, option and validation message is fully translated still shows this one English sentence to its end users, on one of the most commonly hit validation paths there is.

This is a server-side string, not a Console one — I checked both directions before filing:

  • the response body above carries it (read off the browser network panel on a real save, not inferred);
  • it appears in none of the Console bundle chunks (index-*.js, framework-*.js, ui-components-*.js, vendor-*.js — 0 hits, including the 2.8 MB ui-components chunk).

Minimal repro

  1. Any object with a text field declared unique: true — e.g. invoice.serial_no.
  2. Create a record with serial_no = "A-001".
  3. Create a second record with the same serial_no, from the Console form or by POST /api/v1/data/invoice.
  4. → 409 with the body above; the Console banner/toast show the raw English sentence.

The part that looks like an outright inconsistency

The same package already knows how to do better on the bulk / row path. sanitizeRowError resolves the offending column and names it:

var UNNAMED_CONFLICT = "A record with this value already exists.";
function sanitizeRowError(raw) {
  ...
  if (isUniqueViolationError(msg)) {
    const column = uniqueViolationColumn(msg);
    return column ? `A record with this ${column} already exists.` : UNNAMED_CONFLICT;
  }

…while the single-record error envelope hard-codes the unnamed variant even though uniqueViolationColumn is right there and the error object is in hand:

if (isUniqueViolationError2(error)) {
  return {
    status: 409,
    body: {
      error: "A record with this value already exists",
      code: "UNIQUE_VIOLATION",
      ...object ? { object } : {}
    }
  };
}

So the same platform gives two different answers to the same constraint depending on whether the write came in one row at a time or in a batch: the batch path tells the user which field collided, the single-record path does not. On a record with several unique fields, the user is told only that "a value" is taken and has to guess which one.

Why a client cannot work around it

code: "UNIQUE_VIOLATION" is in the body, which is enough to detect the class of failure — but the body carries no field / column. A client that wanted to render its own localized message therefore cannot name the field either; the only thing it can produce is a generic "some unique value is taken", which is strictly worse than what the platform itself already computes on the bulk path.

Note the asymmetry with application-authored errors: an error thrown from an app's own hook has its message surfaced verbatim, so app authors can localize those. It is specifically the platform's built-in constraint errors that are stuck in English.

Expected

Either of these would close it; (b) alone is already enough to unblock clients:

  • (a) resolve the message through the i18n service the way other user-facing platform strings are, so a locale bundle can translate it and an application can override it;
  • (b) include the offending field in the 409 body (field / column, alongside the existing code and object), so a client can build its own localized message — and, while there, reuse uniqueViolationColumn in the single-record branch so its default message reaches parity with the bulk path.

Same treatment would apply to the sibling constants in sanitizeRowError ("... is required.", "The database rejected this row (a value may be invalid or already in use).", "Row failed"), which are user-visible on the import path for the same reason.

Activity

  1. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Triage: pm:queue + domain:cli (+ bug).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. self-assigned this
    on Aug 12, 2026
  3. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Claim: PM loop round 5 — domain:cli seat.

    • Session: session_01B3Kurx8qufrDzNjk4rag7V (account hotlong)
    • Branch: claude/issue-7821-unique-violation-field
    • Worktree: dedicated (cloud container)
    • Domain: domain:cli
    • File surface: packages/rest — the single-record error envelope's isUniqueViolationError branch, and whatever it must import to reach uniqueViolationColumn + its tests. Stop on breach; explain in the report.
    • Container & model: M, mode:cloud, model: claude-opus-5.
    • Serial constraints cleared: 0 in flight in this lane; all nine of this shift's PRs are MERGED, including fix(meta): gate _drafts outlets as authoring surfaces (ADR-0106 D5(4), #6599) #7907 which held the rest-server.ts _drafts region. packages/rest is free.

    Note on this card's readability — it is NOT truncated

    An earlier read of this body cut off at POST /api/v1/data/ and looked like the sanitizer swallowing a bare &lt;object&gt;. It is not: a second read through a different API path returns the body whole, bare angle brackets and all. That was read-side truncation in the first tool call — the failure mode that must be ruled out before blaming the sanitizer. Recording it so nobody re-opens the question or "repairs" a body that is fine.

    裁决:实施 (b),⛔ 不做 (a)

    Do (b): include the offending field in the 409 body (alongside the existing code and object), and reuse uniqueViolationColumn in the single-record branch so its default message reaches parity with the bulk path.

    This is a parity fix, not a feature: the platform already resolves the colliding column on the bulk path (sanitizeRowError → uniqueViolationColumn → "A record with this <column> already exists.") and withholds it on the single-record path, with the error object in hand and the helper in the same package. One rule, two implementations, one of them strictly worse — the same shape this lane has been closing all shift.

    ⛔ Do NOT do (a) (routing the message through the i18n service). Not because it is wrong — because it is not this card's decision to make. #7307 reports the identical class on DELETE_RESTRICTED, and the sibling constants in sanitizeRowError ("... is required.", "The database rejected this row…", "Row failed") are the same family. How platform-built-in error copy gets localized is one architectural answer owed to all of them at once, not something to settle inside a 409 body fix. The card itself says (b) alone unblocks clients — that is the scope.

    The premise this ruling hangs on — verify before coding

    uniqueViolationColumn must actually be reachable and reliable at the single-record branch: it resolves the column from the driver's raw error text, so confirm it returns a real column on the single-record error shape and not just on the bulk one, and that it degrades to the unnamed message rather than to undefined/garbage when it cannot parse (e.g. a driver whose message format it does not know). If it does NOT work at that site, ⛔ do not fabricate a parser — report needs_decision with what you measured.

    Pin the consequence, both directions

    • A single-record POST violating a unique field returns 409 with code: 'UNIQUE_VIOLATION', the object, and the field, and a default message naming that field.
    • Parity: the same constraint hit through the bulk/row path still names the field as it does today (⛔ unchanged — do not "unify" by making the bulk path worse).
    • Degradation: an unparseable driver error still returns the 409 with the unnamed message and no bogus field — a wrong field name is worse than none.
    • Assert code and status per ADR-0112, not that it threw.

    Adding a field to a response body is additive, but confirm no envelope/route-ledger gate pins the exact 409 body shape; if one does, update it in the same PR rather than working around it.


    Generated by Claude Code

  4. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor
    {
      "issue": 7821,
      "status": "done",
      "branch": "claude/issue-7821-unique-violation-field",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/7930",
      "premise_still_valid": true,
      "summary": "Implemented (b) only; (a) deliberately not done. `mapDataError`'s `isUniqueViolationError` branch now resolves the colliding column via `uniqueViolationColumn` and puts it on the 409 wire as `field`, with the default message naming it — reaching parity with the bulk path, which is untouched. PREMISE VERIFIED BEFORE CODING against 16 driver shapes on origin/main @ 3c9a67e through the real `mapDataError`: the helper is reachable and reliable at the single-record site, and passing the ERROR OBJECT rather than `error.message` is strictly better than the bulk path's string read — `col(error)`=email vs `col(error.message)`=undefined for node-postgres's `DETAIL:` on `error.detail` and for that shape behind a wrapped `cause`, which is the driver we actually ship. 4/16 shapes name a field (pg DETAIL flattened, pg detail-field, pg detail-via-cause, sqlite table.column); the other 12 degrade to `undefined` — never garbage. Defect confirmed still live on current main: field=undefined and the unnamed sentence on every shape. Disclosure boundary unchanged: user data, index names and the `table.` qualifier still never reach the wire (`sys_user.email` -> `email`); the pre-existing per-dialect withholding assertions are untouched and green. The old 'one fixed sentence' pin was WIDENED (set-equality against exactly two sentences one bare identifier apart), not relaxed, so driver text leaking into body.error still goes red. No envelope or route-ledger gate pins the 409 body shape — `check:route-envelope` passes unchanged and the error-code ledger registers codes, not shapes.",
      "tests": {
        "reverse_verification": "Source reverted, tests kept: 7 assertions RED (2 named-field dialect cases via message, 2 via the object-only detail/cause channels, the two-sentence pin, the additive-field case, the bulk-path parity case). Fix applied: 78 PASS. The degradation half passes both ways by design (it is the invariant), so a 'both halves are populated' guard keeps the new pin non-vacuous.",
        "suite": "packages/rest full vitest suite: 92 files, 1519 tests, all passing.",
        "assertions": "code AND status per ADR-0112 throughout — never toThrow(). Degradation asserted as not.toHaveProperty('field') rather than === undefined, so a field:null/'' on the wire also fails. MySQL pinned by name so deriving a column from an index name would go red.",
        "new_dialect_shapes": "node-postgres DETAIL on error.detail; DETAIL behind a wrapped cause; composite key; SQLite `index 'x'` form.",
        "gates": "check:route-envelope, check:authz-resolver, check:meta-type-normalized (the three dispatch-gates.mjs matched for the source paths); check:changeset-gate-self-tests, check:objectui-changeset, check:empty-changeset, check:release-notes, check-changeset-no-major (.changeset); check:nul-bytes; check:error-code-casing — all green.",
        "type_check_debt": "GREEN. Run after `turbo run build --filter='./packages/*' --filter='./packages/*/*'` exactly as lint.yml does: '33 ledger entries re-measured, 1789 raw tsc errors total, none above its recorded number.' No bullet error line for packages/rest; no ledger entry raised (#6376 respected). packages/rest `tsc --noEmit` also clean.",
        "known_preexisting": "One lint error on rest-server.ts (@typescript-eslint/ban-ts-comment 'Definition for rule was not found') is present identically on pristine origin/main at line 4342, shifted to 4383 by this diff's +41 lines. Confirmed by linting the reverted file in place. Not introduced here; a config-resolution artifact of invoking eslint directly at the repo root."
      },
      "open_questions": [
        "Resolved myself, recorded not asked — WIRE KEY NAME: used `field`, not `column`. The issue offered either; `field` is already this file's established error-body key (rest-server.ts:912 and the per-field VALIDATION_FAILED envelope), and the REST surface is object/field-shaped while `column` is the driver's vocabulary.",
        "Resolved myself — PUNCTUATION LEFT DIVERGENT ON PURPOSE. The bulk path ends its sentences with a period ('A record with this email already exists.'); the single-record path never has. I kept the single-record path period-less so the unnamed sentence stays byte-identical to today. Adding a period would have been an unrequested wire change breaking several unrelated pinned assertions (rest.test.ts, rest-5xx-status-passthrough, rest-unknown-object-heuristic, rest-hook-refusal-status-passthrough) for cosmetics. Parity as ruled = naming the field, which is delivered. Flagging because it is a real, pre-existing, still-live inconsistency between the two paths.",
        "Not a blocker, noted for the (a)-family decision: MySQL deployments get NO field at all, since MySQL's duplicate-entry text names only the index. That is `uniqueViolationColumn`'s documented and deliberate contract (#6544), not a gap introduced here — but it means the localizability half of this card is unresolved for MySQL users in a way `field` alone cannot fix."
      ],
      "out_of_scope_findings": [
        "None filed. The only defects encountered adjacent to this card are the i18n family the ruling explicitly fenced off — #7307 (DELETE_RESTRICTED) and sanitizeRowError's sibling constants ('... is required.', 'The database rejected this row...', 'Row failed') — which are already tracked and were deliberately not folded in. The punctuation divergence above is cosmetic and recorded as an open question rather than filed as a finding."
      ]
    }

    Generated by Claude Code

  5. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    ACCEPTED — PR #7930. Landing on gate-green.

    Reviewed by the domain:cli seat (hotlong, session session_01B3Kurx8qufrDzNjk4rag7V) against the diff. The ruling was honored exactly: (b) only, and the i18n half is explicitly left to the #7307 family rather than smuggled in.

    It found something my dispatch did not know

    I told it to reuse uniqueViolationColumn for parity with the bulk path. It measured that passing the error object rather than error.message resolves more than the bulk path can: sanitizeRowError only ever holds a string, while uniqueViolationColumn additionally reads detail and one step of cause — and that is where the column actually is for the Postgres driver we ship, since node-postgres keeps its DETAIL: Key (email)=(…) line on error.detail and off the message. Measured on origin/main: that shape resolves email from the object and undefined from the message. There is now a dialect sample for exactly that shape, and one for it behind a re-throw, so the object-vs-string distinction is pinned rather than incidental.

    The degradation clause is asserted as a contract, at the right strictness

    expect(r.body).not.toHaveProperty('field') — not field === undefined — with the reason stated: "a field: null or field: '' on the wire is the same defect wearing a different type." Composite keys, SQLite's index 'x' form, and MySQL (always an index name, never a column) all take that path. MySQL is asserted by name rather than left to the table, with the note that if someone later "improves" the helper by deriving email from idx_email_unique, the test goes red — which is the point.

    The modified assertion was widened correctly, not loosened

    This is the part I check hardest, because changing an existing assertion to accommodate your own diff is how a suite quietly loses its teeth. The old test pinned one sentence across all dialects. It now pins exactly two — via set-equality, deliberately not relaxed to a regex — with the reasoning that the invariant was never "one sentence" but "the body is assembled from fixed text and values this package chose, never from driver prose." A third distinct sentence still fails, whatever it says. That preserves the assertion's defensive power instead of trading it away.

    There is also a vacuity guard I did not ask for: expect(NAMED.length).toBeGreaterThan(0) / expect(UNNAMED.length).toBeGreaterThan(0), so deleting the field entries from the sample table cannot make the whole block pass by having nothing left to assert.

    The #6250 security invariant is intact, and re-argued rather than assumed

    The pre-existing assertions — the body must not echo driver prose, the offending value, the index name, or the table. qualifier — are unchanged and still green. The PR explains why the column is the one item in that list that is safe to add: it is a schema identifier the caller already posted, it arrives as a bare [A-Za-z_][A-Za-z0-9_$]* from uniqueViolationColumn and never as driver text, and sys_user.email still yields email so the not.toContain('sys_user.email') limb keeps holding. Adding a field to an error body next to a disclosure guard is precisely where a reviewer should be suspicious, and the diff answers it before being asked.

    Bulk path untouched — my explicit requirement ("converge upward or not at all"). The parity test compares the two paths on one conflict rather than re-testing either.

    Scope

    4 files: packages/rest/src/rest-server.ts, its dialect test, the changeset, and content/docs/protocol/kernel/http-protocol.mdx. That last one is beyond the surface I declared — and it is the right call, not creep: it is the page documenting the 409 wire shape, so shipping the new field without it would leave the protocol docs stating a body that is no longer what the server sends. The Callout correctly presents field as best-effort and optional, tells clients to key on code rather than the message, and states the degradation. ⛔ No content/docs/releases/, no docs/adr/**, no skills paths.

    ⛔ Not flipped yet: ESLint and TypeScript Type Check must both read conclusion: success on the head commit. ⚠️ packages/rest is the package whose TEST_DEBT ratchet caught a sibling PR earlier today (155 → 156) and this PR adds ~225 test lines — if it trips, the fix is to type the new tests, ⛔ never to raise the ledger.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions