fix: surface proper errors for People create/delete constraint violations#21270
Open
Pantkartik wants to merge 3 commits into
Open
fix: surface proper errors for People create/delete constraint violations#21270Pantkartik wants to merge 3 commits into
Pantkartik wants to merge 3 commits into
Conversation
|
👋 Thanks for contributing to Twenty! Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run. Looking forward to your contribution! |
…ions
When createPerson or deletePerson mutations hit Postgres constraint errors
(NOT_NULL_VIOLATION, FOREIGN_KEY_VIOLATION, RESTRICT_VIOLATION,
CHECK_VIOLATION), the catch-all in computeTwentyORMException was throwing a
generic PostgresException('Data validation error.') which the GraphQL layer
then returned as INTERNAL_SERVER_ERROR — masking the real cause entirely.
Root cause: the catch-all block at the bottom of computeTwentyORMException
matches every known Postgres error code (via Object.values(POSTGRESQL_ERROR_CODES).includes())
and discards the original error details, replacing them with the opaque
'Data validation error.' message.
Fix: add specific handling for common data-integrity constraint errors before
the catch-all, converting them to TwentyORMException(INVALID_INPUT) with a
user-friendly message. This surfaces the real reason for the failure to the
client and allows the GraphQL error handler to return BAD_USER_INPUT instead
of INTERNAL_SERVER_ERROR.
Errors now handled explicitly:
- NOT_NULL_VIOLATION (23502): required field missing
- FOREIGN_KEY_VIOLATION (23503): referenced record doesn't exist / can't delete
- RESTRICT_VIOLATION (23001): deletion blocked by dependent records
- CHECK_VIOLATION (23514): field value fails a database check constraint
Fixes twentyhq#21119
3cdd213 to
a74e0c5
Compare
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. 🧭 External PR Quality Review🟠 Needs triage for the following reason(s):
cc @prastoin Checks
Detailed findings (duplicate candidates, standards notes, summary) are in the workflow run logs. Automated pre-review — human approval still required. |
…ta-validation-error
…ng comments Replaces 4 structurally identical if-blocks with a single CONSTRAINT_VIOLATION_MESSAGES map lookup. Adding future constraint handlers is now a one-line map entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #21119 —
createPerson/deletePersonmutations fail with a genericINTERNAL_SERVER_ERROR: "Data validation error."instead of a meaningful error, blocking core CRM record management.Root Cause
The
computeTwentyORMExceptionfunction intwenty-ormcontains a catch-all block that matches every known Postgres error code viaObject.values(POSTGRESQL_ERROR_CODES).includes(errorCode)and discards all error detail, throwing:This
PostgresExceptionis then converted by the GraphQL error handler intoINTERNAL_SERVER_ERROR, masking the real constraint violation. Common mutations likecreatePersonthat hit:NOT_NULL_VIOLATION(23502) — a required field is missingFOREIGN_KEY_VIOLATION(23503) — referenced record missing or deletion blocked by a FKRESTRICT_VIOLATION(23001) — record deletion blocked by a referencing rowCHECK_VIOLATION(23514) — field value fails a DB check constraint...all silently surface as the same opaque
"Data validation error."withINTERNAL_SERVER_ERROR.Already handled correctly before the catch-all:
UNIQUE_VIOLATION→ delegates tohandleDuplicateKeyError✅INVALID_TEXT_REPRESENTATION→TwentyORMException(INVALID_INPUT)✅TwentyORMException(QUERY_READ_TIMEOUT)✅Fix
Add explicit handling before the catch-all for the four most common data-integrity constraint errors, converting them to
TwentyORMException(INVALID_INPUT)with a clear user-facing message. The GraphQL error handler then returnsBAD_USER_INPUT(400) instead ofINTERNAL_SERVER_ERROR(500).Changes
packages/twenty-server/src/engine/twenty-orm/error-handling/compute-twenty-orm-exception.tsAdded specific handling for:
23502NOT_NULL_VIOLATION23503FOREIGN_KEY_VIOLATION23001RESTRICT_VIOLATION23514CHECK_VIOLATIONBefore / After
Before:
{ "data": { "createPerson": null }, "errors": [{ "message": "Data validation error.", "extensions": { "code": "INTERNAL_SERVER_ERROR" } }] }After (e.g. NOT_NULL_VIOLATION):
{ "data": { "createPerson": null }, "errors": [{ "message": "A required field is missing. Please provide all required values and try again.", "extensions": { "code": "BAD_USER_INPUT" } }] }