mirror of
https://github.com/twentyhq/twenty.git
synced 2026-04-18 05:54:42 -04:00
Surface structured validation errors during application install (#19787)
## Summary - Add `WorkspaceMigrationGraphqlApiExceptionInterceptor` to `MarketplaceResolver` and `ApplicationInstallResolver` so validation failures during app install return `METADATA_VALIDATION_FAILED` with structured `extensions.errors` instead of generic `INTERNAL_SERVER_ERROR` - Update SDK `installTarballApp()` to pass the full GraphQL error object (including extensions) through the install flow - Add `formatInstallValidationErrors` utility to format structured validation errors for CLI output - Add integration test verifying structured error responses for invalid navigation menu items and view fields --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -13,7 +13,7 @@ import { manifestUpdateChecksums } from '@/cli/utilities/build/manifest/manifest
|
||||
import { writeManifestToOutput } from '@/cli/utilities/build/manifest/manifest-writer';
|
||||
import { ClientService } from '@/cli/utilities/client/client-service';
|
||||
import { ConfigService } from '@/cli/utilities/config/config-service';
|
||||
import { formatSyncErrorEvents } from '@/cli/utilities/dev/orchestrator/steps/format-sync-error-events';
|
||||
import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors';
|
||||
import { serializeError } from '@/cli/utilities/error/serialize-error';
|
||||
import { FileUploader } from '@/cli/utilities/file/file-uploader';
|
||||
import { runSafe } from '@/cli/utilities/run-safe';
|
||||
@@ -197,7 +197,7 @@ const innerAppDevOnce = async (
|
||||
if (!syncResult.success) {
|
||||
const errorEvents = verbose
|
||||
? null
|
||||
: formatSyncErrorEvents(syncResult.error);
|
||||
: formatManifestValidationErrors(syncResult.error);
|
||||
|
||||
const message = errorEvents
|
||||
? errorEvents.map((event) => event.message).join('\n')
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { ApiService } from '@/cli/utilities/api/api-service';
|
||||
import { readManifestFromFile } from '@/cli/utilities/build/manifest/manifest-reader';
|
||||
import { ConfigService } from '@/cli/utilities/config/config-service';
|
||||
import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors';
|
||||
import { serializeError } from '@/cli/utilities/error/serialize-error';
|
||||
import { runSafe } from '@/cli/utilities/run-safe';
|
||||
import { APP_ERROR_CODES, type CommandResult } from '@/cli/types';
|
||||
|
||||
@@ -34,16 +36,17 @@ const innerAppInstall = async (
|
||||
});
|
||||
|
||||
if (!result.success) {
|
||||
const errorMessage =
|
||||
result.error instanceof Error
|
||||
? result.error.message
|
||||
: String(result.error ?? 'Unknown error');
|
||||
const errorEvents = formatManifestValidationErrors(result.error);
|
||||
|
||||
const message = errorEvents
|
||||
? errorEvents.map((event) => event.message).join('\n')
|
||||
: `Install failed with error: ${serializeError(result.error)}`;
|
||||
|
||||
return {
|
||||
success: false,
|
||||
error: {
|
||||
code: APP_ERROR_CODES.INSTALL_FAILED,
|
||||
message: errorMessage,
|
||||
message,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -161,8 +161,7 @@ export class FileApi {
|
||||
if (response.data.errors) {
|
||||
return {
|
||||
success: false,
|
||||
error:
|
||||
response.data.errors[0]?.message || 'Failed to install application',
|
||||
error: response.data.errors[0] || 'Failed to install application',
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ import {
|
||||
type OrchestratorStateStepEvent,
|
||||
type OrchestratorStateSyncStatus,
|
||||
} from '@/cli/utilities/dev/orchestrator/dev-mode-orchestrator-state';
|
||||
import { formatSyncErrorEvents } from '@/cli/utilities/dev/orchestrator/steps/format-sync-error-events';
|
||||
import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors';
|
||||
import { serializeError } from '@/cli/utilities/error/serialize-error';
|
||||
import { type Manifest } from 'twenty-shared/application';
|
||||
|
||||
@@ -81,7 +81,7 @@ export class SyncApplicationOrchestratorStep {
|
||||
|
||||
const errorEvents = this.verbose
|
||||
? null
|
||||
: formatSyncErrorEvents(syncResult.error);
|
||||
: formatManifestValidationErrors(syncResult.error);
|
||||
|
||||
if (errorEvents) {
|
||||
events.push(...errorEvents);
|
||||
|
||||
@@ -1,26 +1,26 @@
|
||||
import { formatSyncErrorEvents } from '@/cli/utilities/dev/orchestrator/steps/format-sync-error-events';
|
||||
import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors';
|
||||
|
||||
describe('formatSyncErrorEvents', () => {
|
||||
describe('formatManifestValidationErrors', () => {
|
||||
it('should return null for null input', () => {
|
||||
expect(formatSyncErrorEvents(null)).toBeNull();
|
||||
expect(formatManifestValidationErrors(null)).toBeNull();
|
||||
});
|
||||
|
||||
it('should return null for undefined input', () => {
|
||||
expect(formatSyncErrorEvents(undefined)).toBeNull();
|
||||
expect(formatManifestValidationErrors(undefined)).toBeNull();
|
||||
});
|
||||
|
||||
it('should return null for non-object input', () => {
|
||||
expect(formatSyncErrorEvents('string error')).toBeNull();
|
||||
expect(formatSyncErrorEvents(42)).toBeNull();
|
||||
expect(formatManifestValidationErrors('string error')).toBeNull();
|
||||
expect(formatManifestValidationErrors(42)).toBeNull();
|
||||
});
|
||||
|
||||
it('should return null when extensions is missing', () => {
|
||||
expect(formatSyncErrorEvents({ message: 'error' })).toBeNull();
|
||||
expect(formatManifestValidationErrors({ message: 'error' })).toBeNull();
|
||||
});
|
||||
|
||||
it('should return null when extensions.errors is missing', () => {
|
||||
expect(
|
||||
formatSyncErrorEvents({
|
||||
formatManifestValidationErrors({
|
||||
extensions: { summary: { totalErrors: 1 } },
|
||||
}),
|
||||
).toBeNull();
|
||||
@@ -28,14 +28,14 @@ describe('formatSyncErrorEvents', () => {
|
||||
|
||||
it('should return null when extensions.summary is missing', () => {
|
||||
expect(
|
||||
formatSyncErrorEvents({
|
||||
formatManifestValidationErrors({
|
||||
extensions: { errors: {} },
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('should format a single error', () => {
|
||||
const events = formatSyncErrorEvents({
|
||||
const events = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
fieldMetadata: [
|
||||
@@ -72,7 +72,7 @@ describe('formatSyncErrorEvents', () => {
|
||||
});
|
||||
|
||||
it('should format multiple errors across metadata types', () => {
|
||||
const events = formatSyncErrorEvents({
|
||||
const events = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
fieldMetadata: [
|
||||
@@ -103,7 +103,7 @@ describe('formatSyncErrorEvents', () => {
|
||||
});
|
||||
|
||||
it('should format errors with details for both objectMetadata and fieldMetadata', () => {
|
||||
const events = formatSyncErrorEvents({
|
||||
const events = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
objectMetadata: [
|
||||
@@ -170,7 +170,7 @@ describe('formatSyncErrorEvents', () => {
|
||||
});
|
||||
|
||||
it('should include value in details when present', () => {
|
||||
const events = formatSyncErrorEvents({
|
||||
const events = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
fieldMetadata: [
|
||||
@@ -194,7 +194,7 @@ describe('formatSyncErrorEvents', () => {
|
||||
});
|
||||
|
||||
it('should omit details suffix when no value or universalIdentifier', () => {
|
||||
const events = formatSyncErrorEvents({
|
||||
const events = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
fieldMetadata: [
|
||||
@@ -212,7 +212,7 @@ describe('formatSyncErrorEvents', () => {
|
||||
});
|
||||
|
||||
it('should fall back to entries.length when summary count is missing for a metadata type', () => {
|
||||
const events = formatSyncErrorEvents({
|
||||
const events = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
fieldMetadata: [
|
||||
@@ -233,7 +233,7 @@ describe('formatSyncErrorEvents', () => {
|
||||
});
|
||||
|
||||
it('should pluralize correctly for singular and plural counts', () => {
|
||||
const singleError = formatSyncErrorEvents({
|
||||
const singleError = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
objectMetadata: [
|
||||
@@ -249,7 +249,7 @@ describe('formatSyncErrorEvents', () => {
|
||||
expect(singleError?.[0].message).toBe('Sync failed with 1 error');
|
||||
expect(singleError?.[1].message).toBe('objectMetadata: 1 error');
|
||||
|
||||
const multipleErrors = formatSyncErrorEvents({
|
||||
const multipleErrors = formatManifestValidationErrors({
|
||||
extensions: {
|
||||
errors: {
|
||||
objectMetadata: [
|
||||
@@ -15,7 +15,7 @@ type StructuredSyncError = {
|
||||
};
|
||||
};
|
||||
|
||||
export const formatSyncErrorEvents = (
|
||||
export const formatManifestValidationErrors = (
|
||||
error: unknown,
|
||||
): OrchestratorStateStepEvent[] | null => {
|
||||
if (!error || typeof error !== 'object') {
|
||||
@@ -1,4 +1,9 @@
|
||||
import { UseFilters, UseGuards, UsePipes } from '@nestjs/common';
|
||||
import {
|
||||
UseFilters,
|
||||
UseGuards,
|
||||
UseInterceptors,
|
||||
UsePipes,
|
||||
} from '@nestjs/common';
|
||||
import { Args, Mutation, Query } from '@nestjs/graphql';
|
||||
|
||||
import { PermissionFlagType } from 'twenty-shared/constants';
|
||||
@@ -14,10 +19,12 @@ import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspac
|
||||
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
|
||||
import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.guard';
|
||||
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
|
||||
import { WorkspaceMigrationGraphqlApiExceptionInterceptor } from 'src/engine/workspace-manager/workspace-migration/interceptors/workspace-migration-graphql-api-exception.interceptor';
|
||||
|
||||
@UsePipes(ResolverValidationPipe)
|
||||
@MetadataResolver()
|
||||
@UseFilters(ApplicationExceptionFilter, AuthGraphqlApiExceptionFilter)
|
||||
@UseInterceptors(WorkspaceMigrationGraphqlApiExceptionInterceptor)
|
||||
@UseGuards(WorkspaceAuthGuard)
|
||||
export class ApplicationInstallResolver {
|
||||
constructor(
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { UseFilters, UseGuards } from '@nestjs/common';
|
||||
import { UseFilters, UseGuards, UseInterceptors } from '@nestjs/common';
|
||||
import { Args, Mutation, Query } from '@nestjs/graphql';
|
||||
|
||||
import { PermissionFlagType } from 'twenty-shared/constants';
|
||||
@@ -13,6 +13,7 @@ import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorat
|
||||
import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard';
|
||||
import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.guard';
|
||||
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
|
||||
import { WorkspaceMigrationGraphqlApiExceptionInterceptor } from 'src/engine/workspace-manager/workspace-migration/interceptors/workspace-migration-graphql-api-exception.interceptor';
|
||||
import { MarketplaceCatalogSyncCronJob } from 'src/engine/core-modules/application/application-marketplace/crons/marketplace-catalog-sync.cron.job';
|
||||
import { InjectMessageQueue } from 'src/engine/core-modules/message-queue/decorators/message-queue.decorator';
|
||||
import { MessageQueue } from 'src/engine/core-modules/message-queue/message-queue.constants';
|
||||
@@ -20,6 +21,7 @@ import { MessageQueueService } from 'src/engine/core-modules/message-queue/servi
|
||||
|
||||
@MetadataResolver()
|
||||
@UseFilters(ApplicationRegistrationExceptionFilter)
|
||||
@UseInterceptors(WorkspaceMigrationGraphqlApiExceptionInterceptor)
|
||||
@UseGuards(WorkspaceAuthGuard, NoPermissionGuard)
|
||||
export class MarketplaceResolver {
|
||||
constructor(
|
||||
|
||||
@@ -0,0 +1,184 @@
|
||||
import crypto from 'crypto';
|
||||
import { promises as fs } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
import * as tar from 'tar';
|
||||
import { installApplication } from 'test/integration/metadata/suites/application/utils/install-application.util';
|
||||
import { uploadAppTarball } from 'test/integration/metadata/suites/application/utils/upload-app-tarball.util';
|
||||
import { STANDARD_OBJECTS } from 'twenty-shared/metadata';
|
||||
import { type DataSource } from 'typeorm';
|
||||
|
||||
const createTestTarball = async (
|
||||
files: Record<string, string>,
|
||||
): Promise<Buffer> => {
|
||||
const tempId = crypto.randomUUID();
|
||||
const sourceDir = join(tmpdir(), `test-tarball-src-${tempId}`);
|
||||
const tarballPath = join(tmpdir(), `test-tarball-${tempId}.tar.gz`);
|
||||
|
||||
await fs.mkdir(sourceDir, { recursive: true });
|
||||
|
||||
for (const [name, content] of Object.entries(files)) {
|
||||
const filePath = join(sourceDir, name);
|
||||
const dir = filePath.substring(0, filePath.lastIndexOf('/'));
|
||||
|
||||
if (dir !== sourceDir) {
|
||||
await fs.mkdir(dir, { recursive: true });
|
||||
}
|
||||
await fs.writeFile(filePath, content);
|
||||
}
|
||||
|
||||
await tar.create(
|
||||
{
|
||||
file: tarballPath,
|
||||
gzip: true,
|
||||
cwd: sourceDir,
|
||||
},
|
||||
Object.keys(files),
|
||||
);
|
||||
|
||||
const buffer = await fs.readFile(tarballPath);
|
||||
|
||||
await fs.rm(sourceDir, { recursive: true, force: true });
|
||||
await fs.rm(tarballPath, { force: true });
|
||||
|
||||
return buffer;
|
||||
};
|
||||
|
||||
const buildManifestWithCrossEntityIdentifierConflict = (
|
||||
universalIdentifier: string,
|
||||
roleUniversalIdentifier: string,
|
||||
duplicatedUniversalIdentifier: string,
|
||||
) =>
|
||||
JSON.stringify({
|
||||
application: {
|
||||
universalIdentifier,
|
||||
displayName: 'Test App With Cross Entity Identifier Conflict',
|
||||
description:
|
||||
'A test app whose manifest reuses a universalIdentifier across entity types',
|
||||
icon: 'IconTestPipe',
|
||||
defaultRoleUniversalIdentifier: roleUniversalIdentifier,
|
||||
applicationVariables: {},
|
||||
packageJsonChecksum: null,
|
||||
yarnLockChecksum: null,
|
||||
},
|
||||
roles: [
|
||||
{
|
||||
universalIdentifier: roleUniversalIdentifier,
|
||||
label: 'First Role',
|
||||
description: 'First role',
|
||||
},
|
||||
{
|
||||
universalIdentifier: duplicatedUniversalIdentifier,
|
||||
label: 'Second Role',
|
||||
description: 'Second role',
|
||||
objectPermissions: [
|
||||
{
|
||||
universalIdentifier: duplicatedUniversalIdentifier,
|
||||
objectUniversalIdentifier:
|
||||
STANDARD_OBJECTS.company.universalIdentifier,
|
||||
canReadObjectRecords: true,
|
||||
canUpdateObjectRecords: false,
|
||||
canSoftDeleteObjectRecords: false,
|
||||
canDestroyObjectRecords: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
skills: [],
|
||||
agents: [],
|
||||
objects: [],
|
||||
fields: [],
|
||||
logicFunctions: [],
|
||||
frontComponents: [],
|
||||
publicAssets: [],
|
||||
views: [],
|
||||
navigationMenuItems: [],
|
||||
pageLayouts: [],
|
||||
});
|
||||
|
||||
describe('Install application should return structured validation errors', () => {
|
||||
let ds: DataSource;
|
||||
const createdRegistrationIds: string[] = [];
|
||||
const createdApplicationUniversalIdentifiers: string[] = [];
|
||||
|
||||
beforeAll(() => {
|
||||
jest.useRealTimers();
|
||||
ds = globalThis.testDataSource;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
for (const uid of createdApplicationUniversalIdentifiers) {
|
||||
await ds.query(
|
||||
`DELETE FROM core."file" WHERE "applicationId" IN (
|
||||
SELECT id FROM core."application" WHERE "universalIdentifier" = $1
|
||||
)`,
|
||||
[uid],
|
||||
);
|
||||
|
||||
await ds.query(
|
||||
`DELETE FROM core."application" WHERE "universalIdentifier" = $1`,
|
||||
[uid],
|
||||
);
|
||||
}
|
||||
|
||||
for (const id of createdRegistrationIds) {
|
||||
await ds.query(
|
||||
`DELETE FROM core."applicationRegistration" WHERE id = $1`,
|
||||
[id],
|
||||
);
|
||||
}
|
||||
|
||||
jest.useFakeTimers();
|
||||
});
|
||||
|
||||
it('should return METADATA_VALIDATION_FAILED with structured errors when installing an app whose manifest has validation errors', async () => {
|
||||
const universalIdentifier = crypto.randomUUID();
|
||||
const roleUniversalIdentifier = crypto.randomUUID();
|
||||
const duplicatedUniversalIdentifier = crypto.randomUUID();
|
||||
const manifest = buildManifestWithCrossEntityIdentifierConflict(
|
||||
universalIdentifier,
|
||||
roleUniversalIdentifier,
|
||||
duplicatedUniversalIdentifier,
|
||||
);
|
||||
|
||||
const tarball = await createTestTarball({
|
||||
'manifest.json': manifest,
|
||||
'package.json': JSON.stringify({
|
||||
name: 'test-cross-entity-identifier-conflict-app',
|
||||
version: '1.0.0',
|
||||
}),
|
||||
});
|
||||
|
||||
const uploadResult = await uploadAppTarball({
|
||||
tarballBuffer: tarball,
|
||||
universalIdentifier,
|
||||
});
|
||||
|
||||
expect(uploadResult.errors).toBeUndefined();
|
||||
expect(uploadResult.data?.uploadAppTarball.id).toBeDefined();
|
||||
|
||||
const registrationId = uploadResult.data!.uploadAppTarball.id;
|
||||
|
||||
createdRegistrationIds.push(registrationId);
|
||||
createdApplicationUniversalIdentifiers.push(universalIdentifier);
|
||||
|
||||
const { errors } = await installApplication({
|
||||
input: {
|
||||
appRegistrationId: registrationId,
|
||||
},
|
||||
expectToFail: true,
|
||||
});
|
||||
|
||||
expect(errors).toBeDefined();
|
||||
expect(errors.length).toBe(1);
|
||||
|
||||
const [error] = errors;
|
||||
|
||||
expect(error.extensions.code).toBe('METADATA_VALIDATION_FAILED');
|
||||
expect(error.extensions.errors).toBeDefined();
|
||||
expect(error.extensions.summary).toBeDefined();
|
||||
expect(error.extensions.summary.totalErrors).toBeGreaterThan(0);
|
||||
expect(error.extensions.message).toMatch(/Validation failed for/);
|
||||
}, 120000);
|
||||
});
|
||||
Reference in New Issue
Block a user