calculate saving settings only once for an execution

packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts

@@ -185,16 +185,11 @@ describe('Execution Lifecycle Hooks', () => {
 	};
 
 	describe('getWorkflowHooksMain', () => {
+		const createHooks = () =>
+			getWorkflowHooksMain({ executionMode, workflowData, pushRef, retryOf }, executionId);
+
 		beforeEach(() => {
-			hooks = getWorkflowHooksMain(
-				{
-					executionMode,
-					workflowData,
-					pushRef,
-					retryOf,
-				},
-				executionId,
-			);
+			hooks = createHooks();
 		});
 
 		workflowEventTests();
@@ -245,6 +240,7 @@ describe('Execution Lifecycle Hooks', () => {
 
 			it('should save execution progress when enabled', async () => {
 				workflowData.settings = { saveExecutionProgress: true };
+				hooks = createHooks();
 
 				await hooks.executeHookFunctions('nodeExecuteAfter', [
 					nodeName,
@@ -260,6 +256,7 @@ describe('Execution Lifecycle Hooks', () => {
 
 			it('should not save execution progress when disabled', async () => {
 				workflowData.settings = { saveExecutionProgress: false };
+				hooks = createHooks();
 
 				await hooks.executeHookFunctions('nodeExecuteAfter', [
 					nodeName,
@@ -391,6 +388,7 @@ describe('Execution Lifecycle Hooks', () => {
 
 				it('should soft delete manual executions when manual saving is disabled', async () => {
 					hooks.workflowData.settings = { saveManualExecutions: false };
+					hooks = createHooks();
 
 					await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, {}]);
 
@@ -399,6 +397,7 @@ describe('Execution Lifecycle Hooks', () => {
 
 				it('should not soft delete manual executions with waitTill', async () => {
 					hooks.workflowData.settings = { saveManualExecutions: false };
+					hooks = createHooks();
 
 					await hooks.executeHookFunctions('workflowExecuteAfter', [waitingRun, {}]);
 

packages/cli/src/execution-lifecycle/__tests__/save-execution-progress.test.ts

@@ -1,13 +1,12 @@
 import { ErrorReporter } from 'n8n-core';
 import { Logger } from 'n8n-core';
-import type { IRunExecutionData, ITaskData, IWorkflowBase } from 'n8n-workflow';
+import type { IRunExecutionData, ITaskData } from 'n8n-workflow';
 
 import { ExecutionRepository } from '@/databases/repositories/execution.repository';
 import type { IExecutionResponse } from '@/interfaces';
 import { mockInstance } from '@test/mocking';
 
 import { saveExecutionProgress } from '../save-execution-progress';
-import * as fnModule from '../to-save-settings';
 
 mockInstance(Logger);
 const errorReporter = mockInstance(ErrorReporter);
@@ -17,8 +16,8 @@ afterEach(() => {
 	jest.clearAllMocks();
 });
 
-const commonArgs: [IWorkflowBase, string, string, ITaskData, IRunExecutionData, string] = [
-	{} as IWorkflowBase,
+const commonArgs: [string, string, string, ITaskData, IRunExecutionData, string] = [
+	'some-workflow-id',
 	'some-execution-id',
 	'My Node',
 	{} as ITaskData,
@@ -29,40 +28,25 @@ const commonArgs: [IWorkflowBase, string, string, ITaskData, IRunExecutionData,
 const commonSettings = { error: true, success: true, manual: true };
 
 test('should ignore if save settings say so', async () => {
-	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
-		...commonSettings,
-		progress: false,
-	});
-
-	await saveExecutionProgress(...commonArgs);
+	await saveExecutionProgress({ ...commonSettings, progress: false }, ...commonArgs);
 
 	expect(executionRepository.updateExistingExecution).not.toHaveBeenCalled();
 });
 
 test('should ignore on leftover async call', async () => {
-	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
-		...commonSettings,
-		progress: true,
-	});
-
 	executionRepository.findSingleExecution.mockResolvedValue({
 		finished: true,
 	} as IExecutionResponse);
 
-	await saveExecutionProgress(...commonArgs);
+	await saveExecutionProgress({ ...commonSettings, progress: true }, ...commonArgs);
 
 	expect(executionRepository.updateExistingExecution).not.toHaveBeenCalled();
 });
 
 test('should update execution when saving progress is enabled', async () => {
-	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
-		...commonSettings,
-		progress: true,
-	});
-
 	executionRepository.findSingleExecution.mockResolvedValue({} as IExecutionResponse);
 
-	await saveExecutionProgress(...commonArgs);
+	await saveExecutionProgress({ ...commonSettings, progress: true }, ...commonArgs);
 
 	expect(executionRepository.updateExistingExecution).toHaveBeenCalledWith('some-execution-id', {
 		data: {
@@ -82,18 +66,13 @@ test('should update execution when saving progress is enabled', async () => {
 });
 
 test('should report error on failure', async () => {
-	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
-		...commonSettings,
-		progress: true,
-	});
-
 	const error = new Error('Something went wrong');
 
 	executionRepository.findSingleExecution.mockImplementation(() => {
 		throw error;
 	});
 
-	await saveExecutionProgress(...commonArgs);
+	await saveExecutionProgress({ ...commonSettings, progress: true }, ...commonArgs);
 
 	expect(executionRepository.updateExistingExecution).not.toHaveBeenCalled();
 	expect(errorReporter.error).toHaveBeenCalledWith(error);

packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts

@@ -32,7 +32,7 @@ import {
 	prepareExecutionDataForDbUpdate,
 	updateExistingExecution,
 } from './shared/shared-hook-functions';
-import { toSaveSettings } from './to-save-settings';
+import { type ExecutionSavingSettings, toSaveSettings } from './to-save-settings';
 
 function mergeHookFunctions(...hookFunctions: IWorkflowExecuteHooks[]): IWorkflowExecuteHooks {
 	const result: IWorkflowExecuteHooks = {
@@ -212,7 +212,7 @@ function hookFunctionsExternalHooks(): IWorkflowExecuteHooks {
 	};
 }
 
-function hookFunctionsSaveProgress(): IWorkflowExecuteHooks {
+function hookFunctionsSaveProgress(saveSettings: ExecutionSavingSettings): IWorkflowExecuteHooks {
 	return {
 		nodeExecuteAfter: [
 			async function (
@@ -222,7 +222,8 @@ function hookFunctionsSaveProgress(): IWorkflowExecuteHooks {
 				executionData: IRunExecutionData,
 			): Promise<void> {
 				await saveExecutionProgress(
-					this.workflowData,
+					saveSettings,
+					this.workflowData.id,
 					this.executionId,
 					nodeName,
 					data,
@@ -248,7 +249,7 @@ function hookFunctionsFinalizeExecutionStatus(): IWorkflowExecuteHooks {
 /**
  * Returns hook functions to save workflow execution and call error workflow
  */
-function hookFunctionsSave(): IWorkflowExecuteHooks {
+function hookFunctionsSave(saveSettings: ExecutionSavingSettings): IWorkflowExecuteHooks {
 	const logger = Container.get(Logger);
 	const errorReporter = Container.get(ErrorReporter);
 	const executionRepository = Container.get(ExecutionRepository);
@@ -288,8 +289,6 @@ function hookFunctionsSave(): IWorkflowExecuteHooks {
 						}
 					}
 
-					const saveSettings = toSaveSettings(this.workflowData.settings);
-
 					if (isManualMode && !saveSettings.manual && !fullRunData.waitTill) {
 						/**
 						 * When manual executions are not being saved, we only soft-delete
@@ -472,12 +471,13 @@ export function getWorkflowHooksIntegrated(
 	workflowData: IWorkflowBase,
 	userId?: string,
 ): WorkflowHooks {
+	const saveSettings = toSaveSettings(workflowData.settings);
 	const hookFunctions = mergeHookFunctions(
 		hookFunctionsWorkflowEvents(userId),
 		hookFunctionsNodeEvents(),
 		hookFunctionsFinalizeExecutionStatus(),
-		hookFunctionsSave(),
-		hookFunctionsSaveProgress(),
+		hookFunctionsSave(saveSettings),
+		hookFunctionsSaveProgress(saveSettings),
 		hookFunctionsExternalHooks(),
 	);
 	return new WorkflowHooks(hookFunctions, mode, executionId, workflowData);
@@ -492,11 +492,12 @@ export function getWorkflowHooksWorkerExecuter(
 	workflowData: IWorkflowBase,
 	optionalParameters: IWorkflowHooksOptionalParameters = {},
 ): WorkflowHooks {
+	const saveSettings = toSaveSettings(workflowData.settings);
 	const toMerge = [
 		hookFunctionsNodeEvents(),
 		hookFunctionsFinalizeExecutionStatus(),
 		hookFunctionsSaveWorker(),
-		hookFunctionsSaveProgress(),
+		hookFunctionsSaveProgress(saveSettings),
 		hookFunctionsExternalHooks(),
 	];
 
@@ -517,10 +518,11 @@ export function getWorkflowHooksWorkerMain(
 	workflowData: IWorkflowBase,
 	optionalParameters: IWorkflowHooksOptionalParameters = {},
 ): WorkflowHooks {
+	const saveSettings = toSaveSettings(workflowData.settings);
 	const executionRepository = Container.get(ExecutionRepository);
 	const hookFunctions = mergeHookFunctions(
 		hookFunctionsWorkflowEvents(),
-		hookFunctionsSaveProgress(),
+		hookFunctionsSaveProgress(saveSettings),
 		hookFunctionsExternalHooks(),
 		hookFunctionsFinalizeExecutionStatus(),
 		{
@@ -529,8 +531,6 @@ export function getWorkflowHooksWorkerMain(
 					// Don't delete executions before they are finished
 					if (!fullRunData.finished) return;
 
-					const saveSettings = toSaveSettings(this.workflowData.settings);
-
 					const isManualMode = this.mode === 'manual';
 
 					if (isManualMode && !saveSettings.manual && !fullRunData.waitTill) {
@@ -579,13 +579,14 @@ export function getWorkflowHooksMain(
 	data: IWorkflowExecutionDataProcess,
 	executionId: string,
 ): WorkflowHooks {
+	const saveSettings = toSaveSettings(data.workflowData.settings);
 	const hookFunctions = mergeHookFunctions(
 		hookFunctionsWorkflowEvents(),
 		hookFunctionsNodeEvents(),
 		hookFunctionsFinalizeExecutionStatus(),
-		hookFunctionsSave(),
+		hookFunctionsSave(saveSettings),
 		hookFunctionsPush(),
-		hookFunctionsSaveProgress(),
+		hookFunctionsSaveProgress(saveSettings),
 		hookFunctionsExternalHooks(),
 	);
 	return new WorkflowHooks(hookFunctions, data.executionMode, executionId, data.workflowData, {

packages/cli/src/execution-lifecycle/save-execution-progress.ts

@@ -1,21 +1,20 @@
 import { Container } from '@n8n/di';
 import { ErrorReporter, Logger } from 'n8n-core';
-import type { IRunExecutionData, ITaskData, IWorkflowBase } from 'n8n-workflow';
+import type { IRunExecutionData, ITaskData } from 'n8n-workflow';
 
 import { ExecutionRepository } from '@/databases/repositories/execution.repository';
 
-import { toSaveSettings } from './to-save-settings';
+import { type ExecutionSavingSettings } from './to-save-settings';
 
 export async function saveExecutionProgress(
-	workflowData: IWorkflowBase,
+	saveSettings: ExecutionSavingSettings,
+	workflowId: string,
 	executionId: string,
 	nodeName: string,
 	data: ITaskData,
 	executionData: IRunExecutionData,
 	pushRef?: string,
 ) {
-	const saveSettings = toSaveSettings(workflowData.settings);
-
 	if (!saveSettings.progress) return;
 
 	const logger = Container.get(Logger);
@@ -97,7 +96,7 @@ export async function saveExecutionProgress(
 				...error,
 				executionId,
 				pushRef,
-				workflowId: workflowData.id,
+				workflowId,
 			},
 		);
 	}

packages/cli/src/execution-lifecycle/to-save-settings.ts

@@ -2,6 +2,13 @@ import type { IWorkflowSettings } from 'n8n-workflow';
 
 import config from '@/config';
 
+export type ExecutionSavingSettings = {
+	error: boolean | 'all' | 'none';
+	success: boolean | 'all' | 'none';
+	manual: boolean;
+	progress: boolean;
+};
+
 /**
  * Return whether a workflow execution is configured to be saved or not:
  *
@@ -10,7 +17,7 @@ import config from '@/config';
  * - `manual`: Whether to save successful or failed manual executions.
  * - `progress`: Whether to save execution progress, i.e. after each node's execution.
  */
-export function toSaveSettings(workflowSettings: IWorkflowSettings = {}) {
+export function toSaveSettings(workflowSettings: IWorkflowSettings = {}): ExecutionSavingSettings {
 	const DEFAULTS = {
 		ERROR: config.getEnv('executions.saveDataOnError'),
 		SUCCESS: config.getEnv('executions.saveDataOnSuccess'),