fix(n8n Form Node): Redirection update (no-changelog) (#13104)

Co-authored-by: Dana <152518854+dana-gill@users.noreply.github.com>

.prettierignore

@@ -5,6 +5,7 @@ pnpm-lock.yaml
 packages/editor-ui/index.html
 packages/nodes-base/nodes/**/test
 packages/cli/templates/form-trigger.handlebars
+packages/cli/templates/form-trigger-completion.handlebars
 cypress/fixtures
 CHANGELOG.md
 .github/pull_request_template.md

packages/cli/src/active-executions.ts

@@ -174,6 +174,22 @@ export class ActiveExecutions {
 		this.logger.debug('Execution finalized', { executionId });
 	}
 
+	/** Resolve the response promise in an execution. */
+	resolveExecutionResponsePromise(executionId: string) {
+		// TODO: This should probably be refactored.
+		// The reason for adding this method is that the Form node works in 'responseNode' mode
+		// and expects the next Form to 'sendResponse' to redirect to the current Form node.
+		// Resolving responsePromise here is needed to complete the redirection chain; otherwise, a manual reload will be required.
+
+		if (!this.has(executionId)) return;
+		const execution = this.getExecutionOrFail(executionId);
+
+		if (execution.status !== 'waiting' && execution?.responsePromise) {
+			execution.responsePromise.resolve({});
+			this.logger.debug('Execution response promise cleaned', { executionId });
+		}
+	}
+
 	/**
 	 * Returns a promise which will resolve with the data of the execution with the given id
 	 */

packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts

@@ -0,0 +1,97 @@
+import { mock, type MockProxy } from 'jest-mock-extended';
+import type { Workflow, INode, IDataObject } from 'n8n-workflow';
+import { FORM_NODE_TYPE, WAIT_NODE_TYPE } from 'n8n-workflow';
+
+import { autoDetectResponseMode, handleFormRedirectionCase } from '../webhook-helpers';
+import type { IWebhookResponseCallbackData } from '../webhook.types';
+
+describe('autoDetectResponseMode', () => {
+	let workflow: MockProxy<Workflow>;
+
+	beforeEach(() => {
+		workflow = mock<Workflow>();
+		workflow.nodes = {};
+	});
+
+	test('should return undefined if start node is WAIT_NODE_TYPE with resume not equal to form', () => {
+		const workflowStartNode = mock<INode>({
+			type: WAIT_NODE_TYPE,
+			parameters: { resume: 'webhook' },
+		});
+		const result = autoDetectResponseMode(workflowStartNode, workflow, 'POST');
+		expect(result).toBeUndefined();
+	});
+
+	test('should return responseNode when start node is FORM_NODE_TYPE and method is POST', () => {
+		const workflowStartNode = mock<INode>({
+			type: FORM_NODE_TYPE,
+			name: 'startNode',
+			parameters: {},
+		});
+		workflow.getChildNodes.mockReturnValue(['childNode']);
+		workflow.nodes.childNode = mock<INode>({
+			type: WAIT_NODE_TYPE,
+			parameters: { resume: 'form' },
+			disabled: false,
+		});
+		const result = autoDetectResponseMode(workflowStartNode, workflow, 'POST');
+		expect(result).toBe('responseNode');
+	});
+
+	test('should return undefined when start node is FORM_NODE_TYPE with no other form child nodes', () => {
+		const workflowStartNode = mock<INode>({
+			type: FORM_NODE_TYPE,
+			name: 'startNode',
+			parameters: {},
+		});
+		workflow.getChildNodes.mockReturnValue([]);
+		const result = autoDetectResponseMode(workflowStartNode, workflow, 'POST');
+		expect(result).toBeUndefined();
+	});
+
+	test('should return undefined for non-matching node type and method', () => {
+		const workflowStartNode = mock<INode>({ type: 'someOtherNodeType', parameters: {} });
+		const result = autoDetectResponseMode(workflowStartNode, workflow, 'GET');
+		expect(result).toBeUndefined();
+	});
+});
+
+describe('handleFormRedirectionCase', () => {
+	test('should return data unchanged if start node is WAIT_NODE_TYPE with resume not equal to form', () => {
+		const data: IWebhookResponseCallbackData = {
+			responseCode: 302,
+			headers: { location: 'http://example.com' },
+		};
+		const workflowStartNode = mock<INode>({
+			type: WAIT_NODE_TYPE,
+			parameters: { resume: 'webhook' },
+		});
+		const result = handleFormRedirectionCase(data, workflowStartNode);
+		expect(result).toEqual(data);
+	});
+
+	test('should modify data if start node type matches and responseCode is a redirect', () => {
+		const data: IWebhookResponseCallbackData = {
+			responseCode: 302,
+			headers: { location: 'http://example.com' },
+		};
+		const workflowStartNode = mock<INode>({
+			type: FORM_NODE_TYPE,
+			parameters: {},
+		});
+		const result = handleFormRedirectionCase(data, workflowStartNode);
+		expect(result.responseCode).toBe(200);
+		expect(result.data).toEqual({ redirectURL: 'http://example.com' });
+		expect((result?.headers as IDataObject)?.location).toBeUndefined();
+	});
+
+	test('should not modify data if location header is missing', () => {
+		const data: IWebhookResponseCallbackData = { responseCode: 302, headers: {} };
+		const workflowStartNode = mock<INode>({
+			type: FORM_NODE_TYPE,
+			parameters: {},
+		});
+		const result = handleFormRedirectionCase(data, workflowStartNode);
+		expect(result).toEqual(data);
+	});
+});

packages/cli/src/webhooks/waiting-forms.ts

@@ -1,8 +1,7 @@
 import { Service } from '@n8n/di';
-import axios from 'axios';
 import type express from 'express';
 import type { IRunData } from 'n8n-workflow';
-import { FORM_NODE_TYPE, sleep, Workflow } from 'n8n-workflow';
+import { FORM_NODE_TYPE, Workflow } from 'n8n-workflow';
 
 import { ConflictError } from '@/errors/response-errors/conflict.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
@@ -39,25 +38,6 @@ export class WaitingForms extends WaitingWebhooks {
 		});
 	}
 
-	private async reloadForm(req: WaitingWebhookRequest, res: express.Response) {
-		try {
-			await sleep(1000);
-
-			const url = `${req.protocol}://${req.get('host')}${req.originalUrl}`;
-			const page = await axios({ url });
-
-			if (page) {
-				res.send(`
-				<script>
-					setTimeout(function() {
-						window.location.reload();
-					}, 1);
-				</script>
-			`);
-			}
-		} catch (error) {}
-	}
-
 	findCompletionPage(workflow: Workflow, runData: IRunData, lastNodeExecuted: string) {
 		const parentNodes = workflow.getParentNodes(lastNodeExecuted);
 		const lastNode = workflow.nodes[lastNodeExecuted];
@@ -105,11 +85,6 @@ export class WaitingForms extends WaitingWebhooks {
 		}
 
 		if (execution.status === 'running') {
-			if (this.includeForms && req.method === 'GET') {
-				await this.reloadForm(req, res);
-				return { noWebhookResponse: true };
-			}
-
 			throw new ConflictError(`The execution "${executionId}" is running already.`);
 		}
 

packages/cli/src/webhooks/webhook-helpers.ts

@@ -37,7 +37,9 @@ import {
 	createDeferredPromise,
 	ExecutionCancelledError,
 	FORM_NODE_TYPE,
+	FORM_TRIGGER_NODE_TYPE,
 	NodeOperationError,
+	WAIT_NODE_TYPE,
 } from 'n8n-workflow';
 import assert from 'node:assert';
 import { finished } from 'stream/promises';
@@ -101,6 +103,62 @@ export function getWorkflowWebhooks(
 	return returnData;
 }
 
+export function autoDetectResponseMode(
+	workflowStartNode: INode,
+	workflow: Workflow,
+	method: string,
+) {
+	if (workflowStartNode.type === WAIT_NODE_TYPE && workflowStartNode.parameters.resume !== 'form') {
+		return undefined;
+	}
+	if (
+		[FORM_NODE_TYPE, FORM_TRIGGER_NODE_TYPE, WAIT_NODE_TYPE].includes(workflowStartNode.type) &&
+		method === 'POST'
+	) {
+		const connectedNodes = workflow.getChildNodes(workflowStartNode.name);
+
+		for (const nodeName of connectedNodes) {
+			const node = workflow.nodes[nodeName];
+
+			if (node.type === WAIT_NODE_TYPE && node.parameters.resume !== 'form') {
+				continue;
+			}
+
+			if ([FORM_NODE_TYPE, WAIT_NODE_TYPE].includes(node.type) && !node.disabled) {
+				return 'responseNode';
+			}
+		}
+	}
+
+	return undefined;
+}
+
+/**
+ * for formTrigger and form nodes redirection has to be handled by sending redirectURL in response body
+ */
+export const handleFormRedirectionCase = (
+	data: IWebhookResponseCallbackData,
+	workflowStartNode: INode,
+) => {
+	if (workflowStartNode.type === WAIT_NODE_TYPE && workflowStartNode.parameters.resume !== 'form') {
+		return data;
+	}
+
+	if (
+		[FORM_NODE_TYPE, FORM_TRIGGER_NODE_TYPE, WAIT_NODE_TYPE].includes(workflowStartNode.type) &&
+		(data?.headers as IDataObject)?.location &&
+		String(data?.responseCode).startsWith('3')
+	) {
+		data.responseCode = 200;
+		data.data = {
+			redirectURL: (data?.headers as IDataObject)?.location,
+		};
+		(data.headers as IDataObject).location = undefined;
+	}
+
+	return data;
+};
+
 const { formDataFileSizeMax } = Container.get(GlobalConfig).endpoints;
 const parseFormData = createMultiFormDataParser(formDataFileSizeMax);
 
@@ -154,23 +212,8 @@ export async function executeWebhook(
 	// Get the responseMode
 	let responseMode;
 
-	// if this is n8n FormTrigger node, check if there is a Form node in child nodes,
-	// if so, set 'responseMode' to 'formPage' to redirect to URL of that Form later
-	if (nodeType.description.name === 'formTrigger') {
-		const connectedNodes = workflow.getChildNodes(workflowStartNode.name);
-		let hasNextPage = false;
-		for (const nodeName of connectedNodes) {
-			const node = workflow.nodes[nodeName];
-			if (node.type === FORM_NODE_TYPE && !node.disabled) {
-				hasNextPage = true;
-				break;
-			}
-		}
-
-		if (hasNextPage) {
-			responseMode = 'formPage';
-		}
-	}
+	//check if response mode should be set automatically, e.g. multipage form
+	responseMode = autoDetectResponseMode(workflowStartNode, workflow, req.method);
 
 	if (!responseMode) {
 		responseMode = workflow.expression.getSimpleParameterValue(
@@ -201,7 +244,7 @@ export async function executeWebhook(
 		'firstEntryJson',
 	);
 
-	if (!['onReceived', 'lastNode', 'responseNode', 'formPage'].includes(responseMode)) {
+	if (!['onReceived', 'lastNode', 'responseNode'].includes(responseMode)) {
 		// If the mode is not known we error. Is probably best like that instead of using
 		// the default that people know as early as possible (probably already testing phase)
 		// that something does not resolve properly.
@@ -497,28 +540,16 @@ export async function executeWebhook(
 					} else {
 						// TODO: This probably needs some more changes depending on the options on the
 						//       Webhook Response node
-						const headers = response.headers;
-						let responseCode = response.statusCode;
-						let data = response.body as IDataObject;
 
-						// for formTrigger node redirection has to be handled by sending redirectURL in response body
-						if (
-							nodeType.description.name === 'formTrigger' &&
-							headers.location &&
-							String(responseCode).startsWith('3')
-						) {
-							responseCode = 200;
-							data = {
-								redirectURL: headers.location,
-							};
-							headers.location = undefined;
-						}
+						let data: IWebhookResponseCallbackData = {
+							data: response.body as IDataObject,
+							headers: response.headers,
+							responseCode: response.statusCode,
+						};
 
-						responseCallback(null, {
-							data,
-							headers,
-							responseCode,
-						});
+						data = handleFormRedirectionCase(data, workflowStartNode);
+
+						responseCallback(null, data);
 					}
 
 					process.nextTick(() => res.end());
@@ -552,12 +583,6 @@ export async function executeWebhook(
 			responsePromise,
 		);
 
-		if (responseMode === 'formPage' && !didSendResponse) {
-			res.redirect(`${additionalData.formWaitingBaseUrl}/${executionId}`);
-			process.nextTick(() => res.end());
-			didSendResponse = true;
-		}
-
 		Container.get(Logger).debug(
 			`Started execution of workflow "${workflow.name}" from webhook with execution ID ${executionId}`,
 			{ executionId },

packages/cli/src/workflow-runner.ts

@@ -296,6 +296,7 @@ export class WorkflowRunner {
 						fullRunData.finished = false;
 					}
 					fullRunData.status = this.activeExecutions.getStatus(executionId);
+					this.activeExecutions.resolveExecutionResponsePromise(executionId);
 					this.activeExecutions.finalizeExecution(executionId, fullRunData);
 				})
 				.catch(

packages/cli/templates/form-trigger-completion.handlebars

@@ -28,6 +28,8 @@
 	<body>
 		{{#if responseText}}
 			{{{responseText}}}
+		{{else if redirectUrl}}
+			<div>Redirecting to <a href='{{redirectUrl}}'>{{redirectUrl}}</a></div>
 		{{else}}
 			<div class='container'>
 				<section>
@@ -73,9 +75,40 @@
 				</section>
 			</div>
 		{{/if}}
-		<script>
-			fetch('', { method: 'POST', body: {}, }).catch(() => {});
-		</script>
+		{{#if redirectUrl}}
+			<a id='redirectUrl' href='{{redirectUrl}}' style='display: none;'></a>
+		{{/if}}
+	<script>
+			fetch('', {
+					method: 'POST',
+					body: {}
+			})
+			.then(async function (response) {
+					if (response.status === 200) {
+						const redirectUrl = document.getElementById("redirectUrl");
+						if (redirectUrl) {
+							window.location.replace(redirectUrl.href);
+						}
+					}
+
+					const text = await response.text();
+					let json;
+
+					try {
+							json = JSON.parse(text);
+					} catch (e) {}
+
+					if (json?.redirectURL) {
+							const url = json.redirectURL.includes("://")
+									? json.redirectURL
+									: "https://" + json.redirectURL;
+							window.location.replace(url);
+					}
+			})
+			.catch(function (error) {
+					console.error('Error:', error);
+			});
+	</script>
 	</body>
 
 </html>

packages/cli/templates/form-trigger.handlebars

@@ -779,14 +779,20 @@
 								if (json?.redirectURL) {
 									const url = json.redirectURL.includes("://") ? json.redirectURL : "https://" + json.redirectURL;
 									window.location.replace(url);
-								} else if (json?.formSubmittedText) {
+									return;
+								}
+
+								if (json?.formSubmittedText) {
 									form.style.display = 'none';
 									document.querySelector('#submitted-form').style.display = 'block';
 									document.querySelector('#submitted-content').textContent = json.formSubmittedText;
-								} else {
-									document.body.innerHTML = text;
+									return;
+								}
+
+								if (text) {
+									document.body.innerHTML = text;
+									return;
 								}
-								return;
 							}
 
 							if (response.status === 200) {
@@ -814,18 +820,6 @@
 						.catch(function (error) {
 							console.error('Error:', error);
 						});
-
-					const isWaitingForm = window.location.href.includes('form-waiting');
-					if(isWaitingForm) {
-						const interval = setInterval(function() {
-							const isSubmited = document.querySelector('#submitted-form').style.display;
-							if(isSubmited === 'block') {
-								clearInterval(interval);
-								return;
-							}
-							window.location.reload();
-						}, 2000);
-					}
 				}
 			});
 		</script>

packages/nodes-base/nodes/Form/Form.node.ts

@@ -243,7 +243,7 @@ export class Form extends Node {
 			{
 				name: 'default',
 				httpMethod: 'POST',
-				responseMode: 'onReceived',
+				responseMode: 'responseNode',
 				path: '',
 				restartWebhook: true,
 				isFullPath: true,
@@ -384,6 +384,13 @@ export class Form extends Node {
 		const waitTill = configureWaitTillDate(context, 'root');
 		await context.putExecutionToWait(waitTill);
 
+		context.sendResponse({
+			headers: {
+				location: context.evaluateExpression('{{ $execution.resumeFormUrl }}', 0),
+			},
+			statusCode: 307,
+		});
+
 		return [context.getInputData()];
 	}
 }

packages/nodes-base/nodes/Form/formCompletionUtils.ts

@@ -18,13 +18,6 @@ export const renderFormCompletion = async (
 	const options = context.getNodeParameter('options', {}) as { formTitle: string };
 	const responseText = context.getNodeParameter('responseText', '') as string;
 
-	if (redirectUrl) {
-		res.send(
-			`<html><head><meta http-equiv="refresh" content="0; url=${redirectUrl}"></head></html>`,
-		);
-		return { noWebhookResponse: true };
-	}
-
 	let title = options.formTitle;
 	if (!title) {
 		title = context.evaluateExpression(`{{ $('${trigger?.name}').params.formTitle }}`) as string;
@@ -39,6 +32,7 @@ export const renderFormCompletion = async (
 		formTitle: title,
 		appendAttribution,
 		responseText: sanitizeHtml(responseText),
+		redirectUrl,
 	});
 
 	return { noWebhookResponse: true };

packages/nodes-base/nodes/Form/formNodeUtils.ts

@@ -2,8 +2,6 @@ import { type Response } from 'express';
 import {
 	type NodeTypeAndVersion,
 	type IWebhookFunctions,
-	FORM_NODE_TYPE,
-	WAIT_NODE_TYPE,
 	type FormFieldsParameter,
 	type IWebhookResponseData,
 } from 'n8n-workflow';
@@ -43,20 +41,6 @@ export const renderFormNode = async (
 			) as string) || 'Submit';
 	}
 
-	const responseMode = 'onReceived';
-
-	let redirectUrl;
-
-	const connectedNodes = context.getChildNodes(context.getNode().name);
-
-	const hasNextPage = connectedNodes.some(
-		(node) => !node.disabled && (node.type === FORM_NODE_TYPE || node.type === WAIT_NODE_TYPE),
-	);
-
-	if (hasNextPage) {
-		redirectUrl = context.evaluateExpression('{{ $execution.resumeFormUrl }}') as string;
-	}
-
 	const appendAttribution = context.evaluateExpression(
 		`{{ $('${trigger?.name}').params.options?.appendAttribution === false ? false : true }}`,
 	) as boolean;
@@ -67,9 +51,9 @@ export const renderFormNode = async (
 		formTitle: title,
 		formDescription: description,
 		formFields: fields,
-		responseMode,
+		responseMode: 'responseNode',
 		mode,
-		redirectUrl,
+		redirectUrl: undefined,
 		appendAttribution,
 		buttonLabel,
 	});

packages/nodes-base/nodes/Form/test/Form.node.test.ts

@@ -166,7 +166,7 @@ describe('Form Node', () => {
 				formTitle: 'Form Title',
 				n8nWebsiteLink: 'https://n8n.io/?utm_source=n8n-internal&utm_medium=form-trigger',
 				testRun: true,
-				useResponseData: false,
+				useResponseData: true,
 				validForm: true,
 				formSubmittedHeader: undefined,
 			});
@@ -230,6 +230,7 @@ describe('Form Node', () => {
 						appendAttribution: 'test',
 						formTitle: 'test',
 						message: 'Test Message',
+						redirectUrl: '',
 						title: 'Test Title',
 						responseText: '',
 					},
@@ -242,6 +243,7 @@ describe('Form Node', () => {
 						appendAttribution: 'test',
 						formTitle: 'test',
 						message: 'Test Message',
+						redirectUrl: '',
 						title: 'Test Title',
 						responseText: '<div>hey</div>',
 					},
@@ -254,6 +256,7 @@ describe('Form Node', () => {
 						appendAttribution: 'test',
 						formTitle: 'test',
 						message: 'Test Message',
+						redirectUrl: '',
 						title: 'Test Title',
 						responseText: 'my text over here',
 					},
@@ -340,9 +343,14 @@ describe('Form Node', () => {
 			const result = await form.webhook(mockWebhookFunctions);
 
 			expect(result).toEqual({ noWebhookResponse: true });
-			expect(mockResponseObject.send).toHaveBeenCalledWith(
-				'<html><head><meta http-equiv="refresh" content="0; url=https://n8n.io"></head></html>',
-			);
+			expect(mockResponseObject.render).toHaveBeenCalledWith('form-trigger-completion', {
+				appendAttribution: 'test',
+				formTitle: 'test',
+				message: 'Test Message',
+				redirectUrl: 'https://n8n.io',
+				responseText: '',
+				title: 'Test Title',
+			});
 		});
 	});
 });

packages/nodes-base/nodes/Form/utils.ts

@@ -476,7 +476,7 @@ export async function formWebhook(
 	if (method === 'GET') {
 		const formTitle = context.getNodeParameter('formTitle', '') as string;
 		const formDescription = sanitizeHtml(context.getNodeParameter('formDescription', '') as string);
-		const responseMode = context.getNodeParameter('responseMode', '') as string;
+		let responseMode = context.getNodeParameter('responseMode', '') as string;
 
 		let formSubmittedText;
 		let redirectUrl;
@@ -504,15 +504,14 @@ export async function formWebhook(
 			buttonLabel = options.buttonLabel;
 		}
 
-		if (!redirectUrl && node.type !== FORM_TRIGGER_NODE_TYPE) {
-			const connectedNodes = context.getChildNodes(context.getNode().name, {
-				includeNodeParameters: true,
-			});
-			const hasNextPage = isFormConnected(connectedNodes);
+		const connectedNodes = context.getChildNodes(context.getNode().name, {
+			includeNodeParameters: true,
+		});
+		const hasNextPage = isFormConnected(connectedNodes);
 
-			if (hasNextPage) {
-				redirectUrl = context.evaluateExpression('{{ $execution.resumeFormUrl }}') as string;
-			}
+		if (hasNextPage) {
+			redirectUrl = undefined;
+			responseMode = 'responseNode';
 		}
 
 		renderForm({

packages/nodes-base/nodes/Wait/Wait.node.ts

@@ -7,7 +7,12 @@ import type {
 	IDisplayOptions,
 	IWebhookFunctions,
 } from 'n8n-workflow';
-import { NodeOperationError, NodeConnectionType, WAIT_INDEFINITELY } from 'n8n-workflow';
+import {
+	NodeOperationError,
+	NodeConnectionType,
+	WAIT_INDEFINITELY,
+	FORM_TRIGGER_NODE_TYPE,
+} from 'n8n-workflow';
 
 import { updateDisplayOptions } from '../../utils/utilities';
 import {
@@ -459,7 +464,25 @@ export class Wait extends Webhook {
 		const resume = context.getNodeParameter('resume', 0) as string;
 
 		if (['webhook', 'form'].includes(resume)) {
-			return await this.configureAndPutToWait(context);
+			let hasFormTrigger = false;
+
+			if (resume === 'form') {
+				const parentNodes = context.getParentNodes(context.getNode().name);
+				hasFormTrigger = parentNodes.some((node) => node.type === FORM_TRIGGER_NODE_TYPE);
+			}
+
+			const returnData = await this.configureAndPutToWait(context);
+
+			if (resume === 'form' && hasFormTrigger) {
+				context.sendResponse({
+					headers: {
+						location: context.evaluateExpression('{{ $execution.resumeFormUrl }}', 0),
+					},
+					statusCode: 307,
+				});
+			}
+
+			return returnData;
 		}
 
 		let waitTill: Date;

packages/workflow/src/Interfaces.ts

@@ -2031,7 +2031,7 @@ export interface IWebhookResponseData {
 }
 
 export type WebhookResponseData = 'allEntries' | 'firstEntryJson' | 'firstEntryBinary' | 'noData';
-export type WebhookResponseMode = 'onReceived' | 'lastNode' | 'responseNode' | 'formPage';
+export type WebhookResponseMode = 'onReceived' | 'lastNode' | 'responseNode';
 
 export interface INodeTypes {
 	getByName(nodeType: string): INodeType | IVersionedNodeType;