diff --git a/packages/cli/src/webhooks/__tests__/webhook-last-node-response-extractor.test.ts b/packages/cli/src/webhooks/__tests__/webhook-last-node-response-extractor.test.ts index d17568d760a..e0bf016178a 100644 --- a/packages/cli/src/webhooks/__tests__/webhook-last-node-response-extractor.test.ts +++ b/packages/cli/src/webhooks/__tests__/webhook-last-node-response-extractor.test.ts @@ -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, }, }); diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index 8aaf147e163..4d08c892e49 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -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 }; } /** diff --git a/packages/cli/src/webhooks/webhook-last-node-response-extractor.ts b/packages/cli/src/webhooks/webhook-last-node-response-extractor.ts index e99a78b4b4e..a51a6e2e9ef 100644 --- a/packages/cli/src/webhooks/webhook-last-node-response-extractor.ts +++ b/packages/cli/src/webhooks/webhook-last-node-response-extractor.ts @@ -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> { 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 { 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> { 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 { 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; + } } }