Skip to content

Commit

Permalink
fix(core): Ensure failed executions are saved in queue mode (#7744)
Browse files Browse the repository at this point in the history
This PR adds `status` to run data so that
`determineFinalExecutionStatus` resolves correctly on execution failure
and removes the cleanup that is being duplicated in a worker hook.

Followup to #7138

Should fix:
- #7705
-
https://linear.app/n8n/issue/PAY-964/no-execution-found-after-execution-fails
-
https://linear.app/n8n/issue/PAY-1010/execution-deletion-in-queue-mode-not-complying-with-settings
  • Loading branch information
ivov committed Nov 20, 2023
1 parent ad04986 commit b7c5c74
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 30 deletions.
31 changes: 2 additions & 29 deletions packages/cli/src/WorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,11 +559,8 @@ export class WorkflowRunner {
};
}

let racingPromisesResult: JobResponse = {
success: false,
};
try {
racingPromisesResult = await Promise.race(racingPromises);
await Promise.race(racingPromises);
if (clearWatchdogInterval !== undefined) {
clearWatchdogInterval();
}
Expand Down Expand Up @@ -617,6 +614,7 @@ export class WorkflowRunner {
mode: fullExecutionData.mode,
startedAt: fullExecutionData.startedAt,
stoppedAt: fullExecutionData.stoppedAt,
status: fullExecutionData.status,
} as IRun;

if (executionHasPostExecutionPromises) {
Expand All @@ -631,31 +629,6 @@ export class WorkflowRunner {
// Normally also static data should be supplied here but as it only used for sending
// data to editor-UI is not needed.
await hooks.executeHookFunctions('workflowExecuteAfter', [runData]);
try {
// Check if this execution data has to be removed from database
// based on workflow settings.
const workflowSettings = data.workflowData.settings ?? {};
const saveDataErrorExecution =
workflowSettings.saveDataErrorExecution ?? config.getEnv('executions.saveDataOnError');
const saveDataSuccessExecution =
workflowSettings.saveDataSuccessExecution ??
config.getEnv('executions.saveDataOnSuccess');

const workflowDidSucceed = !racingPromisesResult.error;
if (
(workflowDidSucceed && saveDataSuccessExecution === 'none') ||
(!workflowDidSucceed && saveDataErrorExecution === 'none')
) {
await Container.get(ExecutionRepository).hardDelete({
workflowId: data.workflowData.id as string,
executionId,
});
}
// eslint-disable-next-line id-denylist
} catch (err) {
// We don't want errors here to crash n8n. Just log and proceed.
console.log('Error removing saved execution from database. More details: ', err);
}

resolve(runData);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ import { Logger } from '@/Logger';
export function determineFinalExecutionStatus(runData: IRun): ExecutionStatus {
const workflowHasCrashed = runData.status === 'crashed';
const workflowWasCanceled = runData.status === 'canceled';
const workflowHasFailed = runData.status === 'failed';
const workflowDidSucceed =
!runData.data.resultData?.error && !workflowHasCrashed && !workflowWasCanceled;
!runData.data.resultData?.error &&
!workflowHasCrashed &&
!workflowWasCanceled &&
!workflowHasFailed;
let workflowStatusFinal: ExecutionStatus = workflowDidSucceed ? 'success' : 'failed';
if (workflowHasCrashed) workflowStatusFinal = 'crashed';
if (workflowWasCanceled) workflowStatusFinal = 'canceled';
Expand Down

0 comments on commit b7c5c74

Please sign in to comment.