Include root cause errors in workspace migration runner exception message (#23416)
closes https://github.com/twentyhq/core-team-issues/issues/2733 ```diff - "message": "Migration action 'create' for 'index' (universalIdentifier: 67c6811e-...) failed" + "message": "Migration action 'create' for 'index' (universalIdentifier: 67c6811e-...) failed: [workspaceSchema] could not create unique index "IDX_UNIQUE_85951922a2..." (pg code: 23505, detail: Key ("externalId")=(DUPLICATED-VALUE) is duplicated.)" ``` ## Problem When a workspace migration action fails, the `EXECUTION_FAILED` exception message only states which action failed: ``` Migration action 'create' for 'index' (universalIdentifier: 9e20a0f6-7a18-51c5-a422-5dc4dbd1d972) failed ``` The underlying errors (`metadata`, `workspaceSchema`, `actionTranspilation`) are attached to the exception instance but dropped by most surfaces: the SDK CLI only prints `errors[0].message`, server logs only log `error.message`, and the REST/GraphQL paths lose the postgres driver details. Debugging a failed app install (like the stale index universalIdentifier case in the issue) requires guessing. ## Change - New `formatWorkspaceMigrationRunnerExecutionErrors` util that builds a compact one-line summary of the underlying execution errors, including the postgres error code and `detail` for `QueryFailedError`, capped at 1500 chars. - The `EXECUTION_FAILED` exception message now appends that summary: ``` Migration action 'create' for 'index' (universalIdentifier: 9e20a0f6-...) failed: [workspaceSchema] relation "IDX_..." already exists (pg code: 42P07) ``` Since every surface (CLI, server logs, Sentry, REST, GraphQL) shows `error.message`, the root cause now propagates everywhere without touching those surfaces. - Since actions run inside a single transaction, when one branch fails with `25P02 current transaction is aborted` (collateral of the other branch's statement aborting the transaction), the summary keeps only the real root cause. A lone 25P02 error is still shown. ## Notes - The SDK's `getSyncErrorRecoveryHint` matching (`/migration action .* failed/`) still works with the suffixed format. - Commit-time failures from `DEFERRABLE INITIALLY DEFERRED` FK constraints still bypass this path (wrapped as `INTERNAL_SERVER_ERROR` with no action attribution) and are left as a follow-up. ## Tests - New spec for the formatter util (labels, pg code/detail, 25P02 demotion, truncation). - Extended `workspace-migration-runner.exception.spec.ts` with a root-cause message assertion. - Updated `format-upgrade-error-for-storage` snapshots (first line now carries the enriched message). --- _Generated by [Claude Code](https://claude.ai/code/session_01Mu8t74X14oYVkLrFBZHs2o)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23416?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+22
@@ -42,4 +42,26 @@ describe('WorkspaceMigrationRunnerException', () => {
|
||||
"Migration action 'delete' for 'pageLayout' (universalIdentifier: uid-page-layout) failed",
|
||||
);
|
||||
});
|
||||
|
||||
it('includes the underlying execution errors in the message', () => {
|
||||
const action = {
|
||||
type: 'create',
|
||||
metadataName: 'index',
|
||||
flatEntity: {
|
||||
universalIdentifier: '9e20a0f6-7a18-51c5-a422-5dc4dbd1d972',
|
||||
},
|
||||
} as unknown as AllUniversalWorkspaceMigrationAction;
|
||||
|
||||
const exception = new WorkspaceMigrationRunnerException({
|
||||
code: WorkspaceMigrationRunnerExceptionCode.EXECUTION_FAILED,
|
||||
action,
|
||||
errors: {
|
||||
workspaceSchema: new Error('relation "IDX_abc" already exists'),
|
||||
},
|
||||
});
|
||||
|
||||
expect(exception.message).toBe(
|
||||
"Migration action 'create' for 'index' (universalIdentifier: 9e20a0f6-7a18-51c5-a422-5dc4dbd1d972) failed: [workspaceSchema] relation \"IDX_abc\" already exists",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
+7
-1
@@ -4,6 +4,7 @@ import { assertUnreachable, CustomError } from 'twenty-shared/utils';
|
||||
|
||||
import { type FlatEntityMapsExceptionContext } from 'src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception';
|
||||
import { type AllUniversalWorkspaceMigrationAction } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/workspace-migration-action-common';
|
||||
import { formatWorkspaceMigrationRunnerExecutionErrors } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-runner/utils/format-workspace-migration-runner-execution-errors.util';
|
||||
|
||||
export const WorkspaceMigrationRunnerExceptionCode = {
|
||||
INTERNAL_SERVER_ERROR: 'INTERNAL_SERVER_ERROR',
|
||||
@@ -88,9 +89,14 @@ export class WorkspaceMigrationRunnerException extends CustomError {
|
||||
args.action,
|
||||
);
|
||||
const identifierClause = ` (universalIdentifier: ${universalIdentifier})`;
|
||||
const executionErrorsSummary =
|
||||
formatWorkspaceMigrationRunnerExecutionErrors(args.errors);
|
||||
const causeClause = executionErrorsSummary
|
||||
? `: ${executionErrorsSummary}`
|
||||
: '';
|
||||
|
||||
super(
|
||||
`Migration action '${args.action.type}' for '${args.action.metadataName}'${identifierClause} failed`,
|
||||
`Migration action '${args.action.type}' for '${args.action.metadataName}'${identifierClause} failed${causeClause}`,
|
||||
);
|
||||
|
||||
this.code = args.code;
|
||||
|
||||
+114
@@ -0,0 +1,114 @@
|
||||
import { QueryFailedError } from 'typeorm';
|
||||
|
||||
import { formatWorkspaceMigrationRunnerExecutionErrors } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-runner/utils/format-workspace-migration-runner-execution-errors.util';
|
||||
|
||||
const buildQueryFailedError = ({
|
||||
message,
|
||||
code,
|
||||
detail,
|
||||
}: {
|
||||
message: string;
|
||||
code?: string;
|
||||
detail?: string;
|
||||
}): QueryFailedError => {
|
||||
const driverError = new Error(message);
|
||||
|
||||
Object.assign(driverError, { code, detail });
|
||||
|
||||
return new QueryFailedError('INSERT INTO "core"."index"', [], driverError);
|
||||
};
|
||||
|
||||
describe('formatWorkspaceMigrationRunnerExecutionErrors', () => {
|
||||
it('returns undefined when no error is set', () => {
|
||||
expect(formatWorkspaceMigrationRunnerExecutionErrors({})).toBeUndefined();
|
||||
});
|
||||
|
||||
it('formats a plain error with its origin label', () => {
|
||||
expect(
|
||||
formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
metadata: new Error('column "label" cannot be null'),
|
||||
}),
|
||||
).toBe('[metadata] column "label" cannot be null');
|
||||
});
|
||||
|
||||
it('includes postgres code and detail of a QueryFailedError', () => {
|
||||
expect(
|
||||
formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
workspaceSchema: buildQueryFailedError({
|
||||
message: 'relation "IDX_abc" already exists',
|
||||
code: '42P07',
|
||||
}),
|
||||
}),
|
||||
).toBe(
|
||||
'[workspaceSchema] relation "IDX_abc" already exists (pg code: 42P07)',
|
||||
);
|
||||
|
||||
expect(
|
||||
formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
metadata: buildQueryFailedError({
|
||||
message: 'duplicate key value violates unique constraint "UQ_name"',
|
||||
code: '23505',
|
||||
detail: 'Key (name)=(foo) already exists.',
|
||||
}),
|
||||
}),
|
||||
).toBe(
|
||||
'[metadata] duplicate key value violates unique constraint "UQ_name" (pg code: 23505, detail: Key (name)=(foo) already exists.)',
|
||||
);
|
||||
});
|
||||
|
||||
it('joins multiple errors', () => {
|
||||
expect(
|
||||
formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
metadata: new Error('column "label" cannot be null'),
|
||||
workspaceSchema: new Error('table already exists'),
|
||||
}),
|
||||
).toBe(
|
||||
'[metadata] column "label" cannot be null; [workspaceSchema] table already exists',
|
||||
);
|
||||
});
|
||||
|
||||
it('hides a 25P02 aborted-transaction error when a root cause is available', () => {
|
||||
expect(
|
||||
formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
metadata: buildQueryFailedError({
|
||||
message: 'current transaction is aborted',
|
||||
code: '25P02',
|
||||
}),
|
||||
workspaceSchema: buildQueryFailedError({
|
||||
message: 'relation "IDX_abc" already exists',
|
||||
code: '42P07',
|
||||
}),
|
||||
}),
|
||||
).toBe(
|
||||
'[workspaceSchema] relation "IDX_abc" already exists (pg code: 42P07)',
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps a 25P02 error when it is the only one', () => {
|
||||
expect(
|
||||
formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
metadata: buildQueryFailedError({
|
||||
message: 'current transaction is aborted',
|
||||
code: '25P02',
|
||||
}),
|
||||
}),
|
||||
).toBe('[metadata] current transaction is aborted (pg code: 25P02)');
|
||||
});
|
||||
|
||||
it('truncates an oversized summary to the cap, marker included', () => {
|
||||
const summary = formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
metadata: new Error('x'.repeat(5_000)),
|
||||
});
|
||||
|
||||
expect(summary).toHaveLength(1_500);
|
||||
expect(summary?.endsWith(' [truncated]')).toBe(true);
|
||||
});
|
||||
|
||||
it('stringifies a non-Error rejection value', () => {
|
||||
expect(
|
||||
formatWorkspaceMigrationRunnerExecutionErrors({
|
||||
metadata: 'plain string rejection' as unknown as Error,
|
||||
}),
|
||||
).toBe('[metadata] plain string rejection');
|
||||
});
|
||||
});
|
||||
+81
@@ -0,0 +1,81 @@
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { QueryFailedError } from 'typeorm';
|
||||
|
||||
import { type WorkspaceMigrationRunnerExecutionErrors } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-runner/exceptions/workspace-migration-runner.exception';
|
||||
|
||||
const MAX_EXECUTION_ERRORS_SUMMARY_LENGTH = 1_500;
|
||||
|
||||
const TRUNCATION_MARKER = ' [truncated]';
|
||||
|
||||
const POSTGRES_TRANSACTION_ABORTED_CODE = '25P02';
|
||||
|
||||
// Ordered by execution phase: transpilation runs before the metadata and
|
||||
// workspace schema writes.
|
||||
const EXECUTION_ERROR_LABELS = [
|
||||
'actionTranspilation',
|
||||
'metadata',
|
||||
'workspaceSchema',
|
||||
] as const satisfies readonly (keyof WorkspaceMigrationRunnerExecutionErrors)[];
|
||||
|
||||
type ExecutionErrorEntry = {
|
||||
label: (typeof EXECUTION_ERROR_LABELS)[number];
|
||||
error: Error;
|
||||
};
|
||||
|
||||
const getPostgresDriverError = (
|
||||
error: Error,
|
||||
): { code?: string; detail?: string } | undefined =>
|
||||
error instanceof QueryFailedError
|
||||
? (error.driverError as { code?: string; detail?: string } | undefined)
|
||||
: undefined;
|
||||
|
||||
const isTransactionAbortedError = (error: Error): boolean =>
|
||||
getPostgresDriverError(error)?.code === POSTGRES_TRANSACTION_ABORTED_CODE;
|
||||
|
||||
const formatSingleExecutionError = (error: Error): string => {
|
||||
// Rejection reasons are typed as Error but nothing guarantees it at runtime.
|
||||
const baseMessage = error instanceof Error ? error.message : String(error);
|
||||
const driverError = getPostgresDriverError(error);
|
||||
const driverErrorParts = [
|
||||
isDefined(driverError?.code) ? `pg code: ${driverError.code}` : null,
|
||||
isDefined(driverError?.detail) ? `detail: ${driverError.detail}` : null,
|
||||
].filter(isDefined);
|
||||
|
||||
return driverErrorParts.length > 0
|
||||
? `${baseMessage} (${driverErrorParts.join(', ')})`
|
||||
: baseMessage;
|
||||
};
|
||||
|
||||
export const formatWorkspaceMigrationRunnerExecutionErrors = (
|
||||
errors: WorkspaceMigrationRunnerExecutionErrors,
|
||||
): string | undefined => {
|
||||
const entries = EXECUTION_ERROR_LABELS.map((label) => ({
|
||||
label,
|
||||
error: errors[label],
|
||||
})).filter((entry): entry is ExecutionErrorEntry => isDefined(entry.error));
|
||||
|
||||
if (entries.length === 0) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// A 25P02 failure is collateral noise from the statement that aborted the
|
||||
// transaction; hide it whenever a root-cause error is available.
|
||||
const rootCauseEntries = entries.filter(
|
||||
(entry) => !isTransactionAbortedError(entry.error),
|
||||
);
|
||||
const relevantEntries =
|
||||
rootCauseEntries.length > 0 ? rootCauseEntries : entries;
|
||||
|
||||
const summary = relevantEntries
|
||||
.map(
|
||||
(entry) => `[${entry.label}] ${formatSingleExecutionError(entry.error)}`,
|
||||
)
|
||||
.join('; ');
|
||||
|
||||
return summary.length <= MAX_EXECUTION_ERRORS_SUMMARY_LENGTH
|
||||
? summary
|
||||
: `${summary.slice(
|
||||
0,
|
||||
MAX_EXECUTION_ERRORS_SUMMARY_LENGTH - TRUNCATION_MARKER.length,
|
||||
)}${TRUNCATION_MARKER}`;
|
||||
};
|
||||
Reference in New Issue
Block a user