From 8764171989459c87fe00e91e2d7fb97af082c89d Mon Sep 17 00:00:00 2001 From: Ben Hesseldieck <1849459+BHesseldieck@users.noreply.github.com> Date: Thu, 28 Jan 2021 15:44:10 +0100 Subject: [PATCH] :art: small webhook refactorings (#1383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🧹 clean up forgotten leftover * :art: reset req.params * :bug: Bugfix and minor change on request parameters * :zap: Minor improvements on request parameters Co-authored-by: Jan Oberhauser --- packages/cli/src/ActiveWorkflowRunner.ts | 11 ++++++----- packages/cli/src/Server.ts | 6 ------ packages/cli/src/TestWebhooks.ts | 4 ++++ 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 81d723ec3d..2bf2a2f7b9 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -117,6 +117,9 @@ export class ActiveWorkflowRunner { throw new ResponseHelper.ResponseError('The "activeWorkflows" instance did not get initialized yet.', 404, 404); } + // Reset request parameters + req.params = {}; + let webhook = await Db.collections.Webhook?.findOne({ webhookPath: path, method: httpMethod }) as IWebhookDb; let webhookId: string | undefined; @@ -126,10 +129,11 @@ export class ActiveWorkflowRunner { const pathElements = path.split('/'); webhookId = pathElements.shift(); const dynamicWebhooks = await Db.collections.Webhook?.find({ webhookId, method: httpMethod, pathLength: pathElements.length }); - if (dynamicWebhooks === undefined) { + if (dynamicWebhooks === undefined || dynamicWebhooks.length === 0) { // The requested webhook is not registered throw new ResponseHelper.ResponseError(`The requested webhook "${httpMethod} ${path}" is not registered.`, 404, 404); } + // set webhook to the first webhook result // if more results have been returned choose the one with the most route-matches webhook = dynamicWebhooks[0]; @@ -323,10 +327,7 @@ export class ActiveWorkflowRunner { // TODO check if there is standard error code for duplicate key violation that works // with all databases if (error.name === 'QueryFailedError') { - errorMessage = error.parameters.length === 5 - ? `Node [${webhook.node}] can't be saved, please duplicate [${webhook.node}] and delete the currently existing one.` - : `The webhook path [${webhook.webhookPath}] and method [${webhook.method}] already exist.`; - + errorMessage = `The webhook path [${webhook.webhookPath}] and method [${webhook.method}] already exist.`; } else if (error.detail) { // it's a error runnig the webhook methods (checkExists, create) errorMessage = error.detail; diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index bcd2d81ffd..4b38988b71 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -1693,7 +1693,6 @@ class App { let response; try { - delete req.params[0]; response = await this.activeWorkflowRunner.executeWebhook('HEAD', requestUrl, req, res); } catch (error) { ResponseHelper.sendErrorResponse(res, error); @@ -1735,7 +1734,6 @@ class App { let response; try { - delete req.params[0]; response = await this.activeWorkflowRunner.executeWebhook('GET', requestUrl, req, res); } catch (error) { ResponseHelper.sendErrorResponse(res, error); @@ -1757,7 +1755,6 @@ class App { let response; try { - delete req.params[0]; response = await this.activeWorkflowRunner.executeWebhook('POST', requestUrl, req, res); } catch (error) { ResponseHelper.sendErrorResponse(res, error); @@ -1779,7 +1776,6 @@ class App { let response; try { - delete req.params[0]; response = await this.testWebhooks.callTestWebhook('HEAD', requestUrl, req, res); } catch (error) { ResponseHelper.sendErrorResponse(res, error); @@ -1821,7 +1817,6 @@ class App { let response; try { - delete req.params[0]; response = await this.testWebhooks.callTestWebhook('GET', requestUrl, req, res); } catch (error) { ResponseHelper.sendErrorResponse(res, error); @@ -1843,7 +1838,6 @@ class App { let response; try { - delete req.params[0]; response = await this.testWebhooks.callTestWebhook('POST', requestUrl, req, res); } catch (error) { ResponseHelper.sendErrorResponse(res, error); diff --git a/packages/cli/src/TestWebhooks.ts b/packages/cli/src/TestWebhooks.ts index cd463bffaa..580de9c34c 100644 --- a/packages/cli/src/TestWebhooks.ts +++ b/packages/cli/src/TestWebhooks.ts @@ -56,6 +56,9 @@ export class TestWebhooks { async callTestWebhook(httpMethod: WebhookHttpMethod, path: string, request: express.Request, response: express.Response): Promise { let webhookData: IWebhookData | undefined = this.activeWebhooks!.get(httpMethod, path); + // Reset request parameters + request.params = {}; + // check if path is dynamic if (webhookData === undefined) { const pathElements = path.split('/'); @@ -65,6 +68,7 @@ export class TestWebhooks { // The requested webhook is not registered throw new ResponseHelper.ResponseError(`The requested webhook "${httpMethod} ${path}" is not registered.`, 404, 404); } + path = webhookData.path; // extracting params from path path.split('/').forEach((ele, index) => {