feat(n8n Form Trigger Node): Improvements (#7571)
Github issue / Community forum post (link here to close automatically): --------- Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in> Co-authored-by: Giulio Andreini <andreini@netseven.it>
This commit is contained in:
@@ -14,6 +14,7 @@ import { ExternalHooks } from '@/ExternalHooks';
|
||||
import { send, sendErrorResponse } from '@/ResponseHelper';
|
||||
import { rawBodyReader, bodyParser, corsMiddleware } from '@/middlewares';
|
||||
import { TestWebhooks } from '@/TestWebhooks';
|
||||
import { WaitingForms } from '@/WaitingForms';
|
||||
import { WaitingWebhooks } from '@/WaitingWebhooks';
|
||||
import { webhookRequestHandler } from '@/WebhookHelpers';
|
||||
import { generateHostInstanceId } from './databases/utils/generators';
|
||||
@@ -39,6 +40,12 @@ export abstract class AbstractServer {
|
||||
|
||||
protected restEndpoint: string;
|
||||
|
||||
protected endpointForm: string;
|
||||
|
||||
protected endpointFormTest: string;
|
||||
|
||||
protected endpointFormWaiting: string;
|
||||
|
||||
protected endpointWebhook: string;
|
||||
|
||||
protected endpointWebhookTest: string;
|
||||
@@ -63,6 +70,11 @@ export abstract class AbstractServer {
|
||||
this.sslCert = config.getEnv('ssl_cert');
|
||||
|
||||
this.restEndpoint = config.getEnv('endpoints.rest');
|
||||
|
||||
this.endpointForm = config.getEnv('endpoints.form');
|
||||
this.endpointFormTest = config.getEnv('endpoints.formTest');
|
||||
this.endpointFormWaiting = config.getEnv('endpoints.formWaiting');
|
||||
|
||||
this.endpointWebhook = config.getEnv('endpoints.webhook');
|
||||
this.endpointWebhookTest = config.getEnv('endpoints.webhookTest');
|
||||
this.endpointWebhookWaiting = config.getEnv('endpoints.webhookWaiting');
|
||||
@@ -165,10 +177,21 @@ export abstract class AbstractServer {
|
||||
|
||||
// Setup webhook handlers before bodyParser, to let the Webhook node handle binary data in requests
|
||||
if (this.webhooksEnabled) {
|
||||
const activeWorkflowRunner = Container.get(ActiveWorkflowRunner);
|
||||
|
||||
// Register a handler for active forms
|
||||
this.app.all(`/${this.endpointForm}/:path(*)`, webhookRequestHandler(activeWorkflowRunner));
|
||||
|
||||
// Register a handler for active webhooks
|
||||
this.app.all(
|
||||
`/${this.endpointWebhook}/:path(*)`,
|
||||
webhookRequestHandler(Container.get(ActiveWorkflowRunner)),
|
||||
webhookRequestHandler(activeWorkflowRunner),
|
||||
);
|
||||
|
||||
// Register a handler for waiting forms
|
||||
this.app.all(
|
||||
`/${this.endpointFormWaiting}/:path/:suffix?`,
|
||||
webhookRequestHandler(Container.get(WaitingForms)),
|
||||
);
|
||||
|
||||
// Register a handler for waiting webhooks
|
||||
@@ -181,7 +204,8 @@ export abstract class AbstractServer {
|
||||
if (this.testWebhooksEnabled) {
|
||||
const testWebhooks = Container.get(TestWebhooks);
|
||||
|
||||
// Register a handler for test webhooks
|
||||
// Register a handler
|
||||
this.app.all(`/${this.endpointFormTest}/:path(*)`, webhookRequestHandler(testWebhooks));
|
||||
this.app.all(`/${this.endpointWebhookTest}/:path(*)`, webhookRequestHandler(testWebhooks));
|
||||
|
||||
// Removes a test webhook
|
||||
|
||||
@@ -2,7 +2,11 @@
|
||||
import type { Request, Response } from 'express';
|
||||
import { parse, stringify } from 'flatted';
|
||||
import picocolors from 'picocolors';
|
||||
import { ErrorReporterProxy as ErrorReporter, NodeApiError } from 'n8n-workflow';
|
||||
import {
|
||||
ErrorReporterProxy as ErrorReporter,
|
||||
FORM_TRIGGER_PATH_IDENTIFIER,
|
||||
NodeApiError,
|
||||
} from 'n8n-workflow';
|
||||
import { Readable } from 'node:stream';
|
||||
import type {
|
||||
IExecutionDb,
|
||||
@@ -67,6 +71,20 @@ export function sendErrorResponse(res: Response, error: Error) {
|
||||
console.error(picocolors.red(error.httpStatusCode), error.message);
|
||||
}
|
||||
|
||||
//render custom 404 page for form triggers
|
||||
const { originalUrl } = res.req;
|
||||
if (error.errorCode === 404 && originalUrl) {
|
||||
const basePath = originalUrl.split('/')[1];
|
||||
const isLegacyFormTrigger = originalUrl.includes(FORM_TRIGGER_PATH_IDENTIFIER);
|
||||
const isFormTrigger = basePath.includes('form');
|
||||
|
||||
if (isFormTrigger || isLegacyFormTrigger) {
|
||||
const isTestWebhook = basePath.includes('test');
|
||||
res.status(404);
|
||||
return res.render('form-trigger-404', { isTestWebhook });
|
||||
}
|
||||
}
|
||||
|
||||
httpStatusCode = error.httpStatusCode;
|
||||
|
||||
if (error.errorCode) {
|
||||
|
||||
19
packages/cli/src/WaitingForms.ts
Normal file
19
packages/cli/src/WaitingForms.ts
Normal file
@@ -0,0 +1,19 @@
|
||||
import { Service } from 'typedi';
|
||||
|
||||
import type { IExecutionResponse } from '@/Interfaces';
|
||||
import { WaitingWebhooks } from '@/WaitingWebhooks';
|
||||
|
||||
@Service()
|
||||
export class WaitingForms extends WaitingWebhooks {
|
||||
protected override includeForms = true;
|
||||
|
||||
protected override logReceivedWebhook(method: string, executionId: string) {
|
||||
this.logger.debug(`Received waiting-form "${method}" for execution "${executionId}"`);
|
||||
}
|
||||
|
||||
protected disableNode(execution: IExecutionResponse, method?: string) {
|
||||
if (method === 'POST') {
|
||||
execution.data.executionData!.nodeExecutionStack[0].node.disabled = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import type express from 'express';
|
||||
import * as WebhookHelpers from '@/WebhookHelpers';
|
||||
import { NodeTypes } from '@/NodeTypes';
|
||||
import type {
|
||||
IExecutionResponse,
|
||||
IResponseCallbackData,
|
||||
IWebhookManager,
|
||||
IWorkflowDb,
|
||||
@@ -19,8 +20,10 @@ import { NotFoundError } from './errors/response-errors/not-found.error';
|
||||
|
||||
@Service()
|
||||
export class WaitingWebhooks implements IWebhookManager {
|
||||
protected includeForms = false;
|
||||
|
||||
constructor(
|
||||
private readonly logger: Logger,
|
||||
protected readonly logger: Logger,
|
||||
private readonly nodeTypes: NodeTypes,
|
||||
private readonly executionRepository: ExecutionRepository,
|
||||
private readonly ownershipService: OwnershipService,
|
||||
@@ -28,12 +31,21 @@ export class WaitingWebhooks implements IWebhookManager {
|
||||
|
||||
// TODO: implement `getWebhookMethods` for CORS support
|
||||
|
||||
protected logReceivedWebhook(method: string, executionId: string) {
|
||||
this.logger.debug(`Received waiting-webhook "${method}" for execution "${executionId}"`);
|
||||
}
|
||||
|
||||
protected disableNode(execution: IExecutionResponse, _method?: string) {
|
||||
execution.data.executionData!.nodeExecutionStack[0].node.disabled = true;
|
||||
}
|
||||
|
||||
async executeWebhook(
|
||||
req: WaitingWebhookRequest,
|
||||
res: express.Response,
|
||||
): Promise<IResponseCallbackData> {
|
||||
const { path: executionId, suffix } = req.params;
|
||||
this.logger.debug(`Received waiting-webhook "${req.method}" for execution "${executionId}"`);
|
||||
|
||||
this.logReceivedWebhook(req.method, executionId);
|
||||
|
||||
// Reset request parameters
|
||||
req.params = {} as WaitingWebhookRequest['params'];
|
||||
@@ -55,7 +67,7 @@ export class WaitingWebhooks implements IWebhookManager {
|
||||
|
||||
// Set the node as disabled so that the data does not get executed again as it would result
|
||||
// in starting the wait all over again
|
||||
execution.data.executionData!.nodeExecutionStack[0].node.disabled = true;
|
||||
this.disableNode(execution, req.method);
|
||||
|
||||
// Remove waitTill information else the execution would stop
|
||||
execution.data.waitTill = undefined;
|
||||
@@ -97,7 +109,8 @@ export class WaitingWebhooks implements IWebhookManager {
|
||||
(webhook) =>
|
||||
webhook.httpMethod === req.method &&
|
||||
webhook.path === (suffix ?? '') &&
|
||||
webhook.webhookDescription.restartWebhook === true,
|
||||
webhook.webhookDescription.restartWebhook === true &&
|
||||
(webhook.webhookDescription.isForm || false) === this.includeForms,
|
||||
);
|
||||
|
||||
if (webhookData === undefined) {
|
||||
|
||||
@@ -37,7 +37,6 @@ import {
|
||||
BINARY_ENCODING,
|
||||
createDeferredPromise,
|
||||
ErrorReporterProxy as ErrorReporter,
|
||||
FORM_TRIGGER_PATH_IDENTIFIER,
|
||||
NodeHelpers,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
@@ -133,16 +132,7 @@ export const webhookRequestHandler =
|
||||
try {
|
||||
response = await webhookManager.executeWebhook(req, res);
|
||||
} catch (error) {
|
||||
if (
|
||||
error.errorCode === 404 &&
|
||||
(error.message as string).includes(FORM_TRIGGER_PATH_IDENTIFIER)
|
||||
) {
|
||||
const isTestWebhook = req.originalUrl.includes('webhook-test');
|
||||
res.status(404);
|
||||
return res.render('form-trigger-404', { isTestWebhook });
|
||||
} else {
|
||||
return ResponseHelper.sendErrorResponse(res, error as Error);
|
||||
}
|
||||
return ResponseHelper.sendErrorResponse(res, error as Error);
|
||||
}
|
||||
|
||||
// Don't respond, if already responded
|
||||
@@ -560,10 +550,27 @@ 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;
|
||||
}
|
||||
|
||||
responseCallback(null, {
|
||||
data: response.body as IDataObject,
|
||||
headers: response.headers,
|
||||
responseCode: response.statusCode,
|
||||
data,
|
||||
headers,
|
||||
responseCode,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -963,9 +963,11 @@ export async function getBase(
|
||||
): Promise<IWorkflowExecuteAdditionalData> {
|
||||
const urlBaseWebhook = WebhookHelpers.getWebhookBaseUrl();
|
||||
|
||||
const formWaitingBaseUrl = urlBaseWebhook + config.getEnv('endpoints.formWaiting');
|
||||
|
||||
const webhookBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhook');
|
||||
const webhookWaitingBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhookWaiting');
|
||||
const webhookTestBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhookTest');
|
||||
const webhookWaitingBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhookWaiting');
|
||||
|
||||
const variables = await WorkflowHelpers.getVariables();
|
||||
|
||||
@@ -974,6 +976,7 @@ export async function getBase(
|
||||
executeWorkflow,
|
||||
restApiUrl: urlBaseWebhook + config.getEnv('endpoints.rest'),
|
||||
instanceBaseUrl: urlBaseWebhook,
|
||||
formWaitingBaseUrl,
|
||||
webhookBaseUrl,
|
||||
webhookWaitingBaseUrl,
|
||||
webhookTestBaseUrl,
|
||||
|
||||
@@ -668,6 +668,24 @@ export const schema = {
|
||||
env: 'N8N_ENDPOINT_REST',
|
||||
doc: 'Path for rest endpoint',
|
||||
},
|
||||
form: {
|
||||
format: String,
|
||||
default: 'form',
|
||||
env: 'N8N_ENDPOINT_FORM',
|
||||
doc: 'Path for form endpoint',
|
||||
},
|
||||
formTest: {
|
||||
format: String,
|
||||
default: 'form-test',
|
||||
env: 'N8N_ENDPOINT_FORM_TEST',
|
||||
doc: 'Path for test form endpoint',
|
||||
},
|
||||
formWaiting: {
|
||||
format: String,
|
||||
default: 'form-waiting',
|
||||
env: 'N8N_ENDPOINT_FORM_WAIT',
|
||||
doc: 'Path for waiting form endpoint',
|
||||
},
|
||||
webhook: {
|
||||
format: String,
|
||||
default: 'webhook',
|
||||
|
||||
@@ -81,6 +81,9 @@ export class FrontendService {
|
||||
}
|
||||
|
||||
this.settings = {
|
||||
endpointForm: config.getEnv('endpoints.form'),
|
||||
endpointFormTest: config.getEnv('endpoints.formTest'),
|
||||
endpointFormWaiting: config.getEnv('endpoints.formWaiting'),
|
||||
endpointWebhook: config.getEnv('endpoints.webhook'),
|
||||
endpointWebhookTest: config.getEnv('endpoints.webhookTest'),
|
||||
saveDataErrorExecution: config.getEnv('executions.saveDataOnError'),
|
||||
|
||||
Reference in New Issue
Block a user