fix(core): Don't send a executionFinished event to the browser with no run data if the execution has already been cleaned up (#11502)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
Danny Martini 2024-11-04 13:04:22 +01:00 committed by GitHub
parent da18e1ad29
commit d1153f51e8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 33 additions and 8 deletions

View file

@ -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(

View file

@ -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';

View file

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