fix(core): Acquire expression isolate for dynamic node parameter requests (#29671)

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Alexander Gekov 2026-05-04 13:20:05 +03:00 committed by GitHub
parent dc52bbd532
commit 418f1f2edb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 160 additions and 17 deletions

View File

@ -6,6 +6,7 @@ import {
type INodeType,
type IWorkflowExecuteAdditionalData,
type ResourceMapperFields,
Expression,
} from 'n8n-workflow';
import { DynamicNodeParametersService } from '../dynamic-node-parameters.service';
@ -84,6 +85,128 @@ describe('DynamicNodeParametersService', () => {
});
});
describe('expression isolate lifecycle', () => {
let acquireSpy: jest.SpyInstance;
let releaseSpy: jest.SpyInstance;
beforeEach(() => {
acquireSpy = jest.spyOn(Expression.prototype, 'acquireIsolate').mockResolvedValue(undefined);
releaseSpy = jest.spyOn(Expression.prototype, 'releaseIsolate').mockResolvedValue(undefined);
});
it('should acquire and release isolate around getOptionsViaMethodName', async () => {
const loadOptionsMethod = jest.fn().mockResolvedValue([{ name: 'opt', value: 'v' }]);
nodeTypes.getByNameAndVersion.mockReturnValue(
mock<INodeType>({
description: { properties: [] },
methods: { loadOptions: { getOptions: loadOptionsMethod } },
}),
);
await service.getOptionsViaMethodName(
'getOptions',
'',
mock<IWorkflowExecuteAdditionalData>(),
{ name: 'TestNode', version: 1 },
mock<INodeParameters>(),
);
expect(acquireSpy).toHaveBeenCalledTimes(1);
expect(releaseSpy).toHaveBeenCalledTimes(1);
});
it('should release isolate even when the inner method throws', async () => {
const loadOptionsMethod = jest.fn().mockRejectedValue(new Error('boom'));
nodeTypes.getByNameAndVersion.mockReturnValue(
mock<INodeType>({
description: { properties: [] },
methods: { loadOptions: { getOptions: loadOptionsMethod } },
}),
);
await expect(
service.getOptionsViaMethodName(
'getOptions',
'',
mock<IWorkflowExecuteAdditionalData>(),
{ name: 'TestNode', version: 1 },
mock<INodeParameters>(),
),
).rejects.toThrow('boom');
expect(acquireSpy).toHaveBeenCalledTimes(1);
expect(releaseSpy).toHaveBeenCalledTimes(1);
});
it('should acquire and release isolate around getResourceLocatorResults', async () => {
const listSearchMethod = jest
.fn()
.mockResolvedValue({ results: [{ name: 'r', value: 'v' }] });
nodeTypes.getByNameAndVersion.mockReturnValue(
mock<INodeType>({
description: { properties: [] },
methods: { listSearch: { searchModels: listSearchMethod } },
}),
);
await service.getResourceLocatorResults(
'searchModels',
'',
mock<IWorkflowExecuteAdditionalData>(),
{ name: 'TestNode', version: 1 },
mock<INodeParameters>(),
);
expect(acquireSpy).toHaveBeenCalledTimes(1);
expect(releaseSpy).toHaveBeenCalledTimes(1);
});
it('should acquire and release isolate around getResourceMappingFields', async () => {
const resourceMappingMethod = jest.fn().mockResolvedValue({
fields: [{ id: '1', displayName: 'F', defaultMatch: false, required: true, display: true }],
});
nodeTypes.getByNameAndVersion.mockReturnValue(
mock<INodeType>({
description: { properties: [] },
methods: { resourceMapping: { getFields: resourceMappingMethod } },
}),
);
await service.getResourceMappingFields(
'getFields',
'',
mock<IWorkflowExecuteAdditionalData>(),
{ name: 'TestNode', version: 1 },
mock<INodeParameters>(),
);
expect(acquireSpy).toHaveBeenCalledTimes(1);
expect(releaseSpy).toHaveBeenCalledTimes(1);
});
it('should acquire and release isolate around getActionResult', async () => {
const actionHandler = jest.fn().mockResolvedValue({ key: 'value' });
nodeTypes.getByNameAndVersion.mockReturnValue(
mock<INodeType>({
description: { properties: [] },
methods: { actionHandler: { handle: actionHandler } },
}),
);
await service.getActionResult(
'handle',
'',
mock<IWorkflowExecuteAdditionalData>(),
{ name: 'TestNode', version: 1 },
mock<INodeParameters>(),
undefined,
);
expect(acquireSpy).toHaveBeenCalledTimes(1);
expect(releaseSpy).toHaveBeenCalledTimes(1);
});
});
describe('getLocalResourceMappingFields', () => {
it('should remove duplicate resource mapping fields', async () => {
const resourceMappingMethod = jest.fn();

View File

@ -134,10 +134,11 @@ export class DynamicNodeParametersService {
const method = this.getMethod('loadOptions', methodName, nodeType);
const workflow = this.getWorkflow(nodeTypeAndVersion, currentNodeParameters, credentials);
const thisArgs = this.getThisArg(path, additionalData, workflow);
// Need to use untyped call since `this` usage is widespread and we don't have `strictBindCallApply`
// enabled in `tsconfig.json`
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return await method.call(thisArgs);
return await this.withExpressionIsolate(workflow, async () => {
// Need to use untyped call since `this` usage is widespread and we don't have `strictBindCallApply`
// enabled in `tsconfig.json`
return await method.call(thisArgs);
});
}
/** Returns the available options via a loadOptions param */
@ -210,17 +211,19 @@ export class DynamicNodeParametersService {
[],
);
const routingNode = new RoutingNode(executeFunctions, tempNodeType);
const optionsData = await routingNode.runNode();
return await this.withExpressionIsolate(workflow, async () => {
const optionsData = await routingNode.runNode();
if (optionsData?.length === 0) {
return [];
}
if (optionsData?.length === 0) {
return [];
}
if (!Array.isArray(optionsData)) {
throw new UnexpectedError('The returned data is not an array');
}
if (!Array.isArray(optionsData)) {
throw new UnexpectedError('The returned data is not an array');
}
return optionsData[0].map((item) => item.json) as unknown as INodePropertyOptions[];
return optionsData[0].map((item) => item.json) as unknown as INodePropertyOptions[];
});
}
async getResourceLocatorResults(
@ -237,8 +240,9 @@ export class DynamicNodeParametersService {
const method = this.getMethod('listSearch', methodName, nodeType);
const workflow = this.getWorkflow(nodeTypeAndVersion, currentNodeParameters, credentials);
const thisArgs = this.getThisArg(path, additionalData, workflow);
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return await method.call(thisArgs, filter, paginationToken);
return await this.withExpressionIsolate(workflow, async () => {
return await method.call(thisArgs, filter, paginationToken);
});
}
/** Returns the available mapping fields for the ResourceMapper component */
@ -254,7 +258,9 @@ export class DynamicNodeParametersService {
const method = this.getMethod('resourceMapping', methodName, nodeType);
const workflow = this.getWorkflow(nodeTypeAndVersion, currentNodeParameters, credentials);
const thisArgs = this.getThisArg(path, additionalData, workflow);
return this.removeDuplicateResourceMappingFields(await method.call(thisArgs));
return await this.withExpressionIsolate(workflow, async () =>
this.removeDuplicateResourceMappingFields(await method.call(thisArgs)),
);
}
/** Returns the available workflow input mapping fields for the ResourceMapper component */
@ -284,8 +290,22 @@ export class DynamicNodeParametersService {
const method = this.getMethod('actionHandler', handler, nodeType);
const workflow = this.getWorkflow(nodeTypeAndVersion, currentNodeParameters, credentials);
const thisArgs = this.getThisArg(path, additionalData, workflow);
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return await method.call(thisArgs, payload);
return await this.withExpressionIsolate(workflow, async () => {
return await method.call(thisArgs, payload);
});
}
/**
* When N8N_EXPRESSION_ENGINE=vm, expressions run in an isolate that must be acquired
* for this workflow before any code resolves {{ }} in parameters or credentials.
*/
private async withExpressionIsolate<T>(workflow: Workflow, fn: () => Promise<T>): Promise<T> {
await workflow.expression.acquireIsolate();
try {
return await fn();
} finally {
await workflow.expression.releaseIsolate();
}
}
private getMethod(