From d1153f51e80911cbc8f34ba5f038f349b75295c3 Mon Sep 17 00:00:00 2001
From: Danny Martini <danny@n8n.io>
Date: Mon, 4 Nov 2024 13:04:22 +0100
Subject: [PATCH] fix(core): Don't send a `executionFinished` event to the
 browser with no run data if the execution has already been cleaned up
 (#11502)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
---
 .../cli/src/__tests__/workflow-runner.test.ts     | 14 ++++++++++++++
 packages/cli/src/workflow-runner.ts               | 12 +++++++++++-
 packages/core/src/WorkflowExecute.ts              | 15 ++++++++-------
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/packages/cli/src/__tests__/workflow-runner.test.ts b/packages/cli/src/__tests__/workflow-runner.test.ts
index f828880c74..4774746ba7 100644
--- a/packages/cli/src/__tests__/workflow-runner.test.ts
+++ b/packages/cli/src/__tests__/workflow-runner.test.ts
@@ -4,6 +4,7 @@ import Container from 'typedi';
 import { ActiveExecutions } from '@/active-executions';
 import config from '@/config';
 import type { User } from '@/databases/entities/user';
+import { ExecutionNotFoundError } from '@/errors/execution-not-found-error';
 import { Telemetry } from '@/telemetry';
 import { WorkflowRunner } from '@/workflow-runner';
 import { mockInstance } from '@test/mocking';
@@ -64,6 +65,19 @@ test('processError should return early in Bull stalled edge case', async () => {
 	expect(watchedWorkflowExecuteAfter).toHaveBeenCalledTimes(0);
 });
 
+test('processError should return early if the error is `ExecutionNotFoundError`', async () => {
+	const workflow = await createWorkflow({}, owner);
+	const execution = await createExecution({ status: 'success', finished: true }, workflow);
+	await runner.processError(
+		new ExecutionNotFoundError(execution.id),
+		new Date(),
+		'webhook',
+		execution.id,
+		new WorkflowHooks(hookFunctions, 'webhook', execution.id, workflow),
+	);
+	expect(watchedWorkflowExecuteAfter).toHaveBeenCalledTimes(0);
+});
+
 test('processError should process error', async () => {
 	const workflow = await createWorkflow({}, owner);
 	const execution = await createExecution(
diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts
index 02e0b94afd..38ead8896e 100644
--- a/packages/cli/src/workflow-runner.ts
+++ b/packages/cli/src/workflow-runner.ts
@@ -35,6 +35,7 @@ import * as WorkflowHelpers from '@/workflow-helpers';
 import { generateFailedExecutionFromError } from '@/workflow-helpers';
 import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service';
 
+import { ExecutionNotFoundError } from './errors/execution-not-found-error';
 import { EventService } from './events/event.service';
 
 @Service()
@@ -57,12 +58,21 @@ export class WorkflowRunner {
 
 	/** The process did error */
 	async processError(
-		error: ExecutionError,
+		error: ExecutionError | ExecutionNotFoundError,
 		startedAt: Date,
 		executionMode: WorkflowExecuteMode,
 		executionId: string,
 		hooks?: WorkflowHooks,
 	) {
+		// This means the execution was probably cancelled and has already
+		// been cleaned up.
+		//
+		// FIXME: This is a quick fix. The proper fix would be to not remove
+		// the execution from the active executions while it's still running.
+		if (error instanceof ExecutionNotFoundError) {
+			return;
+		}
+
 		ErrorReporter.error(error, { executionId });
 
 		const isQueueMode = config.getEnv('executions.mode') === 'queue';
diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts
index 23b88abd5b..f8c32351bb 100644
--- a/packages/core/src/WorkflowExecute.ts
+++ b/packages/core/src/WorkflowExecute.ts
@@ -45,6 +45,7 @@ import {
 	NodeExecutionOutput,
 	sleep,
 	ErrorReporterProxy,
+	ExecutionCancelledError,
 } from 'n8n-workflow';
 import PCancelable from 'p-cancelable';
 
@@ -154,10 +155,6 @@ export class WorkflowExecute {
 		return this.processRunExecutionData(workflow);
 	}
 
-	static isAbortError(e?: ExecutionBaseError) {
-		return e?.message === 'AbortError';
-	}
-
 	forceInputNodeExecution(workflow: Workflow): boolean {
 		return workflow.settings.executionOrder !== 'v1';
 	}
@@ -1479,7 +1476,7 @@ export class WorkflowExecute {
 							// Add the execution data again so that it can get restarted
 							this.runExecutionData.executionData!.nodeExecutionStack.unshift(executionData);
 							// Only execute the nodeExecuteAfter hook if the node did not get aborted
-							if (!WorkflowExecute.isAbortError(executionError)) {
+							if (!this.isCancelled) {
 								await this.executeHook('nodeExecuteAfter', [
 									executionNode.name,
 									taskData,
@@ -1827,7 +1824,7 @@ export class WorkflowExecute {
 						return await this.processSuccessExecution(
 							startedAt,
 							workflow,
-							new WorkflowOperationError('Workflow has been canceled or timed out'),
+							new ExecutionCancelledError(this.additionalData.executionId ?? 'unknown'),
 							closeFunction,
 						);
 					}
@@ -1928,7 +1925,7 @@ export class WorkflowExecute {
 
 		this.moveNodeMetadata();
 		// Prevent from running the hook if the error is an abort error as it was already handled
-		if (!WorkflowExecute.isAbortError(executionError)) {
+		if (!this.isCancelled) {
 			await this.executeHook('workflowExecuteAfter', [fullRunData, newStaticData]);
 		}
 
@@ -1959,4 +1956,8 @@ export class WorkflowExecute {
 
 		return fullRunData;
 	}
+
+	private get isCancelled() {
+		return this.abortController.signal.aborted;
+	}
 }