consolidate all external hook calls in execution lifecycle hooks

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);
 		});

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,

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

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
+			postExecutePromise.catch((error) => {
-				.then(async (executionData) => {
+				if (error instanceof ExecutionCancelledError) return;
-					try {
+				this.errorReporter.error(error, {
-						await this.externalHooks.run('workflow.postExecute', [
+					extra: { executionId, workflowId },
-							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,
-					);
 				});
+				this.logger.error('There was an error in the post-execution promise', {
+					error,
+					executionId,
+					workflowId,
+				});
+			});
 		}
 
 		return executionId;