From f2666e92ffed2c3983d08e73b1e45a2bd516b90d Mon Sep 17 00:00:00 2001 From: Ben Hesseldieck <1849459+BHesseldieck@users.noreply.github.com> Date: Fri, 13 Nov 2020 23:31:27 +0100 Subject: [PATCH] :zap: Add preExecuteHooks (#1151) * :zap: Save initital data on hook error * :construction: update function interface * :construction: response webhook with error, :bug: fix adding preExecutionHooks to hooks * :fire: remove execute hook * :zap: execute preExecute hooks on integrated workflows Co-authored-by: Jan Oberhauser --- packages/cli/commands/start.ts | 1 - packages/cli/src/WebhookHelpers.ts | 26 ++++++++-------- .../cli/src/WorkflowExecuteAdditionalData.ts | 26 ++++++++++++++-- packages/cli/src/WorkflowRunner.ts | 6 ++-- packages/cli/src/WorkflowRunnerProcess.ts | 13 +++++++- packages/core/src/WorkflowExecute.ts | 30 ++++++++++++++++++- packages/workflow/src/Interfaces.ts | 2 +- packages/workflow/src/WorkflowHooks.ts | 16 ++-------- 8 files changed, 85 insertions(+), 35 deletions(-) diff --git a/packages/cli/commands/start.ts b/packages/cli/commands/start.ts index 8e14b98e2c..d6e16b7cb3 100644 --- a/packages/cli/commands/start.ts +++ b/packages/cli/commands/start.ts @@ -15,7 +15,6 @@ import { Db, ExternalHooks, GenericHelpers, - IExecutionsCurrentSummary, LoadNodesAndCredentials, NodeTypes, Server, diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index 114111a821..6d3504963a 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -222,7 +222,7 @@ export function getWorkflowWebhooksBasic(workflow: Workflow): IWebhookData[] { return; } - // Now that we know that the workflow should run we can return the default respons + // Now that we know that the workflow should run we can return the default response // directly if responseMode it set to "onReceived" and a respone should be sent if (responseMode === 'onReceived' && didSendResponse === false) { // Return response directly and do not wait for the workflow to finish @@ -302,6 +302,19 @@ export function getWorkflowWebhooksBasic(workflow: Workflow): IWebhookData[] { } const returnData = WorkflowHelpers.getDataLastExecutedNodeData(data); + if(data.data.resultData.error || returnData?.error !== undefined) { + if (didSendResponse === false) { + responseCallback(null, { + data: { + message: 'Workflow did error.', + }, + responseCode: 500, + }); + } + didSendResponse = true; + return data; + } + if (returnData === undefined) { if (didSendResponse === false) { responseCallback(null, { @@ -313,17 +326,6 @@ export function getWorkflowWebhooksBasic(workflow: Workflow): IWebhookData[] { } didSendResponse = true; return data; - } else if (returnData.error !== undefined) { - if (didSendResponse === false) { - responseCallback(null, { - data: { - message: 'Workflow did error.', - }, - responseCode: 500, - }); - } - didSendResponse = true; - return data; } const responseData = workflow.expression.getSimpleParameterValue(workflowStartNode, webhookData.webhookDescription['responseData'], 'firstEntryJson'); diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 11c15aee8e..cf755a6eaa 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -202,6 +202,18 @@ function hookFunctionsPush(): IWorkflowExecuteHooks { } +export function hookFunctionsPreExecute(parentProcessMode?: string): IWorkflowExecuteHooks { + const externalHooks = ExternalHooks(); + + return { + workflowExecuteBefore: [ + async function (this: WorkflowHooks, workflow: Workflow): Promise { + await externalHooks.run('workflow.preExecute', [workflow, this.mode]); + }, + ], + }; +} + /** * Returns hook functions to save workflow execution and call error workflow * @@ -337,7 +349,6 @@ export async function executeWorkflow(workflowInfo: IExecuteWorkflowInfo, additi const externalHooks = ExternalHooks(); await externalHooks.init(); - await externalHooks.run('workflow.execute', [workflowData, mode]); const nodeTypes = NodeTypes(); @@ -462,6 +473,10 @@ export async function getBase(credentials: IWorkflowCredentials, currentNodePara export function getWorkflowHooksIntegrated(mode: WorkflowExecuteMode, executionId: string, workflowData: IWorkflowBase, optionalParameters?: IWorkflowHooksOptionalParameters): WorkflowHooks { optionalParameters = optionalParameters || {}; const hookFunctions = hookFunctionsSave(optionalParameters.parentProcessMode); + const preExecuteFunctions = hookFunctionsPreExecute(optionalParameters.parentProcessMode); + for (const key of Object.keys(preExecuteFunctions)) { + hookFunctions[key]!.push.apply(hookFunctions[key], preExecuteFunctions[key]); + } return new WorkflowHooks(hookFunctions, mode, executionId, workflowData, optionalParameters); } @@ -474,12 +489,19 @@ export function getWorkflowHooksIntegrated(mode: WorkflowExecuteMode, executionI * @param {string} executionId * @returns {WorkflowHooks} */ -export function getWorkflowHooksMain(data: IWorkflowExecutionDataProcess, executionId: string): WorkflowHooks { +export function getWorkflowHooksMain(data: IWorkflowExecutionDataProcess, executionId: string, isMainProcess = false): WorkflowHooks { const hookFunctions = hookFunctionsSave(); const pushFunctions = hookFunctionsPush(); for (const key of Object.keys(pushFunctions)) { hookFunctions[key]!.push.apply(hookFunctions[key], pushFunctions[key]); } + if (isMainProcess) { + const preExecuteFunctions = hookFunctionsPreExecute(); + for (const key of Object.keys(preExecuteFunctions)) { + hookFunctions[key]!.push.apply(hookFunctions[key], preExecuteFunctions[key]); + } + } + return new WorkflowHooks(hookFunctions, data.executionMode, executionId, data.workflowData, { sessionId: data.sessionId, retryOf: data.retryOf as string}); } diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 3a5e197f1a..3306282a4e 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -100,9 +100,6 @@ export class WorkflowRunner { * @memberof WorkflowRunner */ async run(data: IWorkflowExecutionDataProcess, loadStaticData?: boolean): Promise { - const externalHooks = ExternalHooks(); - await externalHooks.run('workflow.execute', [data.workflowData, data.executionMode]); - const executionsProcess = config.get('executions.process') as string; let executionId: string; @@ -112,6 +109,7 @@ export class WorkflowRunner { executionId = await this.runSubprocess(data, loadStaticData); } + const externalHooks = ExternalHooks(); if (externalHooks.exists('workflow.postExecute')) { this.activeExecutions.getPostExecutePromise(executionId) .then(async (executionData) => { @@ -148,7 +146,7 @@ export class WorkflowRunner { // Register the active execution const executionId = this.activeExecutions.add(data, undefined); - additionalData.hooks = WorkflowExecuteAdditionalData.getWorkflowHooksMain(data, executionId); + additionalData.hooks = WorkflowExecuteAdditionalData.getWorkflowHooksMain(data, executionId, true); let workflowExecution: PCancelable; if (data.executionData !== undefined) { diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index 894e110297..8fc749b4b8 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -2,6 +2,7 @@ import { CredentialsOverwrites, CredentialTypes, + ExternalHooks, IWorkflowExecutionDataProcessWithExecution, NodeTypes, WorkflowExecuteAdditionalData, @@ -19,6 +20,7 @@ import { INodeTypeData, IRun, ITaskData, + IWorkflowExecuteHooks, Workflow, WorkflowHooks, } from 'n8n-workflow'; @@ -68,6 +70,10 @@ export class WorkflowRunnerProcess { const credentialsOverwrites = CredentialsOverwrites(); await credentialsOverwrites.init(inputData.credentialsOverwrite); + // Load all external hooks + const externalHooks = ExternalHooks(); + await externalHooks.init(); + this.workflow = new Workflow({ id: this.data.workflowData.id as string | undefined, name: this.data.workflowData.name, nodes: this.data.workflowData!.nodes, connections: this.data.workflowData!.connections, active: this.data.workflowData!.active, nodeTypes, staticData: this.data.workflowData!.staticData, settings: this.data.workflowData!.settings}); const additionalData = await WorkflowExecuteAdditionalData.getBase(this.data.credentials); additionalData.hooks = this.getProcessForwardHooks(); @@ -121,7 +127,7 @@ export class WorkflowRunnerProcess { * @returns */ getProcessForwardHooks(): WorkflowHooks { - const hookFunctions = { + const hookFunctions: IWorkflowExecuteHooks = { nodeExecuteBefore: [ async (nodeName: string): Promise => { this.sendHookToParentProcess('nodeExecuteBefore', [nodeName]); @@ -144,6 +150,11 @@ export class WorkflowRunnerProcess { ], }; + const preExecuteFunctions = WorkflowExecuteAdditionalData.hookFunctionsPreExecute(); + for (const key of Object.keys(preExecuteFunctions)) { + hookFunctions[key]!.push.apply(hookFunctions[key], preExecuteFunctions[key]); + } + return new WorkflowHooks(hookFunctions, this.data!.executionMode, this.data!.executionId, this.data!.workflowData, { sessionId: this.data!.sessionId, retryOf: this.data!.retryOf as string }); } diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index 4133db0d9f..4ed49770a3 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -468,7 +468,6 @@ export class WorkflowExecute { this.runExecutionData.startData = {}; } - this.executeHook('workflowExecuteBefore', []); let currentExecutionTry = ''; let lastExecutionTry = ''; @@ -482,6 +481,35 @@ export class WorkflowExecute { }); const returnPromise = (async () => { + try { + await this.executeHook('workflowExecuteBefore', [workflow]); + } catch (error) { + // Set the error that it can be saved correctly + executionError = { + message: error.message, + stack: error.stack, + }; + + // Set the incoming data of the node that it can be saved correctly + executionData = this.runExecutionData.executionData!.nodeExecutionStack[0] as IExecuteData; + this.runExecutionData.resultData = { + runData: { + [executionData.node.name]: [ + { + startTime, + executionTime: (new Date().getTime()) - startTime, + data: ({ + 'main': executionData.data.main, + } as ITaskDataConnections), + }, + ], + }, + lastNodeExecuted: executionData.node.name, + error: executionError, + }; + + throw error; + } executionLoop: while (this.runExecutionData.executionData!.nodeExecutionStack.length !== 0) { diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 492810cf2c..33524176bb 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -716,7 +716,7 @@ export interface IWorkflowExecuteHooks { nodeExecuteAfter?: Array<((nodeName: string, data: ITaskData) => Promise)>; nodeExecuteBefore?: Array<((nodeName: string) => Promise)>; workflowExecuteAfter?: Array<((data: IRun, newStaticData: IDataObject) => Promise)>; - workflowExecuteBefore?: Array<(() => Promise)>; + workflowExecuteBefore?: Array<((workflow: Workflow, data: IRunExecutionData) => Promise)>; } export interface IWorkflowExecuteAdditionalData { diff --git a/packages/workflow/src/WorkflowHooks.ts b/packages/workflow/src/WorkflowHooks.ts index 94a02abddc..92efb468a0 100644 --- a/packages/workflow/src/WorkflowHooks.ts +++ b/packages/workflow/src/WorkflowHooks.ts @@ -28,19 +28,9 @@ export class WorkflowHooks { async executeHookFunctions(hookName: string, parameters: any[]) { // tslint:disable-line:no-any if (this.hookFunctions[hookName] !== undefined && Array.isArray(this.hookFunctions[hookName])) { for (const hookFunction of this.hookFunctions[hookName]!) { - await hookFunction.apply(this, parameters) - .catch((error: Error) => { - // Catch all errors here because when "executeHook" gets called - // we have the most time no "await" and so the errors would so - // not be uncaught by anything. - - // TODO: Add proper logging - console.error(`There was a problem executing hook: "${hookName}"`); - console.error('Parameters:'); - console.error(parameters); - console.error('Error:'); - console.error(error); - }); + // TODO: As catch got removed we should make sure that we catch errors + // where hooks get called + await hookFunction.apply(this, parameters); } } }