From f6d0af5fb81b51d49c9095e036a3ef9b5f580372 Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Thu, 14 Aug 2025 18:38:04 +0200 Subject: [PATCH] Move duplicate key error handling in ORM (#13893) Error available in REST API Screenshot 2025-08-13 at 11 48 19 `curl http://localhost:3000/rest/people \ --request POST \ --header 'Content-Type: application/json' \ --data '{ "emails": { "primaryEmail": "test@test.com" } }'` closes https://github.com/twentyhq/twenty/issues/13567 --- .../interfaces/base-resolver-service.ts | 2 +- .../utils/handle-duplicate-key-error.util.ts | 48 ++++++++++--------- ...nner-graphql-api-exception-handler.util.ts | 21 +------- .../http-exception-handler.service.ts | 25 ++++++++++ .../entity-manager/utils/get-entity-target.ts | 36 ++++++++++++++ .../workspace-entity-manager.ts | 8 +++- .../compute-twenty-orm-exception.ts | 37 +++++++++++++- .../exceptions/twenty-orm.exception.ts | 2 + ...ectRecordNotFoundErrorMessage.util.spec.ts | 2 +- ...tConnectRecordNotFoundErrorMessage.util.ts | 2 +- .../workspace-insert-query-builder.ts | 7 ++- .../workspace-update-query-builder.ts | 14 +++++- ...-orm-graphql-api-exception-handler.util.ts | 2 + 13 files changed, 156 insertions(+), 50 deletions(-) create mode 100644 packages/twenty-server/src/engine/twenty-orm/entity-manager/utils/get-entity-target.ts diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts index 1b1e625966..21bd2a5de3 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts @@ -210,7 +210,7 @@ export abstract class GraphqlQueryBaseResolverService< return resultWithGetters; } catch (error) { - workspaceQueryRunnerGraphqlApiExceptionHandler(error, options); + workspaceQueryRunnerGraphqlApiExceptionHandler(error); } } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/handle-duplicate-key-error.util.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/handle-duplicate-key-error.util.ts index 974df1405a..59f504f497 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/handle-duplicate-key-error.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/handle-duplicate-key-error.util.ts @@ -1,9 +1,12 @@ import { isDefined } from 'twenty-shared/utils'; import { type QueryFailedError } from 'typeorm'; -import { type WorkspaceQueryRunnerOptions } from 'src/engine/api/graphql/workspace-query-runner/interfaces/query-runner-option.interface'; - import { UserInputError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { type ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; +import { + TwentyORMException, + TwentyORMExceptionCode, +} from 'src/engine/twenty-orm/exceptions/twenty-orm.exception'; interface PostgreSQLError extends QueryFailedError { detail?: string; @@ -11,7 +14,7 @@ interface PostgreSQLError extends QueryFailedError { export const handleDuplicateKeyError = ( error: PostgreSQLError, - context: WorkspaceQueryRunnerOptions, + objectMetadata: ObjectMetadataItemWithFieldMaps, ) => { const indexNameMatch = error.message.match(/"([^"]+)"/); @@ -20,28 +23,28 @@ export const handleDuplicateKeyError = ( if (indexNameMatch) { const indexName = indexNameMatch[1]; - const deletedAtFieldMetadataId = - context.objectMetadataItemWithFieldMaps.fieldIdByName['deletedAt']; + const deletedAtFieldMetadataId = objectMetadata.fieldIdByName['deletedAt']; - const affectedColumns = - context.objectMetadataItemWithFieldMaps.indexMetadatas - .find((index) => index.name === indexName) - ?.indexFieldMetadatas?.filter( - (field) => field.fieldMetadataId !== deletedAtFieldMetadataId, - ) - .map((indexField) => { - const fieldMetadata = - context.objectMetadataItemWithFieldMaps.fieldsById[ - indexField.fieldMetadataId - ]; + const affectedColumns = objectMetadata.indexMetadatas + .find((index) => index.name === indexName) + ?.indexFieldMetadatas?.filter( + (field) => field.fieldMetadataId !== deletedAtFieldMetadataId, + ) + .map((indexField) => { + const fieldMetadata = + objectMetadata.fieldsById[indexField.fieldMetadataId]; - return fieldMetadata?.label; - }); + return fieldMetadata?.label; + }); if (!isDefined(affectedColumns)) { - throw new UserInputError(`A duplicate entry was detected`, { - userFriendlyMessage: `This record already exists. Please check your data and try again.`, - }); + throw new TwentyORMException( + `A duplicate entry was detected`, + TwentyORMExceptionCode.DUPLICATE_ENTRY_DETECTED, + { + userFriendlyMessage: `This record already exists. Please check your data and try again.`, + }, + ); } const columnNames = affectedColumns.join(', '); @@ -55,8 +58,9 @@ export const handleDuplicateKeyError = ( ); } - throw new UserInputError( + throw new TwentyORMException( `A duplicate entry was detected. The combination of ${columnNames} must be unique.`, + TwentyORMExceptionCode.DUPLICATE_ENTRY_DETECTED, { userFriendlyMessage: `This combination of ${columnNames.toLowerCase()} already exists. Please use different values.`, }, diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/workspace-query-runner-graphql-api-exception-handler.util.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/workspace-query-runner-graphql-api-exception-handler.util.ts index 0c5992d7bd..1e07e8d394 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/workspace-query-runner-graphql-api-exception-handler.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/workspace-query-runner-graphql-api-exception-handler.util.ts @@ -1,12 +1,7 @@ -import { QueryFailedError } from 'typeorm'; - -import { type WorkspaceQueryRunnerOptions } from 'src/engine/api/graphql/workspace-query-runner/interfaces/query-runner-option.interface'; +import { type QueryFailedError } from 'typeorm'; import { GraphqlQueryRunnerException } from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception'; -import { POSTGRESQL_ERROR_CODES } from 'src/engine/api/graphql/workspace-query-runner/constants/postgres-error-codes.constants'; import { graphqlQueryRunnerExceptionHandler } from 'src/engine/api/graphql/workspace-query-runner/utils/graphql-query-runner-exception-handler.util'; -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 { workspaceExceptionHandler } from 'src/engine/api/graphql/workspace-query-runner/utils/workspace-exception-handler.util'; import { WorkspaceQueryRunnerException } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-runner.exception'; import { ApiKeyException } from 'src/engine/core-modules/api-key/api-key.exception'; @@ -26,22 +21,8 @@ interface QueryFailedErrorWithCode extends QueryFailedError { export const workspaceQueryRunnerGraphqlApiExceptionHandler = ( error: QueryFailedErrorWithCode, - context: WorkspaceQueryRunnerOptions, ) => { switch (true) { - case error instanceof QueryFailedError: { - if ( - error.message.includes('duplicate key value violates unique constraint') - ) { - return handleDuplicateKeyError(error, context); - } - const errorCode = (error as QueryFailedErrorWithCode).code; - - if (POSTGRESQL_ERROR_CODES.includes(errorCode)) { - throw new PostgresException(error.message, errorCode); - } - throw error; - } case error instanceof RecordTransformerException: return recordTransformerGraphqlApiExceptionHandler(error); case error instanceof PermissionsException: diff --git a/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts b/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts index bf606e7e15..6973835511 100644 --- a/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts +++ b/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts @@ -3,6 +3,7 @@ import { type HttpException, Inject, Injectable, + InternalServerErrorException, Scope, } from '@nestjs/common'; import { REQUEST } from '@nestjs/core'; @@ -13,7 +14,12 @@ import { QueryFailedError } from 'typeorm'; import { type ExceptionHandlerUser } from 'src/engine/core-modules/exception-handler/interfaces/exception-handler-user.interface'; import { type ExceptionHandlerWorkspace } from 'src/engine/core-modules/exception-handler/interfaces/exception-handler-workspace.interface'; +import { PostgresException } from 'src/engine/api/graphql/workspace-query-runner/utils/postgres-exception'; import { ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service'; +import { + TwentyORMException, + TwentyORMExceptionCode, +} from 'src/engine/twenty-orm/exceptions/twenty-orm.exception'; import { handleException } from 'src/engine/utils/global-exception-handler.util'; interface RequestAndParams { @@ -84,6 +90,25 @@ export class HttpExceptionHandlerService { statusCode = 400; } + if ( + exception instanceof TwentyORMException && + [ + TwentyORMExceptionCode.INVALID_INPUT, + TwentyORMExceptionCode.DUPLICATE_ENTRY_DETECTED, + TwentyORMExceptionCode.CONNECT_UNIQUE_CONSTRAINT_ERROR, + TwentyORMExceptionCode.CONNECT_NOT_ALLOWED, + TwentyORMExceptionCode.CONNECT_RECORD_NOT_FOUND, + ].includes(exception.code) + ) { + exception = new BadRequestException(exception.message); + statusCode = 400; + } + + if (exception instanceof PostgresException) { + exception = new InternalServerErrorException(exception.message); + statusCode = 500; + } + handleException({ exception, exceptionHandlerService: this.exceptionHandlerService, diff --git a/packages/twenty-server/src/engine/twenty-orm/entity-manager/utils/get-entity-target.ts b/packages/twenty-server/src/engine/twenty-orm/entity-manager/utils/get-entity-target.ts new file mode 100644 index 0000000000..e6b6816753 --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/entity-manager/utils/get-entity-target.ts @@ -0,0 +1,36 @@ +import { isDefined } from 'twenty-shared/utils'; +import { + type EntityTarget, + InstanceChecker, + type ObjectLiteral, + type SaveOptions, +} from 'typeorm'; + +import { type DeepPartialWithNestedRelationFields } from 'src/engine/twenty-orm/entity-manager/types/deep-partial-entity-with-nested-relation-fields.type'; + +export const getEntityTarget = < + Entity extends ObjectLiteral, + T extends DeepPartialWithNestedRelationFields, +>( + targetOrEntity: EntityTarget | Entity | Entity[], + entityOrMaybeOptions: + | T + | T[] + | SaveOptions + | (SaveOptions & { reload: false }), +) => { + const isEntityTarget = + (typeof targetOrEntity === 'function' || + InstanceChecker.isEntitySchema(targetOrEntity) || + typeof targetOrEntity === 'string') && + isDefined(targetOrEntity); + + const entityTarget = isEntityTarget ? targetOrEntity : null; + + if (entityTarget) return entityTarget; + + const entityData = isEntityTarget ? entityOrMaybeOptions : targetOrEntity; + const isEntityArray = Array.isArray(entityData); + + return isEntityArray ? entityData[0]?.constructor : entityData.constructor; +}; diff --git a/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts b/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts index 4a4c767d3c..e2c16e0ab0 100644 --- a/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts +++ b/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts @@ -44,6 +44,7 @@ import { type ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-module import { type WorkspaceDataSource } from 'src/engine/twenty-orm/datasource/workspace.datasource'; import { type DeepPartialWithNestedRelationFields } from 'src/engine/twenty-orm/entity-manager/types/deep-partial-entity-with-nested-relation-fields.type'; import { type QueryDeepPartialEntityWithNestedRelationFields } from 'src/engine/twenty-orm/entity-manager/types/query-deep-partial-entity-with-nested-relation-fields.type'; +import { getEntityTarget } from 'src/engine/twenty-orm/entity-manager/utils/get-entity-target'; import { computeTwentyORMException } from 'src/engine/twenty-orm/error-handling/compute-twenty-orm-exception'; import { RelationNestedQueries } from 'src/engine/twenty-orm/relation-nested-queries/relation-nested-queries'; import { @@ -1230,7 +1231,12 @@ export class WorkspaceEntityManager extends EntityManager { return isEntityArray ? formattedResult : formattedResult[0]; } catch (error) { - throw computeTwentyORMException(error); + const objectMetadataItem = getObjectMetadataFromEntityTarget( + getEntityTarget(targetOrEntity, entityOrMaybeOptions), + this.internalContext, + ); + + throw computeTwentyORMException(error, objectMetadataItem); } } diff --git a/packages/twenty-server/src/engine/twenty-orm/error-handling/compute-twenty-orm-exception.ts b/packages/twenty-server/src/engine/twenty-orm/error-handling/compute-twenty-orm-exception.ts index c5c22d3462..ab7cfdfde2 100644 --- a/packages/twenty-server/src/engine/twenty-orm/error-handling/compute-twenty-orm-exception.ts +++ b/packages/twenty-server/src/engine/twenty-orm/error-handling/compute-twenty-orm-exception.ts @@ -1,12 +1,24 @@ import { t } from '@lingui/core/macro'; +import { isDefined } from 'twenty-shared/utils'; 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 { type ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; import { TwentyORMException, TwentyORMExceptionCode, } from 'src/engine/twenty-orm/exceptions/twenty-orm.exception'; -export const computeTwentyORMException = (error: Error) => { +interface QueryFailedErrorWithCode extends QueryFailedError { + code?: string; +} + +export const computeTwentyORMException = ( + error: Error, + objectMetadata?: ObjectMetadataItemWithFieldMaps, +) => { if (error instanceof QueryFailedError) { if (error.message.includes('Query read timeout')) { return new TwentyORMException( @@ -17,6 +29,29 @@ export const computeTwentyORMException = (error: Error) => { }, ); } + + if ( + error.message.includes( + 'duplicate key value violates unique constraint', + ) && + isDefined(objectMetadata) + ) { + return handleDuplicateKeyError(error, objectMetadata); + } + + if (error.message.includes('invalid input value for')) { + return new TwentyORMException( + error.message, + TwentyORMExceptionCode.INVALID_INPUT, + ); + } + + const errorCode = (error as QueryFailedErrorWithCode).code; + + if (isDefined(errorCode) && POSTGRESQL_ERROR_CODES.includes(errorCode)) { + throw new PostgresException(error.message, errorCode); + } + throw error; } return error; diff --git a/packages/twenty-server/src/engine/twenty-orm/exceptions/twenty-orm.exception.ts b/packages/twenty-server/src/engine/twenty-orm/exceptions/twenty-orm.exception.ts index f62885e476..12d7cb48fc 100644 --- a/packages/twenty-server/src/engine/twenty-orm/exceptions/twenty-orm.exception.ts +++ b/packages/twenty-server/src/engine/twenty-orm/exceptions/twenty-orm.exception.ts @@ -18,4 +18,6 @@ export enum TwentyORMExceptionCode { METHOD_NOT_ALLOWED = 'METHOD_NOT_ALLOWED', ENUM_TYPE_NAME_NOT_FOUND = 'ENUM_TYPE_NAME_NOT_FOUND', QUERY_READ_TIMEOUT = 'QUERY_READ_TIMEOUT', + DUPLICATE_ENTRY_DETECTED = 'DUPLICATE_ENTRY_DETECTED', + INVALID_INPUT = 'INVALID_INPUT', } diff --git a/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/__tests__/formatConnectRecordNotFoundErrorMessage.util.spec.ts b/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/__tests__/formatConnectRecordNotFoundErrorMessage.util.spec.ts index 5d7463d951..bf49117c18 100644 --- a/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/__tests__/formatConnectRecordNotFoundErrorMessage.util.spec.ts +++ b/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/__tests__/formatConnectRecordNotFoundErrorMessage.util.spec.ts @@ -15,7 +15,7 @@ describe('formatConnectRecordNotFoundErrorMessage', () => { errorMessage: 'Expected 1 record to connect to connectFieldName, but found 0 for field1 = value1 and field2 = value2', userFriendlyMessage: - 'Expected 1 record to connect to connectFieldName, but found 0 for field1 = value1 and field2 = value2', + "Can't connect to connectFieldName. No unique record found with condition: field1 = value1 and field2 = value2", }); }); }); diff --git a/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/formatConnectRecordNotFoundErrorMessage.util.ts b/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/formatConnectRecordNotFoundErrorMessage.util.ts index fde8fad8ec..0f0dd311b9 100644 --- a/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/formatConnectRecordNotFoundErrorMessage.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/utils/formatConnectRecordNotFoundErrorMessage.util.ts @@ -13,6 +13,6 @@ export const formatConnectRecordNotFoundErrorMessage = ( return { errorMessage: `Expected 1 record to connect to ${connectFieldName}, but found ${recordToConnectTotal} for ${formattedConnectCondition}`, - userFriendlyMessage: t`Expected 1 record to connect to ${connectFieldName}, but found ${recordToConnectTotal} for ${formattedConnectCondition}`, + userFriendlyMessage: t`Can't connect to ${connectFieldName}. No unique record found with condition: ${formattedConnectCondition}`, }; }; diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-insert-query-builder.ts b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-insert-query-builder.ts index 41ef4f38b9..d4c513c285 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-insert-query-builder.ts +++ b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-insert-query-builder.ts @@ -200,7 +200,12 @@ export class WorkspaceInsertQueryBuilder< identifiers: result.identifiers, }; } catch (error) { - throw computeTwentyORMException(error); + const objectMetadata = getObjectMetadataFromEntityTarget( + this.getMainAliasTarget(), + this.internalContext, + ); + + throw computeTwentyORMException(error, objectMetadata); } } diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts index c5768dd006..656aa7551e 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts +++ b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts @@ -176,7 +176,12 @@ export class WorkspaceUpdateQueryBuilder< affected: result.affected, }; } catch (error) { - throw computeTwentyORMException(error); + const objectMetadata = getObjectMetadataFromEntityTarget( + this.getMainAliasTarget(), + this.internalContext, + ); + + throw computeTwentyORMException(error, objectMetadata); } } @@ -305,7 +310,12 @@ export class WorkspaceUpdateQueryBuilder< affected: results.length, }; } catch (error) { - throw computeTwentyORMException(error); + const objectMetadata = getObjectMetadataFromEntityTarget( + this.getMainAliasTarget(), + this.internalContext, + ); + + throw computeTwentyORMException(error, objectMetadata); } } diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/twenty-orm-graphql-api-exception-handler.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/twenty-orm-graphql-api-exception-handler.util.ts index 1b70ad8a9e..571c4d8006 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/twenty-orm-graphql-api-exception-handler.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/twenty-orm-graphql-api-exception-handler.util.ts @@ -8,6 +8,8 @@ export const twentyORMGraphqlApiExceptionHandler = ( error: TwentyORMException, ) => { switch (error.code) { + case TwentyORMExceptionCode.INVALID_INPUT: + case TwentyORMExceptionCode.DUPLICATE_ENTRY_DETECTED: case TwentyORMExceptionCode.CONNECT_RECORD_NOT_FOUND: case TwentyORMExceptionCode.CONNECT_NOT_ALLOWED: case TwentyORMExceptionCode.CONNECT_UNIQUE_CONSTRAINT_ERROR: