mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-05 02:59:27 +02:00
fix(core): Sub-workflows to respect own timeout settings and global timeout config (#31536)
Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Co-authored-by: Danny Martini <danny@n8n.io>
This commit is contained in:
parent
f25e12de87
commit
0b2a6328cf
|
|
@ -1,5 +1,5 @@
|
|||
import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { ExecutionsConfig, GlobalConfig } from '@n8n/config';
|
||||
import type { WorkflowEntity, User, Project } from '@n8n/db';
|
||||
import {
|
||||
ExecutionRepository,
|
||||
|
|
@ -342,6 +342,199 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
expect(result.data).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('sub-workflow execution timeouts', () => {
|
||||
const now = Date.now();
|
||||
const getDeadlineFromNow = (offsetSeconds: number) => now + offsetSeconds * 1000;
|
||||
const WorkflowExecuteMock: jest.Mock = jest.requireMock('n8n-core').WorkflowExecute;
|
||||
|
||||
let dateSpy: jest.SpyInstance;
|
||||
beforeEach(() => {
|
||||
dateSpy = jest.spyOn(Date, 'now').mockReturnValue(now);
|
||||
WorkflowExecuteMock.mockClear();
|
||||
});
|
||||
afterEach(() => {
|
||||
dateSpy.mockRestore();
|
||||
});
|
||||
|
||||
const getSubWorkflowDeadline = () =>
|
||||
(WorkflowExecuteMock.mock.calls[0][0] as IWorkflowExecuteAdditionalData)
|
||||
.executionTimeoutTimestamp;
|
||||
|
||||
const executeWorkflowWithTimeout = async (opts: {
|
||||
doNotWaitToFinish: boolean;
|
||||
subTimeout?: number;
|
||||
parentDeadline?: number;
|
||||
globalTimeout?: number;
|
||||
globalMaxTimeout?: number;
|
||||
}) => {
|
||||
Container.set(ExecutionsConfig, {
|
||||
timeout: opts.globalTimeout ?? -1,
|
||||
maxTimeout: opts.globalMaxTimeout ?? 3600,
|
||||
} as ExecutionsConfig);
|
||||
|
||||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>(),
|
||||
mock<IWorkflowExecuteAdditionalData>({ executionTimeoutTimestamp: opts.parentDeadline }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: mock<IWorkflowBase>({
|
||||
id: 'sub-id',
|
||||
name: 'Sub Workflow',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
// Pass executionTimeout through even when undefined so jest-mock-extended
|
||||
// keeps it as a real `undefined` rather than auto-mocking it into a function.
|
||||
settings: { executionTimeout: opts.subTimeout },
|
||||
}),
|
||||
doNotWaitToFinish: opts.doNotWaitToFinish,
|
||||
}),
|
||||
);
|
||||
|
||||
if (opts.doNotWaitToFinish) {
|
||||
await new Promise(setImmediate);
|
||||
}
|
||||
};
|
||||
|
||||
describe('doNotWaitToFinish: false (parent waits for sub-workflow result)', () => {
|
||||
it('should use parent deadline when sub-workflow timeout exceeds it', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: 120,
|
||||
parentDeadline: getDeadlineFromNow(10),
|
||||
doNotWaitToFinish: false,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(10));
|
||||
});
|
||||
|
||||
it('should use sub-workflow timeout when shorter than parent deadline', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: 5,
|
||||
parentDeadline: getDeadlineFromNow(60),
|
||||
doNotWaitToFinish: false,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(5));
|
||||
});
|
||||
|
||||
it('should use sub-workflow timeout when parent has no deadline', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: 120,
|
||||
doNotWaitToFinish: false,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(120));
|
||||
});
|
||||
|
||||
it('should cap sub-workflow timeout at global max timeout', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: 7200,
|
||||
parentDeadline: getDeadlineFromNow(7200),
|
||||
doNotWaitToFinish: false,
|
||||
globalMaxTimeout: 3600,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(3600));
|
||||
});
|
||||
|
||||
it('should not cap sub-workflow timeout when global max timeout is disabled (<=0)', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: 7200,
|
||||
doNotWaitToFinish: false,
|
||||
globalMaxTimeout: -1,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(7200));
|
||||
});
|
||||
|
||||
it('should cap the global default timeout at the global max timeout', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
doNotWaitToFinish: false,
|
||||
globalTimeout: 20,
|
||||
globalMaxTimeout: 10,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(10));
|
||||
});
|
||||
|
||||
it('should not cap the global default timeout when global max timeout is disabled (<=0)', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
doNotWaitToFinish: false,
|
||||
globalTimeout: 600,
|
||||
globalMaxTimeout: -1,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(600));
|
||||
});
|
||||
|
||||
it('should fall back to global default when sub-workflow has no timeout', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
parentDeadline: getDeadlineFromNow(60),
|
||||
doNotWaitToFinish: false,
|
||||
globalTimeout: 30,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(30));
|
||||
});
|
||||
|
||||
it('should use parent deadline when sub-workflow has no timeout, and global is unlimited', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
parentDeadline: getDeadlineFromNow(10),
|
||||
doNotWaitToFinish: false,
|
||||
globalTimeout: -1,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(10));
|
||||
});
|
||||
|
||||
it('should set no timeout when neither sub-workflow nor global timeout is set and parent has no deadline', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
globalTimeout: -1,
|
||||
doNotWaitToFinish: false,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should ignore the global default timeout when the sub-workflow timeout is explicitly disabled (-1)', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: -1,
|
||||
doNotWaitToFinish: false,
|
||||
globalTimeout: 600,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should use parent deadline, not the global default, when the sub-workflow timeout is explicitly disabled (-1)', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: -1,
|
||||
parentDeadline: getDeadlineFromNow(60),
|
||||
doNotWaitToFinish: false,
|
||||
globalTimeout: 600,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(60));
|
||||
});
|
||||
});
|
||||
|
||||
describe('doNotWaitToFinish: true (sub-workflow timeout independent of parent)', () => {
|
||||
it('should use sub-workflow own timeout regardless of parent deadline', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: 120,
|
||||
parentDeadline: getDeadlineFromNow(10),
|
||||
doNotWaitToFinish: true,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBe(getDeadlineFromNow(120));
|
||||
});
|
||||
|
||||
it('should set no timeout when sub-workflow has no timeout and global timeout is unlimited', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
parentDeadline: getDeadlineFromNow(10),
|
||||
doNotWaitToFinish: true,
|
||||
globalTimeout: -1,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should ignore the global default timeout when the sub-workflow timeout is explicitly disabled (-1)', async () => {
|
||||
await executeWorkflowWithTimeout({
|
||||
subTimeout: -1,
|
||||
parentDeadline: getDeadlineFromNow(10),
|
||||
doNotWaitToFinish: true,
|
||||
globalTimeout: 600,
|
||||
});
|
||||
expect(getSubWorkflowDeadline()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('getRunData', () => {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@
|
|||
|
||||
import type { PushMessage, PushType } from '@n8n/api-types';
|
||||
import { Logger, ModuleRegistry } from '@n8n/backend-common';
|
||||
import { GlobalConfig, SsrfProtectionConfig } from '@n8n/config';
|
||||
import { ExecutionsConfig, GlobalConfig, SsrfProtectionConfig } from '@n8n/config';
|
||||
import { Time } from '@n8n/constants';
|
||||
import { ExecutionRepository, WorkflowRepository } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { ServiceIdentifier } from '@n8n/di';
|
||||
|
|
@ -201,6 +202,57 @@ export async function getPublishedWorkflowData(
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines a workflow's deadline given its start time, its settings
|
||||
* and global execution config.
|
||||
*/
|
||||
function determineWorkflowDeadline(
|
||||
startTime: number,
|
||||
workflowSettings: IWorkflowSettings | undefined,
|
||||
executionsConfig: ExecutionsConfig,
|
||||
): number | undefined {
|
||||
const effectiveMaxTimeout =
|
||||
executionsConfig.maxTimeout > 0 ? executionsConfig.maxTimeout : Infinity;
|
||||
|
||||
if (workflowSettings?.executionTimeout !== undefined) {
|
||||
// A defined timeout of <= 0 means the workflow's own timeout is explicitly
|
||||
// disabled, so it runs unbounded rather than falling back to the global
|
||||
// default. Otherwise it is clamped to the configurable maximum. This mirrors
|
||||
// the main-workflow path in workflow-runner.ts and job-processor.ts.
|
||||
if (workflowSettings.executionTimeout <= 0) {
|
||||
return undefined;
|
||||
}
|
||||
return (
|
||||
startTime +
|
||||
Math.min(workflowSettings.executionTimeout, effectiveMaxTimeout) * Time.seconds.toMilliseconds
|
||||
);
|
||||
}
|
||||
if (executionsConfig.timeout > 0) {
|
||||
return (
|
||||
startTime +
|
||||
Math.min(executionsConfig.timeout, effectiveMaxTimeout) * Time.seconds.toMilliseconds
|
||||
);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a sub-workflow deadline depending on its parent.
|
||||
*/
|
||||
function resolveSubworkflowDeadline(
|
||||
subWorkflowDeadline: number | undefined,
|
||||
parentDeadline: number | undefined,
|
||||
doNotWaitToFinish: boolean | undefined,
|
||||
): number | undefined {
|
||||
if (doNotWaitToFinish) {
|
||||
return subWorkflowDeadline;
|
||||
}
|
||||
if (parentDeadline !== undefined && subWorkflowDeadline !== undefined) {
|
||||
return Math.min(parentDeadline, subWorkflowDeadline);
|
||||
}
|
||||
return parentDeadline ?? subWorkflowDeadline;
|
||||
}
|
||||
|
||||
/**
|
||||
* Executes the workflow with the given ID
|
||||
*/
|
||||
|
|
@ -443,18 +495,15 @@ async function startExecution(
|
|||
// Propagate streaming state to subworkflows
|
||||
additionalDataIntegrated.streamingEnabled = additionalData.streamingEnabled;
|
||||
|
||||
let subworkflowTimeout = additionalData.executionTimeoutTimestamp;
|
||||
if (workflowSettings?.executionTimeout !== undefined && workflowSettings.executionTimeout > 0) {
|
||||
// We might have received a max timeout timestamp from the parent workflow
|
||||
// If we did, then we get the minimum time between the two timeouts
|
||||
// If no timeout was given from the parent, then we use our timeout.
|
||||
subworkflowTimeout = Math.min(
|
||||
additionalData.executionTimeoutTimestamp || Number.MAX_SAFE_INTEGER,
|
||||
startTime + workflowSettings.executionTimeout * 1000,
|
||||
);
|
||||
}
|
||||
const executionsConfig = Container.get(ExecutionsConfig);
|
||||
|
||||
additionalDataIntegrated.executionTimeoutTimestamp = subworkflowTimeout;
|
||||
const subworkflowDeadline = resolveSubworkflowDeadline(
|
||||
determineWorkflowDeadline(startTime, workflowSettings, executionsConfig),
|
||||
additionalData.executionTimeoutTimestamp,
|
||||
options.doNotWaitToFinish,
|
||||
);
|
||||
|
||||
additionalDataIntegrated.executionTimeoutTimestamp = subworkflowDeadline;
|
||||
|
||||
const runExecutionData = runData.executionData as IRunExecutionData;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user