fix(core): Sync hookFunctionsSave and hookFunctionsSaveWorker (#1…

…2740)
n8n-io · Jan 21, 2025 · d410b8f · d410b8f
1 parent d1b6692
commit d410b8f
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 9 deletions.
diff --git a/packages/cli/src/__tests__/execution-lifecycle-hooks.test.ts b/packages/cli/src/__tests__/execution-lifecycle-hooks.test.ts
@@ -26,6 +26,7 @@ import { mockInstance } from '@test/mocking';
 
 import {
 	getWorkflowHooksMain,
+	getWorkflowHooksWorkerExecuter,
 	getWorkflowHooksWorkerMain,
 } from '../workflow-execute-additional-data';
 
@@ -532,4 +533,85 @@ describe('Execution Lifecycle Hooks', () => {
 			});
 		});
 	});
+
+	describe('getWorkflowHooksWorkerExecuter', () => {
+		let hooks: WorkflowHooks;
+
+		beforeEach(() => {
+			hooks = getWorkflowHooksWorkerExecuter(executionMode, executionId, workflowData, {
+				pushRef,
+				retryOf,
+			});
+		});
+
+		describe('saving static data', () => {
+			it('should skip saving static data for manual executions', async () => {
+				hooks.mode = 'manual';
+
+				await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, staticData]);
+
+				expect(workflowStaticDataService.saveStaticDataById).not.toHaveBeenCalled();
+			});
+
+			it('should save static data for prod executions', async () => {
+				hooks.mode = 'trigger';
+
+				await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, staticData]);
+
+				expect(workflowStaticDataService.saveStaticDataById).toHaveBeenCalledWith(
+					workflowId,
+					staticData,
+				);
+			});
+
+			it('should handle static data saving errors', async () => {
+				hooks.mode = 'trigger';
+				const error = new Error('Static data save failed');
+				workflowStaticDataService.saveStaticDataById.mockRejectedValueOnce(error);
+
+				await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, staticData]);
+
+				expect(errorReporter.error).toHaveBeenCalledWith(error);
+			});
+		});
+
+		describe('error workflow', () => {
+			it('should not execute error workflow for manual executions', async () => {
+				hooks.mode = 'manual';
+
+				await hooks.executeHookFunctions('workflowExecuteAfter', [failedRun, {}]);
+
+				expect(workflowExecutionService.executeErrorWorkflow).not.toHaveBeenCalled();
+			});
+
+			it('should execute error workflow for failed non-manual executions', async () => {
+				hooks.mode = 'trigger';
+				const errorWorkflow = 'error-workflow-id';
+				workflowData.settings = { errorWorkflow };
+				const project = mock<Project>();
+				ownershipService.getWorkflowProjectCached.calledWith(workflowId).mockResolvedValue(project);
+
+				await hooks.executeHookFunctions('workflowExecuteAfter', [failedRun, {}]);
+
+				expect(workflowExecutionService.executeErrorWorkflow).toHaveBeenCalledWith(
+					errorWorkflow,
+					{
+						workflow: {
+							id: workflowId,
+							name: workflowData.name,
+						},
+						execution: {
+							id: executionId,
+							error: expressionError,
+							mode: 'trigger',
+							retryOf,
+							lastNodeExecuted: undefined,
+							url: `http://localhost:5678/workflow/${workflowId}/executions/${executionId}`,
+						},
+					},
+					project,
+				);
+			});
+		});
+	});
 });
diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts
@@ -576,8 +576,11 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks {
 					executionId: this.executionId,
 					workflowId: this.workflowData.id,
 				});
+
+				const isManualMode = this.mode === 'manual';
+
 				try {
-					if (isWorkflowIdValid(this.workflowData.id) && newStaticData) {
+					if (!isManualMode && isWorkflowIdValid(this.workflowData.id) && newStaticData) {
 						// Workflow is saved so update in database
 						try {
 							await Container.get(WorkflowStaticDataService).saveStaticDataById(
@@ -596,7 +599,11 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks {
 					const workflowStatusFinal = determineFinalExecutionStatus(fullRunData);
 					fullRunData.status = workflowStatusFinal;
 
-					if (workflowStatusFinal !== 'success' && workflowStatusFinal !== 'waiting') {
+					if (
+						!isManualMode &&
+						workflowStatusFinal !== 'success' &&
+						workflowStatusFinal !== 'waiting'
+					) {
 						executeErrorWorkflow(
 							this.workflowData,
 							fullRunData,
@@ -615,19 +622,25 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks {
 						retryOf: this.retryOf,
 					});
 
+					// When going into the waiting state, store the pushRef in the execution-data
+					if (fullRunData.waitTill && isManualMode) {
+						fullExecutionData.data.pushRef = this.pushRef;
+					}
+
 					await updateExistingExecution({
 						executionId: this.executionId,
 						workflowId: this.workflowData.id,
 						executionData: fullExecutionData,
 					});
 				} catch (error) {
-					executeErrorWorkflow(
-						this.workflowData,
-						fullRunData,
-						this.mode,
-						this.executionId,
-						this.retryOf,
-					);
+					if (!isManualMode)
+						executeErrorWorkflow(
+							this.workflowData,
+							fullRunData,
+							this.mode,
+							this.executionId,
+							this.retryOf,
+						);
 				} finally {
 					workflowStatisticsService.emit('workflowExecutionCompleted', {
 						workflowData: this.workflowData,