diff --git a/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts b/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts index 46f27d3541..fc04fbaa9e 100644 --- a/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts +++ b/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts @@ -159,6 +159,31 @@ describe('Execution Lifecycle Hooks', () => { }); }; + const externalHooksTests = () => { + describe('workflowExecuteBefore', () => { + it('should run workflow.preExecute hook', async () => { + await hooks.executeHookFunctions('workflowExecuteBefore', [workflow, runExecutionData]); + + expect(externalHooks.run).toHaveBeenCalledWith('workflow.preExecute', [ + workflow, + executionMode, + ]); + }); + }); + + describe('workflowExecuteAfter', () => { + it('should run workflow.postExecute hook', async () => { + await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, {}]); + + expect(externalHooks.run).toHaveBeenCalledWith('workflow.postExecute', [ + successfulRun, + workflowData, + executionId, + ]); + }); + }); + }; + describe('getWorkflowHooksMain', () => { beforeEach(() => { hooks = getWorkflowHooksMain( @@ -174,6 +199,7 @@ describe('Execution Lifecycle Hooks', () => { workflowEventTests(); nodeEventsTests(); + externalHooksTests(); it('should setup the correct set of hooks', () => { expect(hooks).toBeInstanceOf(WorkflowHooks); @@ -187,7 +213,7 @@ describe('Execution Lifecycle Hooks', () => { expect(hookFunctions.nodeExecuteBefore).toHaveLength(2); expect(hookFunctions.nodeExecuteAfter).toHaveLength(3); expect(hookFunctions.workflowExecuteBefore).toHaveLength(3); - expect(hookFunctions.workflowExecuteAfter).toHaveLength(4); + expect(hookFunctions.workflowExecuteAfter).toHaveLength(5); expect(hookFunctions.nodeFetchedData).toHaveLength(1); expect(hookFunctions.sendResponse).toHaveLength(0); }); @@ -507,6 +533,7 @@ describe('Execution Lifecycle Hooks', () => { }); workflowEventTests(); + externalHooksTests(); it('should setup the correct set of hooks', () => { expect(hooks).toBeInstanceOf(WorkflowHooks); @@ -520,7 +547,7 @@ describe('Execution Lifecycle Hooks', () => { expect(hookFunctions.nodeExecuteBefore).toHaveLength(0); expect(hookFunctions.nodeExecuteAfter).toHaveLength(0); expect(hookFunctions.workflowExecuteBefore).toHaveLength(2); - expect(hookFunctions.workflowExecuteAfter).toHaveLength(3); + expect(hookFunctions.workflowExecuteAfter).toHaveLength(4); expect(hookFunctions.nodeFetchedData).toHaveLength(0); expect(hookFunctions.sendResponse).toHaveLength(0); }); @@ -584,6 +611,7 @@ describe('Execution Lifecycle Hooks', () => { }); nodeEventsTests(); + externalHooksTests(); it('should setup the correct set of hooks', () => { expect(hooks).toBeInstanceOf(WorkflowHooks); @@ -680,6 +708,7 @@ describe('Execution Lifecycle Hooks', () => { workflowEventTests(); nodeEventsTests(); + externalHooksTests(); it('should setup the correct set of hooks', () => { expect(hooks).toBeInstanceOf(WorkflowHooks); @@ -693,7 +722,7 @@ describe('Execution Lifecycle Hooks', () => { expect(hookFunctions.nodeExecuteBefore).toHaveLength(1); expect(hookFunctions.nodeExecuteAfter).toHaveLength(2); expect(hookFunctions.workflowExecuteBefore).toHaveLength(2); - expect(hookFunctions.workflowExecuteAfter).toHaveLength(3); + expect(hookFunctions.workflowExecuteAfter).toHaveLength(4); expect(hookFunctions.nodeFetchedData).toHaveLength(1); expect(hookFunctions.sendResponse).toHaveLength(0); }); diff --git a/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts b/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts index 665e4e2732..098e6fa916 100644 --- a/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts +++ b/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts @@ -192,7 +192,7 @@ function hookFunctionsPush(): IWorkflowExecuteHooks { }; } -function hookFunctionsPreExecute(): IWorkflowExecuteHooks { +function hookFunctionsExternalHooks(): IWorkflowExecuteHooks { const externalHooks = Container.get(ExternalHooks); return { workflowExecuteBefore: [ @@ -200,6 +200,20 @@ function hookFunctionsPreExecute(): IWorkflowExecuteHooks { await externalHooks.run('workflow.preExecute', [workflow, this.mode]); }, ], + workflowExecuteAfter: [ + async function (this: WorkflowHooks, fullRunData: IRun) { + await externalHooks.run('workflow.postExecute', [ + fullRunData, + this.workflowData, + this.executionId, + ]); + }, + ], + }; +} + +function hookFunctionsPreExecute(): IWorkflowExecuteHooks { + return { nodeExecuteAfter: [ async function ( this: WorkflowHooks, @@ -365,7 +379,6 @@ function hookFunctionsSave(): IWorkflowExecuteHooks { function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { const logger = Container.get(Logger); const errorReporter = Container.get(ErrorReporter); - const externalHooks = Container.get(ExternalHooks); const workflowStaticDataService = Container.get(WorkflowStaticDataService); const workflowStatisticsService = Container.get(WorkflowStatisticsService); return { @@ -440,15 +453,6 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { }); } }, - async function (this: WorkflowHooks, fullRunData: IRun) { - try { - await externalHooks.run('workflow.postExecute', [ - fullRunData, - this.workflowData, - this.executionId, - ]); - } catch {} - }, ], nodeFetchedData: [ async (workflowId: string, node: INode) => { @@ -474,6 +478,7 @@ export function getWorkflowHooksIntegrated( hookFunctionsFinalizeExecutionStatus(), hookFunctionsSave(), hookFunctionsPreExecute(), + hookFunctionsExternalHooks(), ); return new WorkflowHooks(hookFunctions, mode, executionId, workflowData); } @@ -492,6 +497,7 @@ export function getWorkflowHooksWorkerExecuter( hookFunctionsFinalizeExecutionStatus(), hookFunctionsSaveWorker(), hookFunctionsPreExecute(), + hookFunctionsExternalHooks(), ]; if (mode === 'manual' && Container.get(InstanceSettings).isWorker) { @@ -515,6 +521,7 @@ export function getWorkflowHooksWorkerMain( const hookFunctions = mergeHookFunctions( hookFunctionsWorkflowEvents(), hookFunctionsPreExecute(), + hookFunctionsExternalHooks(), hookFunctionsFinalizeExecutionStatus(), { workflowExecuteAfter: [ @@ -579,6 +586,7 @@ export function getWorkflowHooksMain( hookFunctionsSave(), hookFunctionsPush(), hookFunctionsPreExecute(), + hookFunctionsExternalHooks(), ); return new WorkflowHooks(hookFunctions, data.executionMode, executionId, data.workflowData, { pushRef: data.pushRef, diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index 98a736916a..85d1201ed0 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -38,7 +38,6 @@ import { WorkflowRepository } from '@/databases/repositories/workflow.repository import { EventService } from '@/events/event.service'; import type { AiEventMap, AiEventPayload } from '@/events/maps/ai.event-map'; import { getWorkflowHooksIntegrated } from '@/execution-lifecycle/execution-lifecycle-hooks'; -import { ExternalHooks } from '@/external-hooks'; import type { UpdateExecutionPayload } from '@/interfaces'; import { NodeTypes } from '@/node-types'; import { Push } from '@/push'; @@ -303,9 +302,6 @@ async function startExecution( ); } - const externalHooks = Container.get(ExternalHooks); - await externalHooks.run('workflow.postExecute', [data, workflowData, executionId]); - // subworkflow either finished, or is in status waiting due to a wait node, both cases are considered successes here if (data.finished === true || data.status === 'waiting') { // Workflow did finish successfully diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index da48255faa..915b34e93f 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -26,7 +26,6 @@ import { getWorkflowHooksWorkerExecuter, getWorkflowHooksWorkerMain, } from '@/execution-lifecycle/execution-lifecycle-hooks'; -import { ExternalHooks } from '@/external-hooks'; import { ManualExecutionService } from '@/manual-execution.service'; import { NodeTypes } from '@/node-types'; import type { ScalingService } from '@/scaling/scaling.service'; @@ -49,7 +48,6 @@ export class WorkflowRunner { private readonly errorReporter: ErrorReporter, private readonly activeExecutions: ActiveExecutions, private readonly executionRepository: ExecutionRepository, - private readonly externalHooks: ExternalHooks, private readonly workflowStaticDataService: WorkflowStaticDataService, private readonly nodeTypes: NodeTypes, private readonly permissionChecker: PermissionChecker, @@ -177,24 +175,17 @@ export class WorkflowRunner { data.executionMode === 'manual' ) { const postExecutePromise = this.activeExecutions.getPostExecutePromise(executionId); - postExecutePromise - .then(async (executionData) => { - try { - await this.externalHooks.run('workflow.postExecute', [ - executionData, - data.workflowData, - executionId, - ]); - } catch {} - }) - .catch((error) => { - if (error instanceof ExecutionCancelledError) return; - this.errorReporter.error(error); - this.logger.error( - 'There was a problem running internal hook "onWorkflowPostExecute"', - error, - ); + postExecutePromise.catch((error) => { + if (error instanceof ExecutionCancelledError) return; + this.errorReporter.error(error, { + extra: { executionId, workflowId }, }); + this.logger.error('There was an error in the post-execution promise', { + error, + executionId, + workflowId, + }); + }); } return executionId;