Paul Rastoin
47689e676b
## Context
cc @rashad
Twenty applies metadata changes optimistically to in-memory *flat entity
maps* before persisting them. The utils that mutate these maps throw
`FlatEntityMapsException` on invariant violations, which surface in
Sentry (e.g. during `InstallApplication`) as a **hardcoded, generic
message with no identifying data**:
```
GraphQLError: addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists
```
There was no way to know *which* entity collided — making triage
impossible.
## What this does (two layers)
**Layer 1 — leaf utils emit identifiers**
- `FlatEntityMapsException` gains an optional structured `context`
(`universalIdentifier` / `id` / `applicationId` / `metadataName` /
`relatedMetadataName` / `operation`), read by the Sentry driver's
existing `'context' in exception` → `setExtra` channel.
- All **9 leaf throw sites** append their in-scope identifiers to the
message **and** populate `context`.
**Propagation — context survives the re-wraps**
- On the install path the collision throws in the (unwrapped)
`compute()` step, so the raw exception + context reaches app-sync
intact.
- For the run/build-phase paths, the migration runner and
build-orchestrator re-wraps copy only `.message`; they now also
**forward `context`** so structured data survives there too.
**Layer 2 — human installation error**
- `synchronizeFromManifest` catches flat-entity failures, resolves the
offending `universalIdentifier` to a manifest **object/field label**,
and rethrows `ApplicationException(APPLICATION_INSTALLATION_FAILED)`
with a safe, human `userFriendlyMessage`.
- The leaf `userFriendlyMessage` stays `STANDARD_ERROR_MESSAGE` — the
detailed message never leaks to end users.
- `APPLICATION_INSTALLATION_FAILED` surfaces with the dedicated
`ErrorCode.APPLICATION_INSTALLATION_FAILED` GraphQL code (mirroring the
workspace-migration runner formatter), not `INTERNAL_SERVER_ERROR`.
### Result — client-facing GraphQL error envelope
```json
{
"extensions": {
"code": "APPLICATION_INSTALLATION_FAILED",
"subCode": "APPLICATION_INSTALLATION_FAILED",
"userFriendlyMessage": "We couldn't install \"Test Application\". Its Invoice could not be applied to your workspace."
},
"message": "Installing application 'Test Application' failed [object: Invoice]: addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists (universalIdentifier: ...)",
"name": "GraphQLError"
}
```
## Where the identifier shows up (not just Sentry)
The offending `universalIdentifier` reaches every consumer, not only
Sentry:
- **Sentry (server):** structured `context` extras + the enriched
message (fingerprinted by `code`, so no issue fragmentation).
- **GraphQL response `message`:** un-masked (no `useMaskedErrors`; the
error-handler hook passes `BaseGraphQLError` through as-is), so it
travels over the wire.
- **App-author SDK/CLI terminal:** `twenty-sdk` captures
`errors[0].message`; for this error `formatManifestValidationErrors`
returns `null` (no `extensions.errors`/`summary`), so the orchestrator
falls back to printing the full message, e.g.:
```
✗ Sync failed with error: Installing application 'X' failed [object:
Invoice]: … already exists (universalIdentifier: b1b2c3d4-…)
ℹ Hint: a metadata conflict was detected. Preview the plan with `yarn
twenty dev --once --dry-run`; …
```
The `already exists` / `universalidentifier` substrings also trigger
`getSyncErrorRecoveryHint`, so the author gets an actionable next step.
- **End-user (CRM UI):** only the safe rendered `userFriendlyMessage`
(no UUIDs).
## Design note
`userFriendlyMessage` behaviour of the leaf exceptions is intentionally
unchanged (guardrail). Layer 2 resolves labels for **objects and
fields** (the bulk of metadata); other manifest entity kinds fall back
to an app-name-only human message to avoid brittle manifest-walking —
easy to extend. A future first-class option would be structured
`extensions` (like `METADATA_VALIDATION_FAILED`) + a dedicated SDK
formatter; deferred since the message path already surfaces the detail
in the terminal.
## Tests
- **Unit:** existing through-mutation + runner-exception specs still
pass (they assert on exception **code**, not message). Added a spec for
the enrichment util.
- **Response-format snapshot (verified, green):**
`application-exception-filter.spec.ts` runs the exception filter and
snapshots the exact client-facing GraphQL error envelope shown above.
- **Integration:**
`failing-sync-application-flat-entity-map-conflict.integration-spec.ts`
syncs a manifest whose two objects share a `universalIdentifier`
(collision during manifest map build, before validation) and snapshots
the GraphQL error response via
`expectOneNotInternalServerErrorSnapshot`.
- ⚠️ The integration `.snap` was authored from the identical
deterministic path (verified by the filter unit snapshot) because the
integration suite couldn't be executed in the authoring sandbox. Please
regenerate/confirm with `nx test:integration:with-db-reset` (or `-u`) in
a seeded env.
## Status
Draft — opening for review.
104 lines
3.7 KiB
TypeScript
104 lines
3.7 KiB
TypeScript
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);
|
|
});
|