Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Aborting manual trigger tests should call closeFunction #9980

Merged
merged 2 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions packages/workflow/src/Workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1424,13 +1424,6 @@ export class Workflow {
return { data: null };
}

if (triggerResponse.manualTriggerFunction !== undefined) {
// If a manual trigger function is defined call it and wait till it did run
await triggerResponse.manualTriggerFunction();
}

const response = await triggerResponse.manualTriggerResponse!;

let closeFunction;
if (triggerResponse.closeFunction) {
// In manual mode we return the trigger closeFunction. That allows it to be called directly
Expand All @@ -1439,8 +1432,18 @@ export class Workflow {
// If we would not be able to wait for it to close would it cause problems with "own" mode as the
// process would be killed directly after it and so the acknowledge would not have been finished yet.
closeFunction = triggerResponse.closeFunction;

// Manual testing of Trigger nodes creates an execution. If the execution is cancelled, `closeFunction` should be called to cleanup any open connections/consumers
abortSignal?.addEventListener('abort', closeFunction);
}

if (triggerResponse.manualTriggerFunction !== undefined) {
// If a manual trigger function is defined call it and wait till it did run
await triggerResponse.manualTriggerFunction();
}

const response = await triggerResponse.manualTriggerResponse!;

if (response.length === 0) {
return { data: null, closeFunction };
}
Expand Down
68 changes: 68 additions & 0 deletions packages/workflow/test/Workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@ import type {
IBinaryKeyData,
IConnections,
IDataObject,
IExecuteData,
INode,
INodeExecuteFunctions,
INodeExecutionData,
INodeParameters,
INodeType,
INodeTypeDescription,
INodeTypes,
IRunExecutionData,
ITriggerFunctions,
ITriggerResponse,
IWorkflowExecuteAdditionalData,
NodeParameterValueType,
} from '@/Interfaces';
import { Workflow, type WorkflowParameters } from '@/Workflow';
Expand Down Expand Up @@ -2015,4 +2020,67 @@ describe('Workflow', () => {
]);
});
});

describe('runNode', () => {
const nodeTypes = mock<INodeTypes>();
const triggerNode = mock<INode>();
const triggerResponse = mock<ITriggerResponse>({
closeFunction: jest.fn(),
// This node should never trigger, or return
manualTriggerFunction: async () => await new Promise(() => {}),
});
const triggerNodeType = mock<INodeType>({
description: {
properties: [],
},
execute: undefined,
poll: undefined,
webhook: undefined,
async trigger(this: ITriggerFunctions) {
return triggerResponse;
},
});

nodeTypes.getByNameAndVersion.mockReturnValue(triggerNodeType);

const workflow = new Workflow({
nodeTypes,
nodes: [triggerNode],
connections: {},
active: false,
});

const executionData = mock<IExecuteData>();
const runExecutionData = mock<IRunExecutionData>();
const additionalData = mock<IWorkflowExecuteAdditionalData>();
const nodeExecuteFunctions = mock<INodeExecuteFunctions>();
const triggerFunctions = mock<ITriggerFunctions>();
nodeExecuteFunctions.getExecuteTriggerFunctions.mockReturnValue(triggerFunctions);
const abortController = new AbortController();

test('should call closeFunction when manual trigger is aborted', async () => {
const runPromise = workflow.runNode(
executionData,
runExecutionData,
0,
additionalData,
nodeExecuteFunctions,
'manual',
abortController.signal,
);
// Yield back to the event-loop to let async parts of `runNode` execute
await new Promise((resolve) => setImmediate(resolve));

let isSettled = false;
void runPromise.then(() => {
isSettled = true;
});
expect(isSettled).toBe(false);
expect(abortController.signal.aborted).toBe(false);
expect(triggerResponse.closeFunction).not.toHaveBeenCalled();

abortController.abort();
expect(triggerResponse.closeFunction).toHaveBeenCalled();
});
});
});
Loading