mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-17 17:42:47 +08:00
perf(core): Resolve webhook description fields via declared native resolvers (#35404)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
1a5642821a
commit
18c2d715e3
@@ -1,18 +1,28 @@
|
||||
import { mockLogger } from '@n8n/backend-test-utils';
|
||||
import type { WorkflowsConfig } from '@n8n/config';
|
||||
import type { ExpressionEngineConfig, WorkflowsConfig } from '@n8n/config';
|
||||
import type { WebhookEntity, WorkflowEntity, WorkflowHistory, WorkflowRepository } from '@n8n/db';
|
||||
import type { Response } from 'express';
|
||||
import type {
|
||||
IConnections,
|
||||
IHttpRequestMethods,
|
||||
INode,
|
||||
INodeParameters,
|
||||
INodeProperties,
|
||||
INodeType,
|
||||
IWebhookData,
|
||||
IWebhookDescription,
|
||||
IWorkflowBase,
|
||||
IWorkflowExecuteAdditionalData,
|
||||
Workflow,
|
||||
} from 'n8n-workflow';
|
||||
import type { Mock } from 'vitest';
|
||||
import {
|
||||
fromFunction,
|
||||
fromParameter,
|
||||
WEBHOOK_NODE_TYPE,
|
||||
webhookDescriptionFields,
|
||||
WorkflowExpression,
|
||||
} from 'n8n-workflow';
|
||||
import type { Mock, MockInstance } from 'vitest';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { WebhookNotFoundError } from '@/errors/response-errors/webhook-not-found.error';
|
||||
@@ -39,11 +49,16 @@ describe('LiveWebhooks', () => {
|
||||
const workflowStaticDataService = mock<WorkflowStaticDataService>();
|
||||
const workflowsConfig = mock<WorkflowsConfig>({ useWorkflowPublicationService: false });
|
||||
const workflowPublishedDataService = mock<WorkflowPublishedDataService>();
|
||||
const expressionEngineConfig = mock<ExpressionEngineConfig>({
|
||||
allowWebhookIsolateSkip: true,
|
||||
});
|
||||
|
||||
let liveWebhooks: LiveWebhooks;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
// `clearAllMocks` resets call history, not properties set on a mock
|
||||
expressionEngineConfig.allowWebhookIsolateSkip = true;
|
||||
liveWebhooks = new LiveWebhooks(
|
||||
mockLogger(),
|
||||
nodeTypes,
|
||||
@@ -52,6 +67,7 @@ describe('LiveWebhooks', () => {
|
||||
workflowStaticDataService,
|
||||
workflowsConfig,
|
||||
workflowPublishedDataService,
|
||||
expressionEngineConfig,
|
||||
);
|
||||
|
||||
// Mock WorkflowExecuteAdditionalData.getBase to avoid DI issues
|
||||
@@ -603,4 +619,253 @@ describe('LiveWebhooks', () => {
|
||||
).resolves.toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('webhook-phase isolate acquisition', () => {
|
||||
const nodeName = 'Trigger';
|
||||
const webhookPath = 'my-path';
|
||||
const httpMethod: IHttpRequestMethods = 'GET';
|
||||
|
||||
const nodeProperties: INodeProperties[] = [
|
||||
{ displayName: 'Path', name: 'path', type: 'string', default: '' },
|
||||
{ displayName: 'Method', name: 'httpMethod', type: 'string', default: 'GET' },
|
||||
{ displayName: 'Code', name: 'responseCode', type: 'number', default: 200 },
|
||||
{ displayName: 'Auth', name: 'authentication', type: 'string', default: 'none' },
|
||||
{
|
||||
displayName: 'Options',
|
||||
name: 'options',
|
||||
type: 'collection',
|
||||
default: {},
|
||||
options: [{ displayName: 'Only Run If', name: 'onlyRunIf', type: 'string', default: '' }],
|
||||
},
|
||||
];
|
||||
|
||||
// Mirrors the real Webhook node's description: every templated field is
|
||||
// declared once, so it carries a native resolver.
|
||||
const nativelyResolvableWebhooks: IWebhookDescription[] = [
|
||||
{
|
||||
name: 'default',
|
||||
isFullPath: true,
|
||||
...webhookDescriptionFields({
|
||||
path: fromParameter('path'),
|
||||
httpMethod: fromParameter('httpMethod', 'GET'),
|
||||
responseCode: fromFunction((p: INodeParameters) => p.responseCode as number),
|
||||
}),
|
||||
},
|
||||
];
|
||||
|
||||
const setupMocks = ({
|
||||
nodeType = WEBHOOK_NODE_TYPE,
|
||||
typeVersion = 2.1,
|
||||
parameters = { path: webhookPath, httpMethod },
|
||||
webhooks = nativelyResolvableWebhooks,
|
||||
credentials,
|
||||
}: {
|
||||
nodeType?: string;
|
||||
typeVersion?: number;
|
||||
parameters?: INodeParameters;
|
||||
webhooks?: IWebhookDescription[];
|
||||
credentials?: INode['credentials'];
|
||||
} = {}) => {
|
||||
const node: INode = {
|
||||
id: 'trigger-node',
|
||||
name: nodeName,
|
||||
type: nodeType,
|
||||
typeVersion,
|
||||
position: [0, 0],
|
||||
parameters,
|
||||
credentials,
|
||||
};
|
||||
|
||||
const activeVersion = mock<WorkflowHistory>({
|
||||
versionId: 'v1',
|
||||
workflowId: WORKFLOW_ID,
|
||||
nodes: [node],
|
||||
connections: {},
|
||||
});
|
||||
|
||||
workflowRepository.findOne.mockResolvedValue(
|
||||
mock<WorkflowEntity>({
|
||||
id: WORKFLOW_ID,
|
||||
active: true,
|
||||
activeVersionId: 'v1',
|
||||
nodes: [node],
|
||||
connections: {},
|
||||
staticData: {},
|
||||
activeVersion,
|
||||
shared: [{ role: 'workflow:owner', project: { id: 'project-1', projectRelations: [] } }],
|
||||
}),
|
||||
);
|
||||
|
||||
webhookService.findWebhook.mockResolvedValue(
|
||||
mock<WebhookEntity>({
|
||||
workflowId: WORKFLOW_ID,
|
||||
node: nodeName,
|
||||
webhookPath,
|
||||
method: httpMethod,
|
||||
isDynamic: false,
|
||||
}),
|
||||
);
|
||||
webhookService.getWebhookMethods.mockResolvedValue([httpMethod]);
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue({
|
||||
description: {
|
||||
displayName: 'Trigger',
|
||||
name: nodeType,
|
||||
group: ['trigger'],
|
||||
version: 2.1,
|
||||
description: '',
|
||||
defaults: { name: 'Trigger' },
|
||||
inputs: [],
|
||||
outputs: ['main'],
|
||||
properties: nodeProperties,
|
||||
webhooks,
|
||||
},
|
||||
webhook: vi.fn(),
|
||||
});
|
||||
webhookService.getNodeWebhooks.mockReturnValue([
|
||||
mock<IWebhookData>({
|
||||
httpMethod,
|
||||
path: webhookPath,
|
||||
node: nodeName,
|
||||
webhookDescription: { nodeType: undefined } as never,
|
||||
workflowId: WORKFLOW_ID,
|
||||
}),
|
||||
]);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/require-await
|
||||
(WebhookHelpers.executeWebhook as Mock).mockImplementation(async (...args: unknown[]) => {
|
||||
const webhookCallback = args[args.length - 1] as (
|
||||
error: Error | null,
|
||||
data: object,
|
||||
) => void;
|
||||
void webhookCallback(null, {});
|
||||
});
|
||||
|
||||
return mock<WebhookRequest>({ method: httpMethod, params: { path: webhookPath } });
|
||||
};
|
||||
|
||||
let acquireIsolate: MockInstance<WorkflowExpression['acquireIsolate']>;
|
||||
let releaseIsolate: MockInstance<WorkflowExpression['releaseIsolate']>;
|
||||
|
||||
beforeEach(() => {
|
||||
acquireIsolate = vi
|
||||
.spyOn(WorkflowExpression.prototype, 'acquireIsolate')
|
||||
.mockResolvedValue(true);
|
||||
releaseIsolate = vi
|
||||
.spyOn(WorkflowExpression.prototype, 'releaseIsolate')
|
||||
.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('skips acquisition for a Webhook node with no expression parameters', async () => {
|
||||
const request = setupMocks();
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).not.toHaveBeenCalled();
|
||||
// Still released: a no-op here, but correct if the phase acquired one.
|
||||
expect(releaseIsolate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('acquires when a node parameter holds an expression', async () => {
|
||||
const request = setupMocks({
|
||||
parameters: {
|
||||
path: webhookPath,
|
||||
httpMethod,
|
||||
options: { onlyRunIf: '={{ $json.body.id === 1 }}' },
|
||||
},
|
||||
});
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('acquires for typeVersion 1, whose body parsing evaluates a template', async () => {
|
||||
const request = setupMocks({ typeVersion: 1 });
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still skips for the n8nOAuth2 authentication mode', async () => {
|
||||
const request = setupMocks({
|
||||
parameters: { path: webhookPath, httpMethod, authentication: 'n8nOAuth2' },
|
||||
});
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('acquires for a trigger type that is not on the allowlist', async () => {
|
||||
const request = setupMocks({ nodeType: 'n8n-nodes-base.formTrigger' });
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('acquires when a webhook description has a template it cannot resolve natively', async () => {
|
||||
const request = setupMocks({
|
||||
webhooks: [
|
||||
{
|
||||
name: 'default',
|
||||
httpMethod: 'POST',
|
||||
path: '={{$parameter["path"]}}',
|
||||
responseCode: '={{(function (p) { return p.responseCode; })($parameter)}}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('acquires when a webhook description nests a template it cannot resolve natively', async () => {
|
||||
const request = setupMocks({
|
||||
webhooks: [
|
||||
{
|
||||
name: 'default',
|
||||
httpMethod: 'POST',
|
||||
path: '={{$parameter["path"]}}',
|
||||
responseHeaders: { entries: [{ name: 'x-a', value: '={{ $json.body.a }}' }] },
|
||||
} as unknown as IWebhookDescription,
|
||||
],
|
||||
});
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('acquires when the node type declares no webhooks', async () => {
|
||||
const request = setupMocks({ webhooks: [] });
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still skips for a node that uses credentials', async () => {
|
||||
const request = setupMocks({ credentials: { httpHeaderAuth: { id: '1', name: 'auth' } } });
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('acquires when the kill switch is off', async () => {
|
||||
expressionEngineConfig.allowWebhookIsolateSkip = false;
|
||||
const request = setupMocks();
|
||||
|
||||
await liveWebhooks.executeWebhook(request, mock<Response>());
|
||||
|
||||
expect(acquireIsolate).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
import type {
|
||||
INode,
|
||||
INodeParameters,
|
||||
IWebhookData,
|
||||
IWorkflowDataProxyAdditionalKeys,
|
||||
Workflow,
|
||||
} from 'n8n-workflow';
|
||||
import { fromFunction, fromParameter, webhookDescriptionFields } from 'n8n-workflow';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { WebhookExecutionContext } from '../webhook-execution-context';
|
||||
|
||||
// Every description-field read goes through this class, so the native-resolver
|
||||
// short-circuit has to hold on each one: a field that reaches the engine 500s
|
||||
// on a request that skipped acquiring an isolate.
|
||||
describe('WebhookExecutionContext', () => {
|
||||
const workflow = mock<Workflow>({
|
||||
expression: mock<Workflow['expression']>(),
|
||||
});
|
||||
|
||||
const webhookDescription = {
|
||||
name: 'default' as const,
|
||||
httpMethod: 'POST',
|
||||
...webhookDescriptionFields({
|
||||
path: fromParameter('path'),
|
||||
responseData: fromFunction((p: INodeParameters) =>
|
||||
p.responseMode === 'lastNode' ? 'noData' : undefined,
|
||||
),
|
||||
}),
|
||||
};
|
||||
|
||||
const buildContext = (node: INode) =>
|
||||
new WebhookExecutionContext(
|
||||
workflow,
|
||||
node,
|
||||
// A plain object, not `mock<IWebhookData>`: a deep mock auto-creates a
|
||||
// resolver for every field, defeating the point of these tests.
|
||||
{ webhookDescription } as unknown as IWebhookData,
|
||||
'trigger',
|
||||
mock<IWorkflowDataProxyAdditionalKeys>(),
|
||||
);
|
||||
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
describe('with static node parameters', () => {
|
||||
const context = buildContext(
|
||||
mock<INode>({ name: 'Webhook', parameters: { path: 'my-path', responseMode: 'lastNode' } }),
|
||||
);
|
||||
|
||||
it('resolves a simple value natively without touching the engine', () => {
|
||||
expect(context.evaluateSimpleWebhookDescriptionExpression('path')).toBe('my-path');
|
||||
expect(workflow.expression.getSimpleParameterValue).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('resolves a complex value natively without touching the engine', () => {
|
||||
expect(context.evaluateComplexWebhookDescriptionExpression('responseData')).toBe('noData');
|
||||
expect(workflow.expression.getComplexParameterValue).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns a natively resolved undefined as-is, not the default value', () => {
|
||||
const undefinedContext = buildContext(
|
||||
mock<INode>({ name: 'Webhook', parameters: { responseMode: 'onReceived' } }),
|
||||
);
|
||||
|
||||
expect(
|
||||
undefinedContext.evaluateComplexWebhookDescriptionExpression(
|
||||
'responseData',
|
||||
undefined,
|
||||
'firstEntryJson',
|
||||
),
|
||||
).toBeUndefined();
|
||||
expect(workflow.expression.getComplexParameterValue).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to the engine for a field without a resolver', () => {
|
||||
context.evaluateSimpleWebhookDescriptionExpression('httpMethod');
|
||||
|
||||
expect(workflow.expression.getSimpleParameterValue).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
'POST',
|
||||
'trigger',
|
||||
expect.anything(),
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('with an expression in the node parameters', () => {
|
||||
const context = buildContext(
|
||||
mock<INode>({ name: 'Webhook', parameters: { path: '={{ $json.path }}' } }),
|
||||
);
|
||||
|
||||
it('falls back to the engine even for a field with a resolver', () => {
|
||||
context.evaluateSimpleWebhookDescriptionExpression('path');
|
||||
|
||||
expect(workflow.expression.getSimpleParameterValue).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
'={{$parameter["path"]}}',
|
||||
'trigger',
|
||||
expect.anything(),
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -7,7 +7,12 @@ import type {
|
||||
IWebhookData,
|
||||
IWorkflowExecuteAdditionalData,
|
||||
} from 'n8n-workflow';
|
||||
import { Workflow, WebhookPathTakenError } from 'n8n-workflow';
|
||||
import {
|
||||
Workflow,
|
||||
WebhookPathTakenError,
|
||||
webhookDescriptionFields,
|
||||
fromParameter,
|
||||
} from 'n8n-workflow';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
@@ -590,6 +595,45 @@ describe('WebhookService', () => {
|
||||
expect(webhooks[0].path).not.toMatch(/\s/);
|
||||
expect(webhooks[0].path).toMatch(/\/path$/);
|
||||
});
|
||||
|
||||
test('should resolve declared fields natively, without the expression engine', async () => {
|
||||
const node = {
|
||||
name: 'Webhook',
|
||||
type: 'n8n-nodes-base.webhook',
|
||||
disabled: false,
|
||||
parameters: { path: 'native-path', httpMethod: 'POST' },
|
||||
} as unknown as INode;
|
||||
|
||||
const fields = webhookDescriptionFields({
|
||||
httpMethod: fromParameter('httpMethod', 'GET'),
|
||||
path: fromParameter('path'),
|
||||
});
|
||||
const nodeType = {
|
||||
description: {
|
||||
webhooks: [
|
||||
{
|
||||
name: 'default',
|
||||
...fields,
|
||||
isFullPath: false,
|
||||
restartWebhook: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
} as INodeType;
|
||||
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
|
||||
const engineSpy = vi.spyOn(workflow.expression, 'getSimpleParameterValue');
|
||||
|
||||
const webhooks = webhookService.getNodeWebhooks(workflow, node, additionalData);
|
||||
|
||||
expect(webhooks).toHaveLength(1);
|
||||
expect(webhooks[0]).toMatchObject({ httpMethod: 'POST' });
|
||||
expect(webhooks[0].path).toMatch(/\/native-path$/);
|
||||
// fields with declared resolvers must never engage the expression engine
|
||||
const engineEvaluatedValues = engineSpy.mock.calls.map((call) => call[1]);
|
||||
expect(engineEvaluatedValues).not.toContain(fields.path);
|
||||
expect(engineEvaluatedValues).not.toContain(fields.httpMethod);
|
||||
});
|
||||
});
|
||||
|
||||
describe('createWebhookIfNotExists()', () => {
|
||||
|
||||
@@ -1,9 +1,15 @@
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { WorkflowsConfig } from '@n8n/config';
|
||||
import { ExpressionEngineConfig, WorkflowsConfig } from '@n8n/config';
|
||||
import { WorkflowRepository, type WorkflowEntity, type WorkflowHistory } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { Response } from 'express';
|
||||
import { Workflow, CHAT_TRIGGER_NODE_TYPE } from 'n8n-workflow';
|
||||
import {
|
||||
Workflow,
|
||||
CHAT_TRIGGER_NODE_TYPE,
|
||||
WEBHOOK_NODE_TYPE,
|
||||
nodeParametersAreStatic,
|
||||
webhookDescriptionIsNativelyResolvable,
|
||||
} from 'n8n-workflow';
|
||||
import type { INode, IWebhookData, IHttpRequestMethods, IWorkflowBase } from 'n8n-workflow';
|
||||
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
@@ -41,6 +47,7 @@ export class LiveWebhooks implements IWebhookManager {
|
||||
private readonly workflowStaticDataService: WorkflowStaticDataService,
|
||||
private readonly workflowsConfig: WorkflowsConfig,
|
||||
private readonly workflowPublishedDataService: WorkflowPublishedDataService,
|
||||
private readonly expressionEngineConfig: ExpressionEngineConfig,
|
||||
) {}
|
||||
|
||||
async getWebhookMethods(path: string) {
|
||||
@@ -131,10 +138,15 @@ export class LiveWebhooks implements IWebhookManager {
|
||||
projectId: ownerProjectId,
|
||||
});
|
||||
|
||||
await workflow.expression.acquireIsolate();
|
||||
const startNode = workflow.getNode(webhook.node);
|
||||
|
||||
if (this.webhookPhaseNeedsIsolate(startNode)) {
|
||||
await workflow.expression.acquireIsolate();
|
||||
}
|
||||
|
||||
try {
|
||||
const webhookData = this.webhookService
|
||||
.getNodeWebhooks(workflow, workflow.getNode(webhook.node) as INode, additionalData)
|
||||
.getNodeWebhooks(workflow, startNode as INode, additionalData)
|
||||
.find((w) => w.httpMethod === httpMethod && w.path === webhook.webhookPath) as IWebhookData;
|
||||
|
||||
if (
|
||||
@@ -183,10 +195,38 @@ export class LiveWebhooks implements IWebhookManager {
|
||||
).catch(reject); // ensure the Promise settles even if executeWebhook throws
|
||||
});
|
||||
} finally {
|
||||
// A no-op when the acquire was skipped.
|
||||
await workflow.expression.releaseIsolate();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Expression Engine VM acquisition builds a V8 isolate per request, which is
|
||||
* worth skipping when the webhook phase provably evaluates nothing: every
|
||||
* description field of the trigger resolves natively (see
|
||||
* `webhookDescriptionFields` in n8n-workflow) and the node's own parameters
|
||||
* contain no expressions. Anything not proven below acquires eagerly.
|
||||
*/
|
||||
private webhookPhaseNeedsIsolate(startNode: INode | null): boolean {
|
||||
if (!this.expressionEngineConfig.allowWebhookIsolateSkip) return true;
|
||||
if (startNode === null) return true;
|
||||
|
||||
// Extend only after reviewing the node type's webhook() for
|
||||
// evaluateExpression() calls or other internal evaluations.
|
||||
if (startNode.type !== WEBHOOK_NODE_TYPE) return true;
|
||||
|
||||
// typeVersion 1 body parsing evaluates a hardcoded template.
|
||||
if (startNode.typeVersion === 1) return true;
|
||||
|
||||
if (!nodeParametersAreStatic(startNode)) return true;
|
||||
|
||||
const webhooks = this.nodeTypes.getByNameAndVersion(startNode.type, startNode.typeVersion)
|
||||
?.description.webhooks;
|
||||
if (!webhooks?.length) return true;
|
||||
|
||||
return !webhooks.every(webhookDescriptionIsNativelyResolvable);
|
||||
}
|
||||
|
||||
private async loadWebhookExecutionData(
|
||||
workflowId: string,
|
||||
): Promise<{ workflow: WorkflowEntity; publishedVersion: WorkflowHistory }> {
|
||||
|
||||
@@ -8,6 +8,10 @@ import type {
|
||||
IWebhookDescription,
|
||||
NodeParameterValueType,
|
||||
} from 'n8n-workflow';
|
||||
import { resolveWebhookDescriptionField } from 'n8n-workflow';
|
||||
|
||||
/** The description's evaluable fields — the symbol key holds the resolver map. */
|
||||
type WebhookDescriptionKey = Exclude<keyof IWebhookDescription, symbol>;
|
||||
|
||||
/**
|
||||
* A helper class that holds the context for the webhook execution.
|
||||
@@ -26,10 +30,13 @@ export class WebhookExecutionContext {
|
||||
* Evaluates a simple expression from the webhook description.
|
||||
*/
|
||||
evaluateSimpleWebhookDescriptionExpression<T extends boolean | number | string | unknown[]>(
|
||||
propertyName: keyof IWebhookDescription,
|
||||
propertyName: WebhookDescriptionKey,
|
||||
executeData?: IExecuteData,
|
||||
defaultValue?: T,
|
||||
): T | undefined {
|
||||
const native = this.resolveNatively(propertyName);
|
||||
if (native.resolved) return native.value as T | undefined;
|
||||
|
||||
return this.workflow.expression.getSimpleParameterValue(
|
||||
this.workflowStartNode,
|
||||
this.webhookData.webhookDescription[propertyName],
|
||||
@@ -44,10 +51,13 @@ export class WebhookExecutionContext {
|
||||
* Evaluates a complex expression from the webhook description.
|
||||
*/
|
||||
evaluateComplexWebhookDescriptionExpression<T extends NodeParameterValueType>(
|
||||
propertyName: keyof IWebhookDescription,
|
||||
propertyName: WebhookDescriptionKey,
|
||||
executeData?: IExecuteData,
|
||||
defaultValue?: T,
|
||||
): T | undefined {
|
||||
const native = this.resolveNatively(propertyName);
|
||||
if (native.resolved) return native.value as T | undefined;
|
||||
|
||||
return this.workflow.expression.getComplexParameterValue(
|
||||
this.workflowStartNode,
|
||||
this.webhookData.webhookDescription[propertyName],
|
||||
@@ -57,4 +67,19 @@ export class WebhookExecutionContext {
|
||||
defaultValue,
|
||||
) as T | undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a description field without the expression engine when the field
|
||||
* declares a native resolver and the node's parameters are static (see
|
||||
* `webhookDescriptionFields` in n8n-workflow). Like the engine path, the
|
||||
* resolved value is returned as-is: `defaultValue` only stands in for a
|
||||
* field the description does not define at all.
|
||||
*/
|
||||
private resolveNatively(propertyName: WebhookDescriptionKey) {
|
||||
return resolveWebhookDescriptionField(
|
||||
this.workflowStartNode,
|
||||
this.webhookData.webhookDescription,
|
||||
String(propertyName),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -543,14 +543,7 @@ export async function executeWebhook(
|
||||
responsePropertyName,
|
||||
responseContentType,
|
||||
responseBinaryPropertyName,
|
||||
} = evaluateResponseOptions(
|
||||
workflowStartNode,
|
||||
workflow,
|
||||
req,
|
||||
webhookData,
|
||||
executionMode,
|
||||
additionalKeys,
|
||||
);
|
||||
} = evaluateResponseOptions(context, req);
|
||||
|
||||
if (
|
||||
!['onReceived', 'lastNode', 'responseNode', 'formPage', 'streaming', 'hostedChat'].includes(
|
||||
@@ -1137,71 +1130,44 @@ export async function executeWebhook(
|
||||
/**
|
||||
* Evaluates the response mode, code and data for a webhook node
|
||||
*/
|
||||
function evaluateResponseOptions(
|
||||
workflowStartNode: INode,
|
||||
workflow: Workflow,
|
||||
req: WebhookRequest,
|
||||
webhookData: IWebhookData,
|
||||
executionMode: WorkflowExecuteMode,
|
||||
additionalKeys: IWorkflowDataProxyAdditionalKeys,
|
||||
) {
|
||||
function evaluateResponseOptions(context: WebhookExecutionContext, req: WebhookRequest) {
|
||||
const { workflow, workflowStartNode } = context;
|
||||
|
||||
//check if response mode should be set automatically, e.g. multipage form
|
||||
const responseMode =
|
||||
autoDetectResponseMode(workflowStartNode, workflow, req.method) ??
|
||||
(workflow.expression.getSimpleParameterValue(
|
||||
workflowStartNode,
|
||||
webhookData.webhookDescription.responseMode,
|
||||
executionMode,
|
||||
additionalKeys,
|
||||
context.evaluateSimpleWebhookDescriptionExpression<WebhookResponseMode>(
|
||||
'responseMode',
|
||||
undefined,
|
||||
'onReceived',
|
||||
) as WebhookResponseMode);
|
||||
)!;
|
||||
|
||||
const responseCode = workflow.expression.getSimpleParameterValue(
|
||||
workflowStartNode,
|
||||
webhookData.webhookDescription.responseCode as string,
|
||||
executionMode,
|
||||
additionalKeys,
|
||||
const responseCode = context.evaluateSimpleWebhookDescriptionExpression<number>(
|
||||
'responseCode',
|
||||
undefined,
|
||||
200,
|
||||
) as number;
|
||||
)!;
|
||||
|
||||
// This parameter is used for two different purposes:
|
||||
// 1. as arbitrary string input defined in the workflow in the "respond immediately" mode,
|
||||
// 2. as well as WebhookResponseData config in all the other modes
|
||||
const responseData = workflow.expression.getComplexParameterValue(
|
||||
workflowStartNode,
|
||||
webhookData.webhookDescription.responseData,
|
||||
executionMode,
|
||||
additionalKeys,
|
||||
undefined,
|
||||
'firstEntryJson',
|
||||
) as WebhookResponseData | string | undefined;
|
||||
const responseData = context.evaluateComplexWebhookDescriptionExpression<
|
||||
WebhookResponseData | string
|
||||
>('responseData', undefined, 'firstEntryJson');
|
||||
|
||||
// This is needed for backward compatibility, where only the first main output was checked for data.
|
||||
// We want to keep existing behavior for webhooks, but change for chat triggers, where checking all main outputs makes more sense.
|
||||
// We can unify the behavior in the next major release and get rid of this flag
|
||||
const checkAllMainOutputs = workflowStartNode.type === CHAT_TRIGGER_NODE_TYPE;
|
||||
|
||||
const responsePropertyName = workflow.expression.getSimpleParameterValue(
|
||||
workflowStartNode,
|
||||
webhookData.webhookDescription.responsePropertyName,
|
||||
executionMode,
|
||||
additionalKeys,
|
||||
) as string | undefined;
|
||||
const responsePropertyName =
|
||||
context.evaluateSimpleWebhookDescriptionExpression<string>('responsePropertyName');
|
||||
|
||||
const responseContentType = workflow.expression.getSimpleParameterValue(
|
||||
workflowStartNode,
|
||||
webhookData.webhookDescription.responseContentType,
|
||||
executionMode,
|
||||
additionalKeys,
|
||||
) as string | undefined;
|
||||
const responseContentType =
|
||||
context.evaluateSimpleWebhookDescriptionExpression<string>('responseContentType');
|
||||
|
||||
const responseBinaryPropertyName = workflow.expression.getSimpleParameterValue(
|
||||
workflowStartNode,
|
||||
webhookData.webhookDescription.responseBinaryPropertyName,
|
||||
executionMode,
|
||||
additionalKeys,
|
||||
const responseBinaryPropertyName = context.evaluateSimpleWebhookDescriptionExpression<string>(
|
||||
'responseBinaryPropertyName',
|
||||
undefined,
|
||||
'data',
|
||||
);
|
||||
|
||||
@@ -7,6 +7,7 @@ import { ensureError } from '@n8n/utils/errors/ensure-error';
|
||||
import {
|
||||
isNodeClassInstance,
|
||||
NodeHelpers,
|
||||
resolveWebhookDescriptionField,
|
||||
UnexpectedError,
|
||||
WebhookPathTakenError,
|
||||
} from 'n8n-workflow';
|
||||
@@ -15,6 +16,7 @@ import type {
|
||||
INode,
|
||||
IRunExecutionData,
|
||||
IWebhookData,
|
||||
IWebhookDescription,
|
||||
IWebhookResponseData,
|
||||
IWorkflowExecuteAdditionalData,
|
||||
WebhookSetupMethodNames,
|
||||
@@ -331,7 +333,6 @@ export class WebhookService {
|
||||
}
|
||||
|
||||
const workflowId = workflow.id || '__UNSAVED__';
|
||||
const mode = 'internal';
|
||||
|
||||
const returnData: IWebhookData[] = [];
|
||||
for (const webhookDescription of nodeType.description.webhooks) {
|
||||
@@ -339,13 +340,13 @@ export class WebhookService {
|
||||
continue;
|
||||
}
|
||||
|
||||
let nodeWebhookPath = workflow.expression.getSimpleParameterValue(
|
||||
let nodeWebhookPath = this.evaluateDescriptionProperty(
|
||||
workflow,
|
||||
node,
|
||||
webhookDescription.path,
|
||||
mode,
|
||||
{},
|
||||
webhookDescription,
|
||||
'path',
|
||||
);
|
||||
if (nodeWebhookPath === undefined) {
|
||||
if (nodeWebhookPath === undefined || nodeWebhookPath === null) {
|
||||
this.logger.error(
|
||||
`No webhook path could be found for node "${node.name}" in workflow "${workflowId}".`,
|
||||
);
|
||||
@@ -361,20 +362,18 @@ export class WebhookService {
|
||||
nodeWebhookPath = nodeWebhookPath.slice(0, -1);
|
||||
}
|
||||
|
||||
const isFullPath: boolean = workflow.expression.getSimpleParameterValue(
|
||||
const isFullPath = this.evaluateDescriptionProperty(
|
||||
workflow,
|
||||
node,
|
||||
webhookDescription.isFullPath,
|
||||
'internal',
|
||||
{},
|
||||
undefined,
|
||||
webhookDescription,
|
||||
'isFullPath',
|
||||
false,
|
||||
) as boolean;
|
||||
const restartWebhook: boolean = workflow.expression.getSimpleParameterValue(
|
||||
const restartWebhook = this.evaluateDescriptionProperty(
|
||||
workflow,
|
||||
node,
|
||||
webhookDescription.restartWebhook,
|
||||
'internal',
|
||||
{},
|
||||
undefined,
|
||||
webhookDescription,
|
||||
'restartWebhook',
|
||||
false,
|
||||
) as boolean;
|
||||
const path = NodeHelpers.getNodeWebhookPath(
|
||||
@@ -385,12 +384,11 @@ export class WebhookService {
|
||||
restartWebhook,
|
||||
);
|
||||
|
||||
const webhookMethods = workflow.expression.getSimpleParameterValue(
|
||||
const webhookMethods = this.evaluateDescriptionProperty(
|
||||
workflow,
|
||||
node,
|
||||
webhookDescription.httpMethod,
|
||||
mode,
|
||||
{},
|
||||
undefined,
|
||||
webhookDescription,
|
||||
'httpMethod',
|
||||
'GET',
|
||||
);
|
||||
|
||||
@@ -426,6 +424,32 @@ export class WebhookService {
|
||||
return returnData;
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluates a webhook-description property, preferring the field's native
|
||||
* resolver (see `webhookDescriptionFields` in n8n-workflow) so static-parameter
|
||||
* nodes never engage the expression engine. Falls back to the engine, which
|
||||
* returns plain values as-is and only evaluates `=` templates.
|
||||
*/
|
||||
private evaluateDescriptionProperty(
|
||||
workflow: Workflow,
|
||||
node: INode,
|
||||
webhookDescription: IWebhookDescription,
|
||||
property: string,
|
||||
defaultValue?: string | boolean,
|
||||
) {
|
||||
const native = resolveWebhookDescriptionField(node, webhookDescription, property);
|
||||
if (native.resolved) return native.value;
|
||||
|
||||
return workflow.expression.getSimpleParameterValue(
|
||||
node,
|
||||
webhookDescription[property],
|
||||
'internal',
|
||||
{},
|
||||
undefined,
|
||||
defaultValue,
|
||||
);
|
||||
}
|
||||
|
||||
private async _findWebhookConflicts(
|
||||
workflow: Workflow,
|
||||
checkEntries: Array<{
|
||||
|
||||
Reference in New Issue
Block a user