mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-29 01:39:24 +08:00
fix(core): Wrap a trigger's closeFunction in an expression isolate at creation time (backport to release-candidate/2.35.x) (#36698)
Co-authored-by: Danny Martini <danny@n8n.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Mike Repeć <mike.repec@n8n.io>
This commit is contained in:
committed by
GitHub
parent
898cebc326
commit
eb85ccde78
@@ -62,6 +62,54 @@ describe('TriggersAndPollers', () => {
|
||||
expect(result).toEqual({ test: true });
|
||||
});
|
||||
|
||||
describe('closeFunction isolate wrapping', () => {
|
||||
const originalClose = vi.fn(async () => {});
|
||||
const withIsolate = vi.fn(async (fn: () => Promise<void>) => await fn());
|
||||
|
||||
beforeEach(() => {
|
||||
nodeType.trigger = triggerFn;
|
||||
workflow.expression = { withIsolate } as unknown as Workflow['expression'];
|
||||
});
|
||||
|
||||
it('wraps closeFunction so teardown runs inside workflow.expression.withIsolate', async () => {
|
||||
triggerFn.mockResolvedValue({ closeFunction: originalClose });
|
||||
|
||||
const response = await runTriggerHelper();
|
||||
|
||||
expect(response?.closeFunction).not.toBe(originalClose);
|
||||
expect(originalClose).not.toHaveBeenCalled();
|
||||
|
||||
await response!.closeFunction!();
|
||||
|
||||
expect(withIsolate).toHaveBeenCalledTimes(1);
|
||||
expect(originalClose).toHaveBeenCalledTimes(1);
|
||||
const [isolateOrder] = withIsolate.mock.invocationCallOrder;
|
||||
const [closeOrder] = originalClose.mock.invocationCallOrder;
|
||||
expect(isolateOrder).toBeLessThan(closeOrder);
|
||||
});
|
||||
|
||||
it('wraps closeFunction in manual mode too', async () => {
|
||||
triggerFn.mockResolvedValue({ closeFunction: originalClose });
|
||||
|
||||
const response = await runTriggerHelper('manual');
|
||||
|
||||
expect(response?.closeFunction).not.toBe(originalClose);
|
||||
await response!.closeFunction!();
|
||||
expect(withIsolate).toHaveBeenCalledTimes(1);
|
||||
expect(originalClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('propagates a closeFunction rejection through the wrapper', async () => {
|
||||
const closeError = new Error('close failed');
|
||||
originalClose.mockRejectedValueOnce(closeError);
|
||||
triggerFn.mockResolvedValue({ closeFunction: originalClose });
|
||||
|
||||
const response = await runTriggerHelper();
|
||||
|
||||
await expect(response!.closeFunction!()).rejects.toThrow(closeError);
|
||||
});
|
||||
});
|
||||
|
||||
describe('manual mode', () => {
|
||||
const getMockTriggerFunctions = () => getTriggerFunctions.mock.results[0]?.value;
|
||||
|
||||
|
||||
@@ -1562,8 +1562,9 @@ describe('WorkflowExecute', () => {
|
||||
describe('runNode', () => {
|
||||
const nodeTypes = mock<INodeTypes>();
|
||||
const triggerNode = mock<INode>();
|
||||
const closeFunctionSpy = vi.fn();
|
||||
const triggerResponse = mock<ITriggerResponse>({
|
||||
closeFunction: vi.fn(),
|
||||
closeFunction: closeFunctionSpy,
|
||||
// This node should never trigger, or return
|
||||
manualTriggerFunction: async () => await new Promise(() => {}),
|
||||
});
|
||||
@@ -1618,10 +1619,11 @@ describe('WorkflowExecute', () => {
|
||||
});
|
||||
expect(isSettled).toBe(false);
|
||||
expect(abortController.signal.aborted).toBe(false);
|
||||
expect(triggerResponse.closeFunction).not.toHaveBeenCalled();
|
||||
expect(closeFunctionSpy).not.toHaveBeenCalled();
|
||||
|
||||
abortController.abort();
|
||||
expect(triggerResponse.closeFunction).toHaveBeenCalled();
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(closeFunctionSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -83,10 +83,27 @@ export class TriggersAndPollers {
|
||||
};
|
||||
});
|
||||
|
||||
return triggerResponse;
|
||||
return this.wrapCloseFunctionInIsolate(workflow, triggerResponse);
|
||||
}
|
||||
// In all other modes simply start the trigger
|
||||
return await nodeType.trigger.call(triggerFunctions);
|
||||
return this.wrapCloseFunctionInIsolate(workflow, await nodeType.trigger.call(triggerFunctions));
|
||||
}
|
||||
|
||||
/**
|
||||
* Wraps a trigger's `closeFunction` so teardown holds an expression isolate:
|
||||
* the closure evaluates expressions through this workflow's expression
|
||||
* instance, which is no longer in scope at the eventual close call sites.
|
||||
*/
|
||||
private wrapCloseFunctionInIsolate(
|
||||
workflow: Workflow,
|
||||
response: ITriggerResponse | undefined,
|
||||
): ITriggerResponse | undefined {
|
||||
const closeFunction = response?.closeFunction;
|
||||
if (response && closeFunction) {
|
||||
response.closeFunction = async () =>
|
||||
await workflow.expression.withIsolate(async () => await closeFunction.call(response));
|
||||
}
|
||||
return response;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user