mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-24 23:22:38 +08:00
fix(core): Check all outputs for chat triggers, first output only for webhooks (#20308)
This commit is contained in:
@@ -117,7 +117,7 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should return data from second branch when first is empty', async () => {
|
||||
it('should return error when second branch has data but first is empty (default behavior)', async () => {
|
||||
const jsonData = { foo: 'bar', fromSecondBranch: true };
|
||||
lastNodeTaskData.data = {
|
||||
main: [
|
||||
@@ -132,6 +132,27 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
lastNodeTaskData,
|
||||
);
|
||||
|
||||
assert(!result.ok);
|
||||
expect(result.error).toBeInstanceOf(OperationalError);
|
||||
expect(result.error.message).toBe('No item to return was found');
|
||||
});
|
||||
|
||||
it('should return data from second branch when first is empty and checkAllMainOutputs is true', async () => {
|
||||
const jsonData = { foo: 'bar', fromSecondBranch: true };
|
||||
lastNodeTaskData.data = {
|
||||
main: [
|
||||
[], // First branch is empty
|
||||
[{ json: jsonData }], // Second branch has data
|
||||
],
|
||||
};
|
||||
|
||||
const result = await extractWebhookLastNodeResponse(
|
||||
context,
|
||||
'firstEntryJson',
|
||||
lastNodeTaskData,
|
||||
true, // checkAllMainOutputs = true
|
||||
);
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
result: {
|
||||
@@ -330,7 +351,7 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should return binary data from second branch when first is empty', async () => {
|
||||
it('should return error when second branch has binary but first is empty (default behavior)', async () => {
|
||||
const binaryData: IBinaryData = {
|
||||
data: Buffer.from('binary from second branch').toString(BINARY_ENCODING),
|
||||
mimeType: 'text/plain',
|
||||
@@ -354,6 +375,36 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
lastNodeTaskData,
|
||||
);
|
||||
|
||||
assert(!result.ok);
|
||||
expect(result.error).toBeInstanceOf(OperationalError);
|
||||
expect(result.error.message).toBe('No item was found to return');
|
||||
});
|
||||
|
||||
it('should return binary from second branch when first is empty and checkAllMainOutputs is true', async () => {
|
||||
const binaryData: IBinaryData = {
|
||||
data: Buffer.from('binary from second branch').toString(BINARY_ENCODING),
|
||||
mimeType: 'text/plain',
|
||||
};
|
||||
const nodeExecutionData: INodeExecutionData = {
|
||||
json: {},
|
||||
binary: { data: binaryData },
|
||||
};
|
||||
lastNodeTaskData.data = {
|
||||
main: [
|
||||
[], // First branch is empty
|
||||
[nodeExecutionData], // Second branch has binary data
|
||||
],
|
||||
};
|
||||
|
||||
context.evaluateSimpleWebhookDescriptionExpression.mockReturnValue('data');
|
||||
|
||||
const result = await extractWebhookLastNodeResponse(
|
||||
context,
|
||||
'firstEntryBinary',
|
||||
lastNodeTaskData,
|
||||
true, // checkAllMainOutputs = true
|
||||
);
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
result: {
|
||||
@@ -434,7 +485,7 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should return all entries from second branch when first is empty', async () => {
|
||||
it('should return empty array when second branch has data but first is empty (default behavior)', async () => {
|
||||
const jsonData1 = { item: 1, fromSecondBranch: true };
|
||||
const jsonData2 = { item: 2, fromSecondBranch: true };
|
||||
const jsonData3 = { item: 3, fromSecondBranch: true };
|
||||
@@ -447,6 +498,34 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
|
||||
const result = await extractWebhookLastNodeResponse(context, 'allEntries', lastNodeTaskData);
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
result: {
|
||||
type: 'static',
|
||||
body: [], // Old behavior: returns empty array when first branch is empty
|
||||
contentType: undefined,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('should return all entries from second branch when first is empty and checkAllMainOutputs is true', async () => {
|
||||
const jsonData1 = { item: 1, fromSecondBranch: true };
|
||||
const jsonData2 = { item: 2, fromSecondBranch: true };
|
||||
const jsonData3 = { item: 3, fromSecondBranch: true };
|
||||
lastNodeTaskData.data = {
|
||||
main: [
|
||||
[], // First branch is empty
|
||||
[{ json: jsonData1 }, { json: jsonData2 }, { json: jsonData3 }], // Second branch has data
|
||||
],
|
||||
};
|
||||
|
||||
const result = await extractWebhookLastNodeResponse(
|
||||
context,
|
||||
'allEntries',
|
||||
lastNodeTaskData,
|
||||
true, // checkAllMainOutputs = true
|
||||
);
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
result: {
|
||||
@@ -457,13 +536,14 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should return entries from first non-empty branch only', async () => {
|
||||
it('should return entries from first branch only even when multiple branches have data', async () => {
|
||||
const branch1Data = { item: 'from-first' };
|
||||
const branch2Data = { item: 'from-second' };
|
||||
const branch3Data = { item: 'from-third' };
|
||||
lastNodeTaskData.data = {
|
||||
main: [
|
||||
[], // First branch is empty
|
||||
[{ json: branch2Data }], // Second branch has data - this should be used
|
||||
[{ json: branch1Data }], // First branch has data - this should be used
|
||||
[{ json: branch2Data }], // Second branch also has data - should be ignored
|
||||
[{ json: branch3Data }], // Third branch also has data - should be ignored
|
||||
],
|
||||
};
|
||||
@@ -474,7 +554,7 @@ describe('extractWebhookLastNodeResponse', () => {
|
||||
ok: true,
|
||||
result: {
|
||||
type: 'static',
|
||||
body: [branch2Data], // Only data from second branch
|
||||
body: [branch1Data], // Only data from first branch
|
||||
contentType: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -402,7 +402,7 @@ export async function executeWebhook(
|
||||
additionalData.executionId = executionId;
|
||||
}
|
||||
|
||||
const { responseMode, responseCode, responseData } = evaluateResponseOptions(
|
||||
const { responseMode, responseCode, responseData, checkAllMainOutputs } = evaluateResponseOptions(
|
||||
workflowStartNode,
|
||||
workflow,
|
||||
req,
|
||||
@@ -718,7 +718,9 @@ export async function executeWebhook(
|
||||
context,
|
||||
responseData as WebhookResponseData,
|
||||
lastNodeTaskData,
|
||||
checkAllMainOutputs,
|
||||
);
|
||||
|
||||
if (!result.ok) {
|
||||
responseCallback(result.error, {});
|
||||
didSendResponse = true;
|
||||
@@ -813,7 +815,12 @@ function evaluateResponseOptions(
|
||||
'firstEntryJson',
|
||||
) as WebhookResponseData | string | undefined;
|
||||
|
||||
return { responseMode, responseCode, responseData };
|
||||
// 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;
|
||||
|
||||
return { responseMode, responseCode, responseData, checkAllMainOutputs };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -23,18 +23,27 @@ type StreamResponse = {
|
||||
/**
|
||||
+ * Extracts the response for a webhook when the response mode is set to
|
||||
* `lastNode`.
|
||||
* Note: We can check either all main outputs or just the first one.
|
||||
* For the backward compatibility, by default we only check the first main output.
|
||||
* But when the `checkAllMainOutputs` is set to true, we check all main outputs
|
||||
* until we find one that has data.
|
||||
*/
|
||||
export async function extractWebhookLastNodeResponse(
|
||||
context: WebhookExecutionContext,
|
||||
responseDataType: WebhookResponseData | undefined,
|
||||
lastNodeTaskData: ITaskData,
|
||||
checkAllMainOutputs: boolean = false,
|
||||
): Promise<Result<StaticResponse | StreamResponse, OperationalError>> {
|
||||
if (responseDataType === 'firstEntryJson') {
|
||||
return extractFirstEntryJsonFromTaskData(context, lastNodeTaskData);
|
||||
return extractFirstEntryJsonFromTaskData(context, lastNodeTaskData, checkAllMainOutputs);
|
||||
}
|
||||
|
||||
if (responseDataType === 'firstEntryBinary') {
|
||||
return await extractFirstEntryBinaryFromTaskData(context, lastNodeTaskData);
|
||||
return await extractFirstEntryBinaryFromTaskData(
|
||||
context,
|
||||
lastNodeTaskData,
|
||||
checkAllMainOutputs,
|
||||
);
|
||||
}
|
||||
|
||||
if (responseDataType === 'noData') {
|
||||
@@ -46,15 +55,16 @@ export async function extractWebhookLastNodeResponse(
|
||||
}
|
||||
|
||||
// Default to all entries JSON
|
||||
return extractAllEntriesJsonFromTaskData(lastNodeTaskData);
|
||||
return extractAllEntriesJsonFromTaskData(lastNodeTaskData, checkAllMainOutputs);
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the JSON data of the first item of the first non-empty branch of the last node
|
||||
* Extracts the JSON data of the first item of the last node
|
||||
*/
|
||||
function extractFirstEntryJsonFromTaskData(
|
||||
context: WebhookExecutionContext,
|
||||
lastNodeTaskData: ITaskData,
|
||||
checkAllMainOutputs: boolean = false,
|
||||
): Result<StaticResponse, OperationalError> {
|
||||
const mainOutputs = lastNodeTaskData.data?.main;
|
||||
let firstItem: INodeExecutionData | undefined;
|
||||
@@ -65,6 +75,11 @@ function extractFirstEntryJsonFromTaskData(
|
||||
firstItem = branch[0];
|
||||
break; // Stop after processing the first non-empty branch
|
||||
}
|
||||
|
||||
if (!checkAllMainOutputs) {
|
||||
// If we should not check all main outputs, stop after the first one
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,11 +109,12 @@ function extractFirstEntryJsonFromTaskData(
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the binary data of the first item of the first non-empty branch of the last node
|
||||
* Extracts the binary data of the first item of the last node
|
||||
*/
|
||||
async function extractFirstEntryBinaryFromTaskData(
|
||||
context: WebhookExecutionContext,
|
||||
lastNodeTaskData: ITaskData,
|
||||
checkAllMainOutputs: boolean = false,
|
||||
): Promise<Result<StaticResponse | StreamResponse, OperationalError>> {
|
||||
const mainOutputs = lastNodeTaskData.data?.main;
|
||||
let lastNodeFirstJsonItem: INodeExecutionData | undefined;
|
||||
@@ -110,6 +126,11 @@ async function extractFirstEntryBinaryFromTaskData(
|
||||
lastNodeFirstJsonItem = branch[0];
|
||||
break; // Stop after processing the first non-empty branch
|
||||
}
|
||||
|
||||
if (!checkAllMainOutputs) {
|
||||
// If we should not check all main outputs, stop after the first one
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -161,10 +182,11 @@ async function extractFirstEntryBinaryFromTaskData(
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the JSON data of all the items from the first non-empty branch of the last node
|
||||
* Extracts the JSON data of all the items of the last node
|
||||
*/
|
||||
function extractAllEntriesJsonFromTaskData(
|
||||
lastNodeTaskData: ITaskData,
|
||||
checkAllMainOutputs: boolean = false,
|
||||
): Result<StaticResponse, OperationalError> {
|
||||
const data: unknown[] = [];
|
||||
const mainOutputs = lastNodeTaskData.data?.main;
|
||||
@@ -176,8 +198,14 @@ function extractAllEntriesJsonFromTaskData(
|
||||
for (const entry of branch) {
|
||||
data.push(entry.json);
|
||||
}
|
||||
|
||||
break; // Stop after processing the first non-empty branch
|
||||
}
|
||||
|
||||
if (!checkAllMainOutputs) {
|
||||
// If we should not check all main outputs, stop after the first one
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user