From 66a345ea940552ec4106aff1d4d5707adcbffa72 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Wed, 3 Mar 2021 08:31:55 +0100 Subject: [PATCH] :bug: Fix issue with auto refresh on execution list (#1475) * Fix issue with auto refresh on execution list When auto refresh is on, we used to get executions from backend using the firstId field to filter recent executions. This is a problem when you have executions that do not finish in order, leaving gaps behind. This PR fixes this problem by refreshing the latest 30 executions and correctly adding them to the list. * Fixed an issues with auto refresh on executions ExecutionsList Fixed two bugs, one in frontend which was ignoring the first returned row from the backend and an issue with backend that was not using the overriden version of `executeWorkflow` function for sub sub workflows. * Fixed the display of manual executions when running with queues and improved display of subworkflows * Changing workflow ids array from variable to constant * Added unknown status to workflow execution and changed its color to orange --- packages/cli/src/Server.ts | 15 +++-- .../cli/src/WorkflowExecuteAdditionalData.ts | 4 ++ .../src/components/ExecutionsList.vue | 62 ++++++++++++++++--- .../editor-ui/src/n8n-theme-variables.scss | 2 + 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index bf4a4e6da2..f435e58eab 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -1437,14 +1437,14 @@ class App { limit = parseInt(req.query.limit as string, 10); } - let executingWorkflowIds; + const executingWorkflowIds: string[] = []; if (config.get('executions.mode') === 'queue') { const currentJobs = await Queue.getInstance().getJobs(['active', 'waiting']); - executingWorkflowIds = currentJobs.map(job => job.data.executionId) as string[]; - } else { - executingWorkflowIds = this.activeExecutionsInstance.getActiveExecutions().map(execution => execution.id.toString()) as string[]; + executingWorkflowIds.push(...currentJobs.map(job => job.data.executionId) as string[]); } + // We may have manual executions even with queue so we must account for these. + executingWorkflowIds.push(...this.activeExecutionsInstance.getActiveExecutions().map(execution => execution.id.toString()) as string[]); const countFilter = JSON.parse(JSON.stringify(filter)); countFilter.select = ['id']; @@ -1645,7 +1645,12 @@ class App { if (config.get('executions.mode') === 'queue') { const currentJobs = await Queue.getInstance().getJobs(['active', 'waiting']); - const currentlyRunningExecutionIds = currentJobs.map(job => job.data.executionId); + const currentlyRunningQueueIds = currentJobs.map(job => job.data.executionId); + + const currentlyRunningManualExecutions = this.activeExecutionsInstance.getActiveExecutions(); + const manualExecutionIds = currentlyRunningManualExecutions.map(execution => execution.id); + + const currentlyRunningExecutionIds = currentlyRunningQueueIds.concat(manualExecutionIds); if (currentlyRunningExecutionIds.length === 0) { return []; diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 3e60163466..d1ef25985e 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -520,6 +520,10 @@ export async function executeWorkflow(workflowInfo: IExecuteWorkflowInfo, additi // different webooks const additionalDataIntegrated = await getBase(credentials); additionalDataIntegrated.hooks = getWorkflowHooksIntegrated(runData.executionMode, executionId, workflowData!, { parentProcessMode: additionalData.hooks!.mode }); + // Make sure we pass on the original executeWorkflow function we received + // This one already contains changes to talk to parent process + // and get executionID from `activeExecutions` running on main process + additionalDataIntegrated.executeWorkflow = additionalData.executeWorkflow; // Execute the workflow diff --git a/packages/editor-ui/src/components/ExecutionsList.vue b/packages/editor-ui/src/components/ExecutionsList.vue index da18f3f208..051f85513c 100644 --- a/packages/editor-ui/src/components/ExecutionsList.vue +++ b/packages/editor-ui/src/components/ExecutionsList.vue @@ -88,14 +88,17 @@ Success - + Error + + Unknown + - + @@ -410,12 +413,13 @@ export default mixins( this.$store.commit('setActiveExecutions', activeExecutions); }, async loadAutoRefresh () : Promise { - let firstId: string | number | undefined = 0; - if (this.finishedExecutions.length !== 0) { - firstId = this.finishedExecutions[0].id; - } const filter = this.workflowFilterPast; - const pastExecutionsPromise: Promise = this.restApi().getPastExecutions(filter, this.requestItemsPerRequest, undefined, firstId); + // We cannot use firstId here as some executions finish out of order. Let's say + // You have execution ids 500 to 505 running. + // Suppose 504 finishes before 500, 501, 502 and 503. + // iF you use firstId, filtering id >= 504 you won't + // ever get ids 500, 501, 502 and 503 when they finish + const pastExecutionsPromise: Promise = this.restApi().getPastExecutions(filter, 30); const currentExecutionsPromise: Promise = this.restApi().getCurrentExecutions({}); const results = await Promise.all([pastExecutionsPromise, currentExecutionsPromise]); @@ -428,7 +432,38 @@ export default mixins( this.$store.commit('setActiveExecutions', results[1]); - this.finishedExecutions.unshift.apply(this.finishedExecutions, results[0].results); + const alreadyPresentExecutionIds = this.finishedExecutions.map(exec => exec.id); + for(let i = results[0].results.length - 1; i >= 0; i--) { + const currentItem = results[0].results[i]; + // Check new results from end to start + // Add new items accordingly. + const executionIndex = alreadyPresentExecutionIds.indexOf(currentItem.id); + if (executionIndex !== -1) { + // Execution that we received is already present. + + if (this.finishedExecutions[executionIndex].finished === false && currentItem.finished === true) { + // Concurrency stuff. This might happen if the execution finishes + // prior to saving all information to database. Somewhat rare but + // With auto refresh and several executions, it happens sometimes. + // So we replace the execution data so it displays correctly. + this.finishedExecutions[executionIndex] = currentItem; + } + + continue; + } + + // Find the correct position to place this newcomer + let j; + for (j = this.finishedExecutions.length - 1; j >= 0; j--) { + if (currentItem.id < this.finishedExecutions[j].id) { + this.finishedExecutions.splice(j + 1, 0, currentItem); + break; + } + } + if (j === -1) { + this.finishedExecutions.unshift(currentItem); + } + } this.finishedExecutionsCount = results[0].count; }, async loadFinishedExecutions (): Promise { @@ -554,6 +589,8 @@ export default mixins( return `The workflow execution was a retry of "${entry.retryOf}" and failed.
New retries have to be started from the original execution.`; } else if (entry.retrySuccessId !== undefined) { return `The workflow execution failed but the retry "${entry.retrySuccessId}" was successful.`; + } else if (entry.stoppedAt === null) { + return 'The workflow execution is probably still running but it may have crashed and n8n cannot safely tell. '; } else { return 'The workflow execution failed.'; } @@ -610,6 +647,10 @@ export default mixins( color: $--custom-error-text; background-color: $--custom-error-background; margin-left: 5px; + &.warning { + background-color: $--custom-warning-background; + color: $--custom-warning-text; + } } .selection-options { @@ -640,6 +681,11 @@ export default mixins( background-color: $--custom-success-background; color: $--custom-success-text; } + + &.warning { + background-color: $--custom-warning-background; + color: $--custom-warning-text; + } } .workflow-name { diff --git a/packages/editor-ui/src/n8n-theme-variables.scss b/packages/editor-ui/src/n8n-theme-variables.scss index f0520c5bf1..6561ffe922 100644 --- a/packages/editor-ui/src/n8n-theme-variables.scss +++ b/packages/editor-ui/src/n8n-theme-variables.scss @@ -22,6 +22,8 @@ $--custom-running-background : #ffffe5; $--custom-running-text : #eb9422; $--custom-success-background : #e3f0e4; $--custom-success-text : #40c351; +$--custom-warning-background : #ffffe5; +$--custom-warning-text : #eb9422; $--custom-node-view-background : #faf9fe;