From b171cfbb103f8af63a6fc4f47eb37c77cddc8965 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Wed, 21 Oct 2020 17:50:23 +0200 Subject: [PATCH] :zap: Fix issue with thrown errors for nodes with multiple webhooks --- packages/cli/src/ActiveWorkflowRunner.ts | 9 +++++--- packages/cli/src/TestWebhooks.ts | 21 +++++++++++++++---- packages/core/src/ActiveWebhooks.ts | 5 ++--- .../nodes/Trello/TrelloTrigger.node.ts | 18 ---------------- 4 files changed, 25 insertions(+), 28 deletions(-) diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 8162689f94..856743ed4d 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -258,17 +258,20 @@ export class ActiveWorkflowRunner { await Db.collections.Webhook?.insert(webhook); const webhookExists = await workflow.runWebhookMethod('checkExists', webhookData, NodeExecuteFunctions, mode, false); - if (webhookExists === false) { + if (webhookExists !== true) { // If webhook does not exist yet create it await workflow.runWebhookMethod('create', webhookData, NodeExecuteFunctions, mode, false); } } catch (error) { + try { + await this.removeWorkflowWebhooks(workflow.id as string); + } catch (error) { + console.error(`Could not remove webhooks of workflow "${workflow.id}" because of error: "${error.message}"`); + } let errorMessage = ''; - await Db.collections.Webhook?.delete({ workflowId: workflow.id }); - // if it's a workflow from the the insert // TODO check if there is standard error code for deplicate key violation that works // with all databases diff --git a/packages/cli/src/TestWebhooks.ts b/packages/cli/src/TestWebhooks.ts index 2eb213ebf8..5d81b8dd1e 100644 --- a/packages/cli/src/TestWebhooks.ts +++ b/packages/cli/src/TestWebhooks.ts @@ -156,9 +156,12 @@ export class TestWebhooks { }, 120000); let key: string; + const activatedKey: string[] = []; for (const webhookData of webhooks) { key = this.activeWebhooks!.getWebhookKey(webhookData.httpMethod, webhookData.path); + activatedKey.push(key); + this.testWebhookData[key] = { sessionId, timeout, @@ -166,7 +169,13 @@ export class TestWebhooks { workflowData, }; - await this.activeWebhooks!.add(workflow, webhookData, mode); + try { + await this.activeWebhooks!.add(workflow, webhookData, mode); + } catch (error) { + activatedKey.forEach(deleteKey => delete this.testWebhookData[deleteKey] ); + await this.activeWebhooks!.removeWorkflow(workflow); + throw error; + } } return true; @@ -189,8 +198,6 @@ export class TestWebhooks { continue; } - foundWebhook = true; - clearTimeout(this.testWebhookData[webhookKey].timeout); // Inform editor-ui that webhook got received @@ -207,7 +214,13 @@ export class TestWebhooks { // Remove the webhook delete this.testWebhookData[webhookKey]; - this.activeWebhooks!.removeWorkflow(workflow); + + if (foundWebhook === false) { + // As it removes all webhooks of the workflow execute only once + this.activeWebhooks!.removeWorkflow(workflow); + } + + foundWebhook = true; } return foundWebhook; diff --git a/packages/core/src/ActiveWebhooks.ts b/packages/core/src/ActiveWebhooks.ts index 5b69fab246..f4b1f1f86a 100644 --- a/packages/core/src/ActiveWebhooks.ts +++ b/packages/core/src/ActiveWebhooks.ts @@ -52,7 +52,7 @@ export class ActiveWebhooks { try { const webhookExists = await workflow.runWebhookMethod('checkExists', webhookData, NodeExecuteFunctions, mode, this.testWebhooks); - if (webhookExists === false) { + if (webhookExists !== true) { // If webhook does not exist yet create it await workflow.runWebhookMethod('create', webhookData, NodeExecuteFunctions, mode, this.testWebhooks); @@ -60,7 +60,6 @@ export class ActiveWebhooks { } catch (error) { // If there was a problem unregister the webhook again delete this.webhookUrls[webhookKey]; - delete this.workflowWebhooks[webhookData.workflowId]; throw error; } @@ -159,7 +158,7 @@ export class ActiveWebhooks { /** - * Removes all the webhooks of the given workflow + * Removes all the webhooks of the given workflows */ async removeAll(workflows: Workflow[]): Promise { const removePromises = []; diff --git a/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts b/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts index feb80219da..edc0b3df23 100644 --- a/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts +++ b/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts @@ -68,12 +68,6 @@ export class TrelloTrigger implements INodeType { webhookMethods = { default: { async checkExists(this: IHookFunctions): Promise { - if (this.getWebhookName() === 'setup') { - // Is setup-webhook which only gets used once when - // the webhook gets created so nothing to do. - return true; - } - const credentials = this.getCredentials('trelloApi'); if (credentials === undefined) { @@ -101,12 +95,6 @@ export class TrelloTrigger implements INodeType { return false; }, async create(this: IHookFunctions): Promise { - if (this.getWebhookName() === 'setup') { - // Is setup-webhook which only gets used once when - // the webhook gets created so nothing to do. - return true; - } - const webhookUrl = this.getNodeWebhookUrl('default'); const credentials = this.getCredentials('trelloApi'); @@ -137,12 +125,6 @@ export class TrelloTrigger implements INodeType { return true; }, async delete(this: IHookFunctions): Promise { - if (this.getWebhookName() === 'setup') { - // Is setup-webhook which only gets used once when - // the webhook gets created so nothing to do. - return true; - } - const webhookData = this.getWorkflowStaticData('node'); if (webhookData.webhookId !== undefined) {