fix: surface proper errors for People create/delete constraint violations (#21270)
## Summary Fixes #21119 — `createPerson` / `deletePerson` mutations fail with a generic `INTERNAL_SERVER_ERROR: "Data validation error."` instead of a meaningful error, blocking core CRM record management. ## Root Cause The `computeTwentyORMException` function in `twenty-orm` contains a catch-all block that matches **every known Postgres error code** via `Object.values(POSTGRESQL_ERROR_CODES).includes(errorCode)` and discards all error detail, throwing: ```ts throw new PostgresException('Data validation error.', errorCode); ``` This `PostgresException` is then converted by the GraphQL error handler into `INTERNAL_SERVER_ERROR`, masking the real constraint violation. Common mutations like `createPerson` that hit: - **`NOT_NULL_VIOLATION` (23502)** — a required field is missing - **`FOREIGN_KEY_VIOLATION` (23503)** — referenced record missing or deletion blocked by a FK - **`RESTRICT_VIOLATION` (23001)** — record deletion blocked by a referencing row ...all silently surface as the same opaque `"Data validation error."` with `INTERNAL_SERVER_ERROR`. Already handled correctly before the catch-all: - `UNIQUE_VIOLATION` → delegates to `handleDuplicateKeyError` ✅ - `INVALID_TEXT_REPRESENTATION` → `TwentyORMException(INVALID_INPUT)` ✅ - Query read timeout → `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 returns `BAD_USER_INPUT` (400) instead of `INTERNAL_SERVER_ERROR` (500). ## Changes ### `packages/twenty-server/src/engine/twenty-orm/error-handling/compute-twenty-orm-exception.ts` Added specific handling for: | Postgres Code | Constant | User-facing message | |---|---|---| | `23502` | `NOT_NULL_VIOLATION` | "A required field is missing. Please provide all required values and try again." | | `23503` | `FOREIGN_KEY_VIOLATION` | "This operation references a record that does not exist or cannot be modified due to existing relationships." | | `23001` | `RESTRICT_VIOLATION` | "This record cannot be deleted because it is still referenced by other records." | ## Before / After **Before:** ```json { "data": { "createPerson": null }, "errors": [{ "message": "Data validation error.", "extensions": { "code": "INTERNAL_SERVER_ERROR" } }] } ``` **After (e.g. NOT_NULL_VIOLATION):** ```json { "data": { "createPerson": null }, "errors": [{ "message": "A required field is missing. Please provide all required values and try again.", "extensions": { "code": "BAD_USER_INPUT" } }] } ``` --------- Co-authored-by: Pantkartik <pantkartik@github.com> Co-authored-by: Charles Bochet <charles@twenty.com> Co-authored-by: Etienne <45695613+etiennejouan@users.noreply.github.com>
This commit is contained in:
+198
@@ -0,0 +1,198 @@
|
||||
import { msg } from '@lingui/core/macro';
|
||||
import { QueryFailedError } from 'typeorm';
|
||||
|
||||
import { POSTGRESQL_ERROR_CODES } from 'src/engine/api/graphql/workspace-query-runner/constants/postgres-error-codes.constants';
|
||||
import { handleDuplicateKeyError } from 'src/engine/api/graphql/workspace-query-runner/utils/handle-duplicate-key-error.util';
|
||||
import { PostgresException } from 'src/engine/api/graphql/workspace-query-runner/utils/postgres-exception';
|
||||
import { computeTwentyORMException } from 'src/engine/twenty-orm/error-handling/compute-twenty-orm-exception';
|
||||
import {
|
||||
TwentyORMException,
|
||||
TwentyORMExceptionCode,
|
||||
} from 'src/engine/twenty-orm/exceptions/twenty-orm.exception';
|
||||
|
||||
jest.mock(
|
||||
'src/engine/api/graphql/workspace-query-runner/utils/handle-duplicate-key-error.util',
|
||||
() => ({
|
||||
handleDuplicateKeyError: jest.fn(),
|
||||
}),
|
||||
);
|
||||
|
||||
const handleDuplicateKeyErrorMock = handleDuplicateKeyError as jest.Mock;
|
||||
|
||||
const buildQueryFailedError = (
|
||||
code: string | undefined,
|
||||
message = 'query failed',
|
||||
): QueryFailedError => {
|
||||
const driverError = new Error(message);
|
||||
|
||||
if (code !== undefined) {
|
||||
Object.assign(driverError, { code });
|
||||
}
|
||||
|
||||
return new QueryFailedError('SELECT 1', [], driverError);
|
||||
};
|
||||
|
||||
describe('computeTwentyORMException', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should return an INVALID_INPUT exception with a missing-field message when error is a NOT_NULL_VIOLATION', async () => {
|
||||
const error = buildQueryFailedError(
|
||||
POSTGRESQL_ERROR_CODES.NOT_NULL_VIOLATION,
|
||||
);
|
||||
|
||||
const result = await computeTwentyORMException(error);
|
||||
|
||||
expect(result).toBeInstanceOf(TwentyORMException);
|
||||
expect((result as TwentyORMException).code).toBe(
|
||||
TwentyORMExceptionCode.INVALID_INPUT,
|
||||
);
|
||||
expect((result as TwentyORMException).userFriendlyMessage).toEqual(
|
||||
msg`A required field is missing. Please provide all required values and try again.`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return an INVALID_INPUT exception with a relationship message when error is a FOREIGN_KEY_VIOLATION', async () => {
|
||||
const error = buildQueryFailedError(
|
||||
POSTGRESQL_ERROR_CODES.FOREIGN_KEY_VIOLATION,
|
||||
);
|
||||
|
||||
const result = await computeTwentyORMException(error);
|
||||
|
||||
expect(result).toBeInstanceOf(TwentyORMException);
|
||||
expect((result as TwentyORMException).code).toBe(
|
||||
TwentyORMExceptionCode.INVALID_INPUT,
|
||||
);
|
||||
expect((result as TwentyORMException).userFriendlyMessage).toEqual(
|
||||
msg`This operation references a record that does not exist or cannot be modified due to existing relationships.`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return an INVALID_INPUT exception with a still-referenced message when error is a RESTRICT_VIOLATION', async () => {
|
||||
const error = buildQueryFailedError(
|
||||
POSTGRESQL_ERROR_CODES.RESTRICT_VIOLATION,
|
||||
);
|
||||
|
||||
const result = await computeTwentyORMException(error);
|
||||
|
||||
expect(result).toBeInstanceOf(TwentyORMException);
|
||||
expect((result as TwentyORMException).code).toBe(
|
||||
TwentyORMExceptionCode.INVALID_INPUT,
|
||||
);
|
||||
expect((result as TwentyORMException).userFriendlyMessage).toEqual(
|
||||
msg`This record cannot be deleted because it is still referenced by other records.`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should preserve the original message when surfacing a constraint violation as INVALID_INPUT', async () => {
|
||||
const error = buildQueryFailedError(
|
||||
POSTGRESQL_ERROR_CODES.NOT_NULL_VIOLATION,
|
||||
'null value in column "name" violates not-null constraint',
|
||||
);
|
||||
|
||||
const result = await computeTwentyORMException(error);
|
||||
|
||||
expect((result as TwentyORMException).message).toBe(
|
||||
'null value in column "name" violates not-null constraint',
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw the generic PostgresException when error is a CHECK_VIOLATION (handled by the catch-all)', async () => {
|
||||
const error = buildQueryFailedError(POSTGRESQL_ERROR_CODES.CHECK_VIOLATION);
|
||||
|
||||
await expect(computeTwentyORMException(error)).rejects.toThrow(
|
||||
PostgresException,
|
||||
);
|
||||
await expect(computeTwentyORMException(error)).rejects.toMatchObject({
|
||||
message: 'Data validation error.',
|
||||
code: POSTGRESQL_ERROR_CODES.CHECK_VIOLATION,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return an INVALID_INPUT exception with the original message when error is an INVALID_TEXT_REPRESENTATION', async () => {
|
||||
const error = buildQueryFailedError(
|
||||
POSTGRESQL_ERROR_CODES.INVALID_TEXT_REPRESENTATION,
|
||||
'invalid input syntax for type uuid',
|
||||
);
|
||||
|
||||
const result = await computeTwentyORMException(error);
|
||||
|
||||
expect(result).toBeInstanceOf(TwentyORMException);
|
||||
expect((result as TwentyORMException).code).toBe(
|
||||
TwentyORMExceptionCode.INVALID_INPUT,
|
||||
);
|
||||
expect((result as TwentyORMException).message).toBe(
|
||||
'invalid input syntax for type uuid',
|
||||
);
|
||||
});
|
||||
|
||||
it('should return a QUERY_READ_TIMEOUT exception when error message mentions a query read timeout', async () => {
|
||||
const error = buildQueryFailedError(undefined, 'Query read timeout');
|
||||
|
||||
const result = await computeTwentyORMException(error);
|
||||
|
||||
expect(result).toBeInstanceOf(TwentyORMException);
|
||||
expect((result as TwentyORMException).code).toBe(
|
||||
TwentyORMExceptionCode.QUERY_READ_TIMEOUT,
|
||||
);
|
||||
});
|
||||
|
||||
it('should delegate to handleDuplicateKeyError when error is a UNIQUE_VIOLATION and metadata context is provided', async () => {
|
||||
const error = buildQueryFailedError(
|
||||
POSTGRESQL_ERROR_CODES.UNIQUE_VIOLATION,
|
||||
);
|
||||
const objectMetadata = { nameSingular: 'person' } as never;
|
||||
const entityManager = {} as never;
|
||||
const internalContext = {} as never;
|
||||
const duplicateException = new TwentyORMException(
|
||||
'A duplicate entry was detected',
|
||||
TwentyORMExceptionCode.DUPLICATE_ENTRY_DETECTED,
|
||||
);
|
||||
|
||||
handleDuplicateKeyErrorMock.mockResolvedValue(duplicateException);
|
||||
|
||||
const result = await computeTwentyORMException(
|
||||
error,
|
||||
objectMetadata,
|
||||
entityManager,
|
||||
internalContext,
|
||||
);
|
||||
|
||||
expect(handleDuplicateKeyErrorMock).toHaveBeenCalledWith(
|
||||
error,
|
||||
objectMetadata,
|
||||
internalContext,
|
||||
entityManager,
|
||||
);
|
||||
expect(result).toBe(duplicateException);
|
||||
});
|
||||
|
||||
it('should throw the generic PostgresException when error is a known postgres code without dedicated handling', async () => {
|
||||
const error = buildQueryFailedError(
|
||||
POSTGRESQL_ERROR_CODES.SERIALIZATION_FAILURE,
|
||||
);
|
||||
|
||||
await expect(computeTwentyORMException(error)).rejects.toThrow(
|
||||
PostgresException,
|
||||
);
|
||||
await expect(computeTwentyORMException(error)).rejects.toMatchObject({
|
||||
message: 'Data validation error.',
|
||||
code: POSTGRESQL_ERROR_CODES.SERIALIZATION_FAILURE,
|
||||
});
|
||||
});
|
||||
|
||||
it('should rethrow the original error when error is a QueryFailedError with an unknown code', async () => {
|
||||
const error = buildQueryFailedError('99999');
|
||||
|
||||
await expect(computeTwentyORMException(error)).rejects.toBe(error);
|
||||
});
|
||||
|
||||
it('should return the error unchanged when error is not a QueryFailedError', async () => {
|
||||
const error = new Error('some unrelated error');
|
||||
|
||||
const result = await computeTwentyORMException(error);
|
||||
|
||||
expect(result).toBe(error);
|
||||
});
|
||||
});
|
||||
+18
-1
@@ -1,3 +1,4 @@
|
||||
import { type MessageDescriptor } from '@lingui/core';
|
||||
import { msg } from '@lingui/core/macro';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { QueryFailedError } from 'typeorm';
|
||||
@@ -18,6 +19,12 @@ interface QueryFailedErrorWithCode extends QueryFailedError {
|
||||
code?: string;
|
||||
}
|
||||
|
||||
const CONSTRAINT_VIOLATION_MESSAGES: Record<string, MessageDescriptor> = {
|
||||
[POSTGRESQL_ERROR_CODES.NOT_NULL_VIOLATION]: msg`A required field is missing. Please provide all required values and try again.`,
|
||||
[POSTGRESQL_ERROR_CODES.FOREIGN_KEY_VIOLATION]: msg`This operation references a record that does not exist or cannot be modified due to existing relationships.`,
|
||||
[POSTGRESQL_ERROR_CODES.RESTRICT_VIOLATION]: msg`This record cannot be deleted because it is still referenced by other records.`,
|
||||
};
|
||||
|
||||
export const computeTwentyORMException = async (
|
||||
error: Error,
|
||||
objectMetadata?: FlatObjectMetadata,
|
||||
@@ -53,11 +60,21 @@ export const computeTwentyORMException = async (
|
||||
|
||||
if (errorCode === POSTGRESQL_ERROR_CODES.INVALID_TEXT_REPRESENTATION) {
|
||||
return new TwentyORMException(
|
||||
error.message, // safe and useful
|
||||
error.message,
|
||||
TwentyORMExceptionCode.INVALID_INPUT,
|
||||
);
|
||||
}
|
||||
|
||||
if (isDefined(errorCode) && errorCode in CONSTRAINT_VIOLATION_MESSAGES) {
|
||||
return new TwentyORMException(
|
||||
error.message,
|
||||
TwentyORMExceptionCode.INVALID_INPUT,
|
||||
{
|
||||
userFriendlyMessage: CONSTRAINT_VIOLATION_MESSAGES[errorCode],
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
isDefined(errorCode) &&
|
||||
Object.values(POSTGRESQL_ERROR_CODES).includes(errorCode)
|
||||
|
||||
Reference in New Issue
Block a user