From 3ba7deb337963d40ae70f40ffb2f4eb23cac89b7 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Thu, 7 Dec 2023 10:00:05 +0000 Subject: [PATCH] fix: Ensure external hooks post workflow execute run in queue mode (#7947) ## Summary Since 1.8.x a refactor removed the call to `workflow.postExecute`'s External hook from the execution path. This PR adds it back to the correct place, where workers are supposed to call this, allowing us to avoid having to re-read the execution data in the caller just for the hooks. It is important to have the hooks running in the worker whenever possible to prevent having to read the full execution data in the caller. #### How to test the change: 1. Use the attached hooks file [external-hooks.txt](https://github.com/n8n-io/n8n/files/13597270/external-hooks.txt) setting it up via environment variable using `export EXTERNAL_HOOK_FILES=/path/to/hooks/external-hooks.js` 2. Set up queue mode loading this file in both main and workers 3. See that the message logs will not be displayed without this fix ## Issues fixed Include links to Github issue or Community forum post or **Linear ticket**: ## Review / Merge checklist - [ ] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md)) - [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. - [ ] Tests included. > A bug is not considered fixed, unless a test is added to prevent it from happening again. A feature is not complete without tests. > > *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e). --- .../cli/src/WorkflowExecuteAdditionalData.ts | 16 ++++++++++++++++ packages/cli/src/WorkflowRunner.ts | 6 +++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 9a52e72052..ca1a396968 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -625,6 +625,21 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { // send tracking and event log events, but don't wait for them void internalHooks.onWorkflowPostExecute(this.executionId, this.workflowData, fullRunData); }, + async function (this: WorkflowHooks, fullRunData: IRun, newStaticData: IDataObject) { + const externalHooks = Container.get(ExternalHooks); + if (externalHooks.exists('workflow.postExecute')) { + try { + await externalHooks.run('workflow.postExecute', [ + fullRunData, + this.workflowData, + this.executionId, + ]); + } catch (error) { + ErrorReporter.error(error); + console.error('There was a problem running hook "workflow.postExecute"', error); + } + } + }, ], nodeFetchedData: [ async (workflowId: string, node: INode) => { @@ -1014,6 +1029,7 @@ export function getWorkflowHooksWorkerExecuter( } hookFunctions[key]!.push.apply(hookFunctions[key], preExecuteFunctions[key]); } + return new WorkflowHooks(hookFunctions, mode, executionId, workflowData, optionalParameters); } diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 995033805e..e839333968 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -217,7 +217,11 @@ export class WorkflowRunner { // only run these when not in queue mode or when the execution is manual, // since these calls are now done by the worker directly - if (executionsMode !== 'queue' || data.executionMode === 'manual') { + if ( + executionsMode !== 'queue' || + config.getEnv('generic.instanceType') === 'worker' || + data.executionMode === 'manual' + ) { const postExecutePromise = this.activeExecutions.getPostExecutePromise(executionId); const externalHooks = Container.get(ExternalHooks); postExecutePromise