From 3cd804fe1d0f76c6c5fca63515f91eac8bb140ae Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Mon, 28 Oct 2024 13:51:28 +0100 Subject: [PATCH 001/672] test(editor): Add e2e test to verify Save button behavior when opening and closing Webhook node (#11407) --- ...ebhook-ndv-marks-workflow-as-unsaved.cy.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 cypress/e2e/2270-ADO-opening-webhook-ndv-marks-workflow-as-unsaved.cy.ts diff --git a/cypress/e2e/2270-ADO-opening-webhook-ndv-marks-workflow-as-unsaved.cy.ts b/cypress/e2e/2270-ADO-opening-webhook-ndv-marks-workflow-as-unsaved.cy.ts new file mode 100644 index 0000000000..eede668e1e --- /dev/null +++ b/cypress/e2e/2270-ADO-opening-webhook-ndv-marks-workflow-as-unsaved.cy.ts @@ -0,0 +1,21 @@ +import { WEBHOOK_NODE_NAME } from '../constants'; +import { NDV, WorkflowPage } from '../pages'; + +const workflowPage = new WorkflowPage(); +const ndv = new NDV(); + +describe('ADO-2270 Save button resets on webhook node open', () => { + it('should not reset the save button if webhook node is opened and closed', () => { + workflowPage.actions.visit(); + workflowPage.actions.addInitialNodeToCanvas(WEBHOOK_NODE_NAME); + workflowPage.getters.saveButton().click(); + workflowPage.actions.openNode(WEBHOOK_NODE_NAME); + + ndv.actions.close(); + + cy.ifCanvasVersion( + () => cy.getByTestId('workflow-save-button').should('not.contain', 'Saved'), + () => cy.getByTestId('workflow-save-button').should('contain', 'Saved'), + ); + }); +}); From 3f30a08c8a8ddb49f5dade69135f790091ccc399 Mon Sep 17 00:00:00 2001 From: Ivan Atanasov Date: Mon, 28 Oct 2024 14:42:27 +0100 Subject: [PATCH 002/672] chore: Add vitest extension for vs code test execution (#11414) --- .vscode/extensions.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 681de6c024..0c5abcba47 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -7,6 +7,7 @@ "EditorConfig.EditorConfig", "esbenp.prettier-vscode", "mjmlio.vscode-mjml", - "Vue.volar" + "Vue.volar", + "vitest.explorer" ] } From c56f30ce15aa03c5635bbca746c5d3ede0218cd8 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Mon, 28 Oct 2024 16:14:38 +0200 Subject: [PATCH 003/672] feat: N8N_RUNNERS_MAX_OLD_SPACE_SIZE configuration (no-changelog) (#11419) --- docker/images/n8n/n8n-task-runners.json | 3 +- .../@n8n/config/src/configs/runners.config.ts | 4 ++ packages/@n8n/config/test/config.test.ts | 1 + .../__tests__/task-runner-process.test.ts | 43 ++++++++++++++--- .../cli/src/runners/task-runner-process.ts | 47 +++++++++++++------ 5 files changed, 77 insertions(+), 21 deletions(-) diff --git a/docker/images/n8n/n8n-task-runners.json b/docker/images/n8n/n8n-task-runners.json index 56a48b2d09..4019189589 100644 --- a/docker/images/n8n/n8n-task-runners.json +++ b/docker/images/n8n/n8n-task-runners.json @@ -11,7 +11,8 @@ "N8N_RUNNERS_N8N_URI", "N8N_RUNNERS_MAX_PAYLOAD", "NODE_FUNCTION_ALLOW_BUILTIN", - "NODE_FUNCTION_ALLOW_EXTERNAL" + "NODE_FUNCTION_ALLOW_EXTERNAL", + "NODE_OPTIONS" ], "uid": 2000, "gid": 2000 diff --git a/packages/@n8n/config/src/configs/runners.config.ts b/packages/@n8n/config/src/configs/runners.config.ts index 648959e3f4..f26b96636c 100644 --- a/packages/@n8n/config/src/configs/runners.config.ts +++ b/packages/@n8n/config/src/configs/runners.config.ts @@ -42,4 +42,8 @@ export class TaskRunnersConfig { /** Which task runner to launch from the config */ @Env('N8N_RUNNERS_LAUNCHER_RUNNER') launcherRunner: string = 'javascript'; + + /** The --max-old-space-size option to use for the runner (in MB). Default means node.js will determine it based on the available memory. */ + @Env('N8N_RUNNERS_MAX_OLD_SPACE_SIZE') + maxOldSpaceSize: string = ''; } diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index f605006067..eabcd5c489 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -231,6 +231,7 @@ describe('GlobalConfig', () => { port: 5679, launcherPath: '', launcherRunner: 'javascript', + maxOldSpaceSize: '', }, sentry: { backendDsn: '', diff --git a/packages/cli/src/runners/__tests__/task-runner-process.test.ts b/packages/cli/src/runners/__tests__/task-runner-process.test.ts index 1bae991811..8940428789 100644 --- a/packages/cli/src/runners/__tests__/task-runner-process.test.ts +++ b/packages/cli/src/runners/__tests__/task-runner-process.test.ts @@ -23,7 +23,7 @@ describe('TaskRunnerProcess', () => { runnerConfig.disabled = false; runnerConfig.mode = 'internal_childprocess'; const authService = mock(); - const taskRunnerProcess = new TaskRunnerProcess(runnerConfig, authService); + let taskRunnerProcess = new TaskRunnerProcess(runnerConfig, authService); afterEach(async () => { spawnMock.mockClear(); @@ -40,10 +40,31 @@ describe('TaskRunnerProcess', () => { }); describe('start', () => { - it('should propagate NODE_FUNCTION_ALLOW_BUILTIN and NODE_FUNCTION_ALLOW_EXTERNAL from env', async () => { + beforeEach(() => { + taskRunnerProcess = new TaskRunnerProcess(runnerConfig, authService); + }); + + test.each(['PATH', 'NODE_FUNCTION_ALLOW_BUILTIN', 'NODE_FUNCTION_ALLOW_EXTERNAL'])( + 'should propagate %s from env as is', + async (envVar) => { + jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken'); + process.env[envVar] = 'custom value'; + + await taskRunnerProcess.start(); + + // @ts-expect-error The type is not correct + const options = spawnMock.mock.calls[0][2] as SpawnOptions; + expect(options.env).toEqual( + expect.objectContaining({ + [envVar]: 'custom value', + }), + ); + }, + ); + + it('should pass NODE_OPTIONS env if maxOldSpaceSize is configured', async () => { jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken'); - process.env.NODE_FUNCTION_ALLOW_BUILTIN = '*'; - process.env.NODE_FUNCTION_ALLOW_EXTERNAL = '*'; + runnerConfig.maxOldSpaceSize = '1024'; await taskRunnerProcess.start(); @@ -51,10 +72,20 @@ describe('TaskRunnerProcess', () => { const options = spawnMock.mock.calls[0][2] as SpawnOptions; expect(options.env).toEqual( expect.objectContaining({ - NODE_FUNCTION_ALLOW_BUILTIN: '*', - NODE_FUNCTION_ALLOW_EXTERNAL: '*', + NODE_OPTIONS: '--max-old-space-size=1024', }), ); }); + + it('should not pass NODE_OPTIONS env if maxOldSpaceSize is not configured', async () => { + jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken'); + runnerConfig.maxOldSpaceSize = ''; + + await taskRunnerProcess.start(); + + // @ts-expect-error The type is not correct + const options = spawnMock.mock.calls[0][2] as SpawnOptions; + expect(options.env).not.toHaveProperty('NODE_OPTIONS'); + }); }); }); diff --git a/packages/cli/src/runners/task-runner-process.ts b/packages/cli/src/runners/task-runner-process.ts index 413b74d725..0415b910b1 100644 --- a/packages/cli/src/runners/task-runner-process.ts +++ b/packages/cli/src/runners/task-runner-process.ts @@ -38,6 +38,12 @@ export class TaskRunnerProcess { private isShuttingDown = false; + private readonly passthroughEnvVars = [ + 'PATH', + 'NODE_FUNCTION_ALLOW_BUILTIN', + 'NODE_FUNCTION_ALLOW_EXTERNAL', + ] as const; + constructor( private readonly runnerConfig: TaskRunnersConfig, private readonly authService: TaskRunnerAuthService, @@ -68,26 +74,14 @@ export class TaskRunnerProcess { const startScript = require.resolve('@n8n/task-runner'); return spawn('node', [startScript], { - env: { - PATH: process.env.PATH, - N8N_RUNNERS_GRANT_TOKEN: grantToken, - N8N_RUNNERS_N8N_URI: n8nUri, - N8N_RUNNERS_MAX_PAYLOAD: this.runnerConfig.maxPayload.toString(), - NODE_FUNCTION_ALLOW_BUILTIN: process.env.NODE_FUNCTION_ALLOW_BUILTIN, - NODE_FUNCTION_ALLOW_EXTERNAL: process.env.NODE_FUNCTION_ALLOW_EXTERNAL, - }, + env: this.getProcessEnvVars(grantToken, n8nUri), }); } startLauncher(grantToken: string, n8nUri: string) { return spawn(this.runnerConfig.launcherPath, ['launch', this.runnerConfig.launcherRunner], { env: { - PATH: process.env.PATH, - N8N_RUNNERS_GRANT_TOKEN: grantToken, - N8N_RUNNERS_N8N_URI: n8nUri, - N8N_RUNNERS_MAX_PAYLOAD: this.runnerConfig.maxPayload.toString(), - NODE_FUNCTION_ALLOW_BUILTIN: process.env.NODE_FUNCTION_ALLOW_BUILTIN, - NODE_FUNCTION_ALLOW_EXTERNAL: process.env.NODE_FUNCTION_ALLOW_EXTERNAL, + ...this.getProcessEnvVars(grantToken, n8nUri), // For debug logging if enabled RUST_LOG: process.env.RUST_LOG, }, @@ -155,4 +149,29 @@ export class TaskRunnerProcess { setImmediate(async () => await this.start()); } } + + private getProcessEnvVars(grantToken: string, n8nUri: string) { + const envVars: Record = { + N8N_RUNNERS_GRANT_TOKEN: grantToken, + N8N_RUNNERS_N8N_URI: n8nUri, + N8N_RUNNERS_MAX_PAYLOAD: this.runnerConfig.maxPayload.toString(), + ...this.getPassthroughEnvVars(), + }; + + if (this.runnerConfig.maxOldSpaceSize) { + envVars.NODE_OPTIONS = `--max-old-space-size=${this.runnerConfig.maxOldSpaceSize}`; + } + + return envVars; + } + + private getPassthroughEnvVars() { + return this.passthroughEnvVars.reduce>((env, key) => { + if (process.env[key]) { + env[key] = process.env[key]; + } + + return env; + }, {}); + } } From d4c4db823e7ddbe6707dd16ae74c76aa8efaae4d Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Mon, 28 Oct 2024 16:44:03 +0200 Subject: [PATCH 004/672] feat: Forward logs from task runner to logger (no-changelog) (#11422) --- .../@n8n/config/src/configs/logging.config.ts | 1 + .../__tests__/forward-to-logger.test.ts | 114 ++++++++++++++++++ .../__tests__/task-runner-process.test.ts | 14 ++- packages/cli/src/runners/forward-to-logger.ts | 42 +++++++ .../cli/src/runners/task-runner-process.ts | 13 +- 5 files changed, 175 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/runners/__tests__/forward-to-logger.test.ts create mode 100644 packages/cli/src/runners/forward-to-logger.ts diff --git a/packages/@n8n/config/src/configs/logging.config.ts b/packages/@n8n/config/src/configs/logging.config.ts index 0568eaf791..94e4642223 100644 --- a/packages/@n8n/config/src/configs/logging.config.ts +++ b/packages/@n8n/config/src/configs/logging.config.ts @@ -11,6 +11,7 @@ export const LOG_SCOPES = [ 'redis', 'scaling', 'waiting-executions', + 'task-runner', ] as const; export type LogScope = (typeof LOG_SCOPES)[number]; diff --git a/packages/cli/src/runners/__tests__/forward-to-logger.test.ts b/packages/cli/src/runners/__tests__/forward-to-logger.test.ts new file mode 100644 index 0000000000..64352ab54d --- /dev/null +++ b/packages/cli/src/runners/__tests__/forward-to-logger.test.ts @@ -0,0 +1,114 @@ +import type { Logger } from 'n8n-workflow'; +import { Readable } from 'stream'; + +import { forwardToLogger } from '../forward-to-logger'; + +describe('forwardToLogger', () => { + let logger: Logger; + let stdout: Readable; + let stderr: Readable; + + beforeEach(() => { + logger = { + info: jest.fn(), + error: jest.fn(), + } as unknown as Logger; + + stdout = new Readable({ read() {} }); + stderr = new Readable({ read() {} }); + + jest.resetAllMocks(); + }); + + const pushToStdout = async (data: string) => { + stdout.push(Buffer.from(data)); + stdout.push(null); + // Wait for the next tick to allow the event loop to process the data + await new Promise((resolve) => setImmediate(resolve)); + }; + + const pushToStderr = async (data: string) => { + stderr.push(Buffer.from(data)); + stderr.push(null); + // Wait for the next tick to allow the event loop to process the data + await new Promise((resolve) => setImmediate(resolve)); + }; + + it('should forward stdout data to logger.info', async () => { + forwardToLogger(logger, { stdout, stderr: null }); + + await pushToStdout('Test stdout message'); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(logger.info).toHaveBeenCalledWith('Test stdout message'); + }); + + it('should forward stderr data to logger.error', async () => { + forwardToLogger(logger, { stdout: null, stderr }); + + await pushToStderr('Test stderr message'); + + expect(logger.error).toHaveBeenCalledWith('Test stderr message'); + }); + + it('should remove trailing newline from stdout', async () => { + forwardToLogger(logger, { stdout, stderr: null }); + + await pushToStdout('Test stdout message\n'); + + expect(logger.info).toHaveBeenCalledWith('Test stdout message'); + }); + + it('should remove trailing newline from stderr', async () => { + forwardToLogger(logger, { stdout: null, stderr }); + + await pushToStderr('Test stderr message\n'); + + expect(logger.error).toHaveBeenCalledWith('Test stderr message'); + }); + + it('should forward stderr data to logger.error', async () => { + forwardToLogger(logger, { stdout: null, stderr }); + + await pushToStderr('Test stderr message'); + + expect(logger.error).toHaveBeenCalledWith('Test stderr message'); + }); + + it('should include prefix if provided for stdout', async () => { + const prefix = '[PREFIX]'; + forwardToLogger(logger, { stdout, stderr: null }, prefix); + + await pushToStdout('Message with prefix'); + + expect(logger.info).toHaveBeenCalledWith('[PREFIX] Message with prefix'); + }); + + it('should include prefix if provided for stderr', async () => { + const prefix = '[PREFIX]'; + forwardToLogger(logger, { stdout: null, stderr }, prefix); + + await pushToStderr('Error message with prefix'); + + expect(logger.error).toHaveBeenCalledWith('[PREFIX] Error message with prefix'); + }); + + it('should make sure there is no duplicate space after prefix for stdout', async () => { + const prefix = '[PREFIX] '; + forwardToLogger(logger, { stdout, stderr: null }, prefix); + + await pushToStdout('Message with prefix'); + + expect(logger.info).toHaveBeenCalledWith('[PREFIX] Message with prefix'); + }); + + it('should make sure there is no duplicate space after prefix for stderr', async () => { + const prefix = '[PREFIX] '; + forwardToLogger(logger, { stdout: null, stderr }, prefix); + + await pushToStderr('Error message with prefix'); + + expect(logger.error).toHaveBeenCalledWith('[PREFIX] Error message with prefix'); + }); +}); diff --git a/packages/cli/src/runners/__tests__/task-runner-process.test.ts b/packages/cli/src/runners/__tests__/task-runner-process.test.ts index 8940428789..eb04e3ab8e 100644 --- a/packages/cli/src/runners/__tests__/task-runner-process.test.ts +++ b/packages/cli/src/runners/__tests__/task-runner-process.test.ts @@ -2,9 +2,10 @@ import { TaskRunnersConfig } from '@n8n/config'; import { mock } from 'jest-mock-extended'; import type { ChildProcess, SpawnOptions } from 'node:child_process'; -import { mockInstance } from '../../../test/shared/mocking'; -import type { TaskRunnerAuthService } from '../auth/task-runner-auth.service'; -import { TaskRunnerProcess } from '../task-runner-process'; +import { Logger } from '@/logging/logger.service'; +import type { TaskRunnerAuthService } from '@/runners/auth/task-runner-auth.service'; +import { TaskRunnerProcess } from '@/runners/task-runner-process'; +import { mockInstance } from '@test/mocking'; const spawnMock = jest.fn(() => mock({ @@ -19,11 +20,12 @@ const spawnMock = jest.fn(() => require('child_process').spawn = spawnMock; describe('TaskRunnerProcess', () => { + const logger = mockInstance(Logger); const runnerConfig = mockInstance(TaskRunnersConfig); runnerConfig.disabled = false; runnerConfig.mode = 'internal_childprocess'; const authService = mock(); - let taskRunnerProcess = new TaskRunnerProcess(runnerConfig, authService); + let taskRunnerProcess = new TaskRunnerProcess(logger, runnerConfig, authService); afterEach(async () => { spawnMock.mockClear(); @@ -33,7 +35,7 @@ describe('TaskRunnerProcess', () => { it('should throw if runner mode is external', () => { runnerConfig.mode = 'external'; - expect(() => new TaskRunnerProcess(runnerConfig, authService)).toThrow(); + expect(() => new TaskRunnerProcess(logger, runnerConfig, authService)).toThrow(); runnerConfig.mode = 'internal_childprocess'; }); @@ -41,7 +43,7 @@ describe('TaskRunnerProcess', () => { describe('start', () => { beforeEach(() => { - taskRunnerProcess = new TaskRunnerProcess(runnerConfig, authService); + taskRunnerProcess = new TaskRunnerProcess(logger, runnerConfig, authService); }); test.each(['PATH', 'NODE_FUNCTION_ALLOW_BUILTIN', 'NODE_FUNCTION_ALLOW_EXTERNAL'])( diff --git a/packages/cli/src/runners/forward-to-logger.ts b/packages/cli/src/runners/forward-to-logger.ts new file mode 100644 index 0000000000..0bcc813225 --- /dev/null +++ b/packages/cli/src/runners/forward-to-logger.ts @@ -0,0 +1,42 @@ +import type { Logger } from 'n8n-workflow'; +import type { Readable } from 'stream'; + +/** + * Forwards stdout and stderr of a given producer to the given + * logger's info and error methods respectively. + */ +export function forwardToLogger( + logger: Logger, + producer: { + stdout?: Readable | null; + stderr?: Readable | null; + }, + prefix?: string, +) { + if (prefix) { + prefix = prefix.trimEnd(); + } + + const stringify = (data: Buffer) => { + let str = data.toString(); + + // Remove possible trailing newline (otherwise it's duplicated) + if (str.endsWith('\n')) { + str = str.slice(0, -1); + } + + return prefix ? `${prefix} ${str}` : str; + }; + + if (producer.stdout) { + producer.stdout.on('data', (data: Buffer) => { + logger.info(stringify(data)); + }); + } + + if (producer.stderr) { + producer.stderr.on('data', (data: Buffer) => { + logger.error(stringify(data)); + }); + } +} diff --git a/packages/cli/src/runners/task-runner-process.ts b/packages/cli/src/runners/task-runner-process.ts index 0415b910b1..f4c219ead7 100644 --- a/packages/cli/src/runners/task-runner-process.ts +++ b/packages/cli/src/runners/task-runner-process.ts @@ -4,8 +4,11 @@ import { spawn } from 'node:child_process'; import * as process from 'node:process'; import { Service } from 'typedi'; +import { OnShutdown } from '@/decorators/on-shutdown'; +import { Logger } from '@/logging/logger.service'; + import { TaskRunnerAuthService } from './auth/task-runner-auth.service'; -import { OnShutdown } from '../decorators/on-shutdown'; +import { forwardToLogger } from './forward-to-logger'; type ChildProcess = ReturnType; @@ -38,6 +41,8 @@ export class TaskRunnerProcess { private isShuttingDown = false; + private logger: Logger; + private readonly passthroughEnvVars = [ 'PATH', 'NODE_FUNCTION_ALLOW_BUILTIN', @@ -45,6 +50,7 @@ export class TaskRunnerProcess { ] as const; constructor( + logger: Logger, private readonly runnerConfig: TaskRunnersConfig, private readonly authService: TaskRunnerAuthService, ) { @@ -52,6 +58,8 @@ export class TaskRunnerProcess { this.runnerConfig.mode === 'internal_childprocess' || this.runnerConfig.mode === 'internal_launcher', ); + + this.logger = logger.scoped('task-runner'); } async start() { @@ -64,8 +72,7 @@ export class TaskRunnerProcess { ? this.startLauncher(grantToken, n8nUri) : this.startNode(grantToken, n8nUri); - this.process.stdout?.pipe(process.stdout); - this.process.stderr?.pipe(process.stderr); + forwardToLogger(this.logger, this.process, '[Task Runner]: '); this.monitorProcess(this.process); } From ad292350b3206d92ffe69d0eea01a0c4f396781a Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Mon, 28 Oct 2024 17:47:12 +0200 Subject: [PATCH 005/672] fix(editor): Import copy-pasted workflow only after nodes are added to new canvas (no-changelog) (#11434) --- .../__tests__/useCanvasOperations.spec.ts | 24 +++++++++++++- .../src/composables/useCanvasOperations.ts | 31 ++++--------------- packages/editor-ui/src/types/canvas.ts | 2 ++ packages/editor-ui/src/views/NodeView.v2.vue | 16 ++++++++-- 4 files changed, 45 insertions(+), 28 deletions(-) diff --git a/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts b/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts index d54f7ccbfc..7128c223fe 100644 --- a/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts +++ b/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts @@ -3,6 +3,7 @@ import type { IConnection, Workflow } from 'n8n-workflow'; import { NodeConnectionType, NodeHelpers } from 'n8n-workflow'; import { useCanvasOperations } from '@/composables/useCanvasOperations'; import type { CanvasNode } from '@/types'; +import { CanvasConnectionMode } from '@/types'; import type { ICredentialsResponse, INodeUi, IWorkflowDb } from '@/Interface'; import { RemoveNodeCommand } from '@/models/history'; import { useWorkflowsStore } from '@/stores/workflows.store'; @@ -25,6 +26,7 @@ import { mockedStore } from '@/__tests__/utils'; import { SET_NODE_TYPE, STICKY_NODE_TYPE, STORES } from '@/constants'; import type { Connection } from '@vue-flow/core'; import { useClipboard } from '@/composables/useClipboard'; +import { createCanvasConnectionHandleString } from '@/utils/canvasUtilsV2'; vi.mock('vue-router', async (importOriginal) => { const actual = await importOriginal<{}>(); @@ -966,7 +968,17 @@ describe('useCanvasOperations', () => { const connections = [ { source: nodes[0].id, + sourceHandle: createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Output, + index: 0, + type: NodeConnectionType.Main, + }), target: nodes[1].id, + targetHandle: createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Input, + index: 0, + type: NodeConnectionType.Main, + }), data: { source: { type: NodeConnectionType.Main, index: 0 }, target: { type: NodeConnectionType.Main, index: 0 }, @@ -974,7 +986,17 @@ describe('useCanvasOperations', () => { }, { source: nodes[1].id, + sourceHandle: createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Output, + index: 0, + type: NodeConnectionType.Main, + }), target: nodes[2].id, + targetHandle: createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Input, + index: 0, + type: NodeConnectionType.Main, + }), data: { source: { type: NodeConnectionType.Main, index: 0 }, target: { type: NodeConnectionType.Main, index: 0 }, @@ -993,7 +1015,7 @@ describe('useCanvasOperations', () => { nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeType); const { addConnections } = useCanvasOperations({ router }); - addConnections(connections); + await addConnections(connections); expect(workflowsStore.addConnection).toHaveBeenCalledWith({ connection: [ diff --git a/packages/editor-ui/src/composables/useCanvasOperations.ts b/packages/editor-ui/src/composables/useCanvasOperations.ts index 4ee13581d2..3950e16ecf 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.ts @@ -71,7 +71,6 @@ import { generateOffsets, PUSH_NODES_OFFSET, } from '@/utils/nodeViewUtils'; -import { isValidNodeConnectionType } from '@/utils/typeGuards'; import type { Connection } from '@vue-flow/core'; import type { IConnection, @@ -1374,36 +1373,18 @@ export function useCanvasOperations({ router }: { router: ReturnType; export type CanvasConnectionCreateData = { source: string; + sourceHandle: string; target: string; + targetHandle: string; data: { source: PartialBy; target: PartialBy; diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index 60f4f8983b..95dee594a7 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -46,7 +46,7 @@ import type { CanvasNodeMoveEvent, ConnectStartEvent, } from '@/types'; -import { CanvasNodeRenderType } from '@/types'; +import { CanvasNodeRenderType, CanvasConnectionMode } from '@/types'; import { CHAT_TRIGGER_NODE_TYPE, EnterpriseEditionFeature, @@ -105,6 +105,8 @@ import { useClipboard } from '@/composables/useClipboard'; import { useBeforeUnload } from '@/composables/useBeforeUnload'; import { getResourcePermissions } from '@/permissions'; import NodeViewUnfinishedWorkflowMessage from '@/components/NodeViewUnfinishedWorkflowMessage.vue'; +import { createCanvasConnectionHandleString } from '@/utils/canvasUtilsV2'; +import { isValidNodeConnectionType } from '@/utils/typeGuards'; const LazyNodeCreation = defineAsyncComponent( async () => await import('@/components/Node/NodeCreation.vue'), @@ -879,7 +881,17 @@ async function onAddNodesAndConnections( return { source: fromNode.id, + sourceHandle: createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Output, + type: isValidNodeConnectionType(type) ? type : NodeConnectionType.Main, + index: from.outputIndex ?? 0, + }), target: toNode.id, + targetHandle: createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Input, + type: isValidNodeConnectionType(type) ? type : NodeConnectionType.Main, + index: to.inputIndex ?? 0, + }), data: { source: { index: from.outputIndex ?? 0, @@ -893,7 +905,7 @@ async function onAddNodesAndConnections( }; }); - addConnections(mappedConnections); + await addConnections(mappedConnections); uiStore.resetLastInteractedWith(); selectNodes([addedNodes[addedNodes.length - 1].id]); From c152a3ac56f140a39eea4771a94f5a3082118df7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 29 Oct 2024 08:51:55 +0100 Subject: [PATCH 006/672] fix(core): Ensure job processor does not reprocess amended executions (#11438) --- .../__tests__/job-processor.service.test.ts | 21 +++++++++++++++++++ packages/cli/src/scaling/job-processor.ts | 7 +++++++ packages/cli/src/workflow-runner.ts | 12 ----------- 3 files changed, 28 insertions(+), 12 deletions(-) create mode 100644 packages/cli/src/scaling/__tests__/job-processor.service.test.ts diff --git a/packages/cli/src/scaling/__tests__/job-processor.service.test.ts b/packages/cli/src/scaling/__tests__/job-processor.service.test.ts new file mode 100644 index 0000000000..6a3fa5caa4 --- /dev/null +++ b/packages/cli/src/scaling/__tests__/job-processor.service.test.ts @@ -0,0 +1,21 @@ +import { mock } from 'jest-mock-extended'; + +import type { ExecutionRepository } from '@/databases/repositories/execution.repository'; +import type { IExecutionResponse } from '@/interfaces'; + +import { JobProcessor } from '../job-processor'; +import type { Job } from '../scaling.types'; + +describe('JobProcessor', () => { + it('should refrain from processing a crashed execution', async () => { + const executionRepository = mock(); + executionRepository.findSingleExecution.mockResolvedValue( + mock({ status: 'crashed' }), + ); + const jobProcessor = new JobProcessor(mock(), executionRepository, mock(), mock(), mock()); + + const result = await jobProcessor.processJob(mock()); + + expect(result).toEqual({ success: false }); + }); +}); diff --git a/packages/cli/src/scaling/job-processor.ts b/packages/cli/src/scaling/job-processor.ts index 9a531d3039..6bf2524304 100644 --- a/packages/cli/src/scaling/job-processor.ts +++ b/packages/cli/src/scaling/job-processor.ts @@ -58,6 +58,13 @@ export class JobProcessor { ); } + /** + * Bull's implicit retry mechanism and n8n's execution recovery mechanism may + * cause a crashed execution to be enqueued. We refrain from processing it, + * until we have reworked both mechanisms to prevent this scenario. + */ + if (execution.status === 'crashed') return { success: false }; + const workflowId = execution.workflowData.id; this.logger.info(`Worker started execution ${executionId} (job ${job.id})`, { diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index 4dd5e08714..02e0b94afd 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -14,7 +14,6 @@ import type { IWorkflowExecutionDataProcess, } from 'n8n-workflow'; import { - ApplicationError, ErrorReporterProxy as ErrorReporter, ExecutionCancelledError, Workflow, @@ -381,17 +380,6 @@ export class WorkflowRunner { let job: Job; let hooks: WorkflowHooks; try { - // check to help diagnose PAY-2100 - if ( - data.executionData?.executionData?.nodeExecutionStack?.length === 0 && - config.getEnv('deployment.type') === 'internal' - ) { - await this.executionRepository.setRunning(executionId); // set `startedAt` so we display it correctly in UI - throw new ApplicationError('Execution to enqueue has empty node execution stack', { - extra: { executionData: data.executionData }, - }); - } - job = await this.scalingService.addJob(jobData, { priority: realtime ? 50 : 100 }); hooks = WorkflowExecuteAdditionalData.getWorkflowHooksWorkerMain( From 097f93564c5d22c947e0e120f0967eea0e6b838e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 29 Oct 2024 08:52:07 +0100 Subject: [PATCH 007/672] refactor(core): Port `security` config (#11440) --- .../config/src/configs/security.config.ts | 27 +++++++++++++++++++ packages/@n8n/config/src/index.ts | 5 ++++ packages/@n8n/config/test/config.test.ts | 5 ++++ packages/cli/src/commands/audit.ts | 4 +-- packages/cli/src/config/schema.ts | 23 ---------------- .../credentials-risk-reporter.ts | 5 ++-- .../security-audit/security-audit.service.ts | 8 ++++-- packages/cli/src/services/frontend.service.ts | 5 ++-- .../credentials-risk-reporter.test.ts | 14 +++++++--- .../database-risk-reporter.test.ts | 3 ++- .../filesystem-risk-reporter.test.ts | 3 ++- .../instance-risk-reporter.test.ts | 3 ++- .../nodes-risk-reporter.test.ts | 3 ++- 13 files changed, 69 insertions(+), 39 deletions(-) create mode 100644 packages/@n8n/config/src/configs/security.config.ts diff --git a/packages/@n8n/config/src/configs/security.config.ts b/packages/@n8n/config/src/configs/security.config.ts new file mode 100644 index 0000000000..329e84cc43 --- /dev/null +++ b/packages/@n8n/config/src/configs/security.config.ts @@ -0,0 +1,27 @@ +import { Config, Env } from '../decorators'; + +@Config +export class SecurityConfig { + /** + * Which directories to limit n8n's access to. Separate multiple dirs with semicolon `;`. + * + * @example N8N_RESTRICT_FILE_ACCESS_TO=/home/user/.n8n;/home/user/n8n-data + */ + @Env('N8N_RESTRICT_FILE_ACCESS_TO') + restrictFileAccessTo: string = ''; + + /** + * Whether to block access to all files at: + * - the ".n8n" directory, + * - the static cache dir at ~/.cache/n8n/public, and + * - user-defined config files. + */ + @Env('N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES') + blockFileAccessToN8nFiles: boolean = true; + + /** + * In a [security audit](https://docs.n8n.io/hosting/securing/security-audit/), how many days for a workflow to be considered abandoned if not executed. + */ + @Env('N8N_SECURITY_AUDIT_DAYS_ABANDONED_WORKFLOW') + daysAbandonedWorkflow: number = 90; +} diff --git a/packages/@n8n/config/src/index.ts b/packages/@n8n/config/src/index.ts index 9ebfb12465..1a2a3127ad 100644 --- a/packages/@n8n/config/src/index.ts +++ b/packages/@n8n/config/src/index.ts @@ -13,6 +13,7 @@ import { NodesConfig } from './configs/nodes.config'; import { PublicApiConfig } from './configs/public-api.config'; import { TaskRunnersConfig } from './configs/runners.config'; import { ScalingModeConfig } from './configs/scaling-mode.config'; +import { SecurityConfig } from './configs/security.config'; import { SentryConfig } from './configs/sentry.config'; import { TemplatesConfig } from './configs/templates.config'; import { UserManagementConfig } from './configs/user-management.config'; @@ -22,6 +23,7 @@ import { Config, Env, Nested } from './decorators'; export { Config, Env, Nested } from './decorators'; export { TaskRunnersConfig } from './configs/runners.config'; +export { SecurityConfig } from './configs/security.config'; export { LOG_SCOPES } from './configs/logging.config'; export type { LogScope } from './configs/logging.config'; @@ -106,4 +108,7 @@ export class GlobalConfig { @Nested license: LicenseConfig; + + @Nested + security: SecurityConfig; } diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index eabcd5c489..cd5438e248 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -265,6 +265,11 @@ describe('GlobalConfig', () => { tenantId: 1, cert: '', }, + security: { + restrictFileAccessTo: '', + blockFileAccessToN8nFiles: true, + daysAbandonedWorkflow: 90, + }, }; it('should use all default values when no env variables are defined', () => { diff --git a/packages/cli/src/commands/audit.ts b/packages/cli/src/commands/audit.ts index e86c8c9ab5..e98bb8bce0 100644 --- a/packages/cli/src/commands/audit.ts +++ b/packages/cli/src/commands/audit.ts @@ -1,8 +1,8 @@ +import { SecurityConfig } from '@n8n/config'; import { Flags } from '@oclif/core'; import { ApplicationError } from 'n8n-workflow'; import { Container } from 'typedi'; -import config from '@/config'; import { RISK_CATEGORIES } from '@/security-audit/constants'; import { SecurityAuditService } from '@/security-audit/security-audit.service'; import type { Risk } from '@/security-audit/types'; @@ -26,7 +26,7 @@ export class SecurityAudit extends BaseCommand { }), 'days-abandoned-workflow': Flags.integer({ - default: config.getEnv('security.audit.daysAbandonedWorkflow'), + default: Container.get(SecurityConfig).daysAbandonedWorkflow, description: 'Days for a workflow to be considered abandoned if not executed', }), }; diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 9b711f9766..8bece9199a 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -187,29 +187,6 @@ export const schema = { doc: 'Public URL where the editor is accessible. Also used for emails sent from n8n.', }, - security: { - restrictFileAccessTo: { - doc: 'If set only files in that directories can be accessed. Multiple directories can be separated by semicolon (";").', - format: String, - default: '', - env: 'N8N_RESTRICT_FILE_ACCESS_TO', - }, - blockFileAccessToN8nFiles: { - doc: 'If set to true it will block access to all files in the ".n8n" directory, the static cache dir at ~/.cache/n8n/public, and user defined config files.', - format: Boolean, - default: true, - env: 'N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES', - }, - audit: { - daysAbandonedWorkflow: { - doc: 'Days for a workflow to be considered abandoned if not executed', - format: Number, - default: 90, - env: 'N8N_SECURITY_AUDIT_DAYS_ABANDONED_WORKFLOW', - }, - }, - }, - workflowTagsDisabled: { format: Boolean, default: false, diff --git a/packages/cli/src/security-audit/risk-reporters/credentials-risk-reporter.ts b/packages/cli/src/security-audit/risk-reporters/credentials-risk-reporter.ts index ab7873e808..0c8d84211e 100644 --- a/packages/cli/src/security-audit/risk-reporters/credentials-risk-reporter.ts +++ b/packages/cli/src/security-audit/risk-reporters/credentials-risk-reporter.ts @@ -1,7 +1,7 @@ +import { SecurityConfig } from '@n8n/config'; import type { IWorkflowBase } from 'n8n-workflow'; import { Service } from 'typedi'; -import config from '@/config'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository'; @@ -15,10 +15,11 @@ export class CredentialsRiskReporter implements RiskReporter { private readonly credentialsRepository: CredentialsRepository, private readonly executionRepository: ExecutionRepository, private readonly executionDataRepository: ExecutionDataRepository, + private readonly securityConfig: SecurityConfig, ) {} async report(workflows: WorkflowEntity[]) { - const days = config.getEnv('security.audit.daysAbandonedWorkflow'); + const days = this.securityConfig.daysAbandonedWorkflow; const allExistingCreds = await this.getAllExistingCreds(); const { credsInAnyUse, credsInActiveUse } = await this.getAllCredsInUse(workflows); diff --git a/packages/cli/src/security-audit/security-audit.service.ts b/packages/cli/src/security-audit/security-audit.service.ts index 19582450c4..97b5424a19 100644 --- a/packages/cli/src/security-audit/security-audit.service.ts +++ b/packages/cli/src/security-audit/security-audit.service.ts @@ -1,3 +1,4 @@ +import { SecurityConfig } from '@n8n/config'; import Container, { Service } from 'typedi'; import config from '@/config'; @@ -8,7 +9,10 @@ import { toReportTitle } from '@/security-audit/utils'; @Service() export class SecurityAuditService { - constructor(private readonly workflowRepository: WorkflowRepository) {} + constructor( + private readonly workflowRepository: WorkflowRepository, + private readonly securityConfig: SecurityConfig, + ) {} private reporters: { [name: string]: RiskReporter; @@ -19,7 +23,7 @@ export class SecurityAuditService { await this.initReporters(categories); - const daysFromEnv = config.getEnv('security.audit.daysAbandonedWorkflow'); + const daysFromEnv = this.securityConfig.daysAbandonedWorkflow; if (daysAbandonedWorkflow) { config.set('security.audit.daysAbandonedWorkflow', daysAbandonedWorkflow); diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 144540467f..2916f191f3 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -1,5 +1,5 @@ import type { FrontendSettings, ITelemetrySettings } from '@n8n/api-types'; -import { GlobalConfig } from '@n8n/config'; +import { GlobalConfig, SecurityConfig } from '@n8n/config'; import { createWriteStream } from 'fs'; import { mkdir } from 'fs/promises'; import uniq from 'lodash/uniq'; @@ -46,6 +46,7 @@ export class FrontendService { private readonly mailer: UserManagementMailer, private readonly instanceSettings: InstanceSettings, private readonly urlService: UrlService, + private readonly securityConfig: SecurityConfig, ) { loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes()); void this.generateTypes(); @@ -225,7 +226,7 @@ export class FrontendService { maxCount: config.getEnv('executions.pruneDataMaxCount'), }, security: { - blockFileAccessToN8nFiles: config.getEnv('security.blockFileAccessToN8nFiles'), + blockFileAccessToN8nFiles: this.securityConfig.blockFileAccessToN8nFiles, }, }; } diff --git a/packages/cli/test/integration/security-audit/credentials-risk-reporter.test.ts b/packages/cli/test/integration/security-audit/credentials-risk-reporter.test.ts index 4513beb6bb..b5b4c122df 100644 --- a/packages/cli/test/integration/security-audit/credentials-risk-reporter.test.ts +++ b/packages/cli/test/integration/security-audit/credentials-risk-reporter.test.ts @@ -1,7 +1,8 @@ +import type { SecurityConfig } from '@n8n/config'; +import { mock } from 'jest-mock-extended'; import Container from 'typedi'; import { v4 as uuid } from 'uuid'; -import config from '@/config'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; @@ -15,10 +16,15 @@ import * as testDb from '../shared/test-db'; let securityAuditService: SecurityAuditService; +const securityConfig = mock({ daysAbandonedWorkflow: 90 }); + beforeAll(async () => { await testDb.init(); - securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); + securityAuditService = new SecurityAuditService( + Container.get(WorkflowRepository), + securityConfig, + ); }); beforeEach(async () => { @@ -154,7 +160,7 @@ test('should report credential in not recently executed workflow', async () => { const workflow = await Container.get(WorkflowRepository).save(workflowDetails); const date = new Date(); - date.setDate(date.getDate() - config.getEnv('security.audit.daysAbandonedWorkflow') - 1); + date.setDate(date.getDate() - securityConfig.daysAbandonedWorkflow - 1); const savedExecution = await Container.get(ExecutionRepository).save({ finished: true, @@ -223,7 +229,7 @@ test('should not report credentials in recently executed workflow', async () => const workflow = await Container.get(WorkflowRepository).save(workflowDetails); const date = new Date(); - date.setDate(date.getDate() - config.getEnv('security.audit.daysAbandonedWorkflow') + 1); + date.setDate(date.getDate() - securityConfig.daysAbandonedWorkflow + 1); const savedExecution = await Container.get(ExecutionRepository).save({ finished: true, diff --git a/packages/cli/test/integration/security-audit/database-risk-reporter.test.ts b/packages/cli/test/integration/security-audit/database-risk-reporter.test.ts index d519f97a23..3aef57396b 100644 --- a/packages/cli/test/integration/security-audit/database-risk-reporter.test.ts +++ b/packages/cli/test/integration/security-audit/database-risk-reporter.test.ts @@ -1,3 +1,4 @@ +import { mock } from 'jest-mock-extended'; import Container from 'typedi'; import { v4 as uuid } from 'uuid'; @@ -18,7 +19,7 @@ let securityAuditService: SecurityAuditService; beforeAll(async () => { await testDb.init(); - securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); + securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock()); }); beforeEach(async () => { diff --git a/packages/cli/test/integration/security-audit/filesystem-risk-reporter.test.ts b/packages/cli/test/integration/security-audit/filesystem-risk-reporter.test.ts index 34bcb83b49..ceb306935f 100644 --- a/packages/cli/test/integration/security-audit/filesystem-risk-reporter.test.ts +++ b/packages/cli/test/integration/security-audit/filesystem-risk-reporter.test.ts @@ -1,3 +1,4 @@ +import { mock } from 'jest-mock-extended'; import Container from 'typedi'; import { v4 as uuid } from 'uuid'; @@ -13,7 +14,7 @@ let securityAuditService: SecurityAuditService; beforeAll(async () => { await testDb.init(); - securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); + securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock()); }); beforeEach(async () => { diff --git a/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts b/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts index 4f355cbcbc..928667b518 100644 --- a/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts +++ b/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts @@ -1,3 +1,4 @@ +import { mock } from 'jest-mock-extended'; import { NodeConnectionType } from 'n8n-workflow'; import Container from 'typedi'; import { v4 as uuid } from 'uuid'; @@ -23,7 +24,7 @@ let securityAuditService: SecurityAuditService; beforeAll(async () => { await testDb.init(); - securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); + securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock()); simulateUpToDateInstance(); }); diff --git a/packages/cli/test/integration/security-audit/nodes-risk-reporter.test.ts b/packages/cli/test/integration/security-audit/nodes-risk-reporter.test.ts index 133a574d40..c1fb198b69 100644 --- a/packages/cli/test/integration/security-audit/nodes-risk-reporter.test.ts +++ b/packages/cli/test/integration/security-audit/nodes-risk-reporter.test.ts @@ -1,3 +1,4 @@ +import { mock } from 'jest-mock-extended'; import { Container } from 'typedi'; import { v4 as uuid } from 'uuid'; @@ -24,7 +25,7 @@ let securityAuditService: SecurityAuditService; beforeAll(async () => { await testDb.init(); - securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); + securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock()); }); beforeEach(async () => { From 4f511aab68651caa8fe47f70cd7cdb88bb06a3e2 Mon Sep 17 00:00:00 2001 From: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com> Date: Tue, 29 Oct 2024 08:54:35 +0000 Subject: [PATCH 008/672] fix: Update required node js version in CONTRIBUTING.md (#11437) --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4a67bf00ac..3f19be15e9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,7 +68,7 @@ If you already have VS Code and Docker installed, you can click [here](https://v #### Node.js -[Node.js](https://nodejs.org/en/) version 18.10 or newer is required for development purposes. +[Node.js](https://nodejs.org/en/) version 20.15 or newer is required for development purposes. #### pnpm From 7a8dafe9902fbc0d5001c50579c34959b95211ab Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Tue, 29 Oct 2024 10:45:03 +0100 Subject: [PATCH 009/672] fix(core): Add 'user_id' to `license-community-plus-registered` telemetry event (#11430) --- .../cli/src/events/__tests__/telemetry-event-relay.test.ts | 2 ++ packages/cli/src/events/maps/relay.event-map.ts | 3 ++- packages/cli/src/events/relays/telemetry.event-relay.ts | 2 ++ packages/cli/src/license/__tests__/license.service.test.ts | 3 +++ packages/cli/src/license/license.controller.ts | 5 +++-- packages/cli/src/license/license.service.ts | 4 +++- 6 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts index df65a70ecb..7e98877dc7 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -1061,6 +1061,7 @@ describe('TelemetryEventRelay', () => { describe('Community+ registered', () => { it('should track `license-community-plus-registered` event', () => { const event: RelayEventMap['license-community-plus-registered'] = { + userId: 'user123', email: 'user@example.com', licenseKey: 'license123', }; @@ -1068,6 +1069,7 @@ describe('TelemetryEventRelay', () => { eventService.emit('license-community-plus-registered', event); expect(telemetry.track).toHaveBeenCalledWith('User registered for license community plus', { + user_id: 'user123', email: 'user@example.com', licenseKey: 'license123', }); diff --git a/packages/cli/src/events/maps/relay.event-map.ts b/packages/cli/src/events/maps/relay.event-map.ts index 21b673a2b5..0e72564571 100644 --- a/packages/cli/src/events/maps/relay.event-map.ts +++ b/packages/cli/src/events/maps/relay.event-map.ts @@ -8,7 +8,7 @@ import type { import type { AuthProviderType } from '@/databases/entities/auth-identity'; import type { ProjectRole } from '@/databases/entities/project-relation'; -import type { GlobalRole } from '@/databases/entities/user'; +import type { GlobalRole, User } from '@/databases/entities/user'; import type { IWorkflowDb } from '@/interfaces'; import type { AiEventMap } from './ai.event-map'; @@ -421,6 +421,7 @@ export type RelayEventMap = { }; 'license-community-plus-registered': { + userId: User['id']; email: string; licenseKey: string; }; diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index 7b0cacffaf..fc5cf0a53d 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -236,10 +236,12 @@ export class TelemetryEventRelay extends EventRelay { } private licenseCommunityPlusRegistered({ + userId, email, licenseKey, }: RelayEventMap['license-community-plus-registered']) { this.telemetry.track('User registered for license community plus', { + user_id: userId, email, licenseKey, }); diff --git a/packages/cli/src/license/__tests__/license.service.test.ts b/packages/cli/src/license/__tests__/license.service.test.ts index 9cd9c0ee0b..8ffc1dbf3d 100644 --- a/packages/cli/src/license/__tests__/license.service.test.ts +++ b/packages/cli/src/license/__tests__/license.service.test.ts @@ -94,6 +94,7 @@ describe('LicenseService', () => { .spyOn(axios, 'post') .mockResolvedValueOnce({ data: { title: 'Title', text: 'Text', licenseKey: 'abc-123' } }); const data = await licenseService.registerCommunityEdition({ + userId: '123', email: 'test@ema.il', instanceId: '123', instanceUrl: 'http://localhost', @@ -102,6 +103,7 @@ describe('LicenseService', () => { expect(data).toEqual({ title: 'Title', text: 'Text' }); expect(eventService.emit).toHaveBeenCalledWith('license-community-plus-registered', { + userId: '123', email: 'test@ema.il', licenseKey: 'abc-123', }); @@ -111,6 +113,7 @@ describe('LicenseService', () => { jest.spyOn(axios, 'post').mockRejectedValueOnce(new AxiosError('Failed')); await expect( licenseService.registerCommunityEdition({ + userId: '123', email: 'test@ema.il', instanceId: '123', instanceUrl: 'http://localhost', diff --git a/packages/cli/src/license/license.controller.ts b/packages/cli/src/license/license.controller.ts index db895ef4a0..3c284a25cb 100644 --- a/packages/cli/src/license/license.controller.ts +++ b/packages/cli/src/license/license.controller.ts @@ -4,7 +4,7 @@ import { InstanceSettings } from 'n8n-core'; import { Get, Post, RestController, GlobalScope, Body } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { AuthenticatedRequest, AuthlessRequest, LicenseRequest } from '@/requests'; +import { AuthenticatedRequest, LicenseRequest } from '@/requests'; import { UrlService } from '@/services/url.service'; import { LicenseService } from './license.service'; @@ -41,11 +41,12 @@ export class LicenseController { @Post('/enterprise/community-registered') async registerCommunityEdition( - _req: AuthlessRequest, + req: AuthenticatedRequest, _res: Response, @Body payload: CommunityRegisteredRequestDto, ) { return await this.licenseService.registerCommunityEdition({ + userId: req.user.id, email: payload.email, instanceId: this.instanceSettings.instanceId, instanceUrl: this.urlService.getInstanceBaseUrl(), diff --git a/packages/cli/src/license/license.service.ts b/packages/cli/src/license/license.service.ts index cdd6454036..1419d58b83 100644 --- a/packages/cli/src/license/license.service.ts +++ b/packages/cli/src/license/license.service.ts @@ -61,11 +61,13 @@ export class LicenseService { } async registerCommunityEdition({ + userId, email, instanceId, instanceUrl, licenseType, }: { + userId: User['id']; email: string; instanceId: string; instanceUrl: string; @@ -83,7 +85,7 @@ export class LicenseService { licenseType, }, ); - this.eventService.emit('license-community-plus-registered', { email, licenseKey }); + this.eventService.emit('license-community-plus-registered', { userId, email, licenseKey }); return rest; } catch (e: unknown) { if (e instanceof AxiosError) { From 0ab24c814abd1787268750ba808993ab2735ac52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 29 Oct 2024 11:37:10 +0100 Subject: [PATCH 010/672] fix(editor): Run external hooks after settings have been initialized (#11423) --- packages/editor-ui/src/App.vue | 2 -- packages/editor-ui/src/init.ts | 4 ++++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/editor-ui/src/App.vue b/packages/editor-ui/src/App.vue index 77d371855c..47d2f1268b 100644 --- a/packages/editor-ui/src/App.vue +++ b/packages/editor-ui/src/App.vue @@ -8,7 +8,6 @@ import Modals from '@/components/Modals.vue'; import Telemetry from '@/components/Telemetry.vue'; import AskAssistantFloatingButton from '@/components/AskAssistant/AskAssistantFloatingButton.vue'; import { loadLanguage } from '@/plugins/i18n'; -import { useExternalHooks } from '@/composables/useExternalHooks'; import { APP_MODALS_ELEMENT_ID, HIRING_BANNER, VIEWS } from '@/constants'; import { useRootStore } from '@/stores/root.store'; import { useAssistantStore } from '@/stores/assistant.store'; @@ -46,7 +45,6 @@ watch(defaultLocale, (newLocale) => { onMounted(async () => { setAppZIndexes(); logHiringBanner(); - void useExternalHooks().run('app.mount'); loading.value = false; window.addEventListener('resize', updateGridWidth); await updateGridWidth(); diff --git a/packages/editor-ui/src/init.ts b/packages/editor-ui/src/init.ts index b523393f31..9457063d6d 100644 --- a/packages/editor-ui/src/init.ts +++ b/packages/editor-ui/src/init.ts @@ -4,6 +4,7 @@ import { useRootStore } from '@/stores/root.store'; import { useSettingsStore } from '@/stores/settings.store'; import { useSourceControlStore } from '@/stores/sourceControl.store'; import { useUsersStore } from '@/stores/users.store'; +import { useExternalHooks } from '@/composables/useExternalHooks'; import { initializeCloudHooks } from '@/hooks/register'; import { useVersionsStore } from '@/stores/versions.store'; import { useProjectsStore } from '@/stores/projects.store'; @@ -26,6 +27,9 @@ export async function initializeCore() { const versionsStore = useVersionsStore(); await settingsStore.initialize(); + + void useExternalHooks().run('app.mount'); + if (!settingsStore.isPreviewMode) { await usersStore.initialize(); From 4e3681b9052f202502cf562043dc27ea9bdb4afa Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Tue, 29 Oct 2024 12:39:31 +0200 Subject: [PATCH 011/672] fix: Provide a better error message when task runner disconnects (no-changelog) (#11442) --- .../node-process-oom-detector.test.ts | 43 +++++++++++ .../__tests__/sliding-window-signal.test.ts | 71 +++++++++++++++++++ .../src/runners/__tests__/task-broker.test.ts | 14 ++-- .../errors/task-runner-disconnected-error.ts | 7 ++ .../runners/errors/task-runner-oom-error.ts | 31 ++++++++ .../src/runners/node-process-oom-detector.ts | 34 +++++++++ packages/cli/src/runners/runner-ws-server.ts | 15 ++-- .../cli/src/runners/sliding-window-signal.ts | 59 +++++++++++++++ .../cli/src/runners/task-broker.service.ts | 26 ++++--- .../task-runner-disconnect-analyzer.ts | 60 ++++++++++++++++ .../cli/src/runners/task-runner-process.ts | 19 ++++- .../nodes/Code/JsTaskRunnerSandbox.ts | 8 ++- 12 files changed, 362 insertions(+), 25 deletions(-) create mode 100644 packages/cli/src/runners/__tests__/node-process-oom-detector.test.ts create mode 100644 packages/cli/src/runners/__tests__/sliding-window-signal.test.ts create mode 100644 packages/cli/src/runners/errors/task-runner-disconnected-error.ts create mode 100644 packages/cli/src/runners/errors/task-runner-oom-error.ts create mode 100644 packages/cli/src/runners/node-process-oom-detector.ts create mode 100644 packages/cli/src/runners/sliding-window-signal.ts create mode 100644 packages/cli/src/runners/task-runner-disconnect-analyzer.ts diff --git a/packages/cli/src/runners/__tests__/node-process-oom-detector.test.ts b/packages/cli/src/runners/__tests__/node-process-oom-detector.test.ts new file mode 100644 index 0000000000..5b619e0e08 --- /dev/null +++ b/packages/cli/src/runners/__tests__/node-process-oom-detector.test.ts @@ -0,0 +1,43 @@ +import { spawn } from 'node:child_process'; + +import { NodeProcessOomDetector } from '../node-process-oom-detector'; + +describe('NodeProcessOomDetector', () => { + test('should detect an out-of-memory error in a monitored process', (done) => { + const childProcess = spawn(process.execPath, [ + // set low memory limit + '--max-old-space-size=20', + '-e', + ` + const data = []; + // fill memory until it crashes + while (true) data.push(Array.from({ length: 10_000 }).map(() => Math.random().toString()).join()); + `, + ]); + + const detector = new NodeProcessOomDetector(childProcess); + + childProcess.on('exit', (code) => { + expect(detector.didProcessOom).toBe(true); + expect(code).not.toBe(0); + done(); + }); + }); + + test('should not detect an out-of-memory error in a process that exits normally', (done) => { + const childProcess = spawn(process.execPath, [ + '-e', + ` + console.log("Hello, World!"); + `, + ]); + + const detector = new NodeProcessOomDetector(childProcess); + + childProcess.on('exit', (code) => { + expect(detector.didProcessOom).toBe(false); + expect(code).toBe(0); + done(); + }); + }); +}); diff --git a/packages/cli/src/runners/__tests__/sliding-window-signal.test.ts b/packages/cli/src/runners/__tests__/sliding-window-signal.test.ts new file mode 100644 index 0000000000..56462d186a --- /dev/null +++ b/packages/cli/src/runners/__tests__/sliding-window-signal.test.ts @@ -0,0 +1,71 @@ +import { TypedEmitter } from '../../typed-emitter'; +import { SlidingWindowSignal } from '../sliding-window-signal'; + +type TestEventMap = { + testEvent: string; +}; + +describe('SlidingWindowSignal', () => { + let eventEmitter: TypedEmitter; + let slidingWindowSignal: SlidingWindowSignal; + + beforeEach(() => { + eventEmitter = new TypedEmitter(); + slidingWindowSignal = new SlidingWindowSignal(eventEmitter, 'testEvent', { + windowSizeInMs: 500, + }); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.clearAllMocks(); + }); + + it('should return the last signal if within window size', async () => { + const signal = 'testSignal'; + eventEmitter.emit('testEvent', signal); + + const receivedSignal = await slidingWindowSignal.getSignal(); + + expect(receivedSignal).toBe(signal); + }); + + it('should return null if there is no signal within the window', async () => { + jest.useFakeTimers(); + const receivedSignalPromise = slidingWindowSignal.getSignal(); + jest.advanceTimersByTime(600); + const receivedSignal = await receivedSignalPromise; + + expect(receivedSignal).toBeNull(); + jest.useRealTimers(); + }); + + it('should return null if "exit" event is not emitted before timeout', async () => { + const signal = 'testSignal'; + jest.useFakeTimers(); + const receivedSignalPromise = slidingWindowSignal.getSignal(); + jest.advanceTimersByTime(600); + eventEmitter.emit('testEvent', signal); + + const receivedSignal = await receivedSignalPromise; + expect(receivedSignal).toBeNull(); + jest.useRealTimers(); + }); + + it('should return the signal emitted on "exit" event before timeout', async () => { + jest.useFakeTimers(); + const receivedSignalPromise = slidingWindowSignal.getSignal(); + + // Emit 'exit' with a signal before timeout + const exitSignal = 'exitSignal'; + eventEmitter.emit('testEvent', exitSignal); + + // Advance timers enough to go outside the timeout window + jest.advanceTimersByTime(600); + + const receivedSignal = await receivedSignalPromise; + expect(receivedSignal).toBe(exitSignal); + + jest.useRealTimers(); + }); +}); diff --git a/packages/cli/src/runners/__tests__/task-broker.test.ts b/packages/cli/src/runners/__tests__/task-broker.test.ts index 4a226c5b98..a90bf7662c 100644 --- a/packages/cli/src/runners/__tests__/task-broker.test.ts +++ b/packages/cli/src/runners/__tests__/task-broker.test.ts @@ -110,7 +110,7 @@ describe('TaskBroker', () => { const messageCallback = jest.fn(); taskBroker.registerRunner(runner, messageCallback); - taskBroker.deregisterRunner(runnerId); + taskBroker.deregisterRunner(runnerId, new Error()); const knownRunners = taskBroker.getKnownRunners(); const runnerIds = Object.keys(knownRunners); @@ -138,7 +138,7 @@ describe('TaskBroker', () => { validFor: 1000, validUntil: createValidUntil(1000), }); - taskBroker.deregisterRunner(runnerId); + taskBroker.deregisterRunner(runnerId, new Error()); const offers = taskBroker.getPendingTaskOffers(); expect(offers).toHaveLength(1); @@ -161,10 +161,14 @@ describe('TaskBroker', () => { [taskId]: { id: taskId, requesterId: 'requester1', runnerId, taskType: 'mock' }, task2: { id: 'task2', requesterId: 'requester1', runnerId: 'runner2', taskType: 'mock' }, }); - taskBroker.deregisterRunner(runnerId); + const error = new Error('error'); + taskBroker.deregisterRunner(runnerId, error); - expect(failSpy).toBeCalledWith(taskId, `The Task Runner (${runnerId}) has disconnected`); - expect(rejectSpy).toBeCalledWith(taskId, `The Task Runner (${runnerId}) has disconnected`); + expect(failSpy).toBeCalledWith(taskId, error); + expect(rejectSpy).toBeCalledWith( + taskId, + `The Task Runner (${runnerId}) has disconnected: error`, + ); }); }); diff --git a/packages/cli/src/runners/errors/task-runner-disconnected-error.ts b/packages/cli/src/runners/errors/task-runner-disconnected-error.ts new file mode 100644 index 0000000000..6c7c49450a --- /dev/null +++ b/packages/cli/src/runners/errors/task-runner-disconnected-error.ts @@ -0,0 +1,7 @@ +import { ApplicationError } from 'n8n-workflow'; + +export class TaskRunnerDisconnectedError extends ApplicationError { + constructor(runnerId: string) { + super(`Task runner (${runnerId}) disconnected`); + } +} diff --git a/packages/cli/src/runners/errors/task-runner-oom-error.ts b/packages/cli/src/runners/errors/task-runner-oom-error.ts new file mode 100644 index 0000000000..e52b8b4bea --- /dev/null +++ b/packages/cli/src/runners/errors/task-runner-oom-error.ts @@ -0,0 +1,31 @@ +import { ApplicationError } from 'n8n-workflow'; + +import type { TaskRunner } from '../task-broker.service'; + +export class TaskRunnerOomError extends ApplicationError { + public description: string; + + constructor(runnerId: TaskRunner['id'], isCloudDeployment: boolean) { + super(`Task runner (${runnerId}) ran out of memory.`, { level: 'error' }); + + const fixSuggestions = { + reduceItems: 'Reduce the number of items processed at a time by batching the input.', + increaseMemory: + "Increase the memory available to the task runner with 'N8N_RUNNERS_MAX_OLD_SPACE_SIZE' environment variable.", + upgradePlan: 'Upgrade your cloud plan to increase the available memory.', + }; + + const subtitle = + 'The runner executing the code ran out of memory. This usually happens when there are too many items to process. You can try the following:'; + const suggestions = isCloudDeployment + ? [fixSuggestions.reduceItems, fixSuggestions.upgradePlan] + : [fixSuggestions.reduceItems, fixSuggestions.increaseMemory]; + const suggestionsText = suggestions + .map((suggestion, index) => `${index + 1}. ${suggestion}`) + .join('
'); + + const description = `${subtitle}

${suggestionsText}`; + + this.description = description; + } +} diff --git a/packages/cli/src/runners/node-process-oom-detector.ts b/packages/cli/src/runners/node-process-oom-detector.ts new file mode 100644 index 0000000000..e6debb8551 --- /dev/null +++ b/packages/cli/src/runners/node-process-oom-detector.ts @@ -0,0 +1,34 @@ +import * as a from 'node:assert/strict'; +import type { ChildProcess } from 'node:child_process'; + +/** + * Class to monitor a nodejs process and detect if it runs out of + * memory (OOMs). + */ +export class NodeProcessOomDetector { + public get didProcessOom() { + return this._didProcessOom; + } + + private _didProcessOom = false; + + constructor(processToMonitor: ChildProcess) { + this.monitorProcess(processToMonitor); + } + + private monitorProcess(processToMonitor: ChildProcess) { + a.ok(processToMonitor.stderr, "Can't monitor a process without stderr"); + + processToMonitor.stderr.on('data', this.onStderr); + + processToMonitor.once('exit', () => { + processToMonitor.stderr?.off('data', this.onStderr); + }); + } + + private onStderr = (data: Buffer) => { + if (data.includes('JavaScript heap out of memory')) { + this._didProcessOom = true; + } + }; +} diff --git a/packages/cli/src/runners/runner-ws-server.ts b/packages/cli/src/runners/runner-ws-server.ts index 38b70c97dc..59bb92ff76 100644 --- a/packages/cli/src/runners/runner-ws-server.ts +++ b/packages/cli/src/runners/runner-ws-server.ts @@ -10,6 +10,7 @@ import type { TaskRunnerServerInitResponse, } from './runner-types'; import { TaskBroker, type MessageCallback, type TaskRunner } from './task-broker.service'; +import { TaskRunnerDisconnectAnalyzer } from './task-runner-disconnect-analyzer'; function heartbeat(this: WebSocket) { this.isAlive = true; @@ -22,6 +23,7 @@ export class TaskRunnerService { constructor( private readonly logger: Logger, private readonly taskBroker: TaskBroker, + private readonly disconnectAnalyzer: TaskRunnerDisconnectAnalyzer, ) {} sendMessage(id: TaskRunner['id'], message: N8nMessage.ToRunner.All) { @@ -34,7 +36,7 @@ export class TaskRunnerService { let isConnected = false; - const onMessage = (data: WebSocket.RawData) => { + const onMessage = async (data: WebSocket.RawData) => { try { const buffer = Array.isArray(data) ? Buffer.concat(data) : Buffer.from(data); @@ -45,7 +47,7 @@ export class TaskRunnerService { if (!isConnected && message.type !== 'runner:info') { return; } else if (!isConnected && message.type === 'runner:info') { - this.removeConnection(id); + await this.removeConnection(id); isConnected = true; this.runnerConnections.set(id, connection); @@ -75,10 +77,10 @@ export class TaskRunnerService { }; // Makes sure to remove the session if the connection is closed - connection.once('close', () => { + connection.once('close', async () => { connection.off('pong', heartbeat); connection.off('message', onMessage); - this.removeConnection(id); + await this.removeConnection(id); }); connection.on('message', onMessage); @@ -87,10 +89,11 @@ export class TaskRunnerService { ); } - removeConnection(id: TaskRunner['id']) { + async removeConnection(id: TaskRunner['id']) { const connection = this.runnerConnections.get(id); if (connection) { - this.taskBroker.deregisterRunner(id); + const disconnectReason = await this.disconnectAnalyzer.determineDisconnectReason(id); + this.taskBroker.deregisterRunner(id, disconnectReason); connection.close(); this.runnerConnections.delete(id); } diff --git a/packages/cli/src/runners/sliding-window-signal.ts b/packages/cli/src/runners/sliding-window-signal.ts new file mode 100644 index 0000000000..5954f7bade --- /dev/null +++ b/packages/cli/src/runners/sliding-window-signal.ts @@ -0,0 +1,59 @@ +import type { TypedEmitter } from '../typed-emitter'; + +export type SlidingWindowSignalOpts = { + windowSizeInMs?: number; +}; + +/** + * A class that listens for a specific event on an emitter (signal) and + * provides a sliding window of the last event that was emitted. + */ +export class SlidingWindowSignal { + private lastSignal: TEvents[TEventName] | null = null; + + private lastSignalTime: number = 0; + + private windowSizeInMs: number; + + constructor( + private readonly eventEmitter: TypedEmitter, + private readonly eventName: TEventName, + opts: SlidingWindowSignalOpts = {}, + ) { + const { windowSizeInMs = 500 } = opts; + + this.windowSizeInMs = windowSizeInMs; + + eventEmitter.on(eventName, (signal: TEvents[TEventName]) => { + this.lastSignal = signal; + this.lastSignalTime = Date.now(); + }); + } + + /** + * If an event has been emitted within the last `windowSize` milliseconds, + * that event is returned. Otherwise it will wait for up to `windowSize` + * milliseconds for the event to be emitted. `null` is returned + * if no event is emitted within the window. + */ + public async getSignal(): Promise { + const timeSinceLastEvent = Date.now() - this.lastSignalTime; + if (timeSinceLastEvent <= this.windowSizeInMs) return this.lastSignal; + + return await new Promise((resolve) => { + let timeoutTimerId: NodeJS.Timeout | null = null; + + const onExit = (signal: TEvents[TEventName]) => { + if (timeoutTimerId) clearTimeout(timeoutTimerId); + resolve(signal); + }; + + timeoutTimerId = setTimeout(() => { + this.eventEmitter.off(this.eventName, onExit); + resolve(null); + }); + + this.eventEmitter.once(this.eventName, onExit); + }); + } +} diff --git a/packages/cli/src/runners/task-broker.service.ts b/packages/cli/src/runners/task-broker.service.ts index 40ad6d6e90..d88d677725 100644 --- a/packages/cli/src/runners/task-broker.service.ts +++ b/packages/cli/src/runners/task-broker.service.ts @@ -104,7 +104,7 @@ export class TaskBroker { }); } - deregisterRunner(runnerId: string) { + deregisterRunner(runnerId: string, error: Error) { this.knownRunners.delete(runnerId); // Remove any pending offers @@ -117,8 +117,11 @@ export class TaskBroker { // Fail any tasks for (const task of this.tasks.values()) { if (task.runnerId === runnerId) { - void this.failTask(task.id, `The Task Runner (${runnerId}) has disconnected`); - this.handleRunnerReject(task.id, `The Task Runner (${runnerId}) has disconnected`); + void this.failTask(task.id, error); + this.handleRunnerReject( + task.id, + `The Task Runner (${runnerId}) has disconnected: ${error.message}`, + ); } } } @@ -352,7 +355,7 @@ export class TaskBroker { }); } - private async failTask(taskId: Task['id'], reason: string) { + private async failTask(taskId: Task['id'], error: Error) { const task = this.tasks.get(taskId); if (!task) { return; @@ -362,7 +365,7 @@ export class TaskBroker { await this.messageRequester(task.requesterId, { type: 'broker:taskerror', taskId, - error: reason, + error, }); } @@ -375,11 +378,14 @@ export class TaskBroker { } const runner = this.knownRunners.get(task.runnerId); if (!runner) { - const reason = `Cannot find runner, failed to find runner (${task.runnerId})`; - await this.failTask(taskId, reason); - throw new ApplicationError(reason, { - level: 'error', - }); + const error = new ApplicationError( + `Cannot find runner, failed to find runner (${task.runnerId})`, + { + level: 'error', + }, + ); + await this.failTask(taskId, error); + throw error; } return runner.runner; } diff --git a/packages/cli/src/runners/task-runner-disconnect-analyzer.ts b/packages/cli/src/runners/task-runner-disconnect-analyzer.ts new file mode 100644 index 0000000000..d75a1b9aad --- /dev/null +++ b/packages/cli/src/runners/task-runner-disconnect-analyzer.ts @@ -0,0 +1,60 @@ +import { TaskRunnersConfig } from '@n8n/config'; +import { Service } from 'typedi'; + +import config from '@/config'; + +import { TaskRunnerDisconnectedError } from './errors/task-runner-disconnected-error'; +import { TaskRunnerOomError } from './errors/task-runner-oom-error'; +import { SlidingWindowSignal } from './sliding-window-signal'; +import type { TaskRunner } from './task-broker.service'; +import type { ExitReason, TaskRunnerProcessEventMap } from './task-runner-process'; +import { TaskRunnerProcess } from './task-runner-process'; + +/** + * Analyzes the disconnect reason of a task runner process to provide a more + * meaningful error message to the user. + */ +@Service() +export class TaskRunnerDisconnectAnalyzer { + private readonly exitReasonSignal: SlidingWindowSignal; + + constructor( + private readonly runnerConfig: TaskRunnersConfig, + private readonly taskRunnerProcess: TaskRunnerProcess, + ) { + // When the task runner process is running as a child process, there's + // no determinate time when it exits compared to when the runner disconnects + // (i.e. it's a race condition). Hence we use a sliding window to determine + // the exit reason. As long as we receive the exit signal from the task + // runner process within the window, we can determine the exit reason. + this.exitReasonSignal = new SlidingWindowSignal(this.taskRunnerProcess, 'exit', { + windowSizeInMs: 500, + }); + } + + private get isCloudDeployment() { + return config.get('deployment.type') === 'cloud'; + } + + async determineDisconnectReason(runnerId: TaskRunner['id']): Promise { + const exitCode = await this.awaitExitSignal(); + if (exitCode === 'oom') { + return new TaskRunnerOomError(runnerId, this.isCloudDeployment); + } + + return new TaskRunnerDisconnectedError(runnerId); + } + + private async awaitExitSignal(): Promise { + if (this.runnerConfig.mode === 'external') { + // If the task runner is running in external mode, we don't have + // control over the process and hence cannot determine the exit + // reason. We just return 'unknown' in this case. + return 'unknown'; + } + + const lastExitReason = await this.exitReasonSignal.getSignal(); + + return lastExitReason?.reason ?? 'unknown'; + } +} diff --git a/packages/cli/src/runners/task-runner-process.ts b/packages/cli/src/runners/task-runner-process.ts index f4c219ead7..d453f1d134 100644 --- a/packages/cli/src/runners/task-runner-process.ts +++ b/packages/cli/src/runners/task-runner-process.ts @@ -9,14 +9,24 @@ import { Logger } from '@/logging/logger.service'; import { TaskRunnerAuthService } from './auth/task-runner-auth.service'; import { forwardToLogger } from './forward-to-logger'; +import { NodeProcessOomDetector } from './node-process-oom-detector'; +import { TypedEmitter } from '../typed-emitter'; type ChildProcess = ReturnType; +export type ExitReason = 'unknown' | 'oom'; + +export type TaskRunnerProcessEventMap = { + exit: { + reason: ExitReason; + }; +}; + /** * Manages the JS task runner process as a child process */ @Service() -export class TaskRunnerProcess { +export class TaskRunnerProcess extends TypedEmitter { public get isRunning() { return this.process !== null; } @@ -39,6 +49,8 @@ export class TaskRunnerProcess { private _runPromise: Promise | null = null; + private oomDetector: NodeProcessOomDetector | null = null; + private isShuttingDown = false; private logger: Logger; @@ -54,6 +66,8 @@ export class TaskRunnerProcess { private readonly runnerConfig: TaskRunnersConfig, private readonly authService: TaskRunnerAuthService, ) { + super(); + a.ok( this.runnerConfig.mode === 'internal_childprocess' || this.runnerConfig.mode === 'internal_launcher', @@ -141,6 +155,8 @@ export class TaskRunnerProcess { private monitorProcess(taskRunnerProcess: ChildProcess) { this._runPromise = new Promise((resolve) => { + this.oomDetector = new NodeProcessOomDetector(taskRunnerProcess); + taskRunnerProcess.on('exit', (code) => { this.onProcessExit(code, resolve); }); @@ -149,6 +165,7 @@ export class TaskRunnerProcess { private onProcessExit(_code: number | null, resolveFn: () => void) { this.process = null; + this.emit('exit', { reason: this.oomDetector?.didProcessOom ? 'oom' : 'unknown' }); resolveFn(); // If we are not shutting down, restart the process diff --git a/packages/nodes-base/nodes/Code/JsTaskRunnerSandbox.ts b/packages/nodes-base/nodes/Code/JsTaskRunnerSandbox.ts index 80d4b32445..98d98a28e3 100644 --- a/packages/nodes-base/nodes/Code/JsTaskRunnerSandbox.ts +++ b/packages/nodes-base/nodes/Code/JsTaskRunnerSandbox.ts @@ -60,9 +60,11 @@ export class JsTaskRunnerSandbox { } private throwExecutionError(error: unknown): never { - // The error coming from task runner is not an instance of error, - // so we need to wrap it in an error instance. - if (isWrappableError(error)) { + if (error instanceof Error) { + throw error; + } else if (isWrappableError(error)) { + // The error coming from task runner is not an instance of error, + // so we need to wrap it in an error instance. throw new WrappedExecutionError(error); } From a701d87f5ba94ffc811e424b60e188b26ac6c1c5 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Tue, 29 Oct 2024 13:39:45 +0200 Subject: [PATCH 012/672] feat(editor): Remove edge execution animation on new canvas (#11446) --- packages/editor-ui/src/composables/useCanvasMapping.spec.ts | 3 --- packages/editor-ui/src/composables/useCanvasMapping.ts | 1 - 2 files changed, 4 deletions(-) diff --git a/packages/editor-ui/src/composables/useCanvasMapping.spec.ts b/packages/editor-ui/src/composables/useCanvasMapping.spec.ts index 059b36a988..14914f8275 100644 --- a/packages/editor-ui/src/composables/useCanvasMapping.spec.ts +++ b/packages/editor-ui/src/composables/useCanvasMapping.spec.ts @@ -743,7 +743,6 @@ describe('useCanvasMapping', () => { target, targetHandle, type: 'canvas-edge', - animated: false, }, ]); }); @@ -832,7 +831,6 @@ describe('useCanvasMapping', () => { target: targetA, targetHandle: targetHandleA, type: 'canvas-edge', - animated: false, }, { data: { @@ -855,7 +853,6 @@ describe('useCanvasMapping', () => { targetHandle: targetHandleB, type: 'canvas-edge', markerEnd: MarkerType.ArrowClosed, - animated: false, }, ]); }); diff --git a/packages/editor-ui/src/composables/useCanvasMapping.ts b/packages/editor-ui/src/composables/useCanvasMapping.ts index bb6699a9cb..9858a2b930 100644 --- a/packages/editor-ui/src/composables/useCanvasMapping.ts +++ b/packages/editor-ui/src/composables/useCanvasMapping.ts @@ -507,7 +507,6 @@ export function useCanvasMapping({ data, type, label, - animated: data.status === 'running', markerEnd: MarkerType.ArrowClosed, }; }, From 60cdf0ba44aeb85e93903be41b376be343cca580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 29 Oct 2024 14:41:10 +0100 Subject: [PATCH 013/672] feat(editor): Allow enabling canvas v2 via instance settings (no-changelog) (#11447) --- packages/@n8n/api-types/package.json | 1 + packages/@n8n/api-types/src/frontend-settings.ts | 2 ++ packages/@n8n/config/src/configs/frontend.config.ts | 11 +++++++++++ packages/@n8n/config/src/index.ts | 1 + packages/cli/src/services/frontend.service.ts | 4 +++- packages/editor-ui/src/__tests__/defaults.ts | 1 + .../src/composables/useNodeViewVersionSwitcher.ts | 2 +- packages/editor-ui/src/stores/settings.store.ts | 5 +++++ pnpm-lock.yaml | 10 +++++++++- 9 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 packages/@n8n/config/src/configs/frontend.config.ts diff --git a/packages/@n8n/api-types/package.json b/packages/@n8n/api-types/package.json index 0c4440eb6b..b3d28f18cc 100644 --- a/packages/@n8n/api-types/package.json +++ b/packages/@n8n/api-types/package.json @@ -21,6 +21,7 @@ "dist/**/*" ], "devDependencies": { + "@n8n/config": "workspace:*", "n8n-workflow": "workspace:*" }, "dependencies": { diff --git a/packages/@n8n/api-types/src/frontend-settings.ts b/packages/@n8n/api-types/src/frontend-settings.ts index 5084344aeb..6b2f3231d3 100644 --- a/packages/@n8n/api-types/src/frontend-settings.ts +++ b/packages/@n8n/api-types/src/frontend-settings.ts @@ -1,3 +1,4 @@ +import type { FrontendBetaFeatures } from '@n8n/config'; import type { ExpressionEvaluatorType, LogLevel, WorkflowSettings } from 'n8n-workflow'; export interface IVersionNotificationSettings { @@ -169,4 +170,5 @@ export interface FrontendSettings { security: { blockFileAccessToN8nFiles: boolean; }; + betaFeatures: FrontendBetaFeatures[]; } diff --git a/packages/@n8n/config/src/configs/frontend.config.ts b/packages/@n8n/config/src/configs/frontend.config.ts new file mode 100644 index 0000000000..63f812952f --- /dev/null +++ b/packages/@n8n/config/src/configs/frontend.config.ts @@ -0,0 +1,11 @@ +import { Config, Env } from '../decorators'; +import { StringArray } from '../utils'; + +export type FrontendBetaFeatures = 'canvas_v2'; + +@Config +export class FrontendConfig { + /** Which UI experiments to enable. Separate multiple values with a comma `,` */ + @Env('N8N_UI_BETA_FEATURES') + betaFeatures: StringArray = []; +} diff --git a/packages/@n8n/config/src/index.ts b/packages/@n8n/config/src/index.ts index 1a2a3127ad..c056a1090c 100644 --- a/packages/@n8n/config/src/index.ts +++ b/packages/@n8n/config/src/index.ts @@ -24,6 +24,7 @@ import { Config, Env, Nested } from './decorators'; export { Config, Env, Nested } from './decorators'; export { TaskRunnersConfig } from './configs/runners.config'; export { SecurityConfig } from './configs/security.config'; +export { FrontendBetaFeatures, FrontendConfig } from './configs/frontend.config'; export { LOG_SCOPES } from './configs/logging.config'; export type { LogScope } from './configs/logging.config'; diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 2916f191f3..6cad4a4f24 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -1,5 +1,5 @@ import type { FrontendSettings, ITelemetrySettings } from '@n8n/api-types'; -import { GlobalConfig, SecurityConfig } from '@n8n/config'; +import { GlobalConfig, FrontendConfig, SecurityConfig } from '@n8n/config'; import { createWriteStream } from 'fs'; import { mkdir } from 'fs/promises'; import uniq from 'lodash/uniq'; @@ -47,6 +47,7 @@ export class FrontendService { private readonly instanceSettings: InstanceSettings, private readonly urlService: UrlService, private readonly securityConfig: SecurityConfig, + private readonly frontendConfig: FrontendConfig, ) { loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes()); void this.generateTypes(); @@ -228,6 +229,7 @@ export class FrontendService { security: { blockFileAccessToN8nFiles: this.securityConfig.blockFileAccessToN8nFiles, }, + betaFeatures: this.frontendConfig.betaFeatures, }; } diff --git a/packages/editor-ui/src/__tests__/defaults.ts b/packages/editor-ui/src/__tests__/defaults.ts index dd4fbed9d5..d16678b17c 100644 --- a/packages/editor-ui/src/__tests__/defaults.ts +++ b/packages/editor-ui/src/__tests__/defaults.ts @@ -124,4 +124,5 @@ export const defaultSettings: FrontendSettings = { aiAssistant: { enabled: false, }, + betaFeatures: [], }; diff --git a/packages/editor-ui/src/composables/useNodeViewVersionSwitcher.ts b/packages/editor-ui/src/composables/useNodeViewVersionSwitcher.ts index 336bf388b6..c91da5dcc2 100644 --- a/packages/editor-ui/src/composables/useNodeViewVersionSwitcher.ts +++ b/packages/editor-ui/src/composables/useNodeViewVersionSwitcher.ts @@ -16,7 +16,7 @@ export function useNodeViewVersionSwitcher() { const nodeViewVersion = useLocalStorage( 'NodeView.version', - settingsStore.deploymentType === 'n8n-internal' ? '2' : '1', + settingsStore.isCanvasV2Enabled ? '2' : '1', ); function setNodeViewSwitcherDropdownOpened(visible: boolean) { diff --git a/packages/editor-ui/src/stores/settings.store.ts b/packages/editor-ui/src/stores/settings.store.ts index b7325e4ec5..119fc20ac1 100644 --- a/packages/editor-ui/src/stores/settings.store.ts +++ b/packages/editor-ui/src/stores/settings.store.ts @@ -156,6 +156,10 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => { const isDevRelease = computed(() => settings.value.releaseChannel === 'dev'); + const isCanvasV2Enabled = computed(() => + (settings.value.betaFeatures ?? []).includes('canvas_v2'), + ); + const setSettings = (newSettings: FrontendSettings) => { settings.value = newSettings; userManagement.value = newSettings.userManagement; @@ -418,6 +422,7 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => { saveDataProgressExecution, isCommunityPlan, isAskAiEnabled, + isCanvasV2Enabled, reset, testLdapConnection, getLdapConfig, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9341fbaf3a..fe5ba1e4f7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -254,6 +254,9 @@ importers: specifier: 0.0.15 version: 0.0.15(zod@3.23.8) devDependencies: + '@n8n/config': + specifier: workspace:* + version: link:../config n8n-workflow: specifier: workspace:* version: link:../../workflow @@ -12104,6 +12107,9 @@ packages: vue-component-type-helpers@2.1.6: resolution: {integrity: sha512-ng11B8B/ZADUMMOsRbqv0arc442q7lifSubD0v8oDXIFoMg/mXwAPUunrroIDkY+mcD0dHKccdaznSVp8EoX3w==} + vue-component-type-helpers@2.1.8: + resolution: {integrity: sha512-ii36gDzrYAfOQIkOlo44yceDdT5269gKmNGxf07Qx6seH2U50+tQ2ol02XLhYPmxrh6YabAsOdte8WDrpaO6Tw==} + vue-demi@0.14.10: resolution: {integrity: sha512-nMZBOwuzabUO0nLgIcc6rycZEebF6eeUfaiQx9+WSk8e29IbLvPU9feI6tqW4kTo3hvoYAJkMh8n8D0fuISphg==} engines: {node: '>=12'} @@ -16272,7 +16278,7 @@ snapshots: ts-dedent: 2.2.0 type-fest: 2.19.0 vue: 3.5.11(typescript@5.6.2) - vue-component-type-helpers: 2.1.6 + vue-component-type-helpers: 2.1.8 '@supabase/auth-js@2.65.0': dependencies: @@ -25465,6 +25471,8 @@ snapshots: vue-component-type-helpers@2.1.6: {} + vue-component-type-helpers@2.1.8: {} + vue-demi@0.14.10(vue@3.5.11(typescript@5.6.2)): dependencies: vue: 3.5.11(typescript@5.6.2) From dcd6038c3085135803cdaa546a239359a6d449eb Mon Sep 17 00:00:00 2001 From: Ivan Atanasov Date: Tue, 29 Oct 2024 15:25:35 +0100 Subject: [PATCH 014/672] fix(editor): Change tooltip for workflow with execute workflow trigger (#11374) --- .../src/components/WorkflowActivator.test.ts | 82 +++++++++++++++++++ .../src/components/WorkflowActivator.vue | 21 ++++- .../src/plugins/i18n/locales/en.json | 1 + 3 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 packages/editor-ui/src/components/WorkflowActivator.test.ts diff --git a/packages/editor-ui/src/components/WorkflowActivator.test.ts b/packages/editor-ui/src/components/WorkflowActivator.test.ts new file mode 100644 index 0000000000..62f9481636 --- /dev/null +++ b/packages/editor-ui/src/components/WorkflowActivator.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, vi } from 'vitest'; +import WorkflowActivator from '@/components/WorkflowActivator.vue'; +import userEvent from '@testing-library/user-event'; + +import { useWorkflowsStore } from '@/stores/workflows.store'; + +import { createTestingPinia } from '@pinia/testing'; +import { createComponentRenderer } from '@/__tests__/render'; +import { mockedStore } from '@/__tests__/utils'; +import { EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE } from '@/constants'; + +const renderComponent = createComponentRenderer(WorkflowActivator); +let mockWorkflowsStore: ReturnType>; + +describe('WorkflowActivator', () => { + beforeEach(() => { + createTestingPinia(); + + mockWorkflowsStore = mockedStore(useWorkflowsStore); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('renders correctly', () => { + const renderOptions = { + props: { + workflowActive: false, + workflowId: '1', + workflowPermissions: { update: true }, + }, + }; + + const { getByTestId, getByRole } = renderComponent(renderOptions); + expect(getByTestId('workflow-activator-status')).toBeInTheDocument(); + expect(getByRole('switch')).toBeInTheDocument(); + }); + + it('display an inactive tooltip when there are no nodes available', async () => { + mockWorkflowsStore.workflowId = '1'; + + const { getByTestId, getByRole } = renderComponent({ + props: { + workflowActive: false, + workflowId: '1', + workflowPermissions: { update: true }, + }, + }); + + await userEvent.hover(getByRole('switch')); + expect(getByRole('tooltip')).toBeInTheDocument(); + + expect(getByRole('tooltip')).toHaveTextContent( + 'This workflow has no trigger nodes that require activation', + ); + expect(getByTestId('workflow-activator-status')).toHaveTextContent('Inactive'); + }); + + it('display an inactive tooltip when only execute workflow trigger is available', async () => { + mockWorkflowsStore.workflowId = '1'; + mockWorkflowsStore.workflowTriggerNodes = [ + { type: EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE, disabled: false } as never, + ]; + + const { getByTestId, getByRole } = renderComponent({ + props: { + workflowActive: false, + workflowId: '1', + workflowPermissions: { update: true }, + }, + }); + + await userEvent.hover(getByRole('switch')); + expect(getByRole('tooltip')).toBeInTheDocument(); + + expect(getByRole('tooltip')).toHaveTextContent( + "Execute Workflow Trigger' doesn't require activation as it is triggered by another workflow", + ); + expect(getByTestId('workflow-activator-status')).toHaveTextContent('Inactive'); + }); +}); diff --git a/packages/editor-ui/src/components/WorkflowActivator.vue b/packages/editor-ui/src/components/WorkflowActivator.vue index 6bfe6f799e..8dec6f6620 100644 --- a/packages/editor-ui/src/components/WorkflowActivator.vue +++ b/packages/editor-ui/src/components/WorkflowActivator.vue @@ -7,7 +7,7 @@ import type { VNode } from 'vue'; import { computed, h } from 'vue'; import { useI18n } from '@/composables/useI18n'; import type { PermissionsRecord } from '@/permissions'; -import { PLACEHOLDER_EMPTY_WORKFLOW_ID } from '@/constants'; +import { EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE, PLACEHOLDER_EMPTY_WORKFLOW_ID } from '@/constants'; import WorkflowActivationErrorMessage from './WorkflowActivationErrorMessage.vue'; const props = defineProps<{ @@ -43,6 +43,17 @@ const containsTrigger = computed((): boolean => { return foundTriggers.length > 0; }); +const containsOnlyExecuteWorkflowTrigger = computed((): boolean => { + const foundActiveTriggers = workflowsStore.workflowTriggerNodes.filter( + (trigger) => !trigger.disabled, + ); + const foundTriggers = foundActiveTriggers.filter( + (trigger) => trigger.type === EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE, + ); + + return foundTriggers.length > 0 && foundTriggers.length === foundActiveTriggers.length; +}); + const isNewWorkflow = computed( () => !props.workflowId || @@ -109,7 +120,13 @@ async function displayActivationError() { Click to display error message.", "workflowActivator.thisWorkflowHasNoTriggerNodes": "This workflow has no trigger nodes that require activation", + "workflowActivator.thisWorkflowHasOnlyOneExecuteWorkflowTriggerNode": "'Execute Workflow Trigger' doesn't require activation as it is triggered by another workflow", "workflowDetails.share": "Share", "workflowDetails.active": "Active", "workflowDetails.addTag": "Add tag", From ea47b025fb16c967d4fc73dcacc6e260d2aecd61 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Tue, 29 Oct 2024 16:36:25 +0200 Subject: [PATCH 015/672] fix(editor): Fix adding connections when initializing workspace in templates view on new canvas (#11451) --- packages/editor-ui/src/composables/useCanvasOperations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/editor-ui/src/composables/useCanvasOperations.ts b/packages/editor-ui/src/composables/useCanvasOperations.ts index 3950e16ecf..70e3f048c3 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.ts @@ -1431,7 +1431,7 @@ export function useCanvasOperations({ router }: { router: ReturnType Date: Tue, 29 Oct 2024 21:08:50 +0200 Subject: [PATCH 016/672] feat: Make runner concurrency configurable (no-changelog) (#11448) --- docker/images/n8n/n8n-task-runners.json | 1 + .../@n8n/config/src/configs/runners.config.ts | 4 ++ packages/@n8n/config/test/config.test.ts | 1 + packages/@n8n/task-runner/package.json | 2 + .../src/config/base-runner-config.ts | 16 ++++++++ .../src/config/js-runner-config.ts | 10 +++++ .../task-runner/src/config/main-config.ts | 13 +++++++ .../__tests__/js-task-runner.test.ts | 21 +++++++--- .../src/js-task-runner/js-task-runner.ts | 38 +++++-------------- packages/@n8n/task-runner/src/start.ts | 32 +++------------- packages/@n8n/task-runner/src/task-runner.ts | 38 ++++++++++--------- packages/@n8n/task-runner/tsconfig.json | 2 + .../cli/src/runners/task-runner-process.ts | 1 + pnpm-lock.yaml | 6 +++ 14 files changed, 107 insertions(+), 78 deletions(-) create mode 100644 packages/@n8n/task-runner/src/config/base-runner-config.ts create mode 100644 packages/@n8n/task-runner/src/config/js-runner-config.ts create mode 100644 packages/@n8n/task-runner/src/config/main-config.ts diff --git a/docker/images/n8n/n8n-task-runners.json b/docker/images/n8n/n8n-task-runners.json index 4019189589..699794d504 100644 --- a/docker/images/n8n/n8n-task-runners.json +++ b/docker/images/n8n/n8n-task-runners.json @@ -10,6 +10,7 @@ "N8N_RUNNERS_GRANT_TOKEN", "N8N_RUNNERS_N8N_URI", "N8N_RUNNERS_MAX_PAYLOAD", + "N8N_RUNNERS_MAX_CONCURRENCY", "NODE_FUNCTION_ALLOW_BUILTIN", "NODE_FUNCTION_ALLOW_EXTERNAL", "NODE_OPTIONS" diff --git a/packages/@n8n/config/src/configs/runners.config.ts b/packages/@n8n/config/src/configs/runners.config.ts index f26b96636c..c7be197963 100644 --- a/packages/@n8n/config/src/configs/runners.config.ts +++ b/packages/@n8n/config/src/configs/runners.config.ts @@ -46,4 +46,8 @@ export class TaskRunnersConfig { /** The --max-old-space-size option to use for the runner (in MB). Default means node.js will determine it based on the available memory. */ @Env('N8N_RUNNERS_MAX_OLD_SPACE_SIZE') maxOldSpaceSize: string = ''; + + /** How many concurrent tasks can a runner execute at a time */ + @Env('N8N_RUNNERS_MAX_CONCURRENCY') + maxConcurrency: number = 5; } diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index cd5438e248..07af2c0a0b 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -232,6 +232,7 @@ describe('GlobalConfig', () => { launcherPath: '', launcherRunner: 'javascript', maxOldSpaceSize: '', + maxConcurrency: 5, }, sentry: { backendDsn: '', diff --git a/packages/@n8n/task-runner/package.json b/packages/@n8n/task-runner/package.json index d889bde6f3..fca6b9a22c 100644 --- a/packages/@n8n/task-runner/package.json +++ b/packages/@n8n/task-runner/package.json @@ -22,9 +22,11 @@ "dist/**/*" ], "dependencies": { + "@n8n/config": "workspace:*", "n8n-workflow": "workspace:*", "n8n-core": "workspace:*", "nanoid": "^3.3.6", + "typedi": "catalog:", "ws": "^8.18.0" }, "devDependencies": { diff --git a/packages/@n8n/task-runner/src/config/base-runner-config.ts b/packages/@n8n/task-runner/src/config/base-runner-config.ts new file mode 100644 index 0000000000..01e00c177a --- /dev/null +++ b/packages/@n8n/task-runner/src/config/base-runner-config.ts @@ -0,0 +1,16 @@ +import { Config, Env } from '@n8n/config'; + +@Config +export class BaseRunnerConfig { + @Env('N8N_RUNNERS_N8N_URI') + n8nUri: string = '127.0.0.1:5679'; + + @Env('N8N_RUNNERS_GRANT_TOKEN') + grantToken: string = ''; + + @Env('N8N_RUNNERS_MAX_PAYLOAD') + maxPayloadSize: number = 1024 * 1024 * 1024; + + @Env('N8N_RUNNERS_MAX_CONCURRENCY') + maxConcurrency: number = 5; +} diff --git a/packages/@n8n/task-runner/src/config/js-runner-config.ts b/packages/@n8n/task-runner/src/config/js-runner-config.ts new file mode 100644 index 0000000000..4cba6f1d98 --- /dev/null +++ b/packages/@n8n/task-runner/src/config/js-runner-config.ts @@ -0,0 +1,10 @@ +import { Config, Env } from '@n8n/config'; + +@Config +export class JsRunnerConfig { + @Env('NODE_FUNCTION_ALLOW_BUILTIN') + allowedBuiltInModules: string = ''; + + @Env('NODE_FUNCTION_ALLOW_EXTERNAL') + allowedExternalModules: string = ''; +} diff --git a/packages/@n8n/task-runner/src/config/main-config.ts b/packages/@n8n/task-runner/src/config/main-config.ts new file mode 100644 index 0000000000..a290c0c380 --- /dev/null +++ b/packages/@n8n/task-runner/src/config/main-config.ts @@ -0,0 +1,13 @@ +import { Config, Nested } from '@n8n/config'; + +import { BaseRunnerConfig } from './base-runner-config'; +import { JsRunnerConfig } from './js-runner-config'; + +@Config +export class MainConfig { + @Nested + baseRunnerConfig!: BaseRunnerConfig; + + @Nested + jsRunnerConfig!: JsRunnerConfig; +} diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts index 5f83b23ae1..36f3c5afa2 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts @@ -4,7 +4,6 @@ import fs from 'node:fs'; import { builtinModules } from 'node:module'; import { ValidationError } from '@/js-task-runner/errors/validation-error'; -import type { JsTaskRunnerOpts } from '@/js-task-runner/js-task-runner'; import { JsTaskRunner, type AllCodeTaskData, @@ -13,17 +12,27 @@ import { import type { Task } from '@/task-runner'; import { newAllCodeTaskData, newTaskWithSettings, withPairedItem, wrapIntoJson } from './test-data'; +import type { JsRunnerConfig } from '../../config/js-runner-config'; +import { MainConfig } from '../../config/main-config'; import { ExecutionError } from '../errors/execution-error'; jest.mock('ws'); +const defaultConfig = new MainConfig(); + describe('JsTaskRunner', () => { - const createRunnerWithOpts = (opts: Partial = {}) => + const createRunnerWithOpts = (opts: Partial = {}) => new JsTaskRunner({ - wsUrl: 'ws://localhost', - grantToken: 'grantToken', - maxConcurrency: 1, - ...opts, + baseRunnerConfig: { + ...defaultConfig.baseRunnerConfig, + grantToken: 'grantToken', + maxConcurrency: 1, + n8nUri: 'localhost', + }, + jsRunnerConfig: { + ...defaultConfig.jsRunnerConfig, + ...opts, + }, }); const defaultTaskRunner = createRunnerWithOpts(); diff --git a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts index d7ec7d85f4..40ee12af2c 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts @@ -30,6 +30,7 @@ import { makeSerializable } from './errors/serializable-error'; import type { RequireResolver } from './require-resolver'; import { createRequireResolver } from './require-resolver'; import { validateRunForAllItemsOutput, validateRunForEachItemOutput } from './result-validation'; +import type { MainConfig } from '../config/main-config'; export interface JSExecSettings { code: string; @@ -76,23 +77,6 @@ export interface AllCodeTaskData { additionalData: PartialAdditionalData; } -export interface JsTaskRunnerOpts { - wsUrl: string; - grantToken: string; - maxConcurrency: number; - name?: string; - /** - * List of built-in nodejs modules that are allowed to be required in the - * execution sandbox. Asterisk (*) can be used to allow all. - */ - allowedBuiltInModules?: string; - /** - * List of npm modules that are allowed to be required in the execution - * sandbox. Asterisk (*) can be used to allow all. - */ - allowedExternalModules?: string; -} - type CustomConsole = { log: (...args: unknown[]) => void; }; @@ -100,22 +84,20 @@ type CustomConsole = { export class JsTaskRunner extends TaskRunner { private readonly requireResolver: RequireResolver; - constructor({ - grantToken, - maxConcurrency, - wsUrl, - name = 'JS Task Runner', - allowedBuiltInModules, - allowedExternalModules, - }: JsTaskRunnerOpts) { - super('javascript', wsUrl, grantToken, maxConcurrency, name); + constructor(config: MainConfig, name = 'JS Task Runner') { + super({ + taskType: 'javascript', + name, + ...config.baseRunnerConfig, + }); + const { jsRunnerConfig } = config; const parseModuleAllowList = (moduleList: string) => moduleList === '*' ? null : new Set(moduleList.split(',').map((x) => x.trim())); this.requireResolver = createRequireResolver({ - allowedBuiltInModules: parseModuleAllowList(allowedBuiltInModules ?? ''), - allowedExternalModules: parseModuleAllowList(allowedExternalModules ?? ''), + allowedBuiltInModules: parseModuleAllowList(jsRunnerConfig.allowedBuiltInModules ?? ''), + allowedExternalModules: parseModuleAllowList(jsRunnerConfig.allowedExternalModules ?? ''), }); } diff --git a/packages/@n8n/task-runner/src/start.ts b/packages/@n8n/task-runner/src/start.ts index f5487ba6c2..fcaab84d51 100644 --- a/packages/@n8n/task-runner/src/start.ts +++ b/packages/@n8n/task-runner/src/start.ts @@ -1,27 +1,12 @@ -import { ApplicationError, ensureError } from 'n8n-workflow'; +import { ensureError } from 'n8n-workflow'; +import Container from 'typedi'; +import { MainConfig } from './config/main-config'; import { JsTaskRunner } from './js-task-runner/js-task-runner'; let runner: JsTaskRunner | undefined; let isShuttingDown = false; -type Config = { - n8nUri: string; - grantToken: string; -}; - -function readAndParseConfig(): Config { - const grantToken = process.env.N8N_RUNNERS_GRANT_TOKEN; - if (!grantToken) { - throw new ApplicationError('Missing N8N_RUNNERS_GRANT_TOKEN environment variable'); - } - - return { - n8nUri: process.env.N8N_RUNNERS_N8N_URI ?? '127.0.0.1:5679', - grantToken, - }; -} - function createSignalHandler(signal: string) { return async function onSignal() { if (isShuttingDown) { @@ -46,16 +31,9 @@ function createSignalHandler(signal: string) { } void (async function start() { - const config = readAndParseConfig(); + const config = Container.get(MainConfig); - const wsUrl = `ws://${config.n8nUri}/runners/_ws`; - runner = new JsTaskRunner({ - wsUrl, - grantToken: config.grantToken, - maxConcurrency: 5, - allowedBuiltInModules: process.env.NODE_FUNCTION_ALLOW_BUILTIN, - allowedExternalModules: process.env.NODE_FUNCTION_ALLOW_EXTERNAL, - }); + runner = new JsTaskRunner(config); process.on('SIGINT', createSignalHandler('SIGINT')); process.on('SIGTERM', createSignalHandler('SIGTERM')); diff --git a/packages/@n8n/task-runner/src/task-runner.ts b/packages/@n8n/task-runner/src/task-runner.ts index 356afb69e5..9629cc15d5 100644 --- a/packages/@n8n/task-runner/src/task-runner.ts +++ b/packages/@n8n/task-runner/src/task-runner.ts @@ -1,8 +1,8 @@ import { ApplicationError, type INodeTypeDescription } from 'n8n-workflow'; import { nanoid } from 'nanoid'; -import { URL } from 'node:url'; import { type MessageEvent, WebSocket } from 'ws'; +import type { BaseRunnerConfig } from './config/base-runner-config'; import { TaskRunnerNodeTypes } from './node-types'; import { RPC_ALLOW_LIST, @@ -42,7 +42,10 @@ export interface RPCCallObject { const VALID_TIME_MS = 1000; const VALID_EXTRA_MS = 100; -const DEFAULT_MAX_PAYLOAD_SIZE = 1024 * 1024 * 1024; +export interface TaskRunnerOpts extends BaseRunnerConfig { + taskType: string; + name?: string; +} export abstract class TaskRunner { id: string = nanoid(); @@ -63,22 +66,23 @@ export abstract class TaskRunner { nodeTypes: TaskRunnerNodeTypes = new TaskRunnerNodeTypes([]); - constructor( - public taskType: string, - wsUrl: string, - grantToken: string, - private maxConcurrency: number, - public name?: string, - ) { - const url = new URL(wsUrl); - url.searchParams.append('id', this.id); - this.ws = new WebSocket(url.toString(), { + taskType: string; + + maxConcurrency: number; + + name: string; + + constructor(opts: TaskRunnerOpts) { + this.taskType = opts.taskType; + this.name = opts.name ?? 'Node.js Task Runner SDK'; + this.maxConcurrency = opts.maxConcurrency; + + const wsUrl = `ws://${opts.n8nUri}/runners/_ws?id=${this.id}`; + this.ws = new WebSocket(wsUrl, { headers: { - authorization: `Bearer ${grantToken}`, + authorization: `Bearer ${opts.grantToken}`, }, - maxPayload: process.env.N8N_RUNNERS_MAX_PAYLOAD - ? parseInt(process.env.N8N_RUNNERS_MAX_PAYLOAD) - : DEFAULT_MAX_PAYLOAD_SIZE, + maxPayload: opts.maxPayloadSize, }); this.ws.addEventListener('message', this.receiveMessage); this.ws.addEventListener('close', this.stopTaskOffers); @@ -145,7 +149,7 @@ export abstract class TaskRunner { case 'broker:inforequest': this.send({ type: 'runner:info', - name: this.name ?? 'Node.js Task Runner SDK', + name: this.name, types: [this.taskType], }); break; diff --git a/packages/@n8n/task-runner/tsconfig.json b/packages/@n8n/task-runner/tsconfig.json index db6ad545e3..ddee64ec1f 100644 --- a/packages/@n8n/task-runner/tsconfig.json +++ b/packages/@n8n/task-runner/tsconfig.json @@ -2,6 +2,8 @@ "extends": ["../../../tsconfig.json", "../../../tsconfig.backend.json"], "compilerOptions": { "rootDir": ".", + "emitDecoratorMetadata": true, + "experimentalDecorators": true, "baseUrl": "src", "paths": { "@/*": ["./*"] diff --git a/packages/cli/src/runners/task-runner-process.ts b/packages/cli/src/runners/task-runner-process.ts index d453f1d134..5b31a96ba3 100644 --- a/packages/cli/src/runners/task-runner-process.ts +++ b/packages/cli/src/runners/task-runner-process.ts @@ -179,6 +179,7 @@ export class TaskRunnerProcess extends TypedEmitter { N8N_RUNNERS_GRANT_TOKEN: grantToken, N8N_RUNNERS_N8N_URI: n8nUri, N8N_RUNNERS_MAX_PAYLOAD: this.runnerConfig.maxPayload.toString(), + N8N_RUNNERS_MAX_CONCURRENCY: this.runnerConfig.maxConcurrency.toString(), ...this.getPassthroughEnvVars(), }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fe5ba1e4f7..f527da5bb3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -642,6 +642,9 @@ importers: packages/@n8n/task-runner: dependencies: + '@n8n/config': + specifier: workspace:* + version: link:../config n8n-core: specifier: workspace:* version: link:../../core @@ -651,6 +654,9 @@ importers: nanoid: specifier: ^3.3.6 version: 3.3.7 + typedi: + specifier: 'catalog:' + version: 0.10.0(patch_hash=sk6omkefrosihg7lmqbzh7vfxe) ws: specifier: '>=8.17.1' version: 8.17.1 From cf37e94dd875e9f6ab1f189146fb34e7296af93c Mon Sep 17 00:00:00 2001 From: Eugene Date: Wed, 30 Oct 2024 10:32:08 +0100 Subject: [PATCH 017/672] fix(HTTP Request Tool Node): Fix HTML response optimization issue (#11439) --- .../test/ToolHttpRequest.node.test.ts | 82 ++++++++++++++++--- .../nodes/tools/ToolHttpRequest/utils.ts | 2 +- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts index f18a2437e3..1a99896fff 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts @@ -10,19 +10,19 @@ describe('ToolHttpRequest', () => { const helpers = mock(); const executeFunctions = mock({ helpers }); - describe('Binary response', () => { - beforeEach(() => { - jest.resetAllMocks(); - executeFunctions.getNode.mockReturnValue( - mock({ - type: 'n8n-nodes-base.httpRequest', - name: 'HTTP Request', - typeVersion: 1.1, - }), - ); - executeFunctions.addInputData.mockReturnValue({ index: 0 }); - }); + beforeEach(() => { + jest.resetAllMocks(); + executeFunctions.getNode.mockReturnValue( + mock({ + type: 'n8n-nodes-base.httpRequest', + name: 'HTTP Request', + typeVersion: 1.1, + }), + ); + executeFunctions.addInputData.mockReturnValue({ index: 0 }); + }); + describe('Binary response', () => { it('should return the error when receiving a binary response', async () => { helpers.httpRequest.mockResolvedValue({ body: Buffer.from(''), @@ -237,4 +237,62 @@ describe('ToolHttpRequest', () => { ); }); }); + + describe('Optimize response', () => { + it('should extract body from the response HTML', async () => { + helpers.httpRequest.mockResolvedValue({ + body: ` + + + + +

Test

+ +
+

+ Test content +

+
+ +`, + headers: { + 'content-type': 'text/html', + }, + }); + + executeFunctions.getNodeParameter.mockImplementation( + (paramName: string, _: any, fallback: any) => { + switch (paramName) { + case 'method': + return 'GET'; + case 'url': + return '{url}'; + case 'options': + return {}; + case 'placeholderDefinitions.values': + return []; + case 'optimizeResponse': + return true; + case 'responseType': + return 'html'; + case 'cssSelector': + return 'body'; + default: + return fallback; + } + }, + ); + + const { response } = await httpTool.supplyData.call(executeFunctions, 0); + + const res = await (response as N8nTool).invoke({ + url: 'https://httpbin.org/html', + }); + + expect(helpers.httpRequest).toHaveBeenCalled(); + expect(res).toEqual( + JSON.stringify(['

Test

Test content

'], null, 2), + ); + }); + }); }); diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts index 74f40f0a02..f1d6dfd150 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts @@ -1,5 +1,5 @@ import { Readability } from '@mozilla/readability'; -import cheerio from 'cheerio'; +import * as cheerio from 'cheerio'; import { convert } from 'html-to-text'; import { JSDOM } from 'jsdom'; import get from 'lodash/get'; From 3aa069222b16b15e4e108aa39bb6ac4b51039b2b Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Wed, 30 Oct 2024 06:09:55 -0400 Subject: [PATCH 018/672] refactor(editor): Migrate `UpdatesPanel.vue` to composition API (#11466) --- .../editor-ui/src/components/UpdatesPanel.vue | 56 ++++++++----------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/packages/editor-ui/src/components/UpdatesPanel.vue b/packages/editor-ui/src/components/UpdatesPanel.vue index b96a20c650..fb17fd34ca 100644 --- a/packages/editor-ui/src/components/UpdatesPanel.vue +++ b/packages/editor-ui/src/components/UpdatesPanel.vue @@ -1,38 +1,26 @@ - @@ -45,24 +33,24 @@ export default defineComponent({ >