From 667c15d0dfbdab0c29114d5002eed75031c79b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 18 Jul 2023 15:57:14 +0200 Subject: [PATCH] fix(core): Filter out workflows that failed to activate on startup (#6676) * fix(core): Deactivate on init workflow that should not be retried * fix(core): Filter out workflows with activation errors --- .vscode/launch.json | 1 + packages/cli/src/ActiveWorkflowRunner.ts | 16 +++++--- .../test/unit/ActiveWorkflowRunner.test.ts | 37 +++++++++++++++++-- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 6e81451787..448d745236 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -26,6 +26,7 @@ "name": "Launch n8n with debug", "program": "${workspaceFolder}/packages/cli/bin/n8n", "cwd": "${workspaceFolder}/packages/cli/bin", + // "args": ["start", "--tunnel"], "request": "launch", "skipFiles": ["/**"], "type": "node", diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 1b1c8bb716..66f90b3262 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -134,7 +134,7 @@ export class ActiveWorkflowRunner { } catch (error) { ErrorReporter.error(error); Logger.info( - ' => ERROR: Workflow could not be activated on first try, keep on trying', + ' => ERROR: Workflow could not be activated on first try, keep on trying if not an auth issue', ); // eslint-disable-next-line @typescript-eslint/restrict-template-expressions Logger.info(` ${error.message}`); @@ -148,8 +148,10 @@ export class ActiveWorkflowRunner { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument this.executeErrorWorkflow(error, workflowData, 'internal'); - // Keep on trying to activate the workflow - this.addQueuedWorkflowActivation('init', workflowData); + if (!error.message.includes('Authorization')) { + // Keep on trying to activate the workflow if not an auth issue + this.addQueuedWorkflowActivation('init', workflowData); + } } } Logger.verbose('Finished initializing active workflows (startup)'); @@ -350,7 +352,9 @@ export class ActiveWorkflowRunner { select: ['id'], where: { active: true }, }); - return activeWorkflows.map((workflow) => workflow.id.toString()); + return activeWorkflows + .map((workflow) => workflow.id) + .filter((workflowId) => !this.activationErrors[workflowId]); } else { const active = await Db.collections.Workflow.find({ select: ['id'], @@ -366,7 +370,9 @@ export class ActiveWorkflowRunner { select: ['workflowId'], where, }); - return shared.map((id) => id.workflowId.toString()); + return shared + .map((id) => id.workflowId) + .filter((workflowId) => !this.activationErrors[workflowId]); } } diff --git a/packages/cli/test/unit/ActiveWorkflowRunner.test.ts b/packages/cli/test/unit/ActiveWorkflowRunner.test.ts index 055795936b..39b2222148 100644 --- a/packages/cli/test/unit/ActiveWorkflowRunner.test.ts +++ b/packages/cli/test/unit/ActiveWorkflowRunner.test.ts @@ -1,8 +1,8 @@ import { v4 as uuid } from 'uuid'; import { mocked } from 'jest-mock'; -import type { ICredentialTypes, INodesAndCredentials } from 'n8n-workflow'; -import { LoggerProxy, NodeOperationError, Workflow } from 'n8n-workflow'; +import type { ICredentialTypes, INode, INodesAndCredentials } from 'n8n-workflow'; +import { LoggerProxy, NodeApiError, NodeOperationError, Workflow } from 'n8n-workflow'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import * as Db from '@/Db'; @@ -51,7 +51,7 @@ const generateWorkflows = (count: number): WorkflowEntity[] => { for (let i = 0; i < count; i++) { const workflow = new WorkflowEntity(); Object.assign(workflow, { - id: i + 1, + id: (i + 1).toString(), name: randomName(), active: true, createdAt: new Date(), @@ -260,4 +260,35 @@ describe('ActiveWorkflowRunner', () => { activeWorkflowRunner.executeErrorWorkflow(error, workflowData, 'trigger'); expect(workflowExecuteAdditionalDataExecuteErrorWorkflowSpy).toHaveBeenCalledTimes(1); }); + + describe('init()', () => { + it('should execute error workflow on failure to activate due to 401', async () => { + databaseActiveWorkflowsCount = 1; + + jest.spyOn(ActiveWorkflowRunner.prototype, 'add').mockImplementation(() => { + throw new NodeApiError( + { + id: 'a75dcd1b-9fed-4643-90bd-75933d67936c', + name: 'Github Trigger', + type: 'n8n-nodes-base.githubTrigger', + typeVersion: 1, + position: [0, 0], + } as INode, + { + httpCode: '401', + message: 'Authorization failed - please check your credentials', + }, + ); + }); + + const executeSpy = jest.spyOn(ActiveWorkflowRunner.prototype, 'executeErrorWorkflow'); + + await activeWorkflowRunner.init(); + + const [error, workflow] = executeSpy.mock.calls[0]; + + expect(error.message).toContain('Authorization'); + expect(workflow.id).toBe('1'); + }); + }); });