diff --git a/packages/twenty-server/src/engine/core-modules/application/__tests__/__snapshots__/application-exception-filter.spec.ts.snap b/packages/twenty-server/src/engine/core-modules/application/__tests__/__snapshots__/application-exception-filter.spec.ts.snap new file mode 100644 index 0000000000..fbe7b36486 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/__tests__/__snapshots__/application-exception-filter.spec.ts.snap @@ -0,0 +1,13 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ApplicationExceptionFilter response error format should surface an install conflict as APPLICATION_INSTALLATION_FAILED with a human message 1`] = ` +{ + "extensions": { + "code": "APPLICATION_INSTALLATION_FAILED", + "subCode": "APPLICATION_INSTALLATION_FAILED", + "userFriendlyMessage": "We couldn't install "Test Application". Its object "Invoice" could not be applied to your workspace.", + }, + "message": "Installing application 'Test Application' failed [object: Invoice]: addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists (universalIdentifier: b1b2c3d4-0003-4000-a000-000000000003)", + "name": "GraphQLError", +} +`; diff --git a/packages/twenty-server/src/engine/core-modules/application/__tests__/application-exception-filter.spec.ts b/packages/twenty-server/src/engine/core-modules/application/__tests__/application-exception-filter.spec.ts new file mode 100644 index 0000000000..80cd725336 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/__tests__/application-exception-filter.spec.ts @@ -0,0 +1,93 @@ +import { i18n } from '@lingui/core'; +import { type Manifest } from 'twenty-shared/application'; + +import { ApplicationExceptionFilter } from 'src/engine/core-modules/application/application-exception-filter'; +import { enrichApplicationManifestSyncError } from 'src/engine/core-modules/application/application-manifest/utils/enrich-application-manifest-sync-error.util'; +import { + ApplicationException, + ApplicationExceptionCode, +} from 'src/engine/core-modules/application/application.exception'; +import { + type BaseGraphQLError, + ErrorCode, +} from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { + FlatEntityMapsException, + FlatEntityMapsExceptionCode, +} from 'src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception'; + +const OBJECT_UNIVERSAL_IDENTIFIER = 'b1b2c3d4-0003-4000-a000-000000000003'; + +const manifest = { + application: { displayName: 'Test Application' }, + objects: [ + { + universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER, + labelSingular: 'Invoice', + }, + ], + fields: [], +} as unknown as Manifest; + +const catchAsGraphQLError = (exception: ApplicationException) => { + const filter = new ApplicationExceptionFilter(); + + try { + filter.catch(exception); + } catch (graphqlError) { + return graphqlError as BaseGraphQLError; + } + + throw new Error('ApplicationExceptionFilter did not throw'); +}; + +describe('ApplicationExceptionFilter response error format', () => { + it('should surface an install conflict as APPLICATION_INSTALLATION_FAILED with a human message', () => { + const originalError = new FlatEntityMapsException( + 'addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists (universalIdentifier: b1b2c3d4-0003-4000-a000-000000000003)', + FlatEntityMapsExceptionCode.ENTITY_ALREADY_EXISTS, + { + context: { + universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER, + operation: 'add', + }, + }, + ); + + const enrichedError = enrichApplicationManifestSyncError({ + error: originalError, + manifest, + }) as ApplicationException; + + expect(enrichedError.code).toBe( + ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED, + ); + + const graphqlError = catchAsGraphQLError(enrichedError); + + expect(graphqlError.extensions.code).toBe( + ErrorCode.APPLICATION_INSTALLATION_FAILED, + ); + + expect(graphqlError.context).toEqual({ + universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER, + operation: 'add', + }); + expect(graphqlError.extensions.context).toBeUndefined(); + expect( + JSON.parse(JSON.stringify(graphqlError.toJSON())), + ).not.toHaveProperty('context'); + + expect({ + name: graphqlError.name, + message: graphqlError.message, + extensions: { + code: graphqlError.extensions.code, + subCode: graphqlError.extensions.subCode, + userFriendlyMessage: i18n._( + graphqlError.extensions.userFriendlyMessage, + ), + }, + }).toMatchSnapshot(); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/application/application-exception-filter.ts b/packages/twenty-server/src/engine/core-modules/application/application-exception-filter.ts index f0d555223d..6ee606dadb 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-exception-filter.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-exception-filter.ts @@ -7,6 +7,8 @@ import { ApplicationExceptionCode, } from 'src/engine/core-modules/application/application.exception'; import { + BaseGraphQLError, + ErrorCode, ForbiddenError, InternalServerError, NotFoundError, @@ -41,6 +43,19 @@ export class ApplicationExceptionFilter implements ExceptionFilter { case ApplicationExceptionCode.UPGRADE_FAILED: case ApplicationExceptionCode.INVALID_SERVER_VERSION: throw new InternalServerError(exception); + case ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED: { + const installationError = new BaseGraphQLError( + exception, + ErrorCode.APPLICATION_INSTALLATION_FAILED, + ); + + Object.defineProperty(installationError, 'context', { + value: exception.context, + enumerable: false, + }); + + throw installationError; + } default: { assertUnreachable(exception.code); } diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-sync.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-sync.service.ts index e823c2b24a..821ae18cca 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-sync.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-sync.service.ts @@ -7,6 +7,7 @@ import { isDefined } from 'twenty-shared/utils'; import { PackageJson } from 'type-fest'; import { ApplicationManifestMigrationService } from 'src/engine/core-modules/application/application-manifest/application-manifest-migration.service'; +import { enrichApplicationManifestSyncError } from 'src/engine/core-modules/application/application-manifest/utils/enrich-application-manifest-sync-error.util'; import { buildFromToAllUniversalFlatEntityMaps } from 'src/engine/core-modules/application/application-manifest/utils/build-from-to-all-universal-flat-entity-maps.util'; import { ApplicationTranslationSyncService } from 'src/engine/core-modules/application/application-translation/application-translation-sync.service'; import { getApplicationSubAllFlatEntityMaps } from 'src/engine/core-modules/application/application-manifest/utils/get-application-sub-all-flat-entity-maps.util'; @@ -65,13 +66,24 @@ export class ApplicationSyncService { applicationRegistrationId, }); - const syncResult = - await this.applicationManifestMigrationService.syncMetadataFromManifest({ - manifest, - workspaceId, - ownerFlatApplication, - dryRun, - }); + let syncResult: { + workspaceMigration: WorkspaceMigration; + hasSchemaMetadataChanged: boolean; + }; + + try { + syncResult = + await this.applicationManifestMigrationService.syncMetadataFromManifest( + { + manifest, + workspaceId, + ownerFlatApplication, + dryRun, + }, + ); + } catch (error) { + throw enrichApplicationManifestSyncError({ error, manifest }); + } if (!dryRun && isDefined(ownerFlatApplication.applicationRegistrationId)) { // Translation sync runs after the metadata migration is already applied diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/__tests__/enrich-application-manifest-sync-error.util.spec.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/__tests__/enrich-application-manifest-sync-error.util.spec.ts new file mode 100644 index 0000000000..a03e13f700 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/__tests__/enrich-application-manifest-sync-error.util.spec.ts @@ -0,0 +1,175 @@ +import { type Manifest } from 'twenty-shared/application'; + +import { enrichApplicationManifestSyncError } from 'src/engine/core-modules/application/application-manifest/utils/enrich-application-manifest-sync-error.util'; +import { + ApplicationException, + ApplicationExceptionCode, +} from 'src/engine/core-modules/application/application.exception'; +import { + FlatEntityMapsException, + FlatEntityMapsExceptionCode, +} from 'src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception'; + +const OBJECT_UNIVERSAL_IDENTIFIER = 'object-universal-identifier'; +const FIELD_UNIVERSAL_IDENTIFIER = 'field-universal-identifier'; +const NESTED_FIELD_UNIVERSAL_IDENTIFIER = 'nested-field-universal-identifier'; +const ROLE_UNIVERSAL_IDENTIFIER = 'role-universal-identifier'; +const VIEW_FIELD_UNIVERSAL_IDENTIFIER = 'view-field-universal-identifier'; + +const manifest = { + application: { displayName: 'Stripe' }, + objects: [ + { + universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER, + labelSingular: 'Invoice', + fields: [ + { + universalIdentifier: NESTED_FIELD_UNIVERSAL_IDENTIFIER, + label: 'Due Date', + }, + ], + }, + ], + fields: [ + { + universalIdentifier: FIELD_UNIVERSAL_IDENTIFIER, + label: 'Amount', + }, + ], + roles: [ + { + universalIdentifier: ROLE_UNIVERSAL_IDENTIFIER, + label: 'Support Agent', + }, + ], + viewFields: [ + { + universalIdentifier: VIEW_FIELD_UNIVERSAL_IDENTIFIER, + }, + ], +} as unknown as Manifest; + +describe('enrichApplicationManifestSyncError', () => { + it('should resolve the offending object and produce an installation exception', () => { + const error = new FlatEntityMapsException( + 'addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists', + FlatEntityMapsExceptionCode.ENTITY_ALREADY_EXISTS, + { + context: { + universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER, + operation: 'add', + }, + }, + ); + + const enriched = enrichApplicationManifestSyncError({ error, manifest }); + + expect(enriched).toBeInstanceOf(ApplicationException); + expect((enriched as ApplicationException).code).toBe( + ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED, + ); + expect((enriched as ApplicationException).message).toContain('Stripe'); + expect((enriched as ApplicationException).message).toContain('Invoice'); + expect((enriched as ApplicationException).message).toContain( + 'flat entity to add already exists', + ); + expect((enriched as ApplicationException).context).toEqual({ + universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER, + operation: 'add', + }); + }); + + it('should resolve the offending field by universalIdentifier', () => { + const error = new FlatEntityMapsException( + 'entity malformed', + FlatEntityMapsExceptionCode.ENTITY_MALFORMED, + { context: { universalIdentifier: FIELD_UNIVERSAL_IDENTIFIER } }, + ); + + const enriched = enrichApplicationManifestSyncError({ error, manifest }); + + expect((enriched as ApplicationException).message).toContain('Amount'); + }); + + it('should resolve a field nested in an object manifest by universalIdentifier', () => { + const error = new FlatEntityMapsException( + 'flat entity to add already exists', + FlatEntityMapsExceptionCode.ENTITY_ALREADY_EXISTS, + { context: { universalIdentifier: NESTED_FIELD_UNIVERSAL_IDENTIFIER } }, + ); + + const enriched = enrichApplicationManifestSyncError({ error, manifest }); + + expect((enriched as ApplicationException).message).toContain('field'); + expect((enriched as ApplicationException).message).toContain('Due Date'); + }); + + it('should resolve a labeled non-object/field kind (role) by universalIdentifier', () => { + const error = new FlatEntityMapsException( + 'flat entity to add already exists', + FlatEntityMapsExceptionCode.ENTITY_ALREADY_EXISTS, + { context: { universalIdentifier: ROLE_UNIVERSAL_IDENTIFIER } }, + ); + + const enriched = enrichApplicationManifestSyncError({ error, manifest }); + + expect((enriched as ApplicationException).message).toContain('role'); + expect((enriched as ApplicationException).message).toContain( + 'Support Agent', + ); + }); + + it('should fall back to the entity kind when the resolved entity has no label', () => { + const error = new FlatEntityMapsException( + 'flat entity to add already exists', + FlatEntityMapsExceptionCode.ENTITY_ALREADY_EXISTS, + { context: { universalIdentifier: VIEW_FIELD_UNIVERSAL_IDENTIFIER } }, + ); + + const enriched = enrichApplicationManifestSyncError({ error, manifest }); + + expect((enriched as ApplicationException).message).toContain('view field'); + expect((enriched as ApplicationException).message).not.toContain( + 'undefined', + ); + }); + + it('should still enrich when the identifier is not in the manifest', () => { + const error = new FlatEntityMapsException( + 'entity not found', + FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND, + { context: { universalIdentifier: 'unknown-identifier' } }, + ); + + const enriched = enrichApplicationManifestSyncError({ error, manifest }); + + expect(enriched).toBeInstanceOf(ApplicationException); + expect((enriched as ApplicationException).code).toBe( + ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED, + ); + expect((enriched as ApplicationException).message).toContain('Stripe'); + }); + + it('should extract context forwarded through a wrapper exception', () => { + const wrapperError = { + message: 'wrapped failure', + context: { universalIdentifier: OBJECT_UNIVERSAL_IDENTIFIER }, + }; + + const enriched = enrichApplicationManifestSyncError({ + error: wrapperError, + manifest, + }); + + expect(enriched).toBeInstanceOf(ApplicationException); + expect((enriched as ApplicationException).message).toContain('Invoice'); + }); + + it('should leave non flat-entity errors untouched', () => { + const error = new Error('some unrelated failure'); + + const enriched = enrichApplicationManifestSyncError({ error, manifest }); + + expect(enriched).toBe(error); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/enrich-application-manifest-sync-error.util.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/enrich-application-manifest-sync-error.util.ts new file mode 100644 index 0000000000..659da6746e --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/enrich-application-manifest-sync-error.util.ts @@ -0,0 +1,63 @@ +import { msg } from '@lingui/core/macro'; +import { type Manifest } from 'twenty-shared/application'; +import { isDefined } from 'twenty-shared/utils'; + +import { findManifestEntityDescriptorByUniversalIdentifier } from 'src/engine/core-modules/application/application-manifest/utils/find-manifest-entity-descriptor-by-universal-identifier.util'; +import { + ApplicationException, + ApplicationExceptionCode, +} from 'src/engine/core-modules/application/application.exception'; +import { getFlatEntityMapsExceptionContext } from 'src/engine/metadata-modules/flat-entity/utils/get-flat-entity-maps-exception-context.util'; + +export const enrichApplicationManifestSyncError = ({ + error, + manifest, +}: { + error: unknown; + manifest: Manifest; +}): unknown => { + const context = getFlatEntityMapsExceptionContext(error); + + if (!isDefined(context)) { + return error; + } + + const applicationDisplayName = manifest.application.displayName; + const originalMessage = + error instanceof Error ? error.message : String(error); + + const descriptor = isDefined(context.universalIdentifier) + ? findManifestEntityDescriptorByUniversalIdentifier({ + manifest, + universalIdentifier: context.universalIdentifier, + }) + : undefined; + + if (isDefined(descriptor)) { + const { entityKind, label } = descriptor; + const humanEntity = isDefined(label) + ? `${entityKind} "${label}"` + : entityKind; + const developerDetail = isDefined(label) + ? `${entityKind}: ${label}` + : entityKind; + + return new ApplicationException( + `Installing application '${applicationDisplayName}' failed [${developerDetail}]: ${originalMessage}`, + ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED, + { + userFriendlyMessage: msg`We couldn't install "${applicationDisplayName}". Its ${humanEntity} could not be applied to your workspace.`, + context, + }, + ); + } + + return new ApplicationException( + `Installing application '${applicationDisplayName}' failed: ${originalMessage}`, + ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED, + { + userFriendlyMessage: msg`We couldn't install "${applicationDisplayName}" because some of its metadata could not be applied to your workspace.`, + context, + }, + ); +}; diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/find-manifest-entity-descriptor-by-universal-identifier.util.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/find-manifest-entity-descriptor-by-universal-identifier.util.ts new file mode 100644 index 0000000000..bd04b3185c --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/find-manifest-entity-descriptor-by-universal-identifier.util.ts @@ -0,0 +1,275 @@ +import { type Manifest } from 'twenty-shared/application'; +import { type AllMetadataName } from 'twenty-shared/metadata'; +import { isDefined } from 'twenty-shared/utils'; + +export type ManifestEntityDescriptor = { + entityKind: string; + label?: string; +}; + +type ManifestEntityCandidate = { + universalIdentifier: string; + label?: string; +}; + +type ManifestEntityRegistryEntry = { + entityKind: string; + getCandidates: (manifest: Manifest) => ManifestEntityCandidate[]; +}; + +const NO_MANIFEST_CANDIDATES: ManifestEntityCandidate[] = []; + +const toCandidates = ( + entities: T[] | undefined, + getLabel: (entity: T) => string | undefined, +): ManifestEntityCandidate[] => + (entities ?? []) + .filter((entity): entity is T & { universalIdentifier: string } => + isDefined(entity.universalIdentifier), + ) + .map((entity) => ({ + universalIdentifier: entity.universalIdentifier, + label: getLabel(entity), + })); + +const MANIFEST_ENTITY_REGISTRY: Record< + AllMetadataName, + ManifestEntityRegistryEntry +> = { + objectMetadata: { + entityKind: 'object', + getCandidates: (manifest) => + toCandidates(manifest.objects, (object) => object.labelSingular), + }, + fieldMetadata: { + entityKind: 'field', + getCandidates: (manifest) => [ + ...toCandidates(manifest.fields, (field) => field.label), + ...(manifest.objects ?? []).flatMap((object) => + toCandidates(object.fields, (field) => field.label), + ), + ], + }, + role: { + entityKind: 'role', + getCandidates: (manifest) => + toCandidates(manifest.roles, (role) => role.label), + }, + permissionFlag: { + entityKind: 'permission flag', + getCandidates: (manifest) => + toCandidates( + manifest.permissionFlags, + (permissionFlag) => permissionFlag.label, + ), + }, + skill: { + entityKind: 'skill', + getCandidates: (manifest) => + toCandidates(manifest.skills, (skill) => skill.label), + }, + agent: { + entityKind: 'agent', + getCandidates: (manifest) => + toCandidates(manifest.agents, (agent) => agent.label), + }, + connectionProvider: { + entityKind: 'connection provider', + getCandidates: (manifest) => + toCandidates( + manifest.connectionProviders, + (connectionProvider) => connectionProvider.displayName, + ), + }, + view: { + entityKind: 'view', + getCandidates: (manifest) => + toCandidates(manifest.views, (view) => view.name), + }, + pageLayout: { + entityKind: 'page layout', + getCandidates: (manifest) => + toCandidates(manifest.pageLayouts, (pageLayout) => pageLayout.name), + }, + pageLayoutTab: { + entityKind: 'page layout tab', + getCandidates: (manifest) => [ + ...toCandidates( + manifest.pageLayoutTabs, + (pageLayoutTab) => pageLayoutTab.title, + ), + ...(manifest.pageLayouts ?? []).flatMap((pageLayout) => + toCandidates(pageLayout.tabs, (pageLayoutTab) => pageLayoutTab.title), + ), + ], + }, + pageLayoutWidget: { + entityKind: 'page layout widget', + getCandidates: (manifest) => [ + ...(manifest.pageLayoutTabs ?? []).flatMap((pageLayoutTab) => + toCandidates(pageLayoutTab.widgets, (widget) => widget.title), + ), + ...(manifest.pageLayouts ?? []).flatMap((pageLayout) => + (pageLayout.tabs ?? []).flatMap((pageLayoutTab) => + toCandidates(pageLayoutTab.widgets, (widget) => widget.title), + ), + ), + ], + }, + commandMenuItem: { + entityKind: 'command menu item', + getCandidates: (manifest) => + toCandidates( + manifest.commandMenuItems, + (commandMenuItem) => commandMenuItem.label, + ), + }, + logicFunction: { + entityKind: 'logic function', + getCandidates: (manifest) => + toCandidates( + manifest.logicFunctions, + (logicFunction) => logicFunction.name, + ), + }, + frontComponent: { + entityKind: 'front component', + getCandidates: (manifest) => + toCandidates( + manifest.frontComponents, + (frontComponent) => frontComponent.name, + ), + }, + navigationMenuItem: { + entityKind: 'navigation menu item', + getCandidates: (manifest) => + toCandidates( + manifest.navigationMenuItems, + (navigationMenuItem) => navigationMenuItem.name, + ), + }, + index: { + entityKind: 'index', + getCandidates: (manifest) => + toCandidates(manifest.indexes, () => undefined), + }, + viewField: { + entityKind: 'view field', + getCandidates: (manifest) => [ + ...toCandidates(manifest.viewFields, () => undefined), + ...(manifest.views ?? []).flatMap((view) => + toCandidates(view.fields, () => undefined), + ), + ], + }, + viewFieldGroup: { + entityKind: 'view field group', + getCandidates: (manifest) => + (manifest.views ?? []).flatMap((view) => + toCandidates(view.fieldGroups, (fieldGroup) => fieldGroup.name), + ), + }, + viewGroup: { + entityKind: 'view group', + getCandidates: (manifest) => + (manifest.views ?? []).flatMap((view) => + toCandidates(view.groups, () => undefined), + ), + }, + viewSort: { + entityKind: 'view sort', + getCandidates: (manifest) => + (manifest.views ?? []).flatMap((view) => + toCandidates(view.sorts, () => undefined), + ), + }, + viewFilter: { + entityKind: 'view filter', + getCandidates: (manifest) => + (manifest.views ?? []).flatMap((view) => + toCandidates(view.filters, () => undefined), + ), + }, + viewFilterGroup: { + entityKind: 'view filter group', + getCandidates: (manifest) => + (manifest.views ?? []).flatMap((view) => + toCandidates(view.filterGroups, () => undefined), + ), + }, + objectPermission: { + entityKind: 'object permission', + getCandidates: (manifest) => + (manifest.roles ?? []).flatMap((role) => + toCandidates(role.objectPermissions, () => undefined), + ), + }, + fieldPermission: { + entityKind: 'field permission', + getCandidates: (manifest) => + (manifest.roles ?? []).flatMap((role) => + toCandidates(role.fieldPermissions, () => undefined), + ), + }, + rowLevelPermissionPredicate: { + entityKind: 'row-level permission predicate', + getCandidates: (manifest) => + (manifest.roles ?? []).flatMap((role) => + toCandidates(role.rowLevelPermissionPredicates, () => undefined), + ), + }, + rowLevelPermissionPredicateGroup: { + entityKind: 'row-level permission predicate group', + getCandidates: (manifest) => + (manifest.roles ?? []).flatMap((role) => + toCandidates(role.rowLevelPermissionPredicateGroups, () => undefined), + ), + }, + roleTarget: { + entityKind: 'role target', + getCandidates: () => NO_MANIFEST_CANDIDATES, + }, + rolePermissionFlag: { + entityKind: 'role permission flag', + getCandidates: () => NO_MANIFEST_CANDIDATES, + }, + webhook: { + entityKind: 'webhook', + getCandidates: () => NO_MANIFEST_CANDIDATES, + }, + applicationVariable: { + entityKind: 'application variable', + getCandidates: () => NO_MANIFEST_CANDIDATES, + }, + searchFieldMetadata: { + entityKind: 'search field', + getCandidates: () => NO_MANIFEST_CANDIDATES, + }, +}; + +const MANIFEST_ENTITY_REGISTRY_ENTRIES = Object.values( + MANIFEST_ENTITY_REGISTRY, +); + +export const findManifestEntityDescriptorByUniversalIdentifier = ({ + manifest, + universalIdentifier, +}: { + manifest: Manifest; + universalIdentifier: string; +}): ManifestEntityDescriptor | undefined => { + for (const { + entityKind, + getCandidates, + } of MANIFEST_ENTITY_REGISTRY_ENTRIES) { + const match = getCandidates(manifest).find( + (candidate) => candidate.universalIdentifier === universalIdentifier, + ); + + if (isDefined(match)) { + return { entityKind, label: match.label }; + } + } + + return undefined; +}; diff --git a/packages/twenty-server/src/engine/core-modules/application/application-rest-api-exception.filter.ts b/packages/twenty-server/src/engine/core-modules/application/application-rest-api-exception.filter.ts index 98dc1ff2da..4f04c8dc01 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-rest-api-exception.filter.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-rest-api-exception.filter.ts @@ -42,6 +42,7 @@ const applicationExceptionCodeToHttpStatus = ( case ApplicationExceptionCode.TARBALL_EXTRACTION_FAILED: case ApplicationExceptionCode.UPGRADE_FAILED: case ApplicationExceptionCode.INVALID_SERVER_VERSION: + case ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED: return 500; default: return assertUnreachable(code); diff --git a/packages/twenty-server/src/engine/core-modules/application/application.exception.ts b/packages/twenty-server/src/engine/core-modules/application/application.exception.ts index ba124be317..7bdd2a5460 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application.exception.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application.exception.ts @@ -2,6 +2,7 @@ import { type MessageDescriptor } from '@lingui/core'; import { msg } from '@lingui/core/macro'; import { assertUnreachable } from 'twenty-shared/utils'; +import { type FlatEntityMapsExceptionContext } from 'src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception'; import { CustomException } from 'src/utils/custom-exception'; export enum ApplicationExceptionCode { @@ -25,6 +26,7 @@ export enum ApplicationExceptionCode { SERVER_VERSION_INCOMPATIBLE = 'SERVER_VERSION_INCOMPATIBLE', INVALID_APP_ENGINE_REQUIREMENT = 'INVALID_APP_ENGINE_REQUIREMENT', INVALID_SERVER_VERSION = 'INVALID_SERVER_VERSION', + APPLICATION_INSTALLATION_FAILED = 'APPLICATION_INSTALLATION_FAILED', } const getApplicationExceptionUserFriendlyMessage = ( @@ -71,20 +73,32 @@ const getApplicationExceptionUserFriendlyMessage = ( return msg`The app manifest declares an invalid server version requirement.`; case ApplicationExceptionCode.INVALID_SERVER_VERSION: return msg`The server's APP_VERSION is not a valid semver version. Self-hosted instances must configure a valid APP_VERSION.`; + case ApplicationExceptionCode.APPLICATION_INSTALLATION_FAILED: + return msg`We couldn't install this application because some of its metadata could not be applied to your workspace.`; default: assertUnreachable(code); } }; export class ApplicationException extends CustomException { + context?: FlatEntityMapsExceptionContext; + constructor( message: string, code: ApplicationExceptionCode, - { userFriendlyMessage }: { userFriendlyMessage?: MessageDescriptor } = {}, + { + userFriendlyMessage, + context, + }: { + userFriendlyMessage?: MessageDescriptor; + context?: FlatEntityMapsExceptionContext; + } = {}, ) { super(message, code, { userFriendlyMessage: userFriendlyMessage ?? getApplicationExceptionUserFriendlyMessage(code), }); + + this.context = context; } } diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception.ts b/packages/twenty-server/src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception.ts index c8bb0c7ab9..9e6424aaae 100644 --- a/packages/twenty-server/src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception.ts +++ b/packages/twenty-server/src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception.ts @@ -1,5 +1,6 @@ import { type MessageDescriptor } from '@lingui/core'; import { assertUnreachable } from 'twenty-shared/utils'; +import { z } from 'zod'; import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant'; import { @@ -15,6 +16,19 @@ export const FlatEntityMapsExceptionCode = appendCommonExceptionCode({ ENTITY_MALFORMED: 'ENTITY_MALFORMED', } as const); +export const flatEntityMapsExceptionContextSchema = z.strictObject({ + universalIdentifier: z.string().optional(), + id: z.string().optional(), + applicationId: z.string().optional(), + metadataName: z.string().optional(), + relatedMetadataName: z.string().optional(), + operation: z.enum(['add', 'delete']).optional(), +}); + +export type FlatEntityMapsExceptionContext = z.infer< + typeof flatEntityMapsExceptionContextSchema +>; + const getFlatEntityMapsExceptionUserFriendlyMessage = ( code: keyof typeof FlatEntityMapsExceptionCode, ) => { @@ -33,15 +47,25 @@ const getFlatEntityMapsExceptionUserFriendlyMessage = ( export class FlatEntityMapsException extends CustomException< keyof typeof FlatEntityMapsExceptionCode > { + context?: FlatEntityMapsExceptionContext; + constructor( message: string, code: keyof typeof FlatEntityMapsExceptionCode, - { userFriendlyMessage }: { userFriendlyMessage?: MessageDescriptor } = {}, + { + userFriendlyMessage, + context, + }: { + userFriendlyMessage?: MessageDescriptor; + context?: FlatEntityMapsExceptionContext; + } = {}, ) { super(message, code, { userFriendlyMessage: userFriendlyMessage ?? getFlatEntityMapsExceptionUserFriendlyMessage(code), }); + + this.context = context; } } diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts index 0291e9a643..7894c7974b 100644 --- a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts @@ -128,8 +128,17 @@ export const addFlatEntityToFlatEntityAndRelatedEntityMapsThroughMutationOrThrow ) ) { throw new FlatEntityMapsException( - `Should never occur, invalid flat entity typing. flat ${relatedMetadataName} should contain ${String(flatEntityForeignKeyAggregator)}`, + `Should never occur, invalid flat entity typing. flat ${relatedMetadataName} should contain ${String(flatEntityForeignKeyAggregator)} (metadataName: ${metadataName}, universalIdentifier: ${flatEntity.universalIdentifier})`, FlatEntityMapsExceptionCode.ENTITY_MALFORMED, + { + context: { + id: flatEntity.id, + universalIdentifier: flatEntity.universalIdentifier, + metadataName, + relatedMetadataName, + operation: 'add', + }, + }, ); } diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-maps-or-throw.util.ts b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-maps-or-throw.util.ts index d2800a88bd..e8f653ecb2 100644 --- a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-maps-or-throw.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/add-flat-entity-to-flat-entity-maps-or-throw.util.ts @@ -24,8 +24,16 @@ export const addFlatEntityToFlatEntityMapsOrThrow = < ) ) { throw new FlatEntityMapsException( - 'addFlatEntityToFlatEntityMapsOrThrow: flat entity to add already exists', + `addFlatEntityToFlatEntityMapsOrThrow: flat entity to add already exists (universalIdentifier: ${flatEntity.universalIdentifier})`, FlatEntityMapsExceptionCode.ENTITY_ALREADY_EXISTS, + { + context: { + universalIdentifier: flatEntity.universalIdentifier, + id: flatEntity.id, + applicationId: flatEntity.applicationId, + operation: 'add', + }, + }, ); } diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/delete-flat-entity-from-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/delete-flat-entity-from-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts index d2ee3e3df0..a9394ee836 100644 --- a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/delete-flat-entity-from-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/delete-flat-entity-from-flat-entity-and-related-entity-maps-through-mutation-or-throw.util.ts @@ -110,8 +110,17 @@ export const deleteFlatEntityFromFlatEntityAndRelatedEntityMapsThroughMutationOr ) ) { throw new FlatEntityMapsException( - `Should never occur, invalid flat entity typing. flat ${relatedMetadataName} should contain ${flatEntityForeignKeyAggregator}`, + `Should never occur, invalid flat entity typing. flat ${relatedMetadataName} should contain ${flatEntityForeignKeyAggregator} (metadataName: ${metadataName}, universalIdentifier: ${flatEntity.universalIdentifier})`, FlatEntityMapsExceptionCode.ENTITY_MALFORMED, + { + context: { + id: flatEntity.id, + universalIdentifier: flatEntity.universalIdentifier, + metadataName, + relatedMetadataName, + operation: 'delete', + }, + }, ); } diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/get-flat-entity-maps-exception-context.util.ts b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/get-flat-entity-maps-exception-context.util.ts new file mode 100644 index 0000000000..9818d0e9fb --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/get-flat-entity-maps-exception-context.util.ts @@ -0,0 +1,33 @@ +import { isDefined } from 'twenty-shared/utils'; + +import { + FlatEntityMapsException, + flatEntityMapsExceptionContextSchema, + type FlatEntityMapsExceptionContext, +} from 'src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception'; + +const hasFlatEntityIdentifier = ( + context: FlatEntityMapsExceptionContext, +): boolean => isDefined(context.universalIdentifier) || isDefined(context.id); + +export const getFlatEntityMapsExceptionContext = ( + error: unknown, +): FlatEntityMapsExceptionContext | undefined => { + if (error instanceof FlatEntityMapsException) { + return isDefined(error.context) && hasFlatEntityIdentifier(error.context) + ? error.context + : undefined; + } + + if (isDefined(error) && typeof error === 'object' && 'context' in error) { + const parsedContext = flatEntityMapsExceptionContextSchema.safeParse( + (error as { context?: unknown }).context, + ); + + return parsedContext.success && hasFlatEntityIdentifier(parsedContext.data) + ? parsedContext.data + : undefined; + } + + return undefined; +}; diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration.exception.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration.exception.ts index 0af78b57cd..1149ab4eb2 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration.exception.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration.exception.ts @@ -2,6 +2,7 @@ import { type MessageDescriptor } from '@lingui/core'; import { msg } from '@lingui/core/macro'; import { type WorkspaceMigrationV2ExceptionCode } from 'twenty-shared/metadata'; +import { type FlatEntityMapsExceptionContext } from 'src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception'; import { CustomException } from 'src/utils/custom-exception'; const workspaceMigrationV2ExceptionUserFriendlyMessages: Partial< @@ -11,10 +12,18 @@ const workspaceMigrationV2ExceptionUserFriendlyMessages: Partial< const defaultUserFriendlyMessage = msg`An error occurred during workspace migration.`; export class WorkspaceMigrationV2Exception extends CustomException { + context?: FlatEntityMapsExceptionContext; + constructor( message: string, code: WorkspaceMigrationV2ExceptionCode, - { userFriendlyMessage }: { userFriendlyMessage?: MessageDescriptor } = {}, + { + userFriendlyMessage, + context, + }: { + userFriendlyMessage?: MessageDescriptor; + context?: FlatEntityMapsExceptionContext; + } = {}, ) { super(message, code, { userFriendlyMessage: @@ -22,5 +31,7 @@ export class WorkspaceMigrationV2Exception extends CustomException, + "extensions": { + "code": "APPLICATION_INSTALLATION_FAILED", + "exceptionEventId": Any, + "subCode": "APPLICATION_INSTALLATION_FAILED", + "userFriendlyMessage": "We couldn't install "Test Application". Its field "Due Date" could not be applied to your workspace.", + }, + "message": "Installing application 'Test Application' failed [field: Due Date]: addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists (universalIdentifier: b1b2c3d4-0004-4000-a000-000000000004)", + "name": "GraphQLError", +} +`; + +exports[`Sync application should surface a human error on flat-entity map conflicts should fail with an installation error naming the object when two objects share a universalIdentifier 1`] = ` +{ + "eventId": Any, + "extensions": { + "code": "APPLICATION_INSTALLATION_FAILED", + "exceptionEventId": Any, + "subCode": "APPLICATION_INSTALLATION_FAILED", + "userFriendlyMessage": "We couldn't install "Test Application". Its object "Invoice" could not be applied to your workspace.", + }, + "message": "Installing application 'Test Application' failed [object: Invoice]: addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists (universalIdentifier: b1b2c3d4-0003-4000-a000-000000000003)", + "name": "GraphQLError", +} +`; diff --git a/packages/twenty-server/test/integration/metadata/suites/application/failing-sync-application-flat-entity-map-conflict.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/application/failing-sync-application-flat-entity-map-conflict.integration-spec.ts new file mode 100644 index 0000000000..7d1b4e108e --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application/failing-sync-application-flat-entity-map-conflict.integration-spec.ts @@ -0,0 +1,103 @@ +import { FieldMetadataType } from 'twenty-shared/types'; + +import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util'; +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { buildDefaultObjectManifest } from 'test/integration/metadata/suites/application/utils/build-default-object-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; + +const TEST_APP_ID = 'b1b2c3d4-0001-4000-a000-000000000001'; +const TEST_ROLE_ID = 'b1b2c3d4-0002-4000-a000-000000000002'; +const DUPLICATED_OBJECT_UNIVERSAL_IDENTIFIER = + 'b1b2c3d4-0003-4000-a000-000000000003'; +const DUPLICATED_FIELD_UNIVERSAL_IDENTIFIER = + 'b1b2c3d4-0004-4000-a000-000000000004'; + +describe('Sync application should surface a human error on flat-entity map conflicts', () => { + beforeAll(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Test Flat Entity Map Conflict App', + description: 'App for testing flat-entity map conflict error formatting', + sourcePath: 'test-flat-entity-map-conflict', + }); + }, 60000); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it('should fail with an installation error naming the object when two objects share a universalIdentifier', async () => { + const firstObject = buildDefaultObjectManifest({ + nameSingular: 'invoice', + namePlural: 'invoices', + labelSingular: 'Invoice', + labelPlural: 'Invoices', + universalIdentifier: DUPLICATED_OBJECT_UNIVERSAL_IDENTIFIER, + }); + + const conflictingObject = buildDefaultObjectManifest({ + nameSingular: 'invoiceDuplicate', + namePlural: 'invoiceDuplicates', + labelSingular: 'Invoice Duplicate', + labelPlural: 'Invoice Duplicates', + universalIdentifier: DUPLICATED_OBJECT_UNIVERSAL_IDENTIFIER, + }); + + const manifest = buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + objects: [firstObject, conflictingObject], + }, + }); + + const { errors } = await syncApplication({ + manifest, + expectToFail: true, + }); + + expectOneNotInternalServerErrorSnapshot({ errors }); + }, 60000); + + it('should fail with an installation error naming the field when two fields of an object share a universalIdentifier', async () => { + const objectWithConflictingFields = buildDefaultObjectManifest({ + nameSingular: 'invoice', + namePlural: 'invoices', + labelSingular: 'Invoice', + labelPlural: 'Invoices', + additionalFields: [ + { + universalIdentifier: DUPLICATED_FIELD_UNIVERSAL_IDENTIFIER, + type: FieldMetadataType.DATE_TIME, + name: 'dueDate', + label: 'Due Date', + }, + { + universalIdentifier: DUPLICATED_FIELD_UNIVERSAL_IDENTIFIER, + type: FieldMetadataType.DATE_TIME, + name: 'dueDateDuplicate', + label: 'Due Date Duplicate', + }, + ], + }); + + const manifest = buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + objects: [objectWithConflictingFields], + }, + }); + + const { errors } = await syncApplication({ + manifest, + expectToFail: true, + }); + + expectOneNotInternalServerErrorSnapshot({ errors }); + }, 60000); +});