From a0dd17e1151e668b95dc57367a0b100d00913ea3 Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Tue, 2 May 2023 17:27:05 +0300 Subject: [PATCH] fix(core): Better error message in Webhook node when using the POST method --- packages/cli/src/ActiveWorkflowRunner.ts | 5 ++-- packages/cli/src/TestWebhooks.ts | 9 ++++-- packages/cli/src/utils.ts | 28 +++++++++++++++++++ packages/cli/test/unit/utils.test.ts | 35 ++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 packages/cli/test/unit/utils.test.ts diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 2cd770291e..ca175b303b 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -66,6 +66,7 @@ import { ExternalHooks } from '@/ExternalHooks'; import { whereClause } from './UserManagement/UserManagementHelper'; import { WorkflowsService } from './workflows/workflows.services'; import { START_NODES } from './constants'; +import { webhookNotFoundErrorMessage } from './utils'; const WEBHOOK_PROD_UNREGISTERED_HINT = "The workflow must be active for a production URL to run successfully. You can activate the workflow using the toggle in the top-right of the editor. Note that unlike test URL calls, production URL calls aren't shown on the canvas (only in the executions list)"; @@ -221,7 +222,7 @@ export class ActiveWorkflowRunner { if (dynamicWebhooks === undefined || dynamicWebhooks.length === 0) { // The requested webhook is not registered throw new ResponseHelper.NotFoundError( - `The requested webhook "${httpMethod} ${path}" is not registered.`, + webhookNotFoundErrorMessage(path, httpMethod), WEBHOOK_PROD_UNREGISTERED_HINT, ); } @@ -247,7 +248,7 @@ export class ActiveWorkflowRunner { }); if (webhook === null) { throw new ResponseHelper.NotFoundError( - `The requested webhook "${httpMethod} ${path}" is not registered.`, + webhookNotFoundErrorMessage(path, httpMethod), WEBHOOK_PROD_UNREGISTERED_HINT, ); } diff --git a/packages/cli/src/TestWebhooks.ts b/packages/cli/src/TestWebhooks.ts index e3b7b4fb03..d509a1c030 100644 --- a/packages/cli/src/TestWebhooks.ts +++ b/packages/cli/src/TestWebhooks.ts @@ -18,6 +18,7 @@ import type { IResponseCallbackData, IWorkflowDb } from '@/Interfaces'; import { Push } from '@/push'; import * as ResponseHelper from '@/ResponseHelper'; import * as WebhookHelpers from '@/WebhookHelpers'; +import { webhookNotFoundErrorMessage } from './utils'; const WEBHOOK_TEST_UNREGISTERED_HINT = "Click the 'Execute workflow' button on the canvas, then try again. (In test mode, the webhook only works for one call after you click this button)"; @@ -69,8 +70,9 @@ export class TestWebhooks { webhookData = activeWebhooks.get(httpMethod, pathElements.join('/'), webhookId); if (webhookData === undefined) { // The requested webhook is not registered + const methods = await this.getWebhookMethods(path); throw new ResponseHelper.NotFoundError( - `The requested webhook "${httpMethod} ${path}" is not registered.`, + webhookNotFoundErrorMessage(path, httpMethod, methods), WEBHOOK_TEST_UNREGISTERED_HINT, ); } @@ -95,8 +97,9 @@ export class TestWebhooks { // TODO: Clean that duplication up one day and improve code generally if (testWebhookData[webhookKey] === undefined) { // The requested webhook is not registered + const methods = await this.getWebhookMethods(path); throw new ResponseHelper.NotFoundError( - `The requested webhook "${httpMethod} ${path}" is not registered.`, + webhookNotFoundErrorMessage(path, httpMethod, methods), WEBHOOK_TEST_UNREGISTERED_HINT, ); } @@ -160,7 +163,7 @@ export class TestWebhooks { if (!webhookMethods.length) { // The requested webhook is not registered throw new ResponseHelper.NotFoundError( - `The requested webhook "${path}" is not registered.`, + webhookNotFoundErrorMessage(path), WEBHOOK_TEST_UNREGISTERED_HINT, ); } diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index c99ef97f50..c9eee2f4c8 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -58,3 +58,31 @@ export const separate = (array: T[], test: (element: T) => boolean) => { return [pass, fail]; }; + +export const webhookNotFoundErrorMessage = ( + path: string, + httpMethod?: string, + webhookMethods?: string[], +) => { + let webhookPath = path; + + if (httpMethod) { + webhookPath = `${httpMethod} ${webhookPath}`; + } + + if (webhookMethods?.length && httpMethod) { + let methods = ''; + + if (webhookMethods.length === 1) { + methods = webhookMethods[0]; + } else { + const lastMethod = webhookMethods.pop(); + + methods = `${webhookMethods.join(', ')} or ${lastMethod as string}`; + } + + return `This webhook is not registered for ${httpMethod} requests. Did you mean to make a ${methods} request?`; + } else { + return `The requested webhook "${webhookPath}" is not registered.`; + } +}; diff --git a/packages/cli/test/unit/utils.test.ts b/packages/cli/test/unit/utils.test.ts new file mode 100644 index 0000000000..2925b4b669 --- /dev/null +++ b/packages/cli/test/unit/utils.test.ts @@ -0,0 +1,35 @@ +import { webhookNotFoundErrorMessage } from '../../src/utils'; + +describe('utils test webhookNotFoundErrorMessage ', () => { + it('should return a message with path and method', () => { + const message = webhookNotFoundErrorMessage('webhook12345', 'GET'); + + expect(message).toEqual('The requested webhook "GET webhook12345" is not registered.'); + }); + it('should return a message with path', () => { + const message = webhookNotFoundErrorMessage('webhook12345'); + + expect(message).toEqual('The requested webhook "webhook12345" is not registered.'); + }); + it('should return a message with method with tip', () => { + const message = webhookNotFoundErrorMessage('webhook12345', 'POST', ['GET', 'PUT']); + + expect(message).toEqual( + 'This webhook is not registered for POST requests. Did you mean to make a GET or PUT request?', + ); + }); + it('should return a message with method with tip', () => { + const message = webhookNotFoundErrorMessage('webhook12345', 'POST', ['PUT']); + + expect(message).toEqual( + 'This webhook is not registered for POST requests. Did you mean to make a PUT request?', + ); + }); + it('should return a message with method with tip', () => { + const message = webhookNotFoundErrorMessage('webhook12345', 'POST', ['GET', 'PUT', 'DELETE']); + + expect(message).toEqual( + 'This webhook is not registered for POST requests. Did you mean to make a GET, PUT or DELETE request?', + ); + }); +});