From 69807a5efbf197fb3db7130a5379aae44eda7216 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 25 Mar 2024 17:52:07 +0100 Subject: [PATCH] refactor(core): Unify `failed` and `error` execution status (no-changelog) (#8943) --- packages/cli/src/WorkflowExecuteAdditionalData.ts | 2 +- packages/cli/src/WorkflowHelpers.ts | 2 +- .../1711018413374-RemoveFailedExecutionStatus.ts | 9 +++++++++ packages/cli/src/databases/migrations/mysqldb/index.ts | 2 ++ .../cli/src/databases/migrations/postgresdb/index.ts | 2 ++ packages/cli/src/databases/migrations/sqlite/index.ts | 2 ++ .../src/databases/repositories/execution.repository.ts | 4 ++-- .../cli/src/eventbus/executionDataRecovery.service.ts | 2 +- .../shared/sharedHookFunctions.ts | 4 ++-- packages/cli/src/executions/executionHelpers.ts | 2 +- packages/cli/test/integration/pruning.service.test.ts | 4 ++-- packages/cli/test/integration/shared/db/executions.ts | 2 +- packages/core/src/NodeExecuteFunctions.ts | 1 - packages/editor-ui/src/components/ExecutionsList.vue | 10 +++++----- .../ExecutionsView/__tests__/ExecutionPreview.test.ts | 2 +- packages/editor-ui/src/components/Node.vue | 5 +---- .../src/components/__tests__/ExecutionsList.test.ts | 2 +- packages/editor-ui/src/mixins/executionsHelpers.ts | 2 +- packages/editor-ui/src/utils/executionUtils.ts | 2 +- packages/workflow/src/ExecutionStatus.ts | 1 - 20 files changed, 36 insertions(+), 26 deletions(-) create mode 100644 packages/cli/src/databases/migrations/common/1711018413374-RemoveFailedExecutionStatus.ts diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 091c663019..5b38259dce 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -865,7 +865,7 @@ async function executeWorkflow( mode: 'integrated', startedAt: new Date(), stoppedAt: new Date(), - status: 'failed', + status: 'error', }; // When failing, we might not have finished the execution // Therefore, database might not contain finished errors. diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index f95498fb9d..2354c6a4d3 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -61,7 +61,7 @@ export function generateFailedExecutionFromError( mode, startedAt: new Date(), stoppedAt: new Date(), - status: 'failed', + status: 'error', }; } diff --git a/packages/cli/src/databases/migrations/common/1711018413374-RemoveFailedExecutionStatus.ts b/packages/cli/src/databases/migrations/common/1711018413374-RemoveFailedExecutionStatus.ts new file mode 100644 index 0000000000..2f48fdbc1f --- /dev/null +++ b/packages/cli/src/databases/migrations/common/1711018413374-RemoveFailedExecutionStatus.ts @@ -0,0 +1,9 @@ +import type { IrreversibleMigration, MigrationContext } from '@db/types'; + +export class RemoveFailedExecutionStatus1711018413374 implements IrreversibleMigration { + async up({ escape, runQuery }: MigrationContext) { + const executionEntity = escape.tableName('execution_entity'); + + await runQuery(`UPDATE ${executionEntity} SET status = 'error' WHERE status = 'failed';`); + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index 8ce92158cf..b8878cab43 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -52,6 +52,7 @@ import { AddWorkflowMetadata1695128658538 } from '../common/1695128658538-AddWor import { ModifyWorkflowHistoryNodesAndConnections1695829275184 } from '../common/1695829275184-ModifyWorkflowHistoryNodesAndConnections'; import { AddGlobalAdminRole1700571993961 } from '../common/1700571993961-AddGlobalAdminRole'; import { DropRoleMapping1705429061930 } from '../common/1705429061930-DropRoleMapping'; +import { RemoveFailedExecutionStatus1711018413374 } from '../common/1711018413374-RemoveFailedExecutionStatus'; export const mysqlMigrations: Migration[] = [ InitialMigration1588157391238, @@ -107,4 +108,5 @@ export const mysqlMigrations: Migration[] = [ ModifyWorkflowHistoryNodesAndConnections1695829275184, AddGlobalAdminRole1700571993961, DropRoleMapping1705429061930, + RemoveFailedExecutionStatus1711018413374, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index 8ae3733976..de177ebbc7 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -51,6 +51,7 @@ import { MigrateToTimestampTz1694091729095 } from './1694091729095-MigrateToTime import { ModifyWorkflowHistoryNodesAndConnections1695829275184 } from '../common/1695829275184-ModifyWorkflowHistoryNodesAndConnections'; import { AddGlobalAdminRole1700571993961 } from '../common/1700571993961-AddGlobalAdminRole'; import { DropRoleMapping1705429061930 } from '../common/1705429061930-DropRoleMapping'; +import { RemoveFailedExecutionStatus1711018413374 } from '../common/1711018413374-RemoveFailedExecutionStatus'; export const postgresMigrations: Migration[] = [ InitialMigration1587669153312, @@ -105,4 +106,5 @@ export const postgresMigrations: Migration[] = [ ModifyWorkflowHistoryNodesAndConnections1695829275184, AddGlobalAdminRole1700571993961, DropRoleMapping1705429061930, + RemoveFailedExecutionStatus1711018413374, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index 7db45788ac..2e0bfca4a7 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -49,6 +49,7 @@ import { AddWorkflowMetadata1695128658538 } from '../common/1695128658538-AddWor import { ModifyWorkflowHistoryNodesAndConnections1695829275184 } from '../common/1695829275184-ModifyWorkflowHistoryNodesAndConnections'; import { AddGlobalAdminRole1700571993961 } from '../common/1700571993961-AddGlobalAdminRole'; import { DropRoleMapping1705429061930 } from './1705429061930-DropRoleMapping'; +import { RemoveFailedExecutionStatus1711018413374 } from '../common/1711018413374-RemoveFailedExecutionStatus'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422, @@ -101,6 +102,7 @@ const sqliteMigrations: Migration[] = [ ModifyWorkflowHistoryNodesAndConnections1695829275184, AddGlobalAdminRole1700571993961, DropRoleMapping1705429061930, + RemoveFailedExecutionStatus1711018413374, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index dd04ec673f..44b607fb9d 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -575,7 +575,7 @@ export class ExecutionRepository extends Repository { } else if (status === 'waiting') { condition.status = 'waiting'; } else if (status === 'error') { - condition.status = In(['error', 'crashed', 'failed']); + condition.status = In(['error', 'crashed']); } return condition; @@ -682,7 +682,7 @@ export class ExecutionRepository extends Repository { ) { const where: FindOptionsWhere = { id: In(activeExecutionIds), - status: Not(In(['finished', 'stopped', 'failed', 'crashed'] as ExecutionStatus[])), + status: Not(In(['finished', 'stopped', 'error', 'crashed'])), }; if (filter) { diff --git a/packages/cli/src/eventbus/executionDataRecovery.service.ts b/packages/cli/src/eventbus/executionDataRecovery.service.ts index 80251bd92f..8ebacd7885 100644 --- a/packages/cli/src/eventbus/executionDataRecovery.service.ts +++ b/packages/cli/src/eventbus/executionDataRecovery.service.ts @@ -155,7 +155,7 @@ export class ExecutionDataRecoveryService { } if (applyToDb) { - const newStatus = executionEntry.status === 'failed' ? 'failed' : 'crashed'; + const newStatus = executionEntry.status === 'error' ? 'error' : 'crashed'; await this.executionRepository.updateExistingExecution(executionId, { data: executionData, status: newStatus, diff --git a/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts b/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts index dc520d90b2..7a2e401919 100644 --- a/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts +++ b/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts @@ -10,13 +10,13 @@ import { Logger } from '@/Logger'; export function determineFinalExecutionStatus(runData: IRun): ExecutionStatus { const workflowHasCrashed = runData.status === 'crashed'; const workflowWasCanceled = runData.status === 'canceled'; - const workflowHasFailed = runData.status === 'failed'; + const workflowHasFailed = runData.status === 'error'; const workflowDidSucceed = !runData.data.resultData?.error && !workflowHasCrashed && !workflowWasCanceled && !workflowHasFailed; - let workflowStatusFinal: ExecutionStatus = workflowDidSucceed ? 'success' : 'failed'; + let workflowStatusFinal: ExecutionStatus = workflowDidSucceed ? 'success' : 'error'; if (workflowHasCrashed) workflowStatusFinal = 'crashed'; if (workflowWasCanceled) workflowStatusFinal = 'canceled'; if (runData.waitTill) workflowStatusFinal = 'waiting'; diff --git a/packages/cli/src/executions/executionHelpers.ts b/packages/cli/src/executions/executionHelpers.ts index ff57f10682..f58a07761e 100644 --- a/packages/cli/src/executions/executionHelpers.ts +++ b/packages/cli/src/executions/executionHelpers.ts @@ -13,7 +13,7 @@ export function getStatusUsingPreviousExecutionStatusMethod( } else if (execution.finished) { return 'success'; } else if (execution.stoppedAt !== null) { - return 'failed'; + return 'error'; } else { return 'unknown'; } diff --git a/packages/cli/test/integration/pruning.service.test.ts b/packages/cli/test/integration/pruning.service.test.ts index 59b601eb3a..b8eb07dba1 100644 --- a/packages/cli/test/integration/pruning.service.test.ts +++ b/packages/cli/test/integration/pruning.service.test.ts @@ -99,7 +99,7 @@ describe('softDeleteOnPruningCycle()', () => { ['unknown', { startedAt: now, stoppedAt: now }], ['canceled', { startedAt: now, stoppedAt: now }], ['crashed', { startedAt: now, stoppedAt: now }], - ['failed', { startedAt: now, stoppedAt: now }], + ['error', { startedAt: now, stoppedAt: now }], ['success', { finished: true, startedAt: now, stoppedAt: now }], ])('should prune %s executions', async (status, attributes) => { const executions = [ @@ -192,7 +192,7 @@ describe('softDeleteOnPruningCycle()', () => { ['unknown', { startedAt: yesterday, stoppedAt: yesterday }], ['canceled', { startedAt: yesterday, stoppedAt: yesterday }], ['crashed', { startedAt: yesterday, stoppedAt: yesterday }], - ['failed', { startedAt: yesterday, stoppedAt: yesterday }], + ['error', { startedAt: yesterday, stoppedAt: yesterday }], ['success', { finished: true, startedAt: yesterday, stoppedAt: yesterday }], ])('should prune %s executions', async (status, attributes) => { const execution = await createExecution({ status, ...attributes }, workflow); diff --git a/packages/cli/test/integration/shared/db/executions.ts b/packages/cli/test/integration/shared/db/executions.ts index f5c6e5ddae..7e791a08fe 100644 --- a/packages/cli/test/integration/shared/db/executions.ts +++ b/packages/cli/test/integration/shared/db/executions.ts @@ -55,7 +55,7 @@ export async function createSuccessfulExecution(workflow: WorkflowEntity) { */ export async function createErrorExecution(workflow: WorkflowEntity) { return await createExecution( - { finished: false, stoppedAt: new Date(), status: 'failed' }, + { finished: false, stoppedAt: new Date(), status: 'error' }, workflow, ); } diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index cecabed239..b41fa23671 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -2530,7 +2530,6 @@ const addExecutionDataFunctions = async ( taskData = taskData!; if (data instanceof Error) { - // TODO: Or "failed", what is the difference taskData.executionStatus = 'error'; taskData.error = data; } else { diff --git a/packages/editor-ui/src/components/ExecutionsList.vue b/packages/editor-ui/src/components/ExecutionsList.vue index 189e879e11..682f0d98f6 100644 --- a/packages/editor-ui/src/components/ExecutionsList.vue +++ b/packages/editor-ui/src/components/ExecutionsList.vue @@ -799,7 +799,7 @@ export default defineComponent({ } else if (execution.finished) { status = 'success'; } else if (execution.stoppedAt !== null) { - status = 'failed'; + status = 'error'; } else { status = 'unknown'; } @@ -825,7 +825,7 @@ export default defineComponent({ text = this.i18n.baseText('executionsList.running'); } else if (status === 'success') { text = this.i18n.baseText('executionsList.succeeded'); - } else if (status === 'failed') { + } else if (status === 'error') { text = this.i18n.baseText('executionsList.error'); } else { text = this.i18n.baseText('executionsList.unknown'); @@ -841,7 +841,7 @@ export default defineComponent({ path = 'executionsList.statusWaiting'; } else if (status === 'canceled') { path = 'executionsList.statusCanceled'; - } else if (['crashed', 'failed', 'success'].includes(status)) { + } else if (['crashed', 'error', 'success'].includes(status)) { if (!entry.stoppedAt) { path = 'executionsList.statusTextWithoutTime'; } else { @@ -1032,7 +1032,7 @@ export default defineComponent({ font-weight: var(--font-weight-bold); .crashed &, - .failed & { + .error & { color: var(--color-danger); } @@ -1143,7 +1143,7 @@ export default defineComponent({ } &.crashed td:first-child::before, - &.failed td:first-child::before { + &.error td:first-child::before { background: var(--execution-card-border-error); } diff --git a/packages/editor-ui/src/components/ExecutionsView/__tests__/ExecutionPreview.test.ts b/packages/editor-ui/src/components/ExecutionsView/__tests__/ExecutionPreview.test.ts index 089a094144..9df897f7fd 100644 --- a/packages/editor-ui/src/components/ExecutionsView/__tests__/ExecutionPreview.test.ts +++ b/packages/editor-ui/src/components/ExecutionsView/__tests__/ExecutionPreview.test.ts @@ -56,7 +56,7 @@ const executionDataFactory = (): ExecutionSummary => ({ stoppedAt: faker.date.past(), workflowId: faker.number.int().toString(), workflowName: faker.string.sample(), - status: faker.helpers.arrayElement(['failed', 'success']), + status: faker.helpers.arrayElement(['error', 'success']), nodeExecutionStatus: {}, retryOf: generateUndefinedNullOrString(), retrySuccessId: generateUndefinedNullOrString(), diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index a4e1b56e0f..28ded3cbd2 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -285,10 +285,7 @@ export default defineComponent({ return this.workflowsStore.getWorkflowResultDataByNodeName(this.data?.name || '') || []; }, hasIssues(): boolean { - if ( - this.nodeExecutionStatus && - ['crashed', 'error', 'failed'].includes(this.nodeExecutionStatus) - ) + if (this.nodeExecutionStatus && ['crashed', 'error'].includes(this.nodeExecutionStatus)) return true; if (this.pinnedData.hasData.value) return false; if (this.data?.issues !== undefined && Object.keys(this.data.issues).length) { diff --git a/packages/editor-ui/src/components/__tests__/ExecutionsList.test.ts b/packages/editor-ui/src/components/__tests__/ExecutionsList.test.ts index c3b24d4ed4..7e17fd1b0f 100644 --- a/packages/editor-ui/src/components/__tests__/ExecutionsList.test.ts +++ b/packages/editor-ui/src/components/__tests__/ExecutionsList.test.ts @@ -56,7 +56,7 @@ const executionDataFactory = (): ExecutionSummary => ({ stoppedAt: faker.date.past(), workflowId: faker.number.int().toString(), workflowName: faker.string.sample(), - status: faker.helpers.arrayElement(['failed', 'success']), + status: faker.helpers.arrayElement(['error', 'success']), nodeExecutionStatus: {}, retryOf: generateUndefinedNullOrString(), retrySuccessId: generateUndefinedNullOrString(), diff --git a/packages/editor-ui/src/mixins/executionsHelpers.ts b/packages/editor-ui/src/mixins/executionsHelpers.ts index c429bc1392..507b625b9f 100644 --- a/packages/editor-ui/src/mixins/executionsHelpers.ts +++ b/packages/editor-ui/src/mixins/executionsHelpers.ts @@ -51,7 +51,7 @@ export const executionHelpers = defineComponent({ } else if (execution.status === 'success') { status.name = 'success'; status.label = this.$locale.baseText('executionsList.succeeded'); - } else if (execution.status === 'failed' || execution.status === 'crashed') { + } else if (execution.status === 'error' || execution.status === 'crashed') { status.name = 'error'; status.label = this.$locale.baseText('executionsList.error'); } diff --git a/packages/editor-ui/src/utils/executionUtils.ts b/packages/editor-ui/src/utils/executionUtils.ts index 37db852f5b..6a504b4fcb 100644 --- a/packages/editor-ui/src/utils/executionUtils.ts +++ b/packages/editor-ui/src/utils/executionUtils.ts @@ -31,7 +31,7 @@ export const executionFilterToQueryFilter = ( queryFilter.status = ['waiting']; break; case 'error': - queryFilter.status = ['failed', 'crashed', 'error']; + queryFilter.status = ['crashed', 'error']; break; case 'success': queryFilter.status = ['success']; diff --git a/packages/workflow/src/ExecutionStatus.ts b/packages/workflow/src/ExecutionStatus.ts index fb07e30189..3ec2f385d3 100644 --- a/packages/workflow/src/ExecutionStatus.ts +++ b/packages/workflow/src/ExecutionStatus.ts @@ -2,7 +2,6 @@ export type ExecutionStatus = | 'canceled' | 'crashed' | 'error' - | 'failed' | 'new' | 'running' | 'success'