diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c2f569bf6..ab14dc462e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,50 @@ +# [1.63.0](https://github.com/n8n-io/n8n/compare/n8n@1.62.1...n8n@1.63.0) (2024-10-09) + + +### Bug Fixes + +* **Convert to File Node:** Convert to ICS start date defaults to now ([#11114](https://github.com/n8n-io/n8n/issues/11114)) ([1146c4e](https://github.com/n8n-io/n8n/commit/1146c4e98d8c85c15ac67fa1c3bfb731234531e3)) +* **core:** Allow loading nodes from multiple custom directories ([#11130](https://github.com/n8n-io/n8n/issues/11130)) ([1b84b0e](https://github.com/n8n-io/n8n/commit/1b84b0e5e7485d9f99d61a8ae3df49efadca0745)) +* **core:** Always set `startedAt` when executions start running ([#11098](https://github.com/n8n-io/n8n/issues/11098)) ([722f4a8](https://github.com/n8n-io/n8n/commit/722f4a8b771058800b992a482ad5f644b650960d)) +* **core:** Fix AI nodes not working with new partial execution flow ([#11055](https://github.com/n8n-io/n8n/issues/11055)) ([0eee5df](https://github.com/n8n-io/n8n/commit/0eee5dfd597817819dbe0463a63f671fde53432f)) +* **core:** Print errors that happen before the execution starts on the worker instead of just on the main instance ([#11099](https://github.com/n8n-io/n8n/issues/11099)) ([1d14557](https://github.com/n8n-io/n8n/commit/1d145574611661ecd9ab1a39d815c0ea915b9a1c)) +* **core:** Separate error handlers for main and worker ([#11091](https://github.com/n8n-io/n8n/issues/11091)) ([bb59cc7](https://github.com/n8n-io/n8n/commit/bb59cc71acc9e494e54abc8402d58db39e5a664e)) +* **editor:** Shorten overflowing Node Label in InputLabels on hover and focus ([#11110](https://github.com/n8n-io/n8n/issues/11110)) ([87a0b68](https://github.com/n8n-io/n8n/commit/87a0b68f9009c1c776d937c6ca62096e88c95ed6)) +* **editor:** Add safety to prevent undefined errors ([#11104](https://github.com/n8n-io/n8n/issues/11104)) ([565b117](https://github.com/n8n-io/n8n/commit/565b117a52f8eac9202a1a62c43daf78b293dcf8)) +* **editor:** Fix design system form element sizing ([#11040](https://github.com/n8n-io/n8n/issues/11040)) ([67c3453](https://github.com/n8n-io/n8n/commit/67c3453885bc619fedc8338a6dd0d8d66dead931)) +* **editor:** Fix getInitials when Intl.Segmenter is not supported ([#11103](https://github.com/n8n-io/n8n/issues/11103)) ([7e8955b](https://github.com/n8n-io/n8n/commit/7e8955b322b1d2c84c0f479a5977484d8d5e3135)) +* **editor:** Fix schema view in AI tools ([#11089](https://github.com/n8n-io/n8n/issues/11089)) ([09cfdbd](https://github.com/n8n-io/n8n/commit/09cfdbd1817eba46c935308880fe9f95ded252b0)) +* **editor:** Respect tag querystring filter when listing workflows ([#11029](https://github.com/n8n-io/n8n/issues/11029)) ([59c5ff6](https://github.com/n8n-io/n8n/commit/59c5ff61354302562ba5a2340c66811afdd1523b)) +* **editor:** Show previous nodes autocomplete in AI tool nodes ([#11111](https://github.com/n8n-io/n8n/issues/11111)) ([8566b3a](https://github.com/n8n-io/n8n/commit/8566b3a99939f45ac263830eee30d0d4ade9305c)) +* **editor:** Update Usage page for Community+ edition ([#11074](https://github.com/n8n-io/n8n/issues/11074)) ([3974981](https://github.com/n8n-io/n8n/commit/3974981ea5c67f6f2bbb90a96b405d9d0cfa21af)) +* Fix transaction handling for 'revert' command ([#11145](https://github.com/n8n-io/n8n/issues/11145)) ([a782336](https://github.com/n8n-io/n8n/commit/a7823367f13c3dba0c339eaafaad0199bd524b13)) +* Forbid access to files outside source control work directory ([#11152](https://github.com/n8n-io/n8n/issues/11152)) ([606eedb](https://github.com/n8n-io/n8n/commit/606eedbf1b302e153bd13b7cef80847711e3a9ee)) +* **Gitlab Node:** Author name and email not being set ([#11077](https://github.com/n8n-io/n8n/issues/11077)) ([fce1233](https://github.com/n8n-io/n8n/commit/fce1233b58624d502c9c68f4b32a4bb7d76f1814)) +* Incorrect error message on calling wrong webhook method ([#11093](https://github.com/n8n-io/n8n/issues/11093)) ([d974b01](https://github.com/n8n-io/n8n/commit/d974b015d030c608158ff0c3fa3b7f4cbb8eadd3)) +* **n8n Form Trigger Node:** When clicking on a multiple choice label, the wrong one is selected ([#11059](https://github.com/n8n-io/n8n/issues/11059)) ([948edd1](https://github.com/n8n-io/n8n/commit/948edd1a047cf3dbddb3b0e9ec5de4bac3e97b9f)) +* **NASA Node:** Astronomy-Picture-Of-The-Day fails when it's YouTube video ([#11046](https://github.com/n8n-io/n8n/issues/11046)) ([c70969d](https://github.com/n8n-io/n8n/commit/c70969da2bcabeb33394073a69ccef208311461b)) +* **Postgres PGVector Store Node:** Fix filtering in retriever mode ([#11075](https://github.com/n8n-io/n8n/issues/11075)) ([dbd2ae1](https://github.com/n8n-io/n8n/commit/dbd2ae199506a24c2df4c983111a56f2adf63eee)) +* Show result of waiting execution on canvas after execution complete ([#10815](https://github.com/n8n-io/n8n/issues/10815)) ([90b4bfc](https://github.com/n8n-io/n8n/commit/90b4bfc472ef132d2280b175ae7410dfb8e549b2)) +* **Slack Node:** User id not sent correctly to API when updating user profile ([#11153](https://github.com/n8n-io/n8n/issues/11153)) ([ed9e61c](https://github.com/n8n-io/n8n/commit/ed9e61c46055d8e636a70c9c175d7d4ba596dd48)) + + +### Features + +* **core:** Introduce scoped logging ([#11127](https://github.com/n8n-io/n8n/issues/11127)) ([c68782c](https://github.com/n8n-io/n8n/commit/c68782c633b7ef6253ea705c5a222d4536491fd5)) +* **editor:** Add navigation dropdown component ([#11047](https://github.com/n8n-io/n8n/issues/11047)) ([e081fd1](https://github.com/n8n-io/n8n/commit/e081fd1f0b5a0700017a8dc92f013f0abdbad319)) +* **editor:** Add route for create / edit / share credentials ([#11134](https://github.com/n8n-io/n8n/issues/11134)) ([5697de4](https://github.com/n8n-io/n8n/commit/5697de4429c5d94f25ce1bd14c84fb4266ea47a7)) +* **editor:** Community+ enrollment ([#10776](https://github.com/n8n-io/n8n/issues/10776)) ([92cf860](https://github.com/n8n-io/n8n/commit/92cf860f9f2994442facfddc758bc60f5cbec520)) +* Human in the loop ([#10675](https://github.com/n8n-io/n8n/issues/10675)) ([41228b4](https://github.com/n8n-io/n8n/commit/41228b472de11affc8cd0821284427c2c9e8b421)) +* **OpenAI Node:** Allow to specify thread ID for Assistant -> Message operation ([#11080](https://github.com/n8n-io/n8n/issues/11080)) ([6a2f9e7](https://github.com/n8n-io/n8n/commit/6a2f9e72959fb0e89006b69c31fbcee1ead1cde9)) +* Opt in to additional features on community for existing users ([#11166](https://github.com/n8n-io/n8n/issues/11166)) ([c2adfc8](https://github.com/n8n-io/n8n/commit/c2adfc85451c5103eaad068f882066fd36c4aebe)) + + +### Performance Improvements + +* **core:** Optimize worker healthchecks ([#11092](https://github.com/n8n-io/n8n/issues/11092)) ([19fb728](https://github.com/n8n-io/n8n/commit/19fb728da0839c57603e55da4e407715e6c5b081)) + + + ## [1.62.1](https://github.com/n8n-io/n8n/compare/n8n@1.61.0...n8n@1.62.1) (2024-10-02) diff --git a/cypress/e2e/45-ai-assistant.cy.ts b/cypress/e2e/45-ai-assistant.cy.ts index 6c69a97708..440300a6d5 100644 --- a/cypress/e2e/45-ai-assistant.cy.ts +++ b/cypress/e2e/45-ai-assistant.cy.ts @@ -78,7 +78,7 @@ describe('AI Assistant::enabled', () => { }); it('should start chat session from node error view', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/simple_message_response.json', }).as('chatRequest'); @@ -96,7 +96,7 @@ describe('AI Assistant::enabled', () => { }); it('should render chat input correctly', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/simple_message_response.json', }).as('chatRequest'); @@ -129,7 +129,7 @@ describe('AI Assistant::enabled', () => { }); it('should render and handle quick replies', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/quick_reply_message_response.json', }).as('chatRequest'); @@ -146,7 +146,7 @@ describe('AI Assistant::enabled', () => { }); it('should show quick replies when node is executed after new suggestion', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', (req) => { + cy.intercept('POST', '/rest/ai/chat', (req) => { req.reply((res) => { if (['init-error-helper', 'message'].includes(req.body.payload.type)) { res.send({ statusCode: 200, fixture: 'aiAssistant/simple_message_response.json' }); @@ -177,7 +177,7 @@ describe('AI Assistant::enabled', () => { }); it('should warn before starting a new session', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/simple_message_response.json', }).as('chatRequest'); @@ -204,11 +204,11 @@ describe('AI Assistant::enabled', () => { }); it('should apply code diff to code node', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/code_diff_suggestion_response.json', }).as('chatRequest'); - cy.intercept('POST', '/rest/ai-assistant/chat/apply-suggestion', { + cy.intercept('POST', '/rest/ai/chat/apply-suggestion', { statusCode: 200, fixture: 'aiAssistant/apply_code_diff_response.json', }).as('applySuggestion'); @@ -254,7 +254,7 @@ describe('AI Assistant::enabled', () => { }); it('should end chat session when `end_session` event is received', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/end_session_response.json', }).as('chatRequest'); @@ -268,7 +268,7 @@ describe('AI Assistant::enabled', () => { }); it('should reset session after it ended and sidebar is closed', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', (req) => { + cy.intercept('POST', '/rest/ai/chat', (req) => { req.reply((res) => { if (['init-support-chat'].includes(req.body.payload.type)) { res.send({ statusCode: 200, fixture: 'aiAssistant/simple_message_response.json' }); @@ -296,7 +296,7 @@ describe('AI Assistant::enabled', () => { }); it('Should not reset assistant session when workflow is saved', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/simple_message_response.json', }).as('chatRequest'); @@ -321,7 +321,7 @@ describe('AI Assistant Credential Help', () => { }); it('should start credential help from node credential', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/simple_message_response.json', }).as('chatRequest'); @@ -347,7 +347,7 @@ describe('AI Assistant Credential Help', () => { }); it('should start credential help from credential list', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/simple_message_response.json', }).as('chatRequest'); @@ -446,7 +446,7 @@ describe('General help', () => { }); it('assistant returns code snippet', () => { - cy.intercept('POST', '/rest/ai-assistant/chat', { + cy.intercept('POST', '/rest/ai/chat', { statusCode: 200, fixture: 'aiAssistant/code_snippet_response.json', }).as('chatRequest'); diff --git a/cypress/e2e/6-code-node.cy.ts b/cypress/e2e/6-code-node.cy.ts index 5b422b4589..5a6182c25a 100644 --- a/cypress/e2e/6-code-node.cy.ts +++ b/cypress/e2e/6-code-node.cy.ts @@ -91,28 +91,12 @@ return [] }); describe('Ask AI', () => { - it('tab should display based on experiment', () => { - WorkflowPage.actions.visit(); - cy.window().then((win) => { - win.featureFlags.override('011_ask_AI', 'control'); - WorkflowPage.actions.addInitialNodeToCanvas('Manual'); - WorkflowPage.actions.addNodeToCanvas('Code'); - WorkflowPage.actions.openNode('Code'); - - cy.getByTestId('code-node-tab-ai').should('not.exist'); - - ndv.actions.close(); - win.featureFlags.override('011_ask_AI', undefined); - WorkflowPage.actions.openNode('Code'); - cy.getByTestId('code-node-tab-ai').should('not.exist'); - }); - }); - describe('Enabled', () => { beforeEach(() => { + cy.enableFeature('askAi'); WorkflowPage.actions.visit(); - cy.window().then((win) => { - win.featureFlags.override('011_ask_AI', 'gpt3'); + + cy.window().then(() => { WorkflowPage.actions.addInitialNodeToCanvas('Manual'); WorkflowPage.actions.addNodeToCanvas('Code', true, true); }); @@ -157,7 +141,7 @@ return [] cy.getByTestId('ask-ai-prompt-input').type(prompt); - cy.intercept('POST', '/rest/ask-ai', { + cy.intercept('POST', '/rest/ai/ask-ai', { statusCode: 200, body: { data: { @@ -169,9 +153,7 @@ return [] cy.getByTestId('ask-ai-cta').click(); const askAiReq = cy.wait('@ask-ai'); - askAiReq - .its('request.body') - .should('have.keys', ['question', 'model', 'context', 'n8nVersion']); + askAiReq.its('request.body').should('have.keys', ['question', 'context', 'forNode']); askAiReq.its('context').should('have.keys', ['schema', 'ndvPushRef', 'pushRef']); @@ -195,7 +177,7 @@ return [] ]; handledCodes.forEach(({ code, message }) => { - cy.intercept('POST', '/rest/ask-ai', { + cy.intercept('POST', '/rest/ai/ask-ai', { statusCode: code, status: code, }).as('ask-ai'); diff --git a/package.json b/package.json index feda1c4701..77a80a929b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-monorepo", - "version": "1.62.1", + "version": "1.63.0", "private": true, "engines": { "node": ">=20.15", @@ -69,8 +69,8 @@ ], "overrides": { "@types/node": "^18.16.16", - "chokidar": "3.5.2", - "esbuild": "^0.20.2", + "chokidar": "^4.0.1", + "esbuild": "^0.21.5", "formidable": "3.5.1", "pug": "^3.0.3", "semver": "^7.5.4", diff --git a/packages/@n8n/api-types/package.json b/packages/@n8n/api-types/package.json index ec2bf1bd32..e2614bcf68 100644 --- a/packages/@n8n/api-types/package.json +++ b/packages/@n8n/api-types/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/api-types", - "version": "0.3.0", + "version": "0.4.0", "scripts": { "clean": "rimraf dist .turbo", "dev": "pnpm watch", diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 68c1972a46..41a55f050a 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -2,3 +2,4 @@ export { PasswordUpdateRequestDto } from './user/password-update-request.dto'; export { RoleChangeRequestDto } from './user/role-change-request.dto'; export { SettingsUpdateRequestDto } from './user/settings-update-request.dto'; export { UserUpdateRequestDto } from './user/user-update-request.dto'; +export { CommunityRegisteredRequestDto } from './license/community-registered-request.dto'; diff --git a/packages/@n8n/api-types/src/dto/license/__tests__/community-registered-request.dto.test.ts b/packages/@n8n/api-types/src/dto/license/__tests__/community-registered-request.dto.test.ts new file mode 100644 index 0000000000..84e583e63b --- /dev/null +++ b/packages/@n8n/api-types/src/dto/license/__tests__/community-registered-request.dto.test.ts @@ -0,0 +1,27 @@ +import { CommunityRegisteredRequestDto } from '../community-registered-request.dto'; + +describe('CommunityRegisteredRequestDto', () => { + it('should fail validation for missing email', () => { + const invalidRequest = {}; + + const result = CommunityRegisteredRequestDto.safeParse(invalidRequest); + + expect(result.success).toBe(false); + expect(result.error?.issues[0]).toEqual( + expect.objectContaining({ message: 'Required', path: ['email'] }), + ); + }); + + it('should fail validation for an invalid email', () => { + const invalidRequest = { + email: 'invalid-email', + }; + + const result = CommunityRegisteredRequestDto.safeParse(invalidRequest); + + expect(result.success).toBe(false); + expect(result.error?.issues[0]).toEqual( + expect.objectContaining({ message: 'Invalid email', path: ['email'] }), + ); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/license/community-registered-request.dto.ts b/packages/@n8n/api-types/src/dto/license/community-registered-request.dto.ts new file mode 100644 index 0000000000..9763787767 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/license/community-registered-request.dto.ts @@ -0,0 +1,4 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class CommunityRegisteredRequestDto extends Z.class({ email: z.string().email() }) {} diff --git a/packages/@n8n/api-types/src/frontend-settings.ts b/packages/@n8n/api-types/src/frontend-settings.ts index c70826d4d1..5084344aeb 100644 --- a/packages/@n8n/api-types/src/frontend-settings.ts +++ b/packages/@n8n/api-types/src/frontend-settings.ts @@ -107,6 +107,9 @@ export interface FrontendSettings { aiAssistant: { enabled: boolean; }; + askAi: { + enabled: boolean; + }; deployment: { type: string; }; @@ -154,9 +157,6 @@ export interface FrontendSettings { banners: { dismissed: string[]; }; - ai: { - enabled: boolean; - }; workflowHistory: { pruneTime: number; licensePruneTime: number; diff --git a/packages/@n8n/benchmark/package.json b/packages/@n8n/benchmark/package.json index 2b6979fa45..98edd1dabd 100644 --- a/packages/@n8n/benchmark/package.json +++ b/packages/@n8n/benchmark/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/n8n-benchmark", - "version": "1.6.1", + "version": "1.7.0", "description": "Cli for running benchmark tests for n8n", "main": "dist/index", "scripts": { diff --git a/packages/@n8n/benchmark/scripts/n8n-setups/postgres/docker-compose.yml b/packages/@n8n/benchmark/scripts/n8n-setups/postgres/docker-compose.yml index 2ca26c79b3..3cc08227c1 100644 --- a/packages/@n8n/benchmark/scripts/n8n-setups/postgres/docker-compose.yml +++ b/packages/@n8n/benchmark/scripts/n8n-setups/postgres/docker-compose.yml @@ -7,7 +7,7 @@ services: - ${MOCK_API_DATA_PATH}/mappings:/home/wiremock/mappings postgres: - image: postgres:16 + image: postgres:16.4 restart: always user: root:root environment: diff --git a/packages/@n8n/benchmark/scripts/n8n-setups/scaling-multi-main/docker-compose.yml b/packages/@n8n/benchmark/scripts/n8n-setups/scaling-multi-main/docker-compose.yml index 3298a47556..ca3ad9c23d 100644 --- a/packages/@n8n/benchmark/scripts/n8n-setups/scaling-multi-main/docker-compose.yml +++ b/packages/@n8n/benchmark/scripts/n8n-setups/scaling-multi-main/docker-compose.yml @@ -7,7 +7,7 @@ services: - ${MOCK_API_DATA_PATH}/mappings:/home/wiremock/mappings redis: - image: redis:6-alpine + image: redis:6.2.14-alpine restart: always ports: - 6379:6379 @@ -17,7 +17,7 @@ services: timeout: 3s postgres: - image: postgres:16 + image: postgres:16.4 restart: always environment: - POSTGRES_DB=n8n diff --git a/packages/@n8n/benchmark/scripts/n8n-setups/scaling-single-main/docker-compose.yml b/packages/@n8n/benchmark/scripts/n8n-setups/scaling-single-main/docker-compose.yml index 27590459d2..fe9e3a26c0 100644 --- a/packages/@n8n/benchmark/scripts/n8n-setups/scaling-single-main/docker-compose.yml +++ b/packages/@n8n/benchmark/scripts/n8n-setups/scaling-single-main/docker-compose.yml @@ -7,7 +7,7 @@ services: - ${MOCK_API_DATA_PATH}/mappings:/home/wiremock/mappings redis: - image: redis:6-alpine + image: redis:6.2.14-alpine ports: - 6379:6379 healthcheck: @@ -16,7 +16,7 @@ services: timeout: 3s postgres: - image: postgres:16 + image: postgres:16.4 user: root:root restart: always environment: diff --git a/packages/@n8n/chat/README.md b/packages/@n8n/chat/README.md index 0ed53a5774..6299c80d12 100644 --- a/packages/@n8n/chat/README.md +++ b/packages/@n8n/chat/README.md @@ -184,6 +184,16 @@ createChat({ - **Type**: `string[]` - **Description**: The initial messages to be displayed in the Chat window. +### `allowFileUploads` +- **Type**: `Ref | boolean` +- **Default**: `false` +- **Description**: Whether to allow file uploads in the chat. If set to `true`, users will be able to upload files through the chat interface. + +### `allowedFilesMimeTypes` +- **Type**: `Ref | string` +- **Default**: `''` +- **Description**: A comma-separated list of allowed MIME types for file uploads. Only applicable if `allowFileUploads` is set to `true`. If left empty, all file types are allowed. For example: `'image/*,application/pdf'`. + ## Customization The Chat window is entirely customizable using CSS variables. diff --git a/packages/@n8n/chat/package.json b/packages/@n8n/chat/package.json index 6f3e74fc71..b6379b1fe0 100644 --- a/packages/@n8n/chat/package.json +++ b/packages/@n8n/chat/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/chat", - "version": "0.27.1", + "version": "0.28.0", "scripts": { "dev": "pnpm run storybook", "build": "pnpm build:vite && pnpm build:bundle", diff --git a/packages/@n8n/config/package.json b/packages/@n8n/config/package.json index 6d989f8208..10c8cbcf5b 100644 --- a/packages/@n8n/config/package.json +++ b/packages/@n8n/config/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/config", - "version": "1.12.1", + "version": "1.13.0", "scripts": { "clean": "rimraf dist .turbo", "dev": "pnpm watch", diff --git a/packages/@n8n/config/src/configs/logging.config.ts b/packages/@n8n/config/src/configs/logging.config.ts index 2f68416df4..c29bb6d5a8 100644 --- a/packages/@n8n/config/src/configs/logging.config.ts +++ b/packages/@n8n/config/src/configs/logging.config.ts @@ -1,6 +1,17 @@ import { Config, Env, Nested } from '../decorators'; import { StringArray } from '../utils'; +/** + * Scopes (areas of functionality) to filter logs by. + * + * `executions` -> execution lifecycle + * `license` -> license SDK + * `scaling` -> scaling mode + */ +export const LOG_SCOPES = ['executions', 'license', 'scaling'] as const; + +export type LogScope = (typeof LOG_SCOPES)[number]; + @Config class FileLoggingConfig { /** @@ -44,4 +55,19 @@ export class LoggingConfig { @Nested file: FileLoggingConfig; + + /** + * Scopes to filter logs by. Nothing is filtered by default. + * + * Currently supported log scopes: + * - `executions` + * - `license` + * - `scaling` + * + * @example + * `N8N_LOG_SCOPES=license` + * `N8N_LOG_SCOPES=license,executions` + */ + @Env('N8N_LOG_SCOPES') + scopes: StringArray = []; } diff --git a/packages/@n8n/config/src/configs/scaling-mode.config.ts b/packages/@n8n/config/src/configs/scaling-mode.config.ts index a1f5b2a7d6..05ee6b4841 100644 --- a/packages/@n8n/config/src/configs/scaling-mode.config.ts +++ b/packages/@n8n/config/src/configs/scaling-mode.config.ts @@ -2,7 +2,11 @@ import { Config, Env, Nested } from '../decorators'; @Config class HealthConfig { - /** Whether to enable the worker health check endpoint `/healthz`. */ + /** + * Whether to enable the worker health check endpoints: + * - `/healthz` (worker alive) + * - `/healthz/readiness` (worker connected to migrated database and connected to Redis) + */ @Env('QUEUE_HEALTH_CHECK_ACTIVE') active: boolean = false; diff --git a/packages/@n8n/config/src/index.ts b/packages/@n8n/config/src/index.ts index 3290cac5bb..9044ffa0fa 100644 --- a/packages/@n8n/config/src/index.ts +++ b/packages/@n8n/config/src/index.ts @@ -18,6 +18,9 @@ import { VersionNotificationsConfig } from './configs/version-notifications.conf import { WorkflowsConfig } from './configs/workflows.config'; import { Config, Env, Nested } from './decorators'; +export { LOG_SCOPES } from './configs/logging.config'; +export type { LogScope } from './configs/logging.config'; + @Config export class GlobalConfig { @Nested diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index a0952d0dd0..301022ca3e 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -241,13 +241,13 @@ describe('GlobalConfig', () => { fileSizeMax: 16, location: 'logs/n8n.log', }, + scopes: [], }, }; it('should use all default values when no env variables are defined', () => { process.env = {}; const config = Container.get(GlobalConfig); - expect(deepCopy(config)).toEqual(defaultConfig); expect(mockFs.readFileSync).not.toHaveBeenCalled(); }); diff --git a/packages/@n8n/nodes-langchain/nodes/chains/InformationExtractor/InformationExtractor.node.ts b/packages/@n8n/nodes-langchain/nodes/chains/InformationExtractor/InformationExtractor.node.ts index 0399fc2d5a..7ccfddc5e4 100644 --- a/packages/@n8n/nodes-langchain/nodes/chains/InformationExtractor/InformationExtractor.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/chains/InformationExtractor/InformationExtractor.node.ts @@ -262,7 +262,7 @@ export class InformationExtractor implements INodeType { } const zodSchemaSandbox = getSandboxWithZod(this, jsonSchema, 0); - const zodSchema = (await zodSchemaSandbox.runCode()) as z.ZodSchema; + const zodSchema = await zodSchemaSandbox.runCode>(); parser = OutputFixingParser.fromLLM(llm, StructuredOutputParser.fromZodSchema(zodSchema)); } diff --git a/packages/@n8n/nodes-langchain/nodes/code/Code.node.ts b/packages/@n8n/nodes-langchain/nodes/code/Code.node.ts index 085dec98c2..abe9b01530 100644 --- a/packages/@n8n/nodes-langchain/nodes/code/Code.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/code/Code.node.ts @@ -107,7 +107,7 @@ function getSandbox( } // eslint-disable-next-line @typescript-eslint/unbound-method - const sandbox = new JavaScriptSandbox(context, code, itemIndex, this.helpers, { + const sandbox = new JavaScriptSandbox(context, code, this.helpers, { resolver: vmResolver, }); @@ -368,7 +368,7 @@ export class Code implements INodeType { } const sandbox = getSandbox.call(this, code.supplyData.code, { itemIndex }); - const response = (await sandbox.runCode()) as Tool; + const response = await sandbox.runCode(); return { response: logWrapper(response, this), diff --git a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts index 027ce04e6e..6ce6bff76b 100644 --- a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts @@ -48,7 +48,7 @@ export class N8nStructuredOutputParser extends Structure sandboxedSchema: JavaScriptSandbox, nodeVersion: number, ): Promise>> { - const zodSchema = (await sandboxedSchema.runCode()) as z.ZodSchema; + const zodSchema = await sandboxedSchema.runCode>(); let returnSchema: z.ZodSchema; if (nodeVersion === 1) { diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolCode/ToolCode.node.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolCode/ToolCode.node.ts index df68fb0c6a..7980f5fa9d 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolCode/ToolCode.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolCode/ToolCode.node.ts @@ -199,9 +199,9 @@ export class ToolCode implements INodeType { let sandbox: Sandbox; if (language === 'javaScript') { - sandbox = new JavaScriptSandbox(context, code, index, this.helpers); + sandbox = new JavaScriptSandbox(context, code, this.helpers); } else { - sandbox = new PythonSandbox(context, code, index, this.helpers); + sandbox = new PythonSandbox(context, code, this.helpers); } sandbox.on( @@ -216,7 +216,7 @@ export class ToolCode implements INodeType { const runFunction = async (query: string | IDataObject): Promise => { const sandbox = getSandbox(query, itemIndex); - return await (sandbox.runCode() as Promise); + return await sandbox.runCode(); }; const toolHandler = async (query: string | IDataObject): Promise => { @@ -274,7 +274,7 @@ export class ToolCode implements INodeType { : jsonParse(inputSchema); const zodSchemaSandbox = getSandboxWithZod(this, jsonSchema, 0); - const zodSchema = (await zodSchemaSandbox.runCode()) as DynamicZodObject; + const zodSchema = await zodSchemaSandbox.runCode(); tool = new DynamicStructuredTool({ schema: zodSchema, diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/ToolWorkflow.node.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/ToolWorkflow.node.ts index 0b00e17ac4..352a727d11 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/ToolWorkflow.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/ToolWorkflow.node.ts @@ -530,7 +530,7 @@ export class ToolWorkflow implements INodeType { : jsonParse(inputSchema); const zodSchemaSandbox = getSandboxWithZod(this, jsonSchema, 0); - const zodSchema = (await zodSchemaSandbox.runCode()) as DynamicZodObject; + const zodSchema = await zodSchemaSandbox.runCode(); tool = new DynamicStructuredTool({ schema: zodSchema, diff --git a/packages/@n8n/nodes-langchain/package.json b/packages/@n8n/nodes-langchain/package.json index 34219d3ce6..4df337ed52 100644 --- a/packages/@n8n/nodes-langchain/package.json +++ b/packages/@n8n/nodes-langchain/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/n8n-nodes-langchain", - "version": "1.62.1", + "version": "1.63.0", "description": "", "main": "index.js", "scripts": { @@ -124,18 +124,20 @@ "@types/cheerio": "^0.22.15", "@types/html-to-text": "^9.0.1", "@types/json-schema": "^7.0.15", + "@types/pg": "^8.11.6", "@types/temp": "^0.9.1", "n8n-core": "workspace:*" }, "dependencies": { - "@getzep/zep-cloud": "1.0.11", + "@aws-sdk/client-sso-oidc": "3.666.0", + "@getzep/zep-cloud": "1.0.12", "@getzep/zep-js": "0.9.0", "@google-ai/generativelanguage": "2.6.0", "@google-cloud/resource-manager": "5.3.0", "@google/generative-ai": "0.19.0", "@huggingface/inference": "2.8.0", "@langchain/anthropic": "0.3.1", - "@langchain/aws": "^0.1.0", + "@langchain/aws": "0.1.0", "@langchain/cohere": "0.3.0", "@langchain/community": "0.3.2", "@langchain/core": "catalog:", @@ -149,23 +151,22 @@ "@langchain/qdrant": "0.1.0", "@langchain/redis": "0.1.0", "@langchain/textsplitters": "0.1.0", - "@mozilla/readability": "^0.5.0", + "@mozilla/readability": "0.5.0", "@n8n/typeorm": "0.3.20-12", "@n8n/vm2": "3.9.25", "@pinecone-database/pinecone": "3.0.3", "@qdrant/js-client-rest": "1.11.0", "@supabase/supabase-js": "2.45.4", - "@types/pg": "^8.11.6", - "@xata.io/client": "0.30.0", + "@xata.io/client": "0.28.4", "basic-auth": "catalog:", - "cheerio": "1.0.0-rc.12", + "cheerio": "1.0.0", "cohere-ai": "7.13.2", "d3-dsv": "2.0.0", "epub2": "3.0.2", "form-data": "catalog:", "generate-schema": "2.6.0", "html-to-text": "9.0.5", - "jsdom": "^23.0.1", + "jsdom": "23.0.1", "json-schema-to-zod": "2.1.0", "langchain": "0.3.2", "lodash": "catalog:", diff --git a/packages/@n8n/nodes-langchain/utils/schemaParsing.ts b/packages/@n8n/nodes-langchain/utils/schemaParsing.ts index 8d5f61153d..0591483e2c 100644 --- a/packages/@n8n/nodes-langchain/utils/schemaParsing.ts +++ b/packages/@n8n/nodes-langchain/utils/schemaParsing.ts @@ -57,7 +57,6 @@ export function getSandboxWithZod(ctx: IExecuteFunctions, schema: JSONSchema7, i const itemSchema = new Function('z', 'return (' + zodSchema + ')')(z) return itemSchema `, - itemIndex, ctx.helpers, { resolver: vmResolver }, ); diff --git a/packages/@n8n/task-runner/package.json b/packages/@n8n/task-runner/package.json index 2ce5993bb9..a82b97975d 100644 --- a/packages/@n8n/task-runner/package.json +++ b/packages/@n8n/task-runner/package.json @@ -1,18 +1,19 @@ { "name": "@n8n/task-runner", - "version": "1.0.1", + "version": "1.1.0", "scripts": { "clean": "rimraf dist .turbo", "start": "node dist/start.js", "dev": "pnpm build && pnpm start", "typecheck": "tsc --noEmit", - "build": "tsc -p ./tsconfig.build.json", + "build": "tsc -p ./tsconfig.build.json && tsc-alias -p tsconfig.build.json", "format": "biome format --write src", "format:check": "biome ci src", - "test": "echo \"Error: no tests in this package\" && exit 0", + "test": "jest", + "test:watch": "jest --watch", "lint": "eslint . --quiet", "lintfix": "eslint . --fix", - "watch": "tsc -p tsconfig.build.json --watch" + "watch": "concurrently \"tsc -w -p tsconfig.build.json\" \"tsc-alias -w -p tsconfig.build.json\"" }, "main": "dist/start.js", "module": "src/start.ts", @@ -25,5 +26,8 @@ "n8n-core": "workspace:*", "nanoid": "^3.3.6", "ws": "^8.18.0" + }, + "devDependencies": { + "luxon": "catalog:" } } diff --git a/packages/@n8n/task-runner/src/authenticator.ts b/packages/@n8n/task-runner/src/authenticator.ts index 717af58dd2..7edb4cadf6 100644 --- a/packages/@n8n/task-runner/src/authenticator.ts +++ b/packages/@n8n/task-runner/src/authenticator.ts @@ -11,7 +11,7 @@ export type AuthOpts = { */ export async function authenticate(opts: AuthOpts) { try { - const authEndpoint = `http://${opts.n8nUri}/rest/runners/auth`; + const authEndpoint = `http://${opts.n8nUri}/runners/auth`; const response = await fetch(authEndpoint, { method: 'POST', headers: { diff --git a/packages/@n8n/task-runner/src/code.ts b/packages/@n8n/task-runner/src/code.ts deleted file mode 100644 index 6fcb6cf878..0000000000 --- a/packages/@n8n/task-runner/src/code.ts +++ /dev/null @@ -1,147 +0,0 @@ -import { getAdditionalKeys } from 'n8n-core'; -import { - type INode, - type INodeType, - type ITaskDataConnections, - type IWorkflowExecuteAdditionalData, - WorkflowDataProxy, - type WorkflowParameters, - type IDataObject, - type IExecuteData, - type INodeExecutionData, - type INodeParameters, - type IRunExecutionData, - // type IWorkflowDataProxyAdditionalKeys, - Workflow, - type WorkflowExecuteMode, -} from 'n8n-workflow'; -import * as a from 'node:assert'; -import { runInNewContext, type Context } from 'node:vm'; - -import type { TaskResultData } from './runner-types'; -import { type Task, TaskRunner } from './task-runner'; - -interface JSExecSettings { - code: string; - - // For workflow data proxy - mode: WorkflowExecuteMode; -} - -export interface PartialAdditionalData { - executionId?: string; - restartExecutionId?: string; - restApiUrl: string; - instanceBaseUrl: string; - formWaitingBaseUrl: string; - webhookBaseUrl: string; - webhookWaitingBaseUrl: string; - webhookTestBaseUrl: string; - currentNodeParameters?: INodeParameters; - executionTimeoutTimestamp?: number; - userId?: string; - variables: IDataObject; -} - -export interface AllCodeTaskData { - workflow: Omit; - inputData: ITaskDataConnections; - node: INode; - - runExecutionData: IRunExecutionData; - runIndex: number; - itemIndex: number; - activeNodeName: string; - connectionInputData: INodeExecutionData[]; - siblingParameters: INodeParameters; - mode: WorkflowExecuteMode; - executeData?: IExecuteData; - defaultReturnRunIndex: number; - selfData: IDataObject; - contextNodeName: string; - additionalData: PartialAdditionalData; -} - -export class JsTaskRunner extends TaskRunner { - constructor( - taskType: string, - wsUrl: string, - grantToken: string, - maxConcurrency: number, - name?: string, - ) { - super(taskType, wsUrl, grantToken, maxConcurrency, name ?? 'JS Task Runner'); - } - - async executeTask(task: Task): Promise { - const allData = await this.requestData(task.taskId, 'all'); - - const settings = task.settings; - a.ok(settings, 'JS Code not sent to runner'); - - const workflowParams = allData.workflow; - const workflow = new Workflow({ - ...workflowParams, - nodeTypes: { - getByNameAndVersion() { - return undefined as unknown as INodeType; - }, - getByName() { - return undefined as unknown as INodeType; - }, - getKnownTypes() { - return {}; - }, - }, - }); - - const dataProxy = new WorkflowDataProxy( - workflow, - allData.runExecutionData, - allData.runIndex, - allData.itemIndex, - allData.activeNodeName, - allData.connectionInputData, - allData.siblingParameters, - settings.mode, - getAdditionalKeys( - allData.additionalData as IWorkflowExecuteAdditionalData, - allData.mode, - allData.runExecutionData, - ), - allData.executeData, - allData.defaultReturnRunIndex, - allData.selfData, - allData.contextNodeName, - ); - - const customConsole = { - log: (...args: unknown[]) => { - const logOutput = args - .map((arg) => (typeof arg === 'object' && arg !== null ? JSON.stringify(arg) : arg)) - .join(' '); - console.log('[JS Code]', logOutput); - void this.makeRpcCall(task.taskId, 'logNodeOutput', [logOutput]); - }, - }; - - const context: Context = { - require, - module: {}, - console: customConsole, - - ...dataProxy.getDataProxy(), - ...this.buildRpcCallObject(task.taskId), - }; - - const result = (await runInNewContext( - `module.exports = async function() {${settings.code}\n}()`, - context, - )) as TaskResultData['result']; - - return { - result, - customData: allData.runExecutionData.resultData.metadata, - }; - } -} 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 new file mode 100644 index 0000000000..fec14acc8c --- /dev/null +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts @@ -0,0 +1,455 @@ +import { DateTime } from 'luxon'; +import type { CodeExecutionMode, IDataObject } from 'n8n-workflow'; + +import { ValidationError } from '@/js-task-runner/errors/validation-error'; +import { + JsTaskRunner, + type AllCodeTaskData, + type JSExecSettings, +} from '@/js-task-runner/js-task-runner'; +import type { Task } from '@/task-runner'; + +import { newAllCodeTaskData, newTaskWithSettings, withPairedItem, wrapIntoJson } from './test-data'; + +jest.mock('ws'); + +describe('JsTaskRunner', () => { + const jsTaskRunner = new JsTaskRunner('taskType', 'ws://localhost', 'grantToken', 1); + + const execTaskWithParams = async ({ + task, + taskData, + }: { + task: Task; + taskData: AllCodeTaskData; + }) => { + jest.spyOn(jsTaskRunner, 'requestData').mockResolvedValue(taskData); + return await jsTaskRunner.executeTask(task); + }; + + afterEach(() => { + jest.restoreAllMocks(); + }); + + const executeForAllItems = async ({ + code, + inputItems, + settings, + }: { code: string; inputItems: IDataObject[]; settings?: Partial }) => { + return await execTaskWithParams({ + task: newTaskWithSettings({ + code, + nodeMode: 'runOnceForAllItems', + ...settings, + }), + taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson)), + }); + }; + + const executeForEachItem = async ({ + code, + inputItems, + settings, + }: { code: string; inputItems: IDataObject[]; settings?: Partial }) => { + return await execTaskWithParams({ + task: newTaskWithSettings({ + code, + nodeMode: 'runOnceForEachItem', + ...settings, + }), + taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson)), + }); + }; + + describe('console', () => { + test.each<[CodeExecutionMode]>([['runOnceForAllItems'], ['runOnceForEachItem']])( + 'should make an rpc call for console log in %s mode', + async (nodeMode) => { + jest.spyOn(jsTaskRunner, 'makeRpcCall').mockResolvedValue(undefined); + const task = newTaskWithSettings({ + code: "console.log('Hello', 'world!'); return {}", + nodeMode, + }); + + await execTaskWithParams({ + task, + taskData: newAllCodeTaskData([wrapIntoJson({})]), + }); + + expect(jsTaskRunner.makeRpcCall).toHaveBeenCalledWith(task.taskId, 'logNodeOutput', [ + 'Hello world!', + ]); + }, + ); + }); + + describe('built-in methods and variables available in the context', () => { + const inputItems = [{ a: 1 }]; + + const testExpressionForAllItems = async ( + expression: string, + expected: IDataObject | string | number | boolean, + ) => { + const needsWrapping = typeof expected !== 'object'; + const outcome = await executeForAllItems({ + code: needsWrapping ? `return { val: ${expression} }` : `return ${expression}`, + inputItems, + }); + + expect(outcome.result).toEqual([wrapIntoJson(needsWrapping ? { val: expected } : expected)]); + }; + + const testExpressionForEachItem = async ( + expression: string, + expected: IDataObject | string | number | boolean, + ) => { + const needsWrapping = typeof expected !== 'object'; + const outcome = await executeForEachItem({ + code: needsWrapping ? `return { val: ${expression} }` : `return ${expression}`, + inputItems, + }); + + expect(outcome.result).toEqual([ + withPairedItem(0, wrapIntoJson(needsWrapping ? { val: expected } : expected)), + ]); + }; + + const testGroups = { + // https://docs.n8n.io/code/builtin/current-node-input/ + 'current node input': [ + ['$input.first()', inputItems[0]], + ['$input.last()', inputItems[inputItems.length - 1]], + ['$input.params', { manualTriggerParam: 'empty' }], + ], + // https://docs.n8n.io/code/builtin/output-other-nodes/ + 'output of other nodes': [ + ['$("Trigger").first()', inputItems[0]], + ['$("Trigger").last()', inputItems[inputItems.length - 1]], + ['$("Trigger").params', { manualTriggerParam: 'empty' }], + ], + // https://docs.n8n.io/code/builtin/date-time/ + 'date and time': [ + ['$now', expect.any(DateTime)], + ['$today', expect.any(DateTime)], + ['{dt: DateTime}', { dt: expect.any(Function) }], + ], + // https://docs.n8n.io/code/builtin/jmespath/ + JMESPath: [['{ val: $jmespath([{ f: 1 },{ f: 2 }], "[*].f") }', { val: [1, 2] }]], + // https://docs.n8n.io/code/builtin/n8n-metadata/ + 'n8n metadata': [ + [ + '$execution', + { + id: 'exec-id', + mode: 'test', + resumeFormUrl: 'http://formWaitingBaseUrl/exec-id', + resumeUrl: 'http://webhookWaitingBaseUrl/exec-id', + customData: { + get: expect.any(Function), + getAll: expect.any(Function), + set: expect.any(Function), + setAll: expect.any(Function), + }, + }, + ], + ['$("Trigger").isExecuted', true], + ['$nodeVersion', 2], + ['$prevNode.name', 'Trigger'], + ['$prevNode.outputIndex', 0], + ['$runIndex', 0], + ['{ wf: $workflow }', { wf: { active: true, id: '1', name: 'Test Workflow' } }], + ['$vars', { var: 'value' }], + ], + }; + + for (const [groupName, tests] of Object.entries(testGroups)) { + describe(`${groupName} runOnceForAllItems`, () => { + test.each(tests)( + 'should have the %s available in the context', + async (expression, expected) => { + await testExpressionForAllItems(expression, expected); + }, + ); + }); + + describe(`${groupName} runOnceForEachItem`, () => { + test.each(tests)( + 'should have the %s available in the context', + async (expression, expected) => { + await testExpressionForEachItem(expression, expected); + }, + ); + }); + } + + describe('$env', () => { + it('should have the env available in context when access has not been blocked', async () => { + const outcome = await execTaskWithParams({ + task: newTaskWithSettings({ + code: 'return { val: $env.VAR1 }', + nodeMode: 'runOnceForAllItems', + }), + taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson), { + envProviderState: { + isEnvAccessBlocked: false, + isProcessAvailable: true, + env: { VAR1: 'value' }, + }, + }), + }); + + expect(outcome.result).toEqual([wrapIntoJson({ val: 'value' })]); + }); + + it('should be possible to access env if it has been blocked', async () => { + await expect( + execTaskWithParams({ + task: newTaskWithSettings({ + code: 'return { val: $env.VAR1 }', + nodeMode: 'runOnceForAllItems', + }), + taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson), { + envProviderState: { + isEnvAccessBlocked: true, + isProcessAvailable: true, + env: { VAR1: 'value' }, + }, + }), + }), + ).rejects.toThrow('access to env vars denied'); + }); + + it('should not be possible to iterate $env', async () => { + const outcome = await execTaskWithParams({ + task: newTaskWithSettings({ + code: 'return Object.values($env).concat(Object.keys($env))', + nodeMode: 'runOnceForAllItems', + }), + taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson), { + envProviderState: { + isEnvAccessBlocked: false, + isProcessAvailable: true, + env: { VAR1: '1', VAR2: '2', VAR3: '3' }, + }, + }), + }); + + expect(outcome.result).toEqual([]); + }); + + it("should not expose task runner's env variables even if no env state is received", async () => { + process.env.N8N_RUNNERS_N8N_URI = 'http://127.0.0.1:5679'; + const outcome = await execTaskWithParams({ + task: newTaskWithSettings({ + code: 'return { val: $env.N8N_RUNNERS_N8N_URI }', + nodeMode: 'runOnceForAllItems', + }), + taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson), { + envProviderState: undefined, + }), + }); + + expect(outcome.result).toEqual([wrapIntoJson({ val: undefined })]); + }); + }); + }); + + describe('runOnceForAllItems', () => { + describe('continue on fail', () => { + it('should return an item with the error if continueOnFail is true', async () => { + const outcome = await executeForAllItems({ + code: 'throw new Error("Error message")', + inputItems: [{ a: 1 }], + settings: { continueOnFail: true }, + }); + + expect(outcome).toEqual({ + result: [wrapIntoJson({ error: 'Error message' })], + customData: undefined, + }); + }); + + it('should throw an error if continueOnFail is false', async () => { + await expect( + executeForAllItems({ + code: 'throw new Error("Error message")', + inputItems: [{ a: 1 }], + settings: { continueOnFail: false }, + }), + ).rejects.toThrow('Error message'); + }); + }); + + describe('invalid output', () => { + test.each([['undefined'], ['42'], ['"a string"']])( + 'should throw a ValidationError if the code output is %s', + async (output) => { + await expect( + executeForAllItems({ + code: `return ${output}`, + inputItems: [{ a: 1 }], + }), + ).rejects.toThrow(ValidationError); + }, + ); + + it('should throw a ValidationError if some items are wrapped in json and some are not', async () => { + await expect( + executeForAllItems({ + code: 'return [{b: 1}, {json: {b: 2}}]', + inputItems: [{ a: 1 }], + }), + ).rejects.toThrow(ValidationError); + }); + }); + + it('should return static items', async () => { + const outcome = await executeForAllItems({ + code: 'return [{json: {b: 1}}]', + inputItems: [{ a: 1 }], + }); + + expect(outcome).toEqual({ + result: [wrapIntoJson({ b: 1 })], + customData: undefined, + }); + }); + + it('maps null into an empty array', async () => { + const outcome = await executeForAllItems({ + code: 'return null', + inputItems: [{ a: 1 }], + }); + + expect(outcome).toEqual({ + result: [], + customData: undefined, + }); + }); + + it("should wrap items into json if they aren't", async () => { + const outcome = await executeForAllItems({ + code: 'return [{b: 1}]', + inputItems: [{ a: 1 }], + }); + + expect(outcome).toEqual({ + result: [wrapIntoJson({ b: 1 })], + customData: undefined, + }); + }); + + it('should wrap single item into an array and json', async () => { + const outcome = await executeForAllItems({ + code: 'return {b: 1}', + inputItems: [{ a: 1 }], + }); + + expect(outcome).toEqual({ + result: [wrapIntoJson({ b: 1 })], + customData: undefined, + }); + }); + + test.each([['items'], ['$input.all()'], ["$('Trigger').all()"]])( + 'should have all input items in the context as %s', + async (expression) => { + const outcome = await executeForAllItems({ + code: `return ${expression}`, + inputItems: [{ a: 1 }, { a: 2 }], + }); + + expect(outcome).toEqual({ + result: [wrapIntoJson({ a: 1 }), wrapIntoJson({ a: 2 })], + customData: undefined, + }); + }, + ); + }); + + describe('runForEachItem', () => { + describe('continue on fail', () => { + it('should return an item with the error if continueOnFail is true', async () => { + const outcome = await executeForEachItem({ + code: 'throw new Error("Error message")', + inputItems: [{ a: 1 }, { a: 2 }], + settings: { continueOnFail: true }, + }); + + expect(outcome).toEqual({ + result: [ + withPairedItem(0, wrapIntoJson({ error: 'Error message' })), + withPairedItem(1, wrapIntoJson({ error: 'Error message' })), + ], + customData: undefined, + }); + }); + + it('should throw an error if continueOnFail is false', async () => { + await expect( + executeForEachItem({ + code: 'throw new Error("Error message")', + inputItems: [{ a: 1 }], + settings: { continueOnFail: false }, + }), + ).rejects.toThrow('Error message'); + }); + }); + + describe('invalid output', () => { + test.each([['undefined'], ['42'], ['"a string"'], ['[]'], ['[1,2,3]']])( + 'should throw a ValidationError if the code output is %s', + async (output) => { + await expect( + executeForEachItem({ + code: `return ${output}`, + inputItems: [{ a: 1 }], + }), + ).rejects.toThrow(ValidationError); + }, + ); + }); + + it('should return static items', async () => { + const outcome = await executeForEachItem({ + code: 'return {json: {b: 1}}', + inputItems: [{ a: 1 }], + }); + + expect(outcome).toEqual({ + result: [withPairedItem(0, wrapIntoJson({ b: 1 }))], + customData: undefined, + }); + }); + + it('should filter out null values', async () => { + const outcome = await executeForEachItem({ + code: 'return item.json.a === 1 ? item : null', + inputItems: [{ a: 1 }, { a: 2 }, { a: 3 }], + }); + + expect(outcome).toEqual({ + result: [withPairedItem(0, wrapIntoJson({ a: 1 }))], + customData: undefined, + }); + }); + + test.each([['item'], ['$input.item'], ['{ json: $json }']])( + 'should have the current input item in the context as %s', + async (expression) => { + const outcome = await executeForEachItem({ + code: `return ${expression}`, + inputItems: [{ a: 1 }, { a: 2 }], + }); + + expect(outcome).toEqual({ + result: [ + withPairedItem(0, wrapIntoJson({ a: 1 })), + withPairedItem(1, wrapIntoJson({ a: 2 })), + ], + customData: undefined, + }); + }, + ); + }); +}); diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/test-data.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/test-data.ts new file mode 100644 index 0000000000..b157094619 --- /dev/null +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/test-data.ts @@ -0,0 +1,168 @@ +import type { IDataObject, INode, INodeExecutionData, ITaskData } from 'n8n-workflow'; +import { NodeConnectionType } from 'n8n-workflow'; +import { nanoid } from 'nanoid'; + +import type { AllCodeTaskData, JSExecSettings } from '@/js-task-runner/js-task-runner'; +import type { Task } from '@/task-runner'; + +/** + * Creates a new task with the given settings + */ +export const newTaskWithSettings = ( + settings: Partial & Pick, +): Task => ({ + taskId: '1', + settings: { + workflowMode: 'manual', + continueOnFail: false, + mode: 'manual', + ...settings, + }, + active: true, + cancelled: false, +}); + +/** + * Creates a new node with the given options + */ +export const newNode = (opts: Partial = {}): INode => ({ + id: nanoid(), + name: 'Test Node' + nanoid(), + parameters: {}, + position: [0, 0], + type: 'n8n-nodes-base.code', + typeVersion: 1, + ...opts, +}); + +/** + * Creates a new task data with the given options + */ +export const newTaskData = (opts: Partial & Pick): ITaskData => ({ + startTime: Date.now(), + executionTime: 0, + executionStatus: 'success', + ...opts, +}); + +/** + * Creates a new all code task data with the given options + */ +export const newAllCodeTaskData = ( + codeNodeInputData: INodeExecutionData[], + opts: Partial = {}, +): AllCodeTaskData => { + const codeNode = newNode({ + name: 'JsCode', + parameters: { + mode: 'runOnceForEachItem', + language: 'javaScript', + jsCode: 'return item', + }, + type: 'n8n-nodes-base.code', + typeVersion: 2, + }); + const manualTriggerNode = newNode({ + name: 'Trigger', + type: 'n8n-nodes-base.manualTrigger', + parameters: { + manualTriggerParam: 'empty', + }, + }); + + return { + workflow: { + id: '1', + name: 'Test Workflow', + active: true, + connections: { + [manualTriggerNode.name]: { + main: [[{ node: codeNode.name, type: NodeConnectionType.Main, index: 0 }]], + }, + }, + nodes: [manualTriggerNode, codeNode], + }, + inputData: { + main: [codeNodeInputData], + }, + connectionInputData: codeNodeInputData, + node: codeNode, + runExecutionData: { + startData: {}, + resultData: { + runData: { + [manualTriggerNode.name]: [ + newTaskData({ + source: [], + data: { + main: [codeNodeInputData], + }, + }), + ], + }, + pinData: {}, + lastNodeExecuted: manualTriggerNode.name, + }, + executionData: { + contextData: {}, + nodeExecutionStack: [], + metadata: {}, + waitingExecution: {}, + waitingExecutionSource: {}, + }, + }, + runIndex: 0, + itemIndex: 0, + activeNodeName: codeNode.name, + contextNodeName: codeNode.name, + defaultReturnRunIndex: -1, + siblingParameters: {}, + mode: 'manual', + selfData: {}, + envProviderState: { + env: {}, + isEnvAccessBlocked: true, + isProcessAvailable: true, + }, + additionalData: { + executionId: 'exec-id', + instanceBaseUrl: '', + restartExecutionId: '', + restApiUrl: '', + formWaitingBaseUrl: 'http://formWaitingBaseUrl', + webhookBaseUrl: 'http://webhookBaseUrl', + webhookTestBaseUrl: 'http://webhookTestBaseUrl', + webhookWaitingBaseUrl: 'http://webhookWaitingBaseUrl', + variables: { + var: 'value', + }, + }, + executeData: { + node: codeNode, + data: { + main: [codeNodeInputData], + }, + source: { + main: [{ previousNode: manualTriggerNode.name }], + }, + }, + ...opts, + }; +}; + +/** + * Wraps the given value into an INodeExecutionData object's json property + */ +export const wrapIntoJson = (json: IDataObject): INodeExecutionData => ({ + json, +}); + +/** + * Adds the given index as the pairedItem property to the given INodeExecutionData object + */ +export const withPairedItem = (index: number, data: INodeExecutionData): INodeExecutionData => ({ + ...data, + pairedItem: { + item: index, + }, +}); diff --git a/packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts b/packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts new file mode 100644 index 0000000000..e1fdffc0b6 --- /dev/null +++ b/packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts @@ -0,0 +1,84 @@ +import { ApplicationError } from 'n8n-workflow'; + +export class ExecutionError extends ApplicationError { + description: string | null = null; + + itemIndex: number | undefined = undefined; + + context: { itemIndex: number } | undefined = undefined; + + stack = ''; + + lineNumber: number | undefined = undefined; + + constructor(error: Error & { stack?: string }, itemIndex?: number) { + super(error.message); + this.itemIndex = itemIndex; + + if (this.itemIndex !== undefined) { + this.context = { itemIndex: this.itemIndex }; + } + + this.stack = error.stack ?? ''; + + this.populateFromStack(); + } + + /** + * Populate error `message` and `description` from error `stack`. + */ + private populateFromStack() { + const stackRows = this.stack.split('\n'); + + if (stackRows.length === 0) { + this.message = 'Unknown error'; + } + + const messageRow = stackRows.find((line) => line.includes('Error:')); + const lineNumberRow = stackRows.find((line) => line.includes('Code:')); + const lineNumberDisplay = this.toLineNumberDisplay(lineNumberRow); + + if (!messageRow) { + this.message = `Unknown error ${lineNumberDisplay}`; + return; + } + + const [errorDetails, errorType] = this.toErrorDetailsAndType(messageRow); + + if (errorType) this.description = errorType; + + if (!errorDetails) { + this.message = `Unknown error ${lineNumberDisplay}`; + return; + } + + this.message = `${errorDetails} ${lineNumberDisplay}`; + } + + private toLineNumberDisplay(lineNumberRow?: string) { + const errorLineNumberMatch = lineNumberRow?.match(/Code:(?\d+)/); + + if (!errorLineNumberMatch?.groups?.lineNumber) return null; + + const lineNumber = errorLineNumberMatch.groups.lineNumber; + + this.lineNumber = Number(lineNumber); + + if (!lineNumber) return ''; + + return this.itemIndex === undefined + ? `[line ${lineNumber}]` + : `[line ${lineNumber}, for item ${this.itemIndex}]`; + } + + private toErrorDetailsAndType(messageRow?: string) { + if (!messageRow) return [null, null]; + + const [errorDetails, errorType] = messageRow + .split(':') + .reverse() + .map((i) => i.trim()); + + return [errorDetails, errorType === 'Error' ? null : errorType]; + } +} diff --git a/packages/@n8n/task-runner/src/js-task-runner/errors/validation-error.ts b/packages/@n8n/task-runner/src/js-task-runner/errors/validation-error.ts new file mode 100644 index 0000000000..f2ba712c2c --- /dev/null +++ b/packages/@n8n/task-runner/src/js-task-runner/errors/validation-error.ts @@ -0,0 +1,44 @@ +import { ApplicationError } from 'n8n-workflow'; + +export class ValidationError extends ApplicationError { + description = ''; + + itemIndex: number | undefined = undefined; + + context: { itemIndex: number } | undefined = undefined; + + lineNumber: number | undefined = undefined; + + constructor({ + message, + description, + itemIndex, + lineNumber, + }: { + message: string; + description: string; + itemIndex?: number; + lineNumber?: number; + }) { + super(message); + + this.lineNumber = lineNumber; + this.itemIndex = itemIndex; + + if (this.lineNumber !== undefined && this.itemIndex !== undefined) { + this.message = `${message} [line ${lineNumber}, for item ${itemIndex}]`; + } else if (this.lineNumber !== undefined) { + this.message = `${message} [line ${lineNumber}]`; + } else if (this.itemIndex !== undefined) { + this.message = `${message} [item ${itemIndex}]`; + } else { + this.message = message; + } + + this.description = description; + + if (this.itemIndex !== undefined) { + this.context = { itemIndex: this.itemIndex }; + } + } +} 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 new file mode 100644 index 0000000000..3547e35b6c --- /dev/null +++ b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts @@ -0,0 +1,284 @@ +import { getAdditionalKeys } from 'n8n-core'; +import { + WorkflowDataProxy, + // type IWorkflowDataProxyAdditionalKeys, + Workflow, +} from 'n8n-workflow'; +import type { + CodeExecutionMode, + INode, + INodeType, + ITaskDataConnections, + IWorkflowExecuteAdditionalData, + WorkflowParameters, + IDataObject, + IExecuteData, + INodeExecutionData, + INodeParameters, + IRunExecutionData, + WorkflowExecuteMode, + EnvProviderState, +} from 'n8n-workflow'; +import * as a from 'node:assert'; +import { runInNewContext, type Context } from 'node:vm'; + +import type { TaskResultData } from '@/runner-types'; +import { type Task, TaskRunner } from '@/task-runner'; + +import { validateRunForAllItemsOutput, validateRunForEachItemOutput } from './result-validation'; + +export interface JSExecSettings { + code: string; + nodeMode: CodeExecutionMode; + workflowMode: WorkflowExecuteMode; + continueOnFail: boolean; + + // For workflow data proxy + mode: WorkflowExecuteMode; +} + +export interface PartialAdditionalData { + executionId?: string; + restartExecutionId?: string; + restApiUrl: string; + instanceBaseUrl: string; + formWaitingBaseUrl: string; + webhookBaseUrl: string; + webhookWaitingBaseUrl: string; + webhookTestBaseUrl: string; + currentNodeParameters?: INodeParameters; + executionTimeoutTimestamp?: number; + userId?: string; + variables: IDataObject; +} + +export interface AllCodeTaskData { + workflow: Omit; + inputData: ITaskDataConnections; + node: INode; + + runExecutionData: IRunExecutionData; + runIndex: number; + itemIndex: number; + activeNodeName: string; + connectionInputData: INodeExecutionData[]; + siblingParameters: INodeParameters; + mode: WorkflowExecuteMode; + envProviderState?: EnvProviderState; + executeData?: IExecuteData; + defaultReturnRunIndex: number; + selfData: IDataObject; + contextNodeName: string; + additionalData: PartialAdditionalData; +} + +type CustomConsole = { + log: (...args: unknown[]) => void; +}; + +export class JsTaskRunner extends TaskRunner { + constructor( + taskType: string, + wsUrl: string, + grantToken: string, + maxConcurrency: number, + name?: string, + ) { + super(taskType, wsUrl, grantToken, maxConcurrency, name ?? 'JS Task Runner'); + } + + async executeTask(task: Task): Promise { + const allData = await this.requestData(task.taskId, 'all'); + + const settings = task.settings; + a.ok(settings, 'JS Code not sent to runner'); + + const workflowParams = allData.workflow; + const workflow = new Workflow({ + ...workflowParams, + nodeTypes: { + getByNameAndVersion() { + return undefined as unknown as INodeType; + }, + getByName() { + return undefined as unknown as INodeType; + }, + getKnownTypes() { + return {}; + }, + }, + }); + + const customConsole = { + // Send log output back to the main process. It will take care of forwarding + // it to the UI or printing to console. + log: (...args: unknown[]) => { + const logOutput = args + .map((arg) => (typeof arg === 'object' && arg !== null ? JSON.stringify(arg) : arg)) + .join(' '); + void this.makeRpcCall(task.taskId, 'logNodeOutput', [logOutput]); + }, + }; + + const result = + settings.nodeMode === 'runOnceForAllItems' + ? await this.runForAllItems(task.taskId, settings, allData, workflow, customConsole) + : await this.runForEachItem(task.taskId, settings, allData, workflow, customConsole); + + return { + result, + customData: allData.runExecutionData.resultData.metadata, + }; + } + + /** + * Executes the requested code for all items in a single run + */ + private async runForAllItems( + taskId: string, + settings: JSExecSettings, + allData: AllCodeTaskData, + workflow: Workflow, + customConsole: CustomConsole, + ): Promise { + const dataProxy = this.createDataProxy(allData, workflow, allData.itemIndex); + const inputItems = allData.connectionInputData; + + const context: Context = { + require, + module: {}, + console: customConsole, + + items: inputItems, + ...dataProxy, + ...this.buildRpcCallObject(taskId), + }; + + try { + const result = (await runInNewContext( + `module.exports = async function() {${settings.code}\n}()`, + context, + )) as TaskResultData['result']; + + if (result === null) { + return []; + } + + return validateRunForAllItemsOutput(result); + } catch (error) { + if (settings.continueOnFail) { + return [{ json: { error: this.getErrorMessageFromVmError(error) } }]; + } + + (error as Record).node = allData.node; + throw error; + } + } + + /** + * Executes the requested code for each item in the input data + */ + private async runForEachItem( + taskId: string, + settings: JSExecSettings, + allData: AllCodeTaskData, + workflow: Workflow, + customConsole: CustomConsole, + ): Promise { + const inputItems = allData.connectionInputData; + const returnData: INodeExecutionData[] = []; + + for (let index = 0; index < inputItems.length; index++) { + const item = inputItems[index]; + const dataProxy = this.createDataProxy(allData, workflow, index); + const context: Context = { + require, + module: {}, + console: customConsole, + item, + + ...dataProxy, + ...this.buildRpcCallObject(taskId), + }; + + try { + let result = (await runInNewContext( + `module.exports = async function() {${settings.code}\n}()`, + context, + )) as INodeExecutionData | undefined; + + // Filter out null values + if (result === null) { + continue; + } + + result = validateRunForEachItemOutput(result, index); + if (result) { + returnData.push( + result.binary + ? { + json: result.json, + pairedItem: { item: index }, + binary: result.binary, + } + : { + json: result.json, + pairedItem: { item: index }, + }, + ); + } + } catch (error) { + if (!settings.continueOnFail) { + (error as Record).node = allData.node; + throw error; + } + + returnData.push({ + json: { error: this.getErrorMessageFromVmError(error) }, + pairedItem: { + item: index, + }, + }); + } + } + + return returnData; + } + + private createDataProxy(allData: AllCodeTaskData, workflow: Workflow, itemIndex: number) { + return new WorkflowDataProxy( + workflow, + allData.runExecutionData, + allData.runIndex, + itemIndex, + allData.activeNodeName, + allData.connectionInputData, + allData.siblingParameters, + allData.mode, + getAdditionalKeys( + allData.additionalData as IWorkflowExecuteAdditionalData, + allData.mode, + allData.runExecutionData, + ), + allData.executeData, + allData.defaultReturnRunIndex, + allData.selfData, + allData.contextNodeName, + // Make sure that even if we don't receive the envProviderState for + // whatever reason, we don't expose the task runner's env to the code + allData.envProviderState ?? { + env: {}, + isEnvAccessBlocked: false, + isProcessAvailable: true, + }, + ).getDataProxy(); + } + + private getErrorMessageFromVmError(error: unknown): string { + if (typeof error === 'object' && !!error && 'message' in error) { + return error.message as string; + } + + return JSON.stringify(error); + } +} diff --git a/packages/@n8n/task-runner/src/js-task-runner/obj-utils.ts b/packages/@n8n/task-runner/src/js-task-runner/obj-utils.ts new file mode 100644 index 0000000000..1e49e475d2 --- /dev/null +++ b/packages/@n8n/task-runner/src/js-task-runner/obj-utils.ts @@ -0,0 +1,5 @@ +export function isObject(maybe: unknown): maybe is { [key: string]: unknown } { + return ( + typeof maybe === 'object' && maybe !== null && !Array.isArray(maybe) && !(maybe instanceof Date) + ); +} diff --git a/packages/@n8n/task-runner/src/js-task-runner/result-validation.ts b/packages/@n8n/task-runner/src/js-task-runner/result-validation.ts new file mode 100644 index 0000000000..b7d0ffc5fc --- /dev/null +++ b/packages/@n8n/task-runner/src/js-task-runner/result-validation.ts @@ -0,0 +1,116 @@ +import { normalizeItems } from 'n8n-core'; +import type { INodeExecutionData } from 'n8n-workflow'; + +import { ValidationError } from './errors/validation-error'; +import { isObject } from './obj-utils'; + +export const REQUIRED_N8N_ITEM_KEYS = new Set(['json', 'binary', 'pairedItem', 'error']); + +function validateTopLevelKeys(item: INodeExecutionData, itemIndex: number) { + for (const key in item) { + if (Object.prototype.hasOwnProperty.call(item, key)) { + if (REQUIRED_N8N_ITEM_KEYS.has(key)) return; + + throw new ValidationError({ + message: `Unknown top-level item key: ${key}`, + description: 'Access the properties of an item under `.json`, e.g. `item.json`', + itemIndex, + }); + } + } +} + +function validateItem({ json, binary }: INodeExecutionData, itemIndex: number) { + if (json === undefined || !isObject(json)) { + throw new ValidationError({ + message: "A 'json' property isn't an object", + description: "In the returned data, every key named 'json' must point to an object.", + itemIndex, + }); + } + + if (binary !== undefined && !isObject(binary)) { + throw new ValidationError({ + message: "A 'binary' property isn't an object", + description: "In the returned data, every key named 'binary' must point to an object.", + itemIndex, + }); + } +} + +/** + * Validates the output of a code node in 'Run for All Items' mode. + */ +export function validateRunForAllItemsOutput( + executionResult: INodeExecutionData | INodeExecutionData[] | undefined, +) { + if (typeof executionResult !== 'object') { + throw new ValidationError({ + message: "Code doesn't return items properly", + description: 'Please return an array of objects, one for each item you would like to output.', + }); + } + + if (Array.isArray(executionResult)) { + /** + * If at least one top-level key is an n8n item key (`json`, `binary`, etc.), + * then require all item keys to be an n8n item key. + * + * If no top-level key is an n8n key, then skip this check, allowing non-n8n + * item keys to be wrapped in `json` when normalizing items below. + */ + const mustHaveTopLevelN8nKey = executionResult.some((item) => + Object.keys(item).find((key) => REQUIRED_N8N_ITEM_KEYS.has(key)), + ); + + if (mustHaveTopLevelN8nKey) { + for (let index = 0; index < executionResult.length; index++) { + const item = executionResult[index]; + validateTopLevelKeys(item, index); + } + } + } + + const returnData = normalizeItems(executionResult); + returnData.forEach(validateItem); + return returnData; +} + +/** + * Validates the output of a code node in 'Run for Each Item' mode for single item + */ +export function validateRunForEachItemOutput( + executionResult: INodeExecutionData | undefined, + itemIndex: number, +) { + if (typeof executionResult !== 'object') { + throw new ValidationError({ + message: "Code doesn't return an object", + description: `Please return an object representing the output item. ('${executionResult}' was returned instead.)`, + itemIndex, + }); + } + + if (Array.isArray(executionResult)) { + const firstSentence = + executionResult.length > 0 + ? `An array of ${typeof executionResult[0]}s was returned.` + : 'An empty array was returned.'; + throw new ValidationError({ + message: "Code doesn't return a single object", + description: `${firstSentence} If you need to output multiple items, please use the 'Run Once for All Items' mode instead.`, + itemIndex, + }); + } + + const [returnData] = normalizeItems([executionResult]); + + validateItem(returnData, itemIndex); + + // If at least one top-level key is a supported item key (`json`, `binary`, etc.), + // and another top-level key is unrecognized, then the user mis-added a property + // directly on the item, when they intended to add it on the `json` property + validateTopLevelKeys(returnData, itemIndex); + + return returnData; +} diff --git a/packages/@n8n/task-runner/src/start.ts b/packages/@n8n/task-runner/src/start.ts index 8838ea5b95..7570a11780 100644 --- a/packages/@n8n/task-runner/src/start.ts +++ b/packages/@n8n/task-runner/src/start.ts @@ -2,7 +2,10 @@ import { ApplicationError, ensureError } from 'n8n-workflow'; import * as a from 'node:assert/strict'; import { authenticate } from './authenticator'; -import { JsTaskRunner } from './code'; +import { JsTaskRunner } from './js-task-runner/js-task-runner'; + +let runner: JsTaskRunner | undefined; +let isShuttingDown = false; type Config = { n8nUri: string; @@ -20,12 +23,35 @@ function readAndParseConfig(): Config { } return { - n8nUri: process.env.N8N_RUNNERS_N8N_URI ?? 'localhost:5678', + n8nUri: process.env.N8N_RUNNERS_N8N_URI ?? '127.0.0.1:5679', authToken, grantToken, }; } +function createSignalHandler(signal: string) { + return async function onSignal() { + if (isShuttingDown) { + return; + } + + console.log(`Received ${signal} signal, shutting down...`); + + isShuttingDown = true; + try { + if (runner) { + await runner.stop(); + runner = undefined; + } + } catch (e) { + const error = ensureError(e); + console.error('Error stopping task runner', { error }); + } finally { + process.exit(0); + } + }; +} + void (async function start() { const config = readAndParseConfig(); @@ -40,7 +66,10 @@ void (async function start() { } const wsUrl = `ws://${config.n8nUri}/runners/_ws`; - new JsTaskRunner('javascript', wsUrl, grantToken, 5); + runner = new JsTaskRunner('javascript', wsUrl, grantToken, 5); + + process.on('SIGINT', createSignalHandler('SIGINT')); + process.on('SIGTERM', createSignalHandler('SIGTERM')); })().catch((e) => { const error = ensureError(e); console.error('Task runner failed to start', { error }); diff --git a/packages/@n8n/task-runner/src/task-runner.ts b/packages/@n8n/task-runner/src/task-runner.ts index 6971df6bb4..89402d885c 100644 --- a/packages/@n8n/task-runner/src/task-runner.ts +++ b/packages/@n8n/task-runner/src/task-runner.ts @@ -257,11 +257,8 @@ export abstract class TaskRunner { const data = await this.executeTask(task); this.taskDone(taskId, data); } catch (e) { - if (ensureError(e)) { - this.taskErrored(taskId, (e as Error).message); - } else { - this.taskErrored(taskId, e); - } + const error = ensureError(e); + this.taskErrored(taskId, error); } } @@ -359,4 +356,36 @@ export abstract class TaskRunner { } return rpcObject; } + + /** Close the connection gracefully and wait until has been closed */ + async stop() { + this.stopTaskOffers(); + + await this.waitUntilAllTasksAreDone(); + + await this.closeConnection(); + } + + private async closeConnection() { + // 1000 is the standard close code + // https://www.rfc-editor.org/rfc/rfc6455.html#section-7.1.5 + this.ws.close(1000, 'Shutting down'); + + await new Promise((resolve) => { + this.ws.once('close', resolve); + }); + } + + private async waitUntilAllTasksAreDone(maxWaitTimeInMs = 30_000) { + // TODO: Make maxWaitTimeInMs configurable + const start = Date.now(); + + while (this.runningTasks.size > 0) { + if (Date.now() - start > maxWaitTimeInMs) { + throw new ApplicationError('Timeout while waiting for tasks to finish'); + } + + await new Promise((resolve) => setTimeout(resolve, 100)); + } + } } diff --git a/packages/cli/BREAKING-CHANGES.md b/packages/cli/BREAKING-CHANGES.md index 012892c6ae..bdb1ff5890 100644 --- a/packages/cli/BREAKING-CHANGES.md +++ b/packages/cli/BREAKING-CHANGES.md @@ -6,11 +6,13 @@ This list shows all the versions which include breaking changes and how to upgra ### What changed? -The worker server used to bind to IPv6 by default. It now binds to IPv4 by default. +1. The worker server used to bind to IPv6 by default. It now binds to IPv4 by default. +2. The worker server's `/healthz` used to report healthy status based on database and Redis checks. It now reports healthy status regardless of database and Redis status, and the database and Redis checks are part of `/healthz/readiness`. ### When is action necessary? -If you experience a port conflict error when starting a worker server using its default port, set a different port for the worker server with `QUEUE_HEALTH_CHECK_PORT`. +1. If you experience a port conflict error when starting a worker server using its default port, set a different port for the worker server with `QUEUE_HEALTH_CHECK_PORT`. +2. If you are relying on database and Redis checks for worker health status, switch to checking `/healthz/readiness` instead of `/healthz`. ## 1.57.0 diff --git a/packages/cli/package.json b/packages/cli/package.json index 9c7664d8a8..8cb7d03a72 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "n8n", - "version": "1.62.1", + "version": "1.63.0", "description": "n8n Workflow Automation Tool", "main": "dist/index", "types": "dist/index.d.ts", @@ -51,12 +51,12 @@ "!dist/**/e2e.*" ], "devDependencies": { - "@redocly/cli": "^1.6.0", + "@redocly/cli": "^1.25.5", "@types/aws4": "^1.5.1", "@types/bcryptjs": "^2.4.2", "@types/compression": "1.0.1", "@types/convict": "^6.1.1", - "@types/cookie-parser": "^1.4.2", + "@types/cookie-parser": "^1.4.7", "@types/express": "catalog:", "@types/flat": "^5.0.5", "@types/formidable": "^3.4.5", @@ -76,7 +76,6 @@ "@types/xml2js": "catalog:", "@types/yamljs": "^0.2.31", "@vvo/tzdb": "^6.141.0", - "chokidar": "^3.5.2", "concurrently": "^8.2.0", "ioredis-mock": "^8.8.1", "mjml": "^4.15.3", @@ -94,7 +93,7 @@ "@n8n/permissions": "workspace:*", "@n8n/task-runner": "workspace:*", "@n8n/typeorm": "0.3.20-12", - "@n8n_io/ai-assistant-sdk": "1.9.4", + "@n8n_io/ai-assistant-sdk": "1.10.3", "@n8n_io/license-sdk": "2.13.1", "@oclif/core": "4.0.7", "@rudderstack/rudder-sdk-node": "2.0.9", @@ -111,14 +110,14 @@ "class-validator": "0.14.0", "compression": "1.7.4", "convict": "6.2.4", - "cookie-parser": "1.4.6", + "cookie-parser": "1.4.7", "csrf": "3.1.0", "curlconverter": "3.21.0", "dotenv": "8.6.0", - "express": "4.21.0", + "express": "4.21.1", "express-async-errors": "3.1.1", "express-handlebars": "7.1.2", - "express-openapi-validator": "5.3.3", + "express-openapi-validator": "5.3.7", "express-prom-bundle": "6.6.0", "express-rate-limit": "7.2.0", "fast-glob": "catalog:", @@ -145,7 +144,7 @@ "nodemailer": "6.9.9", "oauth-1.0a": "2.2.6", "open": "7.4.2", - "openapi-types": "10.0.0", + "openapi-types": "12.1.3", "otpauth": "9.1.1", "p-cancelable": "2.1.1", "p-lazy": "3.1.0", @@ -166,7 +165,7 @@ "sqlite3": "5.1.7", "sse-channel": "4.0.0", "sshpk": "1.17.0", - "swagger-ui-express": "5.0.0", + "swagger-ui-express": "5.0.1", "syslog-client": "1.1.1", "typedi": "catalog:", "uuid": "catalog:", diff --git a/packages/cli/src/__tests__/license.test.ts b/packages/cli/src/__tests__/license.test.ts index 35da918abb..67a92b95cd 100644 --- a/packages/cli/src/__tests__/license.test.ts +++ b/packages/cli/src/__tests__/license.test.ts @@ -5,7 +5,7 @@ import type { InstanceSettings } from 'n8n-core'; import config from '@/config'; import { N8N_VERSION } from '@/constants'; import { License } from '@/license'; -import type { Logger } from '@/logging/logger.service'; +import { mockLogger } from '@test/mocking'; jest.mock('@n8n_io/license-sdk'); @@ -25,37 +25,39 @@ describe('License', () => { }); let license: License; - const logger = mock(); const instanceSettings = mock({ instanceId: MOCK_INSTANCE_ID, instanceType: 'main', }); beforeEach(async () => { - license = new License(logger, instanceSettings, mock(), mock(), mock()); + license = new License(mockLogger(), instanceSettings, mock(), mock(), mock()); await license.init(); }); test('initializes license manager', async () => { - expect(LicenseManager).toHaveBeenCalledWith({ - autoRenewEnabled: true, - autoRenewOffset: MOCK_RENEW_OFFSET, - offlineMode: false, - renewOnInit: true, - deviceFingerprint: expect.any(Function), - productIdentifier: `n8n-${N8N_VERSION}`, - logger, - loadCertStr: expect.any(Function), - saveCertStr: expect.any(Function), - onFeatureChange: expect.any(Function), - collectUsageMetrics: expect.any(Function), - collectPassthroughData: expect.any(Function), - server: MOCK_SERVER_URL, - tenantId: 1, - }); + expect(LicenseManager).toHaveBeenCalledWith( + expect.objectContaining({ + autoRenewEnabled: true, + autoRenewOffset: MOCK_RENEW_OFFSET, + offlineMode: false, + renewOnInit: true, + deviceFingerprint: expect.any(Function), + productIdentifier: `n8n-${N8N_VERSION}`, + loadCertStr: expect.any(Function), + saveCertStr: expect.any(Function), + onFeatureChange: expect.any(Function), + collectUsageMetrics: expect.any(Function), + collectPassthroughData: expect.any(Function), + server: MOCK_SERVER_URL, + tenantId: 1, + }), + ); }); test('initializes license manager for worker', async () => { + const logger = mockLogger(); + license = new License( logger, mock({ instanceType: 'worker' }), @@ -64,22 +66,23 @@ describe('License', () => { mock(), ); await license.init(); - expect(LicenseManager).toHaveBeenCalledWith({ - autoRenewEnabled: false, - autoRenewOffset: MOCK_RENEW_OFFSET, - offlineMode: true, - renewOnInit: false, - deviceFingerprint: expect.any(Function), - productIdentifier: `n8n-${N8N_VERSION}`, - logger, - loadCertStr: expect.any(Function), - saveCertStr: expect.any(Function), - onFeatureChange: expect.any(Function), - collectUsageMetrics: expect.any(Function), - collectPassthroughData: expect.any(Function), - server: MOCK_SERVER_URL, - tenantId: 1, - }); + expect(LicenseManager).toHaveBeenCalledWith( + expect.objectContaining({ + autoRenewEnabled: false, + autoRenewOffset: MOCK_RENEW_OFFSET, + offlineMode: true, + renewOnInit: false, + deviceFingerprint: expect.any(Function), + productIdentifier: `n8n-${N8N_VERSION}`, + loadCertStr: expect.any(Function), + saveCertStr: expect.any(Function), + onFeatureChange: expect.any(Function), + collectUsageMetrics: expect.any(Function), + collectPassthroughData: expect.any(Function), + server: MOCK_SERVER_URL, + tenantId: 1, + }), + ); }); test('attempts to activate license with provided key', async () => { @@ -196,7 +199,7 @@ describe('License', () => { it('should enable renewal', async () => { config.set('multiMainSetup.enabled', false); - await new License(mock(), mock(), mock(), mock(), mock()).init(); + await new License(mockLogger(), mock(), mock(), mock(), mock()).init(); expect(LicenseManager).toHaveBeenCalledWith( expect.objectContaining({ autoRenewEnabled: true, renewOnInit: true }), @@ -208,7 +211,7 @@ describe('License', () => { it('should disable renewal', async () => { config.set('license.autoRenewEnabled', false); - await new License(mock(), mock(), mock(), mock(), mock()).init(); + await new License(mockLogger(), mock(), mock(), mock(), mock()).init(); expect(LicenseManager).toHaveBeenCalledWith( expect.objectContaining({ autoRenewEnabled: false, renewOnInit: false }), @@ -226,7 +229,7 @@ describe('License', () => { config.set('multiMainSetup.instanceType', status); config.set('license.autoRenewEnabled', false); - await new License(mock(), mock(), mock(), mock(), mock()).init(); + await new License(mockLogger(), mock(), mock(), mock(), mock()).init(); expect(LicenseManager).toHaveBeenCalledWith( expect.objectContaining({ autoRenewEnabled: false, renewOnInit: false }), @@ -241,7 +244,7 @@ describe('License', () => { config.set('multiMainSetup.instanceType', status); config.set('license.autoRenewEnabled', false); - await new License(mock(), mock(), mock(), mock(), mock()).init(); + await new License(mockLogger(), mock(), mock(), mock(), mock()).init(); expect(LicenseManager).toHaveBeenCalledWith( expect.objectContaining({ autoRenewEnabled: false, renewOnInit: false }), @@ -252,7 +255,7 @@ describe('License', () => { config.set('multiMainSetup.enabled', true); config.set('multiMainSetup.instanceType', 'leader'); - await new License(mock(), mock(), mock(), mock(), mock()).init(); + await new License(mockLogger(), mock(), mock(), mock(), mock()).init(); expect(LicenseManager).toHaveBeenCalledWith( expect.objectContaining({ autoRenewEnabled: true, renewOnInit: true }), @@ -264,7 +267,7 @@ describe('License', () => { describe('reinit', () => { it('should reinitialize license manager', async () => { - const license = new License(mock(), mock(), mock(), mock(), mock()); + const license = new License(mockLogger(), mock(), mock(), mock(), mock()); await license.init(); const initSpy = jest.spyOn(license, 'init'); diff --git a/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts b/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts new file mode 100644 index 0000000000..bcf485445f --- /dev/null +++ b/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts @@ -0,0 +1,37 @@ +import { mock } from 'jest-mock-extended'; +import type { DirectoryLoader } from 'n8n-core'; + +import { LoadNodesAndCredentials } from '../load-nodes-and-credentials'; + +describe('LoadNodesAndCredentials', () => { + describe('resolveIcon', () => { + let instance: LoadNodesAndCredentials; + + beforeEach(() => { + instance = new LoadNodesAndCredentials(mock(), mock(), mock()); + instance.loaders.package1 = mock({ + directory: '/icons/package1', + }); + }); + + it('should return undefined if the loader for the package is not found', () => { + const result = instance.resolveIcon('unknownPackage', '/icons/unknownPackage/icon.png'); + expect(result).toBeUndefined(); + }); + + it('should return undefined if the resolved file path is outside the loader directory', () => { + const result = instance.resolveIcon('package1', '/some/other/path/icon.png'); + expect(result).toBeUndefined(); + }); + + it('should return the file path if the file is within the loader directory', () => { + const result = instance.resolveIcon('package1', '/icons/package1/icon.png'); + expect(result).toBe('/icons/package1/icon.png'); + }); + + it('should return undefined if the URL is outside the package directory', () => { + const result = instance.resolveIcon('package1', '/icons/package1/../../../etc/passwd'); + expect(result).toBeUndefined(); + }); + }); +}); diff --git a/packages/cli/src/__tests__/wait-tracker.test.ts b/packages/cli/src/__tests__/wait-tracker.test.ts index 9ca3a66d33..8f9f31da00 100644 --- a/packages/cli/src/__tests__/wait-tracker.test.ts +++ b/packages/cli/src/__tests__/wait-tracker.test.ts @@ -5,6 +5,7 @@ import type { IExecutionResponse } from '@/interfaces'; import type { MultiMainSetup } from '@/services/orchestration/main/multi-main-setup.ee'; import { OrchestrationService } from '@/services/orchestration.service'; import { WaitTracker } from '@/wait-tracker'; +import { mockLogger } from '@test/mocking'; jest.useFakeTimers(); @@ -21,7 +22,7 @@ describe('WaitTracker', () => { let waitTracker: WaitTracker; beforeEach(() => { waitTracker = new WaitTracker( - mock(), + mockLogger(), executionRepository, mock(), mock(), diff --git a/packages/cli/src/active-workflow-manager.ts b/packages/cli/src/active-workflow-manager.ts index 988a238887..9c5ef15f38 100644 --- a/packages/cli/src/active-workflow-manager.ts +++ b/packages/cli/src/active-workflow-manager.ts @@ -750,7 +750,7 @@ export class ActiveWorkflowManager { const wasRemoved = await this.activeWorkflows.remove(workflowId); if (wasRemoved) { - this.logger.warn(`Removed triggers and pollers for workflow "${workflowId}"`, { + this.logger.debug(`Removed triggers and pollers for workflow "${workflowId}"`, { workflowId, }); } diff --git a/packages/cli/src/commands/base-command.ts b/packages/cli/src/commands/base-command.ts index 7403dd337a..c33fe064c7 100644 --- a/packages/cli/src/commands/base-command.ts +++ b/packages/cli/src/commands/base-command.ts @@ -2,7 +2,12 @@ import 'reflect-metadata'; import { GlobalConfig } from '@n8n/config'; import { Command, Errors } from '@oclif/core'; import { BinaryDataService, InstanceSettings, ObjectStoreService } from 'n8n-core'; -import { ApplicationError, ErrorReporterProxy as ErrorReporter, sleep } from 'n8n-workflow'; +import { + ApplicationError, + ensureError, + ErrorReporterProxy as ErrorReporter, + sleep, +} from 'n8n-workflow'; import { Container } from 'typedi'; import type { AbstractServer } from '@/abstract-server'; @@ -283,8 +288,9 @@ export abstract class BaseCommand extends Command { this.logger.debug('Attempting license activation'); await this.license.activate(activationKey); this.logger.debug('License init complete'); - } catch (e) { - this.logger.error('Could not activate license', e as Error); + } catch (e: unknown) { + const error = ensureError(e); + this.logger.error('Could not activate license', { error }); } } } diff --git a/packages/cli/src/commands/db/__tests__/revert.test.ts b/packages/cli/src/commands/db/__tests__/revert.test.ts index 463c9d617c..ce3911b2b6 100644 --- a/packages/cli/src/commands/db/__tests__/revert.test.ts +++ b/packages/cli/src/commands/db/__tests__/revert.test.ts @@ -170,3 +170,38 @@ test('revert the last migration if it has a down migration', async () => { expect(dataSource.undoLastMigration).toHaveBeenCalled(); expect(dataSource.destroy).toHaveBeenCalled(); }); + +test("don't use transaction if the last migration has transaction = false", async () => { + // + // ARRANGE + // + class TestMigration implements ReversibleMigration { + name = 'ReversibleMigration'; + + transaction = false as const; + + async up() {} + + async down() {} + } + + const migrationsInDb: Migration[] = [ + { id: 1, timestamp: Date.now(), name: 'ReversibleMigration' }, + ]; + const dataSource = mock({ migrations: [new TestMigration()] }); + + const migrationExecutor = mock(); + migrationExecutor.getExecutedMigrations.mockResolvedValue(migrationsInDb); + + // + // ACT + // + await main(logger, dataSource, migrationExecutor); + + // + // ASSERT + // + expect(dataSource.undoLastMigration).toHaveBeenCalledWith({ + transaction: 'none', + }); +}); diff --git a/packages/cli/src/commands/db/revert.ts b/packages/cli/src/commands/db/revert.ts index 7823506ee0..4510044405 100644 --- a/packages/cli/src/commands/db/revert.ts +++ b/packages/cli/src/commands/db/revert.ts @@ -55,7 +55,9 @@ export async function main( return; } - await connection.undoLastMigration(); + await connection.undoLastMigration({ + transaction: lastMigrationInstance.transaction === false ? 'none' : 'each', + }); await connection.destroy(); } diff --git a/packages/cli/src/commands/webhook.ts b/packages/cli/src/commands/webhook.ts index a0f9e10f80..5a5d656c8c 100644 --- a/packages/cli/src/commands/webhook.ts +++ b/packages/cli/src/commands/webhook.ts @@ -112,10 +112,9 @@ export class Webhook extends BaseCommand { async initOrchestration() { await Container.get(OrchestrationWebhookService).init(); + Container.get(PubSubHandler).init(); const subscriber = Container.get(Subscriber); await subscriber.subscribe('n8n.commands'); subscriber.setCommandMessageHandler(); - - Container.get(PubSubHandler).init(); } } diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index 8c1aabf74a..6345db6763 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -8,10 +8,10 @@ import { EventMessageGeneric } from '@/eventbus/event-message-classes/event-mess import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; import { LogStreamingEventRelay } from '@/events/relays/log-streaming.event-relay'; import { JobProcessor } from '@/scaling/job-processor'; -import { Publisher } from '@/scaling/pubsub/publisher.service'; +import { PubSubHandler } from '@/scaling/pubsub/pubsub-handler'; +import { Subscriber } from '@/scaling/pubsub/subscriber.service'; import type { ScalingService } from '@/scaling/scaling.service'; import type { WorkerServerEndpointsConfig } from '@/scaling/worker-server'; -import { OrchestrationHandlerWorkerService } from '@/services/orchestration/worker/orchestration.handler.worker.service'; import { OrchestrationWorkerService } from '@/services/orchestration/worker/orchestration.worker.service'; import { BaseCommand } from './base-command'; @@ -128,12 +128,11 @@ export class Worker extends BaseCommand { */ async initOrchestration() { await Container.get(OrchestrationWorkerService).init(); - await Container.get(OrchestrationHandlerWorkerService).initWithOptions({ - queueModeId: this.queueModeId, - publisher: Container.get(Publisher), - getRunningJobIds: () => this.jobProcessor.getRunningJobIds(), - getRunningJobsSummary: () => this.jobProcessor.getRunningJobsSummary(), - }); + + Container.get(PubSubHandler).init(); + const subscriber = Container.get(Subscriber); + await subscriber.subscribe('n8n.commands'); + subscriber.setCommandMessageHandler(); } async setConcurrency() { diff --git a/packages/cli/src/concurrency/__tests__/concurrency-control.service.test.ts b/packages/cli/src/concurrency/__tests__/concurrency-control.service.test.ts index 6774708099..6511ae4d03 100644 --- a/packages/cli/src/concurrency/__tests__/concurrency-control.service.test.ts +++ b/packages/cli/src/concurrency/__tests__/concurrency-control.service.test.ts @@ -11,13 +11,13 @@ import type { ExecutionRepository } from '@/databases/repositories/execution.rep import { InvalidConcurrencyLimitError } from '@/errors/invalid-concurrency-limit.error'; import type { EventService } from '@/events/event.service'; import type { IExecutingWorkflowData } from '@/interfaces'; -import type { Logger } from '@/logging/logger.service'; import type { Telemetry } from '@/telemetry'; +import { mockLogger } from '@test/mocking'; import { ConcurrencyQueue } from '../concurrency-queue'; describe('ConcurrencyControlService', () => { - const logger = mock(); + const logger = mockLogger(); const executionRepository = mock(); const telemetry = mock(); const eventService = mock(); diff --git a/packages/cli/src/concurrency/concurrency-control.service.ts b/packages/cli/src/concurrency/concurrency-control.service.ts index 1665279352..cf537870f2 100644 --- a/packages/cli/src/concurrency/concurrency-control.service.ts +++ b/packages/cli/src/concurrency/concurrency-control.service.ts @@ -8,7 +8,6 @@ import { UnknownExecutionModeError } from '@/errors/unknown-execution-mode.error import { EventService } from '@/events/event.service'; import type { IExecutingWorkflowData } from '@/interfaces'; import { Logger } from '@/logging/logger.service'; -import type { LogMetadata } from '@/logging/types'; import { Telemetry } from '@/telemetry'; import { ConcurrencyQueue } from './concurrency-queue'; @@ -34,6 +33,8 @@ export class ConcurrencyControlService { private readonly telemetry: Telemetry, private readonly eventService: EventService, ) { + this.logger = this.logger.withScope('executions'); + this.productionLimit = config.getEnv('executions.concurrency.productionLimit'); if (this.productionLimit === 0) { @@ -46,7 +47,6 @@ export class ConcurrencyControlService { if (this.productionLimit === -1 || config.getEnv('executions.mode') === 'queue') { this.isEnabled = false; - this.log('Service disabled'); return; } @@ -65,12 +65,12 @@ export class ConcurrencyControlService { }); this.productionQueue.on('execution-throttled', ({ executionId }) => { - this.log('Execution throttled', { executionId }); + this.logger.debug('Execution throttled', { executionId }); this.eventService.emit('execution-throttled', { executionId }); }); this.productionQueue.on('execution-released', async (executionId) => { - this.log('Execution released', { executionId }); + this.logger.debug('Execution released', { executionId }); }); } @@ -144,9 +144,9 @@ export class ConcurrencyControlService { // ---------------------------------- private logInit() { - this.log('Enabled'); + this.logger.debug('Enabled'); - this.log( + this.logger.debug( [ 'Production execution concurrency is', this.productionLimit === -1 ? 'unlimited' : 'limited to ' + this.productionLimit.toString(), @@ -171,10 +171,6 @@ export class ConcurrencyControlService { throw new UnknownExecutionModeError(mode); } - private log(message: string, metadata?: LogMetadata) { - this.logger.debug(['[Concurrency Control]', message].join(' '), metadata); - } - private shouldReport(capacity: number) { return config.getEnv('deployment.type') === 'cloud' && this.limitsToReport.includes(capacity); } diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 5d458ca376..04512e8be9 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -91,6 +91,7 @@ export const LICENSE_FEATURES = { PROJECT_ROLE_EDITOR: 'feat:projectRole:editor', PROJECT_ROLE_VIEWER: 'feat:projectRole:viewer', AI_ASSISTANT: 'feat:aiAssistant', + ASK_AI: 'feat:askAi', COMMUNITY_NODES_CUSTOM_REGISTRY: 'feat:communityNodes:customRegistry', } as const; diff --git a/packages/cli/src/controllers/ai-assistant.controller.ts b/packages/cli/src/controllers/ai.controller.ts similarity index 65% rename from packages/cli/src/controllers/ai-assistant.controller.ts rename to packages/cli/src/controllers/ai.controller.ts index c910be0a24..1957db2971 100644 --- a/packages/cli/src/controllers/ai-assistant.controller.ts +++ b/packages/cli/src/controllers/ai.controller.ts @@ -7,18 +7,18 @@ import { WritableStream } from 'node:stream/web'; import { Post, RestController } from '@/decorators'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; import { AiAssistantRequest } from '@/requests'; -import { AiAssistantService } from '@/services/ai-assistant.service'; +import { AiService } from '@/services/ai.service'; type FlushableResponse = Response & { flush: () => void }; -@RestController('/ai-assistant') -export class AiAssistantController { - constructor(private readonly aiAssistantService: AiAssistantService) {} +@RestController('/ai') +export class AiController { + constructor(private readonly aiService: AiService) {} @Post('/chat', { rateLimit: { limit: 100 } }) async chat(req: AiAssistantRequest.Chat, res: FlushableResponse) { try { - const aiResponse = await this.aiAssistantService.chat(req.body, req.user); + const aiResponse = await this.aiService.chat(req.body, req.user); if (aiResponse.body) { res.header('Content-type', 'application/json-lines').flush(); await aiResponse.body.pipeTo( @@ -40,10 +40,21 @@ export class AiAssistantController { @Post('/chat/apply-suggestion') async applySuggestion( - req: AiAssistantRequest.ApplySuggestion, + req: AiAssistantRequest.ApplySuggestionPayload, ): Promise { try { - return await this.aiAssistantService.applySuggestion(req.body, req.user); + return await this.aiService.applySuggestion(req.body, req.user); + } catch (e) { + assert(e instanceof Error); + ErrorReporterProxy.error(e); + throw new InternalServerError(`Something went wrong: ${e.message}`); + } + } + + @Post('/ask-ai') + async askAi(req: AiAssistantRequest.AskAiPayload): Promise { + try { + return await this.aiService.askAi(req.body, req.user); } catch (e) { assert(e instanceof Error); ErrorReporterProxy.error(e); diff --git a/packages/cli/src/controllers/e2e.controller.ts b/packages/cli/src/controllers/e2e.controller.ts index 06c4f68c7e..9c5a1ff36d 100644 --- a/packages/cli/src/controllers/e2e.controller.ts +++ b/packages/cli/src/controllers/e2e.controller.ts @@ -92,6 +92,7 @@ export class E2EController { [LICENSE_FEATURES.PROJECT_ROLE_VIEWER]: false, [LICENSE_FEATURES.AI_ASSISTANT]: false, [LICENSE_FEATURES.COMMUNITY_NODES_CUSTOM_REGISTRY]: false, + [LICENSE_FEATURES.ASK_AI]: false, }; private numericFeatures: Record = { diff --git a/packages/cli/src/controllers/orchestration.controller.ts b/packages/cli/src/controllers/orchestration.controller.ts index a5235d1169..db1d690a3e 100644 --- a/packages/cli/src/controllers/orchestration.controller.ts +++ b/packages/cli/src/controllers/orchestration.controller.ts @@ -28,11 +28,4 @@ export class OrchestrationController { if (!this.licenseService.isWorkerViewLicensed()) return; return await this.orchestrationService.getWorkerStatus(); } - - @GlobalScope('orchestration:list') - @Post('/worker/ids') - async getWorkerIdsAll() { - if (!this.licenseService.isWorkerViewLicensed()) return; - return await this.orchestrationService.getWorkerIds(); - } } diff --git a/packages/cli/src/databases/migrations/common/1724951148974-AddApiKeysTable.ts b/packages/cli/src/databases/migrations/common/1724951148974-AddApiKeysTable.ts index 547be1a8fb..a9ea5626bb 100644 --- a/packages/cli/src/databases/migrations/common/1724951148974-AddApiKeysTable.ts +++ b/packages/cli/src/databases/migrations/common/1724951148974-AddApiKeysTable.ts @@ -1,8 +1,8 @@ import type { ApiKey } from '@/databases/entities/api-key'; -import type { MigrationContext } from '@/databases/types'; +import type { MigrationContext, ReversibleMigration } from '@/databases/types'; import { generateNanoId } from '@/databases/utils/generators'; -export class AddApiKeysTable1724951148974 { +export class AddApiKeysTable1724951148974 implements ReversibleMigration { async up({ queryRunner, escape, @@ -55,4 +55,55 @@ export class AddApiKeysTable1724951148974 { // Drop apiKey column on user's table await queryRunner.query(`ALTER TABLE ${userTable} DROP COLUMN ${apiKeyColumn};`); } + + async down({ + queryRunner, + runQuery, + schemaBuilder: { dropTable, addColumns, createIndex, column }, + escape, + isMysql, + }: MigrationContext) { + const userTable = escape.tableName('user'); + const userApiKeysTable = escape.tableName('user_api_keys'); + const apiKeyColumn = escape.columnName('apiKey'); + const userIdColumn = escape.columnName('userId'); + const idColumn = escape.columnName('id'); + const createdAtColumn = escape.columnName('createdAt'); + + await addColumns('user', [column('apiKey').varchar()]); + + await createIndex('user', ['apiKey'], true); + + const queryToGetUsersApiKeys = isMysql + ? ` + SELECT ${userIdColumn}, + ${apiKeyColumn}, + ${createdAtColumn} + FROM ${userApiKeysTable} u + WHERE ${createdAtColumn} = (SELECT Min(${createdAtColumn}) + FROM ${userApiKeysTable} + WHERE ${userIdColumn} = u.${userIdColumn});` + : ` + SELECT DISTINCT ON + (${userIdColumn}) ${userIdColumn}, + ${apiKeyColumn}, ${createdAtColumn} + FROM ${userApiKeysTable} + ORDER BY ${userIdColumn}, ${createdAtColumn} ASC;`; + + const oldestApiKeysPerUser = (await queryRunner.query(queryToGetUsersApiKeys)) as Array< + Partial + >; + + await Promise.all( + oldestApiKeysPerUser.map( + async (user: { userId: string; apiKey: string }) => + await runQuery( + `UPDATE ${userTable} SET ${apiKeyColumn} = :apiKey WHERE ${idColumn} = :userId`, + user, + ), + ), + ); + + await dropTable('user_api_keys'); + } } diff --git a/packages/cli/src/databases/migrations/sqlite/1724951148974-AddApiKeysTable.ts b/packages/cli/src/databases/migrations/sqlite/1724951148974-AddApiKeysTable.ts index 19d27aeb85..71dcecdfb3 100644 --- a/packages/cli/src/databases/migrations/sqlite/1724951148974-AddApiKeysTable.ts +++ b/packages/cli/src/databases/migrations/sqlite/1724951148974-AddApiKeysTable.ts @@ -1,8 +1,10 @@ import type { ApiKey } from '@/databases/entities/api-key'; -import type { MigrationContext } from '@/databases/types'; +import type { MigrationContext, ReversibleMigration } from '@/databases/types'; import { generateNanoId } from '@/databases/utils/generators'; -export class AddApiKeysTable1724951148974 { +export class AddApiKeysTable1724951148974 implements ReversibleMigration { + transaction = false as const; + async up({ queryRunner, tablePrefix, runQuery }: MigrationContext) { const tableName = `${tablePrefix}user_api_keys`; @@ -74,4 +76,52 @@ export class AddApiKeysTable1724951148974 { // Rename the temporary table to users await queryRunner.query('ALTER TABLE users_new RENAME TO user;'); } + + async down({ + queryRunner, + runQuery, + tablePrefix, + schemaBuilder: { dropTable, createIndex }, + escape, + }: MigrationContext) { + const userApiKeysTable = escape.tableName('user_api_keys'); + const apiKeyColumn = escape.columnName('apiKey'); + const userIdColumn = escape.columnName('userId'); + const idColumn = escape.columnName('id'); + const createdAtColumn = escape.columnName('createdAt'); + + const queryToGetUsersApiKeys = ` + SELECT + ${userIdColumn}, + ${apiKeyColumn}, + ${createdAtColumn} + FROM + ${userApiKeysTable} + WHERE + ${createdAtColumn} IN( + SELECT + MIN(${createdAtColumn}) + FROM ${userApiKeysTable} + GROUP BY ${userIdColumn});`; + + const oldestApiKeysPerUser = (await queryRunner.query(queryToGetUsersApiKeys)) as Array< + Partial + >; + + await queryRunner.query(`ALTER TABLE ${tablePrefix}user ADD COLUMN "apiKey" varchar;`); + + await createIndex('user', ['apiKey'], true); + + await Promise.all( + oldestApiKeysPerUser.map( + async (user: { userId: string; apiKey: string }) => + await runQuery( + `UPDATE ${tablePrefix}user SET ${apiKeyColumn} = :apiKey WHERE ${idColumn} = :userId`, + user, + ), + ), + ); + + await dropTable('user_api_keys'); + } } diff --git a/packages/cli/src/databases/repositories/__tests__/execution.repository.test.ts b/packages/cli/src/databases/repositories/__tests__/execution.repository.test.ts index 48f3119780..10d1371f37 100644 --- a/packages/cli/src/databases/repositories/__tests__/execution.repository.test.ts +++ b/packages/cli/src/databases/repositories/__tests__/execution.repository.test.ts @@ -12,7 +12,9 @@ import { mockInstance, mockEntityManager } from '@test/mocking'; describe('ExecutionRepository', () => { const entityManager = mockEntityManager(ExecutionEntity); - const globalConfig = mockInstance(GlobalConfig, { logging: { outputs: ['console'] } }); + const globalConfig = mockInstance(GlobalConfig, { + logging: { outputs: ['console'], scopes: [] }, + }); const binaryDataService = mockInstance(BinaryDataService); const executionRepository = Container.get(ExecutionRepository); const mockDate = new Date('2023-12-28 12:34:56.789Z'); diff --git a/packages/cli/src/environments/source-control/__tests__/source-control.service.test.ts b/packages/cli/src/environments/source-control/__tests__/source-control.service.test.ts new file mode 100644 index 0000000000..06dab4f97c --- /dev/null +++ b/packages/cli/src/environments/source-control/__tests__/source-control.service.test.ts @@ -0,0 +1,46 @@ +import { mock } from 'jest-mock-extended'; +import { InstanceSettings } from 'n8n-core'; + +import { SourceControlPreferencesService } from '@/environments/source-control/source-control-preferences.service.ee'; +import { SourceControlService } from '@/environments/source-control/source-control.service.ee'; + +describe('SourceControlService', () => { + const preferencesService = new SourceControlPreferencesService( + new InstanceSettings(), + mock(), + mock(), + ); + const sourceControlService = new SourceControlService( + mock(), + mock(), + preferencesService, + mock(), + mock(), + mock(), + mock(), + ); + + describe('pushWorkfolder', () => { + it('should throw an error if a file is given that is not in the workfolder', async () => { + jest.spyOn(sourceControlService, 'sanityCheck').mockResolvedValue(undefined); + + await expect( + sourceControlService.pushWorkfolder({ + fileNames: [ + { + file: '/etc/passwd', + id: 'test', + name: 'secret-file', + type: 'file', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: new Date().toISOString(), + pushed: false, + }, + ], + }), + ).rejects.toThrow('File path /etc/passwd is invalid'); + }); + }); +}); diff --git a/packages/cli/src/environments/source-control/source-control-helper.ee.ts b/packages/cli/src/environments/source-control/source-control-helper.ee.ts index 35298d560a..00a9875741 100644 --- a/packages/cli/src/environments/source-control/source-control-helper.ee.ts +++ b/packages/cli/src/environments/source-control/source-control-helper.ee.ts @@ -1,10 +1,13 @@ import { generateKeyPairSync } from 'crypto'; import { constants as fsConstants, mkdirSync, accessSync } from 'fs'; +import { ApplicationError } from 'n8n-workflow'; +import { ok } from 'node:assert/strict'; import path from 'path'; import { Container } from 'typedi'; import { License } from '@/license'; import { Logger } from '@/logging/logger.service'; +import { isContainedWithin } from '@/utils/path-util'; import { SOURCE_CONTROL_GIT_KEY_COMMENT, @@ -163,3 +166,24 @@ export function getTrackingInformationFromPostPushResult(result: SourceControlle uniques.filter((file) => file.pushed && file.file.startsWith('variable_stubs')).length ?? 0, }; } + +/** + * Normalizes and validates the given source controlled file path. Ensures + * the path is absolute and contained within the git folder. + * + * @throws {ApplicationError} If the path is not within the git folder + */ +export function normalizeAndValidateSourceControlledFilePath( + gitFolderPath: string, + filePath: string, +) { + ok(path.isAbsolute(gitFolderPath), 'gitFolder must be an absolute path'); + + const normalizedPath = path.isAbsolute(filePath) ? filePath : path.join(gitFolderPath, filePath); + + if (!isContainedWithin(gitFolderPath, filePath)) { + throw new ApplicationError(`File path ${filePath} is invalid`); + } + + return normalizedPath; +} diff --git a/packages/cli/src/environments/source-control/source-control-import.service.ee.ts b/packages/cli/src/environments/source-control/source-control-import.service.ee.ts index b08ae27dd8..b5012d2762 100644 --- a/packages/cli/src/environments/source-control/source-control-import.service.ee.ts +++ b/packages/cli/src/environments/source-control/source-control-import.service.ee.ts @@ -2,7 +2,12 @@ import { In } from '@n8n/typeorm'; import glob from 'fast-glob'; import { Credentials, InstanceSettings } from 'n8n-core'; -import { ApplicationError, jsonParse, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; +import { + ApplicationError, + jsonParse, + ErrorReporterProxy as ErrorReporter, + ensureError, +} from 'n8n-workflow'; import { readFile as fsReadFile } from 'node:fs/promises'; import path from 'path'; import { Container, Service } from 'typedi'; @@ -274,8 +279,9 @@ export class SourceControlImportService { this.logger.debug(`Reactivating workflow id ${existingWorkflow.id}`); await workflowManager.add(existingWorkflow.id, 'activate'); // update the versionId of the workflow to match the imported workflow - } catch (error) { - this.logger.error(`Failed to activate workflow ${existingWorkflow.id}`, error as Error); + } catch (e) { + const error = ensureError(e); + this.logger.error(`Failed to activate workflow ${existingWorkflow.id}`, { error }); } finally { await Container.get(WorkflowRepository).update( { id: existingWorkflow.id }, @@ -377,8 +383,9 @@ export class SourceControlImportService { await fsReadFile(candidate.file, { encoding: 'utf8' }), { fallbackValue: { tags: [], mappings: [] } }, ); - } catch (error) { - this.logger.error(`Failed to import tags from file ${candidate.file}`, error as Error); + } catch (e) { + const error = ensureError(e); + this.logger.error(`Failed to import tags from file ${candidate.file}`, { error }); return; } @@ -444,8 +451,8 @@ export class SourceControlImportService { await fsReadFile(candidate.file, { encoding: 'utf8' }), { fallbackValue: [] }, ); - } catch (error) { - this.logger.error(`Failed to import tags from file ${candidate.file}`, error as Error); + } catch (e) { + this.logger.error(`Failed to import tags from file ${candidate.file}`, { error: e }); return; } const overriddenKeys = Object.keys(valueOverrides ?? {}); diff --git a/packages/cli/src/environments/source-control/source-control.controller.ee.ts b/packages/cli/src/environments/source-control/source-control.controller.ee.ts index 7e41fd7cae..7b8b2a7266 100644 --- a/packages/cli/src/environments/source-control/source-control.controller.ee.ts +++ b/packages/cli/src/environments/source-control/source-control.controller.ee.ts @@ -170,6 +170,7 @@ export class SourceControlController { if (this.sourceControlPreferencesService.isBranchReadOnly()) { throw new BadRequestError('Cannot push onto read-only branch.'); } + try { await this.sourceControlService.setGitUserDetails( `${req.user.firstName} ${req.user.lastName}`, diff --git a/packages/cli/src/environments/source-control/source-control.service.ee.ts b/packages/cli/src/environments/source-control/source-control.service.ee.ts index 32f0b39ef7..58c213f03c 100644 --- a/packages/cli/src/environments/source-control/source-control.service.ee.ts +++ b/packages/cli/src/environments/source-control/source-control.service.ee.ts @@ -25,6 +25,7 @@ import { getTrackingInformationFromPrePushResult, getTrackingInformationFromPullResult, getVariablesPath, + normalizeAndValidateSourceControlledFilePath, sourceControlFoldersExistCheck, } from './source-control-helper.ee'; import { SourceControlImportService } from './source-control-import.service.ee'; @@ -80,7 +81,7 @@ export class SourceControlService { }); } - private async sanityCheck(): Promise { + public async sanityCheck(): Promise { try { const foldersExisted = sourceControlFoldersExistCheck( [this.gitFolder, this.sshFolder], @@ -217,8 +218,20 @@ export class SourceControlService { throw new BadRequestError('Cannot push onto read-only branch.'); } + const filesToPush = options.fileNames.map((file) => { + const normalizedPath = normalizeAndValidateSourceControlledFilePath( + this.gitFolder, + file.file, + ); + + return { + ...file, + file: normalizedPath, + }; + }); + // only determine file status if not provided by the frontend - let statusResult: SourceControlledFile[] = options.fileNames; + let statusResult: SourceControlledFile[] = filesToPush; if (statusResult.length === 0) { statusResult = (await this.getStatus({ direction: 'push', @@ -240,7 +253,7 @@ export class SourceControlService { const filesToBePushed = new Set(); const filesToBeDeleted = new Set(); - options.fileNames.forEach((e) => { + filesToPush.forEach((e) => { if (e.status !== 'deleted') { filesToBePushed.add(e.file); } else { @@ -250,12 +263,12 @@ export class SourceControlService { this.sourceControlExportService.rmFilesFromExportFolder(filesToBeDeleted); - const workflowsToBeExported = options.fileNames.filter( + const workflowsToBeExported = filesToPush.filter( (e) => e.type === 'workflow' && e.status !== 'deleted', ); await this.sourceControlExportService.exportWorkflowsToWorkFolder(workflowsToBeExported); - const credentialsToBeExported = options.fileNames.filter( + const credentialsToBeExported = filesToPush.filter( (e) => e.type === 'credential' && e.status !== 'deleted', ); const credentialExportResult = @@ -269,11 +282,11 @@ export class SourceControlService { }); } - if (options.fileNames.find((e) => e.type === 'tags')) { + if (filesToPush.find((e) => e.type === 'tags')) { await this.sourceControlExportService.exportTagsToWorkFolder(); } - if (options.fileNames.find((e) => e.type === 'variables')) { + if (filesToPush.find((e) => e.type === 'variables')) { await this.sourceControlExportService.exportVariablesToWorkFolder(); } @@ -281,7 +294,7 @@ export class SourceControlService { for (let i = 0; i < statusResult.length; i++) { // eslint-disable-next-line @typescript-eslint/no-loop-func - if (options.fileNames.find((file) => file.file === statusResult[i].file)) { + if (filesToPush.find((file) => file.file === statusResult[i].file)) { statusResult[i].pushed = true; } } diff --git a/packages/cli/src/eventbus/message-event-bus-writer/message-event-bus-log-writer.ts b/packages/cli/src/eventbus/message-event-bus-writer/message-event-bus-log-writer.ts index b9177faa07..6c6a928a67 100644 --- a/packages/cli/src/eventbus/message-event-bus-writer/message-event-bus-log-writer.ts +++ b/packages/cli/src/eventbus/message-event-bus-writer/message-event-bus-log-writer.ts @@ -149,7 +149,7 @@ export class MessageEventBusLogWriter { this._worker = new Worker(workerFileName); if (this.worker) { this.worker.on('messageerror', async (error) => { - this.logger.error('Event Bus Log Writer thread error, attempting to restart...', error); + this.logger.error('Event Bus Log Writer thread error, attempting to restart...', { error }); await MessageEventBusLogWriter.instance.startThread(); }); return true; 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 6eecb8e812..df65a70ecb 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -1057,4 +1057,20 @@ describe('TelemetryEventRelay', () => { ); }); }); + + describe('Community+ registered', () => { + it('should track `license-community-plus-registered` event', () => { + const event: RelayEventMap['license-community-plus-registered'] = { + email: 'user@example.com', + licenseKey: 'license123', + }; + + eventService.emit('license-community-plus-registered', event); + + expect(telemetry.track).toHaveBeenCalledWith('User registered for license community plus', { + email: 'user@example.com', + licenseKey: 'license123', + }); + }); + }); }); diff --git a/packages/cli/src/events/maps/pub-sub.event-map.ts b/packages/cli/src/events/maps/pub-sub.event-map.ts index 9237e79d13..a1134470bb 100644 --- a/packages/cli/src/events/maps/pub-sub.event-map.ts +++ b/packages/cli/src/events/maps/pub-sub.event-map.ts @@ -80,25 +80,5 @@ export type PubSubCommandMap = { }; export type PubSubWorkerResponseMap = { - // #region Lifecycle - - 'restart-event-bus': { - result: 'success' | 'error'; - error?: string; - }; - - 'reload-external-secrets-providers': { - result: 'success' | 'error'; - error?: string; - }; - - // #endregion - - // #region Worker view - - 'get-worker-id': never; - 'get-worker-status': WorkerStatus; - - // #endregion }; diff --git a/packages/cli/src/events/maps/relay.event-map.ts b/packages/cli/src/events/maps/relay.event-map.ts index a495820283..21b673a2b5 100644 --- a/packages/cli/src/events/maps/relay.event-map.ts +++ b/packages/cli/src/events/maps/relay.event-map.ts @@ -420,6 +420,11 @@ export type RelayEventMap = { success: boolean; }; + 'license-community-plus-registered': { + email: string; + licenseKey: string; + }; + // #endregion // #region Variable diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index c813926bf1..11d84751d0 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -54,6 +54,7 @@ export class TelemetryEventRelay extends EventRelay { 'source-control-user-finished-push-ui': (event) => this.sourceControlUserFinishedPushUi(event), 'license-renewal-attempted': (event) => this.licenseRenewalAttempted(event), + 'license-community-plus-registered': (event) => this.licenseCommunityPlusRegistered(event), 'variable-created': () => this.variableCreated(), 'external-secrets-provider-settings-saved': (event) => this.externalSecretsProviderSettingsSaved(event), @@ -234,6 +235,16 @@ export class TelemetryEventRelay extends EventRelay { }); } + private licenseCommunityPlusRegistered({ + email, + licenseKey, + }: RelayEventMap['license-community-plus-registered']) { + this.telemetry.track('User registered for license community plus', { + email, + licenseKey, + }); + } + // #endregion // #region Variable diff --git a/packages/cli/src/execution-lifecycle-hooks/shared/shared-hook-functions.ts b/packages/cli/src/execution-lifecycle-hooks/shared/shared-hook-functions.ts index 9596dd35df..68fd528f14 100644 --- a/packages/cli/src/execution-lifecycle-hooks/shared/shared-hook-functions.ts +++ b/packages/cli/src/execution-lifecycle-hooks/shared/shared-hook-functions.ts @@ -1,5 +1,5 @@ import pick from 'lodash/pick'; -import type { ExecutionStatus, IRun, IWorkflowBase } from 'n8n-workflow'; +import { ensureError, type ExecutionStatus, type IRun, type IWorkflowBase } from 'n8n-workflow'; import { Container } from 'typedi'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; @@ -95,7 +95,8 @@ export async function updateExistingExecution(parameters: { ); } } catch (e) { - logger.error(`Failed to save metadata for execution ID ${executionId}`, e as Error); + const error = ensureError(e); + logger.error(`Failed to save metadata for execution ID ${executionId}`, { error }); } if (executionData.finished === true && executionData.retryOf !== undefined) { diff --git a/packages/cli/src/license.ts b/packages/cli/src/license.ts index 0cde2bd922..c6e8dfa19f 100644 --- a/packages/cli/src/license.ts +++ b/packages/cli/src/license.ts @@ -37,7 +37,9 @@ export class License { private readonly orchestrationService: OrchestrationService, private readonly settingsRepository: SettingsRepository, private readonly licenseMetricsService: LicenseMetricsService, - ) {} + ) { + this.logger = this.logger.withScope('license'); + } /** * Whether this instance should renew the license - on init and periodically. @@ -109,9 +111,9 @@ export class License { await this.manager.initialize(); this.logger.debug('License initialized'); - } catch (e: unknown) { - if (e instanceof Error) { - this.logger.error('Could not initialize license manager sdk', e); + } catch (error: unknown) { + if (error instanceof Error) { + this.logger.error('Could not initialize license manager sdk', { error }); } } } @@ -253,6 +255,10 @@ export class License { return this.isFeatureEnabled(LICENSE_FEATURES.AI_ASSISTANT); } + isAskAiEnabled() { + return this.isFeatureEnabled(LICENSE_FEATURES.ASK_AI); + } + isAdvancedExecutionFiltersEnabled() { return this.isFeatureEnabled(LICENSE_FEATURES.ADVANCED_EXECUTION_FILTERS); } diff --git a/packages/cli/src/license/__tests__/license.service.test.ts b/packages/cli/src/license/__tests__/license.service.test.ts index 5fe4d6c692..77afe04a2c 100644 --- a/packages/cli/src/license/__tests__/license.service.test.ts +++ b/packages/cli/src/license/__tests__/license.service.test.ts @@ -1,4 +1,5 @@ import type { TEntitlement } from '@n8n_io/license-sdk'; +import axios, { AxiosError } from 'axios'; import { mock } from 'jest-mock-extended'; import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; @@ -7,6 +8,8 @@ import type { EventService } from '@/events/event.service'; import type { License } from '@/license'; import { LicenseErrors, LicenseService } from '@/license/license.service'; +jest.mock('axios'); + describe('LicenseService', () => { const license = mock(); const workflowRepository = mock(); @@ -84,4 +87,37 @@ describe('LicenseService', () => { }); }); }); + + describe('registerCommunityEdition', () => { + test('on success', async () => { + jest + .spyOn(axios, 'post') + .mockResolvedValueOnce({ data: { title: 'Title', text: 'Text', licenseKey: 'abc-123' } }); + const data = await licenseService.registerCommunityEdition({ + email: 'test@ema.il', + instanceId: '123', + instanceUrl: 'http://localhost', + licenseType: 'community-registered', + }); + + expect(data).toEqual({ title: 'Title', text: 'Text' }); + expect(eventService.emit).toHaveBeenCalledWith('license-community-plus-registered', { + email: 'test@ema.il', + licenseKey: 'abc-123', + }); + }); + + test('on failure', async () => { + jest.spyOn(axios, 'post').mockRejectedValueOnce(new AxiosError('Failed')); + await expect( + licenseService.registerCommunityEdition({ + email: 'test@ema.il', + instanceId: '123', + instanceUrl: 'http://localhost', + licenseType: 'community-registered', + }), + ).rejects.toThrowError('Failed'); + expect(eventService.emit).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/cli/src/license/license.controller.ts b/packages/cli/src/license/license.controller.ts index e1644046bb..db895ef4a0 100644 --- a/packages/cli/src/license/license.controller.ts +++ b/packages/cli/src/license/license.controller.ts @@ -1,14 +1,21 @@ +import { CommunityRegisteredRequestDto } from '@n8n/api-types'; import type { AxiosError } from 'axios'; +import { InstanceSettings } from 'n8n-core'; -import { Get, Post, RestController, GlobalScope } from '@/decorators'; +import { Get, Post, RestController, GlobalScope, Body } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { AuthenticatedRequest, LicenseRequest } from '@/requests'; +import { AuthenticatedRequest, AuthlessRequest, LicenseRequest } from '@/requests'; +import { UrlService } from '@/services/url.service'; import { LicenseService } from './license.service'; @RestController('/license') export class LicenseController { - constructor(private readonly licenseService: LicenseService) {} + constructor( + private readonly licenseService: LicenseService, + private readonly instanceSettings: InstanceSettings, + private readonly urlService: UrlService, + ) {} @Get('/') async getLicenseData() { @@ -32,6 +39,20 @@ export class LicenseController { } } + @Post('/enterprise/community-registered') + async registerCommunityEdition( + _req: AuthlessRequest, + _res: Response, + @Body payload: CommunityRegisteredRequestDto, + ) { + return await this.licenseService.registerCommunityEdition({ + email: payload.email, + instanceId: this.instanceSettings.instanceId, + instanceUrl: this.urlService.getInstanceBaseUrl(), + licenseType: 'community-registered', + }); + } + @Post('/activate') @GlobalScope('license:manage') async activateLicense(req: LicenseRequest.Activate) { diff --git a/packages/cli/src/license/license.service.ts b/packages/cli/src/license/license.service.ts index ee0e27bccb..43f9961334 100644 --- a/packages/cli/src/license/license.service.ts +++ b/packages/cli/src/license/license.service.ts @@ -1,4 +1,5 @@ -import axios from 'axios'; +import axios, { AxiosError } from 'axios'; +import { ensureError } from 'n8n-workflow'; import { Service } from 'typedi'; import type { User } from '@/databases/entities/user'; @@ -13,8 +14,7 @@ type LicenseError = Error & { errorId?: keyof typeof LicenseErrors }; export const LicenseErrors = { SCHEMA_VALIDATION: 'Activation key is in the wrong format', - RESERVATION_EXHAUSTED: - 'Activation key has been used too many times. Please contact sales@n8n.io if you would like to extend it', + RESERVATION_EXHAUSTED: 'Activation key has been used too many times', RESERVATION_EXPIRED: 'Activation key has expired', NOT_FOUND: 'Activation key not found', RESERVATION_CONFLICT: 'Activation key not found', @@ -60,6 +60,43 @@ export class LicenseService { }); } + async registerCommunityEdition({ + email, + instanceId, + instanceUrl, + licenseType, + }: { + email: string; + instanceId: string; + instanceUrl: string; + licenseType: string; + }): Promise<{ title: string; text: string }> { + try { + const { + data: { licenseKey, ...rest }, + } = await axios.post<{ title: string; text: string; licenseKey: string }>( + 'https://enterprise.n8n.io/community-registered', + { + email, + instanceId, + instanceUrl, + licenseType, + }, + ); + this.eventService.emit('license-community-plus-registered', { email, licenseKey }); + return rest; + } catch (e: unknown) { + if (e instanceof AxiosError) { + const error = e as AxiosError<{ message: string }>; + const errorMsg = error.response?.data?.message ?? e.message; + throw new BadRequestError('Failed to register community edition: ' + errorMsg); + } else { + this.logger.error('Failed to register community edition', { error: ensureError(e) }); + throw new BadRequestError('Failed to register community edition'); + } + } + } + getManagementJwt(): string { return this.license.getManagementJwt(); } diff --git a/packages/cli/src/load-nodes-and-credentials.ts b/packages/cli/src/load-nodes-and-credentials.ts index 662558e3d6..a5cff3764f 100644 --- a/packages/cli/src/load-nodes-and-credentials.ts +++ b/packages/cli/src/load-nodes-and-credentials.ts @@ -29,6 +29,7 @@ import { inE2ETests, } from '@/constants'; import { Logger } from '@/logging/logger.service'; +import { isContainedWithin } from '@/utils/path-util'; interface LoadedNodesAndCredentials { nodes: INodeTypeData; @@ -155,14 +156,13 @@ export class LoadNodesAndCredentials { resolveIcon(packageName: string, url: string): string | undefined { const loader = this.loaders[packageName]; - if (loader) { - const pathPrefix = `/icons/${packageName}/`; - const filePath = path.resolve(loader.directory, url.substring(pathPrefix.length)); - if (!path.relative(loader.directory, filePath).includes('..')) { - return filePath; - } + if (!loader) { + return undefined; } - return undefined; + const pathPrefix = `/icons/${packageName}/`; + const filePath = path.resolve(loader.directory, url.substring(pathPrefix.length)); + + return isContainedWithin(loader.directory, filePath) ? filePath : undefined; } getCustomDirectories(): string[] { @@ -260,7 +260,7 @@ export class LoadNodesAndCredentials { dir: string, ) { const loader = new constructor(dir, this.excludeNodes, this.includeNodes); - if (loader.packageName in this.loaders) { + if (loader instanceof PackageDirectoryLoader && loader.packageName in this.loaders) { throw new ApplicationError( picocolors.red( `nodes package ${loader.packageName} is already loaded.\n Please delete this second copy at path ${dir}`, diff --git a/packages/cli/src/logging/__tests__/logger.service.test.ts b/packages/cli/src/logging/__tests__/logger.service.test.ts index f699443909..d01a709639 100644 --- a/packages/cli/src/logging/__tests__/logger.service.test.ts +++ b/packages/cli/src/logging/__tests__/logger.service.test.ts @@ -11,6 +11,7 @@ describe('Logger', () => { logging: { level: 'info', outputs: ['console'], + scopes: [], }, }); @@ -30,6 +31,7 @@ describe('Logger', () => { logging: { level: 'info', outputs: ['file'], + scopes: [], file: { fileSizeMax: 100, fileCountMax: 16, @@ -56,6 +58,7 @@ describe('Logger', () => { logging: { level: 'error', outputs: ['console'], + scopes: [], }, }); @@ -74,6 +77,7 @@ describe('Logger', () => { logging: { level: 'warn', outputs: ['console'], + scopes: [], }, }); @@ -92,6 +96,7 @@ describe('Logger', () => { logging: { level: 'info', outputs: ['console'], + scopes: [], }, }); @@ -110,6 +115,7 @@ describe('Logger', () => { logging: { level: 'debug', outputs: ['console'], + scopes: [], }, }); @@ -128,6 +134,7 @@ describe('Logger', () => { logging: { level: 'silent', outputs: ['console'], + scopes: [], }, }); diff --git a/packages/cli/src/logging/logger.service.ts b/packages/cli/src/logging/logger.service.ts index 9a1b804859..8bdb9177de 100644 --- a/packages/cli/src/logging/logger.service.ts +++ b/packages/cli/src/logging/logger.service.ts @@ -1,11 +1,15 @@ +import type { LogScope } from '@n8n/config'; import { GlobalConfig } from '@n8n/config'; import callsites from 'callsites'; +import type { TransformableInfo } from 'logform'; import { InstanceSettings } from 'n8n-core'; import { LoggerProxy, LOG_LEVELS } from 'n8n-workflow'; import path, { basename } from 'node:path'; +import pc from 'picocolors'; import { Service } from 'typedi'; import winston from 'winston'; +import { inDevelopment, inProduction } from '@/constants'; import { isObjectLiteral } from '@/utils'; import { noOp } from './constants'; @@ -13,10 +17,16 @@ import type { LogLocationMetadata, LogLevel, LogMetadata } from './types'; @Service() export class Logger { - private readonly internalLogger: winston.Logger; + private internalLogger: winston.Logger; private readonly level: LogLevel; + private readonly scopes: Set; + + private get isScopingEnabled() { + return this.scopes.size > 0; + } + constructor( private readonly globalConfig: GlobalConfig, private readonly instanceSettings: InstanceSettings, @@ -33,15 +43,30 @@ export class Logger { if (!isSilent) { this.setLevel(); - const { outputs } = this.globalConfig.logging; + const { outputs, scopes } = this.globalConfig.logging; if (outputs.includes('console')) this.setConsoleTransport(); if (outputs.includes('file')) this.setFileTransport(); + + this.scopes = new Set(scopes); } LoggerProxy.init(this); } + private setInternalLogger(internalLogger: winston.Logger) { + this.internalLogger = internalLogger; + } + + withScope(scope: LogScope) { + const scopedLogger = new Logger(this.globalConfig, this.instanceSettings); + const childLogger = this.internalLogger.child({ scope }); + + scopedLogger.setInternalLogger(childLogger); + + return scopedLogger; + } + private log(level: LogLevel, message: string, metadata: LogMetadata) { const location: LogLocationMetadata = {}; @@ -61,8 +86,7 @@ export class Logger { for (const logLevel of LOG_LEVELS) { if (levels[logLevel] > levels[this.level]) { - // winston defines `{ error: 0, warn: 1, info: 2, debug: 5 }` - // so numerically higher (less severe) log levels become no-op + // numerically higher (less severe) log levels become no-op // to prevent overhead from `callsites` calls Object.defineProperty(this, logLevel, { value: noOp }); } @@ -71,24 +95,72 @@ export class Logger { private setConsoleTransport() { const format = - this.level === 'debug' - ? winston.format.combine( - winston.format.metadata(), - winston.format.timestamp(), - winston.format.colorize({ all: true }), - winston.format.printf(({ level, message, timestamp, metadata }) => { - const _metadata = this.toPrintable(metadata); - return `${timestamp} | ${level.padEnd(18)} | ${message}${_metadata}`; - }), - ) - : winston.format.printf(({ message }: { message: string }) => message); + this.level === 'debug' && inDevelopment + ? this.debugDevConsoleFormat() + : this.level === 'debug' && inProduction + ? this.debugProdConsoleFormat() + : winston.format.printf(({ message }: { message: string }) => message); this.internalLogger.add(new winston.transports.Console({ format })); } + private scopeFilter() { + return winston.format((info: TransformableInfo & { metadata: LogMetadata }) => { + const shouldIncludeScope = info.metadata.scope && this.scopes.has(info.metadata.scope); + + if (this.isScopingEnabled && !shouldIncludeScope) return false; + + return info; + })(); + } + + private debugDevConsoleFormat() { + return winston.format.combine( + winston.format.metadata(), + winston.format.timestamp({ format: () => this.devTsFormat() }), + winston.format.colorize({ all: true }), + this.scopeFilter(), + winston.format.printf(({ level: _level, message, timestamp, metadata: _metadata }) => { + const SEPARATOR = ' '.repeat(3); + const LOG_LEVEL_COLUMN_WIDTH = 15; // 5 columns + ANSI color codes + const level = _level.toLowerCase().padEnd(LOG_LEVEL_COLUMN_WIDTH, ' '); + const metadata = this.toPrintable(_metadata); + return [timestamp, level, message + ' ' + pc.dim(metadata)].join(SEPARATOR); + }), + ); + } + + private debugProdConsoleFormat() { + return winston.format.combine( + winston.format.metadata(), + winston.format.timestamp(), + this.scopeFilter(), + winston.format.printf(({ level, message, timestamp, metadata }) => { + const _metadata = this.toPrintable(metadata); + return `${timestamp} | ${level.padEnd(5)} | ${message}${_metadata ? ' ' + _metadata : ''}`; + }), + ); + } + + private devTsFormat() { + const now = new Date(); + const pad = (num: number, digits: number = 2) => num.toString().padStart(digits, '0'); + const hours = pad(now.getHours()); + const minutes = pad(now.getMinutes()); + const seconds = pad(now.getSeconds()); + const milliseconds = pad(now.getMilliseconds(), 3); + return `${hours}:${minutes}:${seconds}.${milliseconds}`; + } + private toPrintable(metadata: unknown) { if (isObjectLiteral(metadata) && Object.keys(metadata).length > 0) { - return ' ' + JSON.stringify(metadata); + return inProduction + ? JSON.stringify(metadata) + : JSON.stringify(metadata) + .replace(/{"/g, '{ "') + .replace(/,"/g, ', "') + .replace(/:/g, ': ') + .replace(/}/g, ' }'); // spacing for readability } return ''; diff --git a/packages/cli/src/logging/types.ts b/packages/cli/src/logging/types.ts index 94b02d8ad7..b6022c0bf6 100644 --- a/packages/cli/src/logging/types.ts +++ b/packages/cli/src/logging/types.ts @@ -1,7 +1,14 @@ +import type { LogScope } from '@n8n/config'; + import type { LOG_LEVELS } from './constants'; export type LogLevel = (typeof LOG_LEVELS)[number]; -export type LogLocationMetadata = Partial<{ file: string; function: string }>; +export type LogMetadata = { + [key: string]: unknown; + scope?: LogScope; + file?: string; + function?: string; +}; -export type LogMetadata = Record | Error; +export type LogLocationMetadata = Pick; diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index b8fa4b99ca..e25a244f5f 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -586,5 +586,6 @@ export declare namespace AiAssistantRequest { type Chat = AuthenticatedRequest<{}, {}, AiAssistantSDK.ChatRequestPayload>; type SuggestionPayload = { sessionId: string; suggestionId: string }; - type ApplySuggestion = AuthenticatedRequest<{}, {}, SuggestionPayload>; + type ApplySuggestionPayload = AuthenticatedRequest<{}, {}, SuggestionPayload>; + type AskAiPayload = AuthenticatedRequest<{}, {}, AiAssistantSDK.AskAiRequestPayload>; } diff --git a/packages/cli/src/runners/task-managers/task-manager.ts b/packages/cli/src/runners/task-managers/task-manager.ts index 9f7e492fbe..b2ca87bc9c 100644 --- a/packages/cli/src/runners/task-managers/task-manager.ts +++ b/packages/cli/src/runners/task-managers/task-manager.ts @@ -1,16 +1,17 @@ -import { - type IExecuteFunctions, - type Workflow, - type IRunExecutionData, - type INodeExecutionData, - type ITaskDataConnections, - type INode, - type WorkflowParameters, - type INodeParameters, - type WorkflowExecuteMode, - type IExecuteData, - type IDataObject, - type IWorkflowExecuteAdditionalData, +import type { + EnvProviderState, + IExecuteFunctions, + Workflow, + IRunExecutionData, + INodeExecutionData, + ITaskDataConnections, + INode, + WorkflowParameters, + INodeParameters, + WorkflowExecuteMode, + IExecuteData, + IDataObject, + IWorkflowExecuteAdditionalData, } from 'n8n-workflow'; import { nanoid } from 'nanoid'; @@ -42,6 +43,7 @@ export interface TaskData { connectionInputData: INodeExecutionData[]; siblingParameters: INodeParameters; mode: WorkflowExecuteMode; + envProviderState: EnvProviderState; executeData?: IExecuteData; defaultReturnRunIndex: number; selfData: IDataObject; @@ -76,6 +78,7 @@ export interface AllCodeTaskData { connectionInputData: INodeExecutionData[]; siblingParameters: INodeParameters; mode: WorkflowExecuteMode; + envProviderState: EnvProviderState; executeData?: IExecuteData; defaultReturnRunIndex: number; selfData: IDataObject; @@ -137,6 +140,7 @@ export class TaskManager { connectionInputData: INodeExecutionData[], siblingParameters: INodeParameters, mode: WorkflowExecuteMode, + envProviderState: EnvProviderState, executeData?: IExecuteData, defaultReturnRunIndex = -1, selfData: IDataObject = {}, @@ -153,6 +157,7 @@ export class TaskManager { itemIndex, siblingParameters, mode, + envProviderState, executeData, defaultReturnRunIndex, selfData, @@ -311,6 +316,7 @@ export class TaskManager { contextNodeName: jd.contextNodeName, defaultReturnRunIndex: jd.defaultReturnRunIndex, mode: jd.mode, + envProviderState: jd.envProviderState, node: jd.node, runExecutionData: jd.runExecutionData, runIndex: jd.runIndex, diff --git a/packages/cli/src/scaling/__tests__/publisher.service.test.ts b/packages/cli/src/scaling/__tests__/publisher.service.test.ts index f26c93cc42..3386a5c648 100644 --- a/packages/cli/src/scaling/__tests__/publisher.service.test.ts +++ b/packages/cli/src/scaling/__tests__/publisher.service.test.ts @@ -52,7 +52,7 @@ describe('Publisher', () => { expect(client.publish).toHaveBeenCalledWith( 'n8n.commands', - JSON.stringify({ ...msg, senderId: queueModeId }), + JSON.stringify({ ...msg, senderId: queueModeId, selfSend: false, debounce: true }), ); }); }); @@ -61,7 +61,7 @@ describe('Publisher', () => { it('should publish worker response into `n8n.worker-response` pubsub channel', async () => { const publisher = new Publisher(mock(), redisClientService); const msg = mock({ - command: 'reload-external-secrets-providers', + command: 'get-worker-status', }); await publisher.publishWorkerResponse(msg); diff --git a/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts b/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts index c637b2faf9..fdb7c8dd23 100644 --- a/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts +++ b/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts @@ -7,7 +7,9 @@ import type { ExternalSecretsManager } from '@/external-secrets/external-secrets import type { License } from '@/license'; import type { CommunityPackagesService } from '@/services/community-packages.service'; +import type { Publisher } from '../pubsub/publisher.service'; import { PubSubHandler } from '../pubsub/pubsub-handler'; +import type { WorkerStatus } from '../worker-status'; describe('PubSubHandler', () => { const eventService = new EventService(); @@ -15,13 +17,19 @@ describe('PubSubHandler', () => { const eventbus = mock(); const externalSecretsManager = mock(); const communityPackagesService = mock(); + const publisher = mock(); + const workerStatus = mock(); + + afterEach(() => { + eventService.removeAllListeners(); + }); describe('in webhook process', () => { const instanceSettings = mock({ instanceType: 'webhook' }); it('should set up handlers in webhook process', () => { // @ts-expect-error Spying on private method - const setupWebhookHandlersSpy = jest.spyOn(PubSubHandler.prototype, 'setupWebhookHandlers'); + const setupHandlersSpy = jest.spyOn(PubSubHandler.prototype, 'setupHandlers'); new PubSubHandler( eventService, @@ -30,9 +38,18 @@ describe('PubSubHandler', () => { eventbus, externalSecretsManager, communityPackagesService, + publisher, + workerStatus, ).init(); - expect(setupWebhookHandlersSpy).toHaveBeenCalled(); + expect(setupHandlersSpy).toHaveBeenCalledWith({ + 'reload-license': expect.any(Function), + 'restart-event-bus': expect.any(Function), + 'reload-external-secrets-providers': expect.any(Function), + 'community-package-install': expect.any(Function), + 'community-package-update': expect.any(Function), + 'community-package-uninstall': expect.any(Function), + }); }); it('should reload license on `reload-license` event', () => { @@ -43,6 +60,8 @@ describe('PubSubHandler', () => { eventbus, externalSecretsManager, communityPackagesService, + publisher, + workerStatus, ).init(); eventService.emit('reload-license'); @@ -58,6 +77,8 @@ describe('PubSubHandler', () => { eventbus, externalSecretsManager, communityPackagesService, + publisher, + workerStatus, ).init(); eventService.emit('restart-event-bus'); @@ -73,6 +94,8 @@ describe('PubSubHandler', () => { eventbus, externalSecretsManager, communityPackagesService, + publisher, + workerStatus, ).init(); eventService.emit('reload-external-secrets-providers'); @@ -88,6 +111,8 @@ describe('PubSubHandler', () => { eventbus, externalSecretsManager, communityPackagesService, + publisher, + workerStatus, ).init(); eventService.emit('community-package-install', { @@ -109,6 +134,8 @@ describe('PubSubHandler', () => { eventbus, externalSecretsManager, communityPackagesService, + publisher, + workerStatus, ).init(); eventService.emit('community-package-update', { @@ -130,6 +157,8 @@ describe('PubSubHandler', () => { eventbus, externalSecretsManager, communityPackagesService, + publisher, + workerStatus, ).init(); eventService.emit('community-package-uninstall', { @@ -139,4 +168,102 @@ describe('PubSubHandler', () => { expect(communityPackagesService.removeNpmPackage).toHaveBeenCalledWith('test-package'); }); }); + + describe('in worker process', () => { + const instanceSettings = mock({ instanceType: 'worker' }); + + it('should set up handlers in worker process', () => { + // @ts-expect-error Spying on private method + const setupHandlersSpy = jest.spyOn(PubSubHandler.prototype, 'setupHandlers'); + + new PubSubHandler( + eventService, + instanceSettings, + license, + eventbus, + externalSecretsManager, + communityPackagesService, + publisher, + workerStatus, + ).init(); + + expect(setupHandlersSpy).toHaveBeenCalledWith({ + 'reload-license': expect.any(Function), + 'restart-event-bus': expect.any(Function), + 'reload-external-secrets-providers': expect.any(Function), + 'community-package-install': expect.any(Function), + 'community-package-update': expect.any(Function), + 'community-package-uninstall': expect.any(Function), + 'get-worker-status': expect.any(Function), + }); + }); + + it('should reload license on `reload-license` event', () => { + new PubSubHandler( + eventService, + instanceSettings, + license, + eventbus, + externalSecretsManager, + communityPackagesService, + publisher, + workerStatus, + ).init(); + + eventService.emit('reload-license'); + + expect(license.reload).toHaveBeenCalled(); + }); + + it('should restart event bus on `restart-event-bus` event', () => { + new PubSubHandler( + eventService, + instanceSettings, + license, + eventbus, + externalSecretsManager, + communityPackagesService, + publisher, + workerStatus, + ).init(); + + eventService.emit('restart-event-bus'); + + expect(eventbus.restart).toHaveBeenCalled(); + }); + + it('should reload providers on `reload-external-secrets-providers` event', () => { + new PubSubHandler( + eventService, + instanceSettings, + license, + eventbus, + externalSecretsManager, + communityPackagesService, + publisher, + workerStatus, + ).init(); + + eventService.emit('reload-external-secrets-providers'); + + expect(externalSecretsManager.reloadAllProviders).toHaveBeenCalled(); + }); + + it('should generate status on `get-worker-status` event', () => { + new PubSubHandler( + eventService, + instanceSettings, + license, + eventbus, + externalSecretsManager, + communityPackagesService, + publisher, + workerStatus, + ).init(); + + eventService.emit('get-worker-status'); + + expect(workerStatus.generateStatus).toHaveBeenCalled(); + }); + }); }); diff --git a/packages/cli/src/scaling/__tests__/scaling.service.test.ts b/packages/cli/src/scaling/__tests__/scaling.service.test.ts index f1ae78f838..a6c14ab964 100644 --- a/packages/cli/src/scaling/__tests__/scaling.service.test.ts +++ b/packages/cli/src/scaling/__tests__/scaling.service.test.ts @@ -6,7 +6,7 @@ import { ApplicationError } from 'n8n-workflow'; import Container from 'typedi'; import type { OrchestrationService } from '@/services/orchestration.service'; -import { mockInstance } from '@test/mocking'; +import { mockInstance, mockLogger } from '@test/mocking'; import { JOB_TYPE_NAME, QUEUE_NAME } from '../constants'; import type { JobProcessor } from '../job-processor'; @@ -74,7 +74,7 @@ describe('ScalingService', () => { instanceSettings.markAsLeader(); scalingService = new ScalingService( - mock(), + mockLogger(), mock(), jobProcessor, globalConfig, diff --git a/packages/cli/src/scaling/__tests__/worker-server.test.ts b/packages/cli/src/scaling/__tests__/worker-server.test.ts index 7f3a21a778..778d403bf2 100644 --- a/packages/cli/src/scaling/__tests__/worker-server.test.ts +++ b/packages/cli/src/scaling/__tests__/worker-server.test.ts @@ -50,10 +50,10 @@ describe('WorkerServer', () => { globalConfig, mock(), mock(), - mock(), externalHooks, mock({ instanceType: 'webhook' }), prometheusMetricsService, + mock(), ), ).toThrowError(AssertionError); }); @@ -75,10 +75,10 @@ describe('WorkerServer', () => { globalConfig, mock(), mock(), - mock(), externalHooks, instanceSettings, prometheusMetricsService, + mock(), ); expect(procesExitSpy).toHaveBeenCalledWith(1); @@ -102,10 +102,10 @@ describe('WorkerServer', () => { globalConfig, mock(), mock(), - mock(), externalHooks, instanceSettings, prometheusMetricsService, + mock(), ); const CREDENTIALS_OVERWRITE_ENDPOINT = 'credentials/overwrites'; @@ -137,10 +137,10 @@ describe('WorkerServer', () => { globalConfig, mock(), mock(), - mock(), externalHooks, instanceSettings, prometheusMetricsService, + mock(), ); await workerServer.init({ health: true, overwrites: false, metrics: true }); @@ -158,10 +158,10 @@ describe('WorkerServer', () => { globalConfig, mock(), mock(), - mock(), externalHooks, instanceSettings, prometheusMetricsService, + mock(), ); await expect( workerServer.init({ health: false, overwrites: false, metrics: false }), @@ -176,10 +176,10 @@ describe('WorkerServer', () => { globalConfig, mock(), mock(), - mock(), externalHooks, instanceSettings, prometheusMetricsService, + mock(), ); server.listen.mockImplementation((...args: unknown[]) => { diff --git a/packages/cli/src/scaling/constants.ts b/packages/cli/src/scaling/constants.ts index f1e55d7ab1..348f156896 100644 --- a/packages/cli/src/scaling/constants.ts +++ b/packages/cli/src/scaling/constants.ts @@ -7,3 +7,17 @@ export const COMMAND_PUBSUB_CHANNEL = 'n8n.commands'; /** Pubsub channel for messages sent by workers in response to commands from main processes. */ export const WORKER_RESPONSE_PUBSUB_CHANNEL = 'n8n.worker-response'; + +/** + * Commands that should be sent to the sender as well, e.g. during workflow activation and + * deactivation in multi-main setup. */ +export const SELF_SEND_COMMANDS = new Set([ + 'add-webhooks-triggers-and-pollers', + 'remove-triggers-and-pollers', +]); + +/** + * Commands that should not be debounced when received, e.g. during webhook handling in + * multi-main setup. + */ +export const IMMEDIATE_COMMANDS = new Set(['relay-execution-lifecycle-event']); diff --git a/packages/cli/src/scaling/pubsub/publisher.service.ts b/packages/cli/src/scaling/pubsub/publisher.service.ts index 7a35b94c3e..bfcede6542 100644 --- a/packages/cli/src/scaling/pubsub/publisher.service.ts +++ b/packages/cli/src/scaling/pubsub/publisher.service.ts @@ -6,6 +6,7 @@ import { Logger } from '@/logging/logger.service'; import { RedisClientService } from '@/services/redis-client.service'; import type { PubSub } from './pubsub.types'; +import { IMMEDIATE_COMMANDS, SELF_SEND_COMMANDS } from '../constants'; /** * Responsible for publishing messages into the pubsub channels used by scaling mode. @@ -43,7 +44,12 @@ export class Publisher { async publishCommand(msg: Omit) { await this.client.publish( 'n8n.commands', - JSON.stringify({ ...msg, senderId: config.getEnv('redis.queueModeId') }), + JSON.stringify({ + ...msg, + senderId: config.getEnv('redis.queueModeId'), + selfSend: SELF_SEND_COMMANDS.has(msg.command), + debounce: !IMMEDIATE_COMMANDS.has(msg.command), + }), ); this.logger.debug(`Published ${msg.command} to command channel`); diff --git a/packages/cli/src/scaling/pubsub/pubsub-handler.ts b/packages/cli/src/scaling/pubsub/pubsub-handler.ts index 8b7a91e4dd..215549e1b9 100644 --- a/packages/cli/src/scaling/pubsub/pubsub-handler.ts +++ b/packages/cli/src/scaling/pubsub/pubsub-handler.ts @@ -1,12 +1,17 @@ import { InstanceSettings } from 'n8n-core'; import { Service } from 'typedi'; +import config from '@/config'; import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; import { EventService } from '@/events/event.service'; import type { PubSubEventMap } from '@/events/maps/pub-sub.event-map'; import { ExternalSecretsManager } from '@/external-secrets/external-secrets-manager.ee'; import { License } from '@/license'; +import { Publisher } from '@/scaling/pubsub/publisher.service'; import { CommunityPackagesService } from '@/services/community-packages.service'; +import { assertNever } from '@/utils'; + +import { WorkerStatus } from '../worker-status'; /** * Responsible for handling events emitted from messages received via a pubsub channel. @@ -20,10 +25,32 @@ export class PubSubHandler { private readonly eventbus: MessageEventBus, private readonly externalSecretsManager: ExternalSecretsManager, private readonly communityPackagesService: CommunityPackagesService, + private readonly publisher: Publisher, + private readonly workerStatus: WorkerStatus, ) {} init() { - if (this.instanceSettings.instanceType === 'webhook') this.setupWebhookHandlers(); + switch (this.instanceSettings.instanceType) { + case 'webhook': + this.setupHandlers(this.commonHandlers); + break; + case 'worker': + this.setupHandlers({ + ...this.commonHandlers, + 'get-worker-status': async () => + await this.publisher.publishWorkerResponse({ + workerId: config.getEnv('redis.queueModeId'), + command: 'get-worker-status', + payload: this.workerStatus.generateStatus(), + }), + }); + break; + case 'main': + // TODO + break; + default: + assertNever(this.instanceSettings.instanceType); + } } private setupHandlers( @@ -40,22 +67,27 @@ export class PubSubHandler { } } - // #region Webhook process - - private setupWebhookHandlers() { - this.setupHandlers({ - 'reload-license': async () => await this.license.reload(), - 'restart-event-bus': async () => await this.eventbus.restart(), - 'reload-external-secrets-providers': async () => - await this.externalSecretsManager.reloadAllProviders(), - 'community-package-install': async ({ packageName, packageVersion }) => - await this.communityPackagesService.installOrUpdateNpmPackage(packageName, packageVersion), - 'community-package-update': async ({ packageName, packageVersion }) => - await this.communityPackagesService.installOrUpdateNpmPackage(packageName, packageVersion), - 'community-package-uninstall': async ({ packageName }) => - await this.communityPackagesService.removeNpmPackage(packageName), - }); - } - - // #endregion + /** Handlers shared by webhook and worker processes. */ + private commonHandlers: { + [K in keyof Pick< + PubSubEventMap, + | 'reload-license' + | 'restart-event-bus' + | 'reload-external-secrets-providers' + | 'community-package-install' + | 'community-package-update' + | 'community-package-uninstall' + >]: (event: PubSubEventMap[K]) => Promise; + } = { + 'reload-license': async () => await this.license.reload(), + 'restart-event-bus': async () => await this.eventbus.restart(), + 'reload-external-secrets-providers': async () => + await this.externalSecretsManager.reloadAllProviders(), + 'community-package-install': async ({ packageName, packageVersion }) => + await this.communityPackagesService.installOrUpdateNpmPackage(packageName, packageVersion), + 'community-package-update': async ({ packageName, packageVersion }) => + await this.communityPackagesService.installOrUpdateNpmPackage(packageName, packageVersion), + 'community-package-uninstall': async ({ packageName }) => + await this.communityPackagesService.removeNpmPackage(packageName), + }; } diff --git a/packages/cli/src/scaling/pubsub/pubsub.types.ts b/packages/cli/src/scaling/pubsub/pubsub.types.ts index ac83659212..a356503348 100644 --- a/packages/cli/src/scaling/pubsub/pubsub.types.ts +++ b/packages/cli/src/scaling/pubsub/pubsub.types.ts @@ -22,6 +22,12 @@ export namespace PubSub { senderId: string; targets?: string[]; command: CommandKey; + + /** Whether the command should be sent to the sender as well. */ + selfSend?: boolean; + + /** Whether the command should be debounced when received. */ + debounce?: boolean; } & (PubSubCommandMap[CommandKey] extends never ? { payload?: never } // some commands carry no payload : { payload: PubSubCommandMap[CommandKey] }); @@ -80,18 +86,6 @@ export namespace PubSub { _ToWorkerResponse >; - namespace WorkerResponses { - export type RestartEventBus = ToWorkerResponse<'restart-event-bus'>; - export type ReloadExternalSecretsProviders = - ToWorkerResponse<'reload-external-secrets-providers'>; - export type GetWorkerId = ToWorkerResponse<'get-worker-id'>; - export type GetWorkerStatus = ToWorkerResponse<'get-worker-status'>; - } - /** Response sent via the `n8n.worker-response` pubsub channel. */ - export type WorkerResponse = - | WorkerResponses.RestartEventBus - | WorkerResponses.ReloadExternalSecretsProviders - | WorkerResponses.GetWorkerId - | WorkerResponses.GetWorkerStatus; + export type WorkerResponse = ToWorkerResponse<'get-worker-status'>; } diff --git a/packages/cli/src/scaling/pubsub/subscriber.service.ts b/packages/cli/src/scaling/pubsub/subscriber.service.ts index f9a2567f8d..7586b52ebc 100644 --- a/packages/cli/src/scaling/pubsub/subscriber.service.ts +++ b/packages/cli/src/scaling/pubsub/subscriber.service.ts @@ -70,12 +70,15 @@ export class Subscriber { // #region Commands setCommandMessageHandler() { - const handlerFn = debounce((str: string) => { - const msg = this.parseCommandMessage(str); - if (msg) this.eventService.emit(msg.command, msg.payload); - }, 300); + const handlerFn = (msg: PubSub.Command) => this.eventService.emit(msg.command, msg.payload); + const debouncedHandlerFn = debounce(handlerFn, 300); - this.setMessageHandler('n8n.commands', handlerFn); + this.setMessageHandler('n8n.commands', (str: string) => { + const msg = this.parseCommandMessage(str); + if (!msg) return; + if (msg.debounce) debouncedHandlerFn(msg); + else handlerFn(msg); + }); } private parseCommandMessage(str: string) { @@ -91,7 +94,10 @@ export class Subscriber { const queueModeId = config.getEnv('redis.queueModeId'); - if (msg.senderId === queueModeId || (msg.targets && !msg.targets.includes(queueModeId))) { + if ( + !msg.selfSend && + (msg.senderId === queueModeId || (msg.targets && !msg.targets.includes(queueModeId))) + ) { this.logger.debug('Disregarding message - not for this instance', msg); return null; diff --git a/packages/cli/src/scaling/scaling.service.ts b/packages/cli/src/scaling/scaling.service.ts index f9d805140d..f35b4348a6 100644 --- a/packages/cli/src/scaling/scaling.service.ts +++ b/packages/cli/src/scaling/scaling.service.ts @@ -47,7 +47,9 @@ export class ScalingService { private readonly instanceSettings: InstanceSettings, private readonly orchestrationService: OrchestrationService, private readonly eventService: EventService, - ) {} + ) { + this.logger = this.logger.withScope('scaling'); + } // #region Lifecycle @@ -77,7 +79,7 @@ export class ScalingService { this.scheduleQueueMetrics(); - this.logger.debug('[ScalingService] Queue setup completed'); + this.logger.debug('Queue setup completed'); } setupWorker(concurrency: number) { @@ -91,7 +93,7 @@ export class ScalingService { // Errors thrown here will be sent to the main instance by bull. Logging // them out and rethrowing them allows to find out which worker had the // issue. - this.logger.error('[ScalingService] Executing a job errored', { + this.logger.error('Executing a job errored', { jobId: job.id, executionId: job.data.executionId, error, @@ -101,19 +103,19 @@ export class ScalingService { } }); - this.logger.debug('[ScalingService] Worker setup completed'); + this.logger.debug('Worker setup completed'); } @OnShutdown(HIGHEST_SHUTDOWN_PRIORITY) async stop() { await this.queue.pause(true, true); - this.logger.debug('[ScalingService] Queue paused'); + this.logger.debug('Queue paused'); this.stopQueueRecovery(); this.stopQueueMetrics(); - this.logger.debug('[ScalingService] Queue recovery and metrics stopped'); + this.logger.debug('Queue recovery and metrics stopped'); let count = 0; @@ -159,7 +161,7 @@ export class ScalingService { const job = await this.queue.add(JOB_TYPE_NAME, jobData, jobOptions); - this.logger.info(`[ScalingService] Added job ${job.id} (execution ${jobData.executionId})`); + this.logger.info(`Added job ${job.id} (execution ${jobData.executionId})`); return job; } @@ -180,16 +182,16 @@ export class ScalingService { try { if (await job.isActive()) { await job.progress({ kind: 'abort-job' }); // being processed by worker - this.logger.debug('[ScalingService] Stopped active job', props); + this.logger.debug('Stopped active job', props); return true; } await job.remove(); // not yet picked up, or waiting for next pickup (stalled) - this.logger.debug('[ScalingService] Stopped inactive job', props); + this.logger.debug('Stopped inactive job', props); return true; } catch (error: unknown) { await job.progress({ kind: 'abort-job' }); - this.logger.error('[ScalingService] Failed to stop job', { ...props, error }); + this.logger.error('Failed to stop job', { ...props, error }); return false; } } @@ -233,12 +235,12 @@ export class ScalingService { * Even if Redis recovers, worker will remain unable to process jobs. */ if (error.message.includes('Error initializing Lua scripts')) { - this.logger.error('[ScalingService] Fatal error initializing worker', { error }); - this.logger.error('[ScalingService] Exiting process...'); + this.logger.error('Fatal error initializing worker', { error }); + this.logger.error('Exiting process...'); process.exit(1); } - this.logger.error('[ScalingService] Queue errored', { error }); + this.logger.error('Queue errored', { error }); throw error; }); @@ -251,7 +253,7 @@ export class ScalingService { this.queue.on('error', (error: Error) => { if ('code' in error && error.code === 'ECONNREFUSED') return; // handled by RedisClientService.retryStrategy - this.logger.error('[ScalingService] Queue errored', { error }); + this.logger.error('Queue errored', { error }); throw error; }); @@ -361,10 +363,10 @@ export class ScalingService { const nextWaitMs = await this.recoverFromQueue(); this.scheduleQueueRecovery(nextWaitMs); } catch (error) { - this.logger.error('[ScalingService] Failed to recover dangling executions from queue', { + this.logger.error('Failed to recover dangling executions from queue', { msg: this.toErrorMsg(error), }); - this.logger.error('[ScalingService] Retrying...'); + this.logger.error('Retrying...'); this.scheduleQueueRecovery(); } @@ -372,7 +374,7 @@ export class ScalingService { const wait = [this.queueRecoveryContext.waitMs / Time.minutes.toMilliseconds, 'min'].join(' '); - this.logger.debug(`[ScalingService] Scheduled queue recovery check for next ${wait}`); + this.logger.debug(`Scheduled queue recovery check for next ${wait}`); } private stopQueueRecovery() { @@ -389,7 +391,7 @@ export class ScalingService { const storedIds = await this.executionRepository.getInProgressExecutionIds(batchSize); if (storedIds.length === 0) { - this.logger.debug('[ScalingService] Completed queue recovery check, no dangling executions'); + this.logger.debug('Completed queue recovery check, no dangling executions'); return waitMs; } @@ -398,23 +400,22 @@ export class ScalingService { const queuedIds = new Set(runningJobs.map((job) => job.data.executionId)); if (queuedIds.size === 0) { - this.logger.debug('[ScalingService] Completed queue recovery check, no dangling executions'); + this.logger.debug('Completed queue recovery check, no dangling executions'); return waitMs; } const danglingIds = storedIds.filter((id) => !queuedIds.has(id)); if (danglingIds.length === 0) { - this.logger.debug('[ScalingService] Completed queue recovery check, no dangling executions'); + this.logger.debug('Completed queue recovery check, no dangling executions'); return waitMs; } await this.executionRepository.markAsCrashed(danglingIds); - this.logger.info( - '[ScalingService] Completed queue recovery check, recovered dangling executions', - { danglingIds }, - ); + this.logger.info('Completed queue recovery check, recovered dangling executions', { + danglingIds, + }); // if this cycle used up the whole batch size, it is possible for there to be // dangling executions outside this check, so speed up next cycle diff --git a/packages/cli/src/scaling/worker-server.ts b/packages/cli/src/scaling/worker-server.ts index 3343ce4e49..3cf6995882 100644 --- a/packages/cli/src/scaling/worker-server.ts +++ b/packages/cli/src/scaling/worker-server.ts @@ -2,7 +2,6 @@ import { GlobalConfig } from '@n8n/config'; import type { Application } from 'express'; import express from 'express'; import { InstanceSettings } from 'n8n-core'; -import { ensureError } from 'n8n-workflow'; import { strict as assert } from 'node:assert'; import http from 'node:http'; import type { Server } from 'node:http'; @@ -12,14 +11,13 @@ import { CredentialsOverwrites } from '@/credentials-overwrites'; import * as Db from '@/db'; import { CredentialsOverwritesAlreadySetError } from '@/errors/credentials-overwrites-already-set.error'; import { NonJsonBodyError } from '@/errors/non-json-body.error'; -import { ServiceUnavailableError } from '@/errors/response-errors/service-unavailable.error'; import { ExternalHooks } from '@/external-hooks'; import type { ICredentialsOverwrite } from '@/interfaces'; import { Logger } from '@/logging/logger.service'; import { PrometheusMetricsService } from '@/metrics/prometheus-metrics.service'; import { rawBodyReader, bodyParser } from '@/middlewares'; import * as ResponseHelper from '@/response-helper'; -import { ScalingService } from '@/scaling/scaling.service'; +import { RedisClientService } from '@/services/redis-client.service'; export type WorkerServerEndpointsConfig = { /** Whether the `/healthz` endpoint is enabled. */ @@ -52,11 +50,11 @@ export class WorkerServer { constructor( private readonly globalConfig: GlobalConfig, private readonly logger: Logger, - private readonly scalingService: ScalingService, private readonly credentialsOverwrites: CredentialsOverwrites, private readonly externalHooks: ExternalHooks, private readonly instanceSettings: InstanceSettings, private readonly prometheusMetricsService: PrometheusMetricsService, + private readonly redisClientService: RedisClientService, ) { assert(this.instanceSettings.instanceType === 'worker'); @@ -94,11 +92,14 @@ export class WorkerServer { } private async mountEndpoints() { - if (this.endpointsConfig.health) { - this.app.get('/healthz', async (req, res) => await this.healthcheck(req, res)); + const { health, overwrites, metrics } = this.endpointsConfig; + + if (health) { + this.app.get('/healthz', async (_, res) => res.send({ status: 'ok' })); + this.app.get('/healthz/readiness', async (_, res) => await this.readiness(_, res)); } - if (this.endpointsConfig.overwrites) { + if (overwrites) { const { endpoint } = this.globalConfig.credentials.overwrite; this.app.post(`/${endpoint}`, rawBodyReader, bodyParser, (req, res) => @@ -106,39 +107,20 @@ export class WorkerServer { ); } - if (this.endpointsConfig.metrics) { + if (metrics) { await this.prometheusMetricsService.init(this.app); } } - private async healthcheck(_req: express.Request, res: express.Response) { - this.logger.debug('[WorkerServer] Health check started'); + private async readiness(_req: express.Request, res: express.Response) { + const isReady = + Db.connectionState.connected && + Db.connectionState.migrated && + this.redisClientService.isConnected(); - try { - await Db.getConnection().query('SELECT 1'); - } catch (value) { - this.logger.error('[WorkerServer] No database connection', ensureError(value)); - - return ResponseHelper.sendErrorResponse( - res, - new ServiceUnavailableError('No database connection'), - ); - } - - try { - await this.scalingService.pingQueue(); - } catch (value) { - this.logger.error('[WorkerServer] No Redis connection', ensureError(value)); - - return ResponseHelper.sendErrorResponse( - res, - new ServiceUnavailableError('No Redis connection'), - ); - } - - this.logger.debug('[WorkerServer] Health check succeeded'); - - ResponseHelper.sendSuccessResponse(res, { status: 'ok' }, true, 200); + return isReady + ? res.status(200).send({ status: 'ok' }) + : res.status(503).send({ status: 'error' }); } private handleOverwrites( diff --git a/packages/cli/src/scaling/worker-status.ts b/packages/cli/src/scaling/worker-status.ts new file mode 100644 index 0000000000..cddccc7e1f --- /dev/null +++ b/packages/cli/src/scaling/worker-status.ts @@ -0,0 +1,43 @@ +import os from 'node:os'; +import { Service } from 'typedi'; + +import config from '@/config'; +import { N8N_VERSION } from '@/constants'; + +import { JobProcessor } from './job-processor'; + +@Service() +export class WorkerStatus { + constructor(private readonly jobProcessor: JobProcessor) {} + + generateStatus() { + return { + workerId: config.getEnv('redis.queueModeId'), + runningJobsSummary: this.jobProcessor.getRunningJobsSummary(), + freeMem: os.freemem(), + totalMem: os.totalmem(), + uptime: process.uptime(), + loadAvg: os.loadavg(), + cpus: this.getOsCpuString(), + arch: os.arch(), + platform: os.platform(), + hostname: os.hostname(), + interfaces: Object.values(os.networkInterfaces()).flatMap((interfaces) => + (interfaces ?? [])?.map((net) => ({ + family: net.family, + address: net.address, + internal: net.internal, + })), + ), + version: N8N_VERSION, + }; + } + + private getOsCpuString() { + const cpus = os.cpus(); + + if (cpus.length === 0) return 'no CPU info'; + + return `${cpus.length}x ${cpus[0].model} - speed: ${cpus[0].speed}`; + } +} diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index b83e2bdb2a..9d84b9d1e3 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -39,7 +39,7 @@ import '@/controllers/annotation-tags.controller.ee'; import '@/controllers/auth.controller'; import '@/controllers/binary-data.controller'; import '@/controllers/curl.controller'; -import '@/controllers/ai-assistant.controller'; +import '@/controllers/ai.controller'; import '@/controllers/dynamic-node-parameters.controller'; import '@/controllers/invitation.controller'; import '@/controllers/me.controller'; diff --git a/packages/cli/src/services/__tests__/orchestration.service.test.ts b/packages/cli/src/services/__tests__/orchestration.service.test.ts index 7bbd797000..1fab4705bb 100644 --- a/packages/cli/src/services/__tests__/orchestration.service.test.ts +++ b/packages/cli/src/services/__tests__/orchestration.service.test.ts @@ -1,3 +1,4 @@ +import type { WorkerStatus } from '@n8n/api-types'; import type Redis from 'ioredis'; import { mock } from 'jest-mock-extended'; import { InstanceSettings } from 'n8n-core'; @@ -34,12 +35,10 @@ mockInstance(ActiveWorkflowManager); let queueModeId: string; -const workerRestartEventBusResponse: PubSub.WorkerResponse = { +const workerStatusResponse: PubSub.WorkerResponse = { workerId: 'test', - command: 'restart-event-bus', - payload: { - result: 'success', - }, + command: 'get-worker-status', + payload: mock(), }; describe('Orchestration Service', () => { @@ -74,10 +73,10 @@ describe('Orchestration Service', () => { test('should handle worker responses', async () => { const response = await handleWorkerResponseMessageMain( - JSON.stringify(workerRestartEventBusResponse), + JSON.stringify(workerStatusResponse), mock(), ); - expect(response?.command).toEqual('restart-event-bus'); + expect(response?.command).toEqual('get-worker-status'); }); test('should handle command messages from others', async () => { @@ -94,10 +93,10 @@ describe('Orchestration Service', () => { test('should reject command messages from itself', async () => { const response = await handleCommandMessageMain( - JSON.stringify({ ...workerRestartEventBusResponse, senderId: queueModeId }), + JSON.stringify({ ...workerStatusResponse, senderId: queueModeId }), ); expect(response).toBeDefined(); - expect(response!.command).toEqual('restart-event-bus'); + expect(response!.command).toEqual('get-worker-status'); expect(response!.senderId).toEqual(queueModeId); expect(eventBus.restart).not.toHaveBeenCalled(); }); @@ -105,7 +104,7 @@ describe('Orchestration Service', () => { test('should send command messages', async () => { // @ts-expect-error Private field jest.spyOn(os.publisher, 'publishCommand').mockImplementation(async () => {}); - await os.getWorkerIds(); + await os.getWorkerStatus(); // @ts-expect-error Private field expect(os.publisher.publishCommand).toHaveBeenCalled(); // @ts-expect-error Private field diff --git a/packages/cli/src/services/ai-assistant.service.ts b/packages/cli/src/services/ai.service.ts similarity index 85% rename from packages/cli/src/services/ai-assistant.service.ts rename to packages/cli/src/services/ai.service.ts index 76b6e3fffb..a7b07219b5 100644 --- a/packages/cli/src/services/ai-assistant.service.ts +++ b/packages/cli/src/services/ai.service.ts @@ -3,7 +3,6 @@ import type { AiAssistantSDK } from '@n8n_io/ai-assistant-sdk'; import { AiAssistantClient } from '@n8n_io/ai-assistant-sdk'; import { assert, type IUser } from 'n8n-workflow'; import { Service } from 'typedi'; -import type { Response } from 'undici'; import config from '@/config'; import type { AiAssistantRequest } from '@/requests'; @@ -12,7 +11,7 @@ import { N8N_VERSION } from '../constants'; import { License } from '../license'; @Service() -export class AiAssistantService { +export class AiService { private client: AiAssistantClient | undefined; constructor( @@ -40,7 +39,7 @@ export class AiAssistantService { }); } - async chat(payload: AiAssistantSDK.ChatRequestPayload, user: IUser): Promise { + async chat(payload: AiAssistantSDK.ChatRequestPayload, user: IUser) { if (!this.client) { await this.init(); } @@ -57,4 +56,13 @@ export class AiAssistantService { return await this.client.applySuggestion(payload, { id: user.id }); } + + async askAi(payload: AiAssistantSDK.AskAiRequestPayload, user: IUser) { + if (!this.client) { + await this.init(); + } + assert(this.client, 'Assistant client not setup'); + + return await this.client.askAi(payload, { id: user.id }); + } } diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index ea8a135ff2..a83158e96e 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -212,8 +212,8 @@ export class FrontendService { banners: { dismissed: [], }, - ai: { - enabled: config.getEnv('ai.enabled'), + askAi: { + enabled: false, }, workflowHistory: { pruneTime: -1, @@ -274,6 +274,7 @@ export class FrontendService { const isS3Available = config.getEnv('binaryDataManager.availableModes').includes('s3'); const isS3Licensed = this.license.isBinaryDataS3Licensed(); const isAiAssistantEnabled = this.license.isAiAssistantEnabled(); + const isAskAiEnabled = this.license.isAskAiEnabled(); this.settings.license.planName = this.license.getPlanName(); this.settings.license.consumerId = this.license.getConsumerId(); @@ -330,6 +331,10 @@ export class FrontendService { this.settings.aiAssistant.enabled = isAiAssistantEnabled; } + if (isAskAiEnabled) { + this.settings.askAi.enabled = isAskAiEnabled; + } + this.settings.mfa.enabled = config.get('mfa.enabled'); this.settings.executionMode = config.getEnv('executions.mode'); diff --git a/packages/cli/src/services/import.service.ts b/packages/cli/src/services/import.service.ts index a486ff8396..2402863bab 100644 --- a/packages/cli/src/services/import.service.ts +++ b/packages/cli/src/services/import.service.ts @@ -88,8 +88,7 @@ export class ImportService { try { await replaceInvalidCredentials(workflow); } catch (e) { - const error = e instanceof Error ? e : new Error(`${e}`); - this.logger.error('Failed to replace invalid credential', error); + this.logger.error('Failed to replace invalid credential', { error: e }); } } diff --git a/packages/cli/src/services/orchestration.service.ts b/packages/cli/src/services/orchestration.service.ts index 666fe48ac6..b8aba46285 100644 --- a/packages/cli/src/services/orchestration.service.ts +++ b/packages/cli/src/services/orchestration.service.ts @@ -128,16 +128,6 @@ export class OrchestrationService { }); } - async getWorkerIds() { - if (!this.sanityCheck()) return; - - const command = 'get-worker-id'; - - this.logger.debug(`Sending "${command}" to command channel`); - - await this.publisher.publishCommand({ command }); - } - // ---------------------------------- // activations // ---------------------------------- diff --git a/packages/cli/src/services/orchestration/main/handle-command-message-main.ts b/packages/cli/src/services/orchestration/main/handle-command-message-main.ts index 909f5976a5..7af2fa6f56 100644 --- a/packages/cli/src/services/orchestration/main/handle-command-message-main.ts +++ b/packages/cli/src/services/orchestration/main/handle-command-message-main.ts @@ -27,17 +27,11 @@ export async function handleCommandMessageMain(messageString: string) { `RedisCommandHandler(main): Received command message ${message.command} from ${message.senderId}`, ); - const selfSendingAllowed = [ - 'add-webhooks-triggers-and-pollers', - 'remove-triggers-and-pollers', - ].includes(message.command); - if ( - !selfSendingAllowed && + !message.selfSend && (message.senderId === queueModeId || (message.targets && !message.targets.includes(queueModeId))) ) { - // Skipping command message because it's not for this instance logger.debug( `Skipping command message ${message.command} because it's not for this instance.`, ); diff --git a/packages/cli/src/services/orchestration/main/handle-worker-response-message-main.ts b/packages/cli/src/services/orchestration/main/handle-worker-response-message-main.ts index a3b5912fb4..1a99382c19 100644 --- a/packages/cli/src/services/orchestration/main/handle-worker-response-message-main.ts +++ b/packages/cli/src/services/orchestration/main/handle-worker-response-message-main.ts @@ -4,6 +4,7 @@ import Container from 'typedi'; import { Logger } from '@/logging/logger.service'; import { WORKER_RESPONSE_PUBSUB_CHANNEL } from '@/scaling/constants'; import type { PubSub } from '@/scaling/pubsub/pubsub.types'; +import { assertNever } from '@/utils'; import type { MainResponseReceivedHandlerOptions } from './types'; import { Push } from '../../../push'; @@ -32,12 +33,8 @@ export async function handleWorkerResponseMessageMain( status: workerResponse.payload, }); break; - case 'get-worker-id': - break; default: - Container.get(Logger).debug( - `Received worker response ${workerResponse.command} from ${workerResponse.workerId}`, - ); + assertNever(workerResponse.command); } return workerResponse; diff --git a/packages/cli/src/services/orchestration/worker/handle-command-message-worker.ts b/packages/cli/src/services/orchestration/worker/handle-command-message-worker.ts deleted file mode 100644 index ae11ac96fe..0000000000 --- a/packages/cli/src/services/orchestration/worker/handle-command-message-worker.ts +++ /dev/null @@ -1,153 +0,0 @@ -import { jsonParse } from 'n8n-workflow'; -import os from 'node:os'; -import Container from 'typedi'; - -import { N8N_VERSION } from '@/constants'; -import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; -import { ExternalSecretsManager } from '@/external-secrets/external-secrets-manager.ee'; -import { License } from '@/license'; -import { Logger } from '@/logging/logger.service'; -import { COMMAND_PUBSUB_CHANNEL } from '@/scaling/constants'; -import type { PubSub } from '@/scaling/pubsub/pubsub.types'; -import { CommunityPackagesService } from '@/services/community-packages.service'; - -import type { WorkerCommandReceivedHandlerOptions } from './types'; -import { debounceMessageReceiver, getOsCpuString } from '../helpers'; - -// eslint-disable-next-line complexity -export async function getWorkerCommandReceivedHandler( - messageString: string, - options: WorkerCommandReceivedHandlerOptions, -) { - if (!messageString) return; - - const logger = Container.get(Logger); - let message: PubSub.Command; - try { - message = jsonParse(messageString); - } catch { - logger.debug( - `Received invalid message via channel ${COMMAND_PUBSUB_CHANNEL}: "${messageString}"`, - ); - return; - } - if (message) { - logger.debug( - `RedisCommandHandler(worker): Received command message ${message.command} from ${message.senderId}`, - ); - if (message.targets && !message.targets.includes(options.queueModeId)) { - return; // early return if the message is not for this worker - } - switch (message.command) { - case 'get-worker-status': - if (!debounceMessageReceiver(message, 500)) return; - await options.publisher.publishWorkerResponse({ - workerId: options.queueModeId, - command: 'get-worker-status', - payload: { - workerId: options.queueModeId, - runningJobsSummary: options.getRunningJobsSummary(), - freeMem: os.freemem(), - totalMem: os.totalmem(), - uptime: process.uptime(), - loadAvg: os.loadavg(), - cpus: getOsCpuString(), - arch: os.arch(), - platform: os.platform(), - hostname: os.hostname(), - interfaces: Object.values(os.networkInterfaces()).flatMap((interfaces) => - (interfaces ?? [])?.map((net) => ({ - family: net.family, - address: net.address, - internal: net.internal, - })), - ), - version: N8N_VERSION, - }, - }); - break; - case 'get-worker-id': - if (!debounceMessageReceiver(message, 500)) return; - await options.publisher.publishWorkerResponse({ - workerId: options.queueModeId, - command: 'get-worker-id', - }); - break; - case 'restart-event-bus': - if (!debounceMessageReceiver(message, 500)) return; - try { - await Container.get(MessageEventBus).restart(); - await options.publisher.publishWorkerResponse({ - workerId: options.queueModeId, - command: 'restart-event-bus', - payload: { - result: 'success', - }, - }); - } catch (error) { - await options.publisher.publishWorkerResponse({ - workerId: options.queueModeId, - command: 'restart-event-bus', - payload: { - result: 'error', - error: (error as Error).message, - }, - }); - } - break; - case 'reload-external-secrets-providers': - if (!debounceMessageReceiver(message, 500)) return; - try { - await Container.get(ExternalSecretsManager).reloadAllProviders(); - await options.publisher.publishWorkerResponse({ - workerId: options.queueModeId, - command: 'reload-external-secrets-providers', - payload: { - result: 'success', - }, - }); - } catch (error) { - await options.publisher.publishWorkerResponse({ - workerId: options.queueModeId, - command: 'reload-external-secrets-providers', - payload: { - result: 'error', - error: (error as Error).message, - }, - }); - } - break; - case 'community-package-install': - case 'community-package-update': - case 'community-package-uninstall': - if (!debounceMessageReceiver(message, 500)) return; - const { packageName } = message.payload; - const communityPackagesService = Container.get(CommunityPackagesService); - if (message.command === 'community-package-uninstall') { - await communityPackagesService.removeNpmPackage(packageName); - } else { - await communityPackagesService.installOrUpdateNpmPackage( - packageName, - message.payload.packageVersion, - ); - } - break; - case 'reload-license': - if (!debounceMessageReceiver(message, 500)) return; - await Container.get(License).reload(); - break; - default: - if ( - message.command === 'relay-execution-lifecycle-event' || - message.command === 'clear-test-webhooks' - ) { - break; // meant only for main - } - - logger.debug( - `Received unknown command via channel ${COMMAND_PUBSUB_CHANNEL}: "${message.command}"`, - ); - break; - } - } -} diff --git a/packages/cli/src/services/orchestration/worker/orchestration.handler.worker.service.ts b/packages/cli/src/services/orchestration/worker/orchestration.handler.worker.service.ts deleted file mode 100644 index 06113d7344..0000000000 --- a/packages/cli/src/services/orchestration/worker/orchestration.handler.worker.service.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { Service } from 'typedi'; - -import { Subscriber } from '@/scaling/pubsub/subscriber.service'; - -import { getWorkerCommandReceivedHandler } from './handle-command-message-worker'; -import type { WorkerCommandReceivedHandlerOptions } from './types'; -import { OrchestrationHandlerService } from '../../orchestration.handler.base.service'; - -@Service() -export class OrchestrationHandlerWorkerService extends OrchestrationHandlerService { - constructor(private readonly subscriber: Subscriber) { - super(); - } - - async initSubscriber(options: WorkerCommandReceivedHandlerOptions) { - await this.subscriber.subscribe('n8n.commands'); - - this.subscriber.setMessageHandler('n8n.commands', async (message: string) => { - await getWorkerCommandReceivedHandler(message, options); - }); - } -} diff --git a/packages/cli/src/services/redis-client.service.ts b/packages/cli/src/services/redis-client.service.ts index f205a756c5..5eaa6edc1d 100644 --- a/packages/cli/src/services/redis-client.service.ts +++ b/packages/cli/src/services/redis-client.service.ts @@ -40,6 +40,10 @@ export class RedisClientService extends TypedEmitter { this.registerListeners(); } + isConnected() { + return !this.lostConnection; + } + createClient(arg: { type: RedisClientType; extraOptions?: RedisOptions }) { const client = this.clusterNodes().length > 0 diff --git a/packages/cli/src/utils/__tests__/path-util.test.ts b/packages/cli/src/utils/__tests__/path-util.test.ts new file mode 100644 index 0000000000..d67e97f9e0 --- /dev/null +++ b/packages/cli/src/utils/__tests__/path-util.test.ts @@ -0,0 +1,24 @@ +import { isContainedWithin } from '../path-util'; + +describe('isContainedWithin', () => { + it('should return true when parent and child paths are the same', () => { + expect(isContainedWithin('/some/parent/folder', '/some/parent/folder')).toBe(true); + }); + + test.each([ + ['/some/parent/folder', '/some/parent/folder/subfolder/file.txt'], + ['/some/parent/folder', '/some/parent/folder/../folder/subfolder/file.txt'], + ['/some/parent/folder/', '/some/parent/folder/subfolder/file.txt'], + ['/some/parent/folder', '/some/parent/folder/subfolder/'], + ])('should return true for parent %s and child %s', (parent, child) => { + expect(isContainedWithin(parent, child)).toBe(true); + }); + + test.each([ + ['/some/parent/folder', '/some/other/folder/file.txt'], + ['/some/parent/folder', '/some/parent/folder_but_not_really'], + ['/one/path', '/another/path'], + ])('should return false for parent %s and child %s', (parent, child) => { + expect(isContainedWithin(parent, child)).toBe(false); + }); +}); diff --git a/packages/cli/src/utils/path-util.ts b/packages/cli/src/utils/path-util.ts new file mode 100644 index 0000000000..f42dc01890 --- /dev/null +++ b/packages/cli/src/utils/path-util.ts @@ -0,0 +1,16 @@ +import * as path from 'node:path'; + +/** + * Checks if the given childPath is contained within the parentPath. Resolves + * the paths before comparing them, so that relative paths are also supported. + */ +export function isContainedWithin(parentPath: string, childPath: string): boolean { + parentPath = path.resolve(parentPath); + childPath = path.resolve(childPath); + + if (parentPath === childPath) { + return true; + } + + return childPath.startsWith(parentPath + path.sep); +} diff --git a/packages/cli/src/wait-tracker.ts b/packages/cli/src/wait-tracker.ts index 82b42c39df..e999c30401 100644 --- a/packages/cli/src/wait-tracker.ts +++ b/packages/cli/src/wait-tracker.ts @@ -28,7 +28,9 @@ export class WaitTracker { private readonly ownershipService: OwnershipService, private readonly workflowRunner: WorkflowRunner, private readonly orchestrationService: OrchestrationService, - ) {} + ) { + this.logger = this.logger.withScope('executions'); + } has(executionId: string) { return this.waitingExecutions[executionId] !== undefined; @@ -50,7 +52,7 @@ export class WaitTracker { } private startTracking() { - this.logger.debug('Wait tracker started tracking waiting executions'); + this.logger.debug('Started tracking waiting executions'); // Poll every 60 seconds a list of upcoming executions this.mainTimer = setInterval(() => { @@ -61,7 +63,7 @@ export class WaitTracker { } async getWaitingExecutions() { - this.logger.debug('Wait tracker querying database for waiting executions'); + this.logger.debug('Querying database for waiting executions'); const executions = await this.executionRepository.getWaitingExecutions(); @@ -71,7 +73,7 @@ export class WaitTracker { const executionIds = executions.map((execution) => execution.id).join(', '); this.logger.debug( - `Wait tracker found ${executions.length} executions. Setting timer for IDs: ${executionIds}`, + `Found ${executions.length} executions. Setting timer for IDs: ${executionIds}`, ); // Add timers for each waiting execution that they get started at the correct time @@ -99,7 +101,7 @@ export class WaitTracker { } startExecution(executionId: string) { - this.logger.debug(`Wait tracker resuming execution ${executionId}`, { executionId }); + this.logger.debug(`Resuming execution ${executionId}`, { executionId }); delete this.waitingExecutions[executionId]; (async () => { @@ -141,7 +143,7 @@ export class WaitTracker { } stopTracking() { - this.logger.debug('Wait tracker shutting down'); + this.logger.debug('Shutting down wait tracking'); clearInterval(this.mainTimer); Object.keys(this.waitingExecutions).forEach((executionId) => { diff --git a/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts b/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts index 7a8dd8b854..892d87e773 100644 --- a/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts +++ b/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts @@ -63,7 +63,7 @@ describe('WaitingWebhooks', () => { * Arrange */ executionRepository.findSingleExecution.mockResolvedValue( - mock({ finished: true }), + mock({ finished: true, workflowData: { nodes: [] } }), ); /** diff --git a/packages/cli/src/webhooks/waiting-webhooks.ts b/packages/cli/src/webhooks/waiting-webhooks.ts index 922de9d869..e644c065f3 100644 --- a/packages/cli/src/webhooks/waiting-webhooks.ts +++ b/packages/cli/src/webhooks/waiting-webhooks.ts @@ -1,5 +1,11 @@ import type express from 'express'; -import { NodeHelpers, Workflow } from 'n8n-workflow'; +import { + type INodes, + type IWorkflowBase, + NodeHelpers, + SEND_AND_WAIT_OPERATION, + Workflow, +} from 'n8n-workflow'; import { Service } from 'typedi'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; @@ -42,6 +48,29 @@ export class WaitingWebhooks implements IWebhookManager { execution.data.executionData!.nodeExecutionStack[0].node.disabled = true; } + private isSendAndWaitRequest(nodes: INodes, suffix: string | undefined) { + return ( + suffix && + Object.keys(nodes).some( + (node) => + nodes[node].id === suffix && nodes[node].parameters.operation === SEND_AND_WAIT_OPERATION, + ) + ); + } + + private getWorkflow(workflowData: IWorkflowBase) { + return new Workflow({ + id: workflowData.id, + name: workflowData.name, + nodes: workflowData.nodes, + connections: workflowData.connections, + active: workflowData.active, + nodeTypes: this.nodeTypes, + staticData: workflowData.staticData, + settings: workflowData.settings, + }); + } + async executeWebhook( req: WaitingWebhookRequest, res: express.Response, @@ -66,10 +95,21 @@ export class WaitingWebhooks implements IWebhookManager { throw new ConflictError(`The execution "${executionId} is running already.`); } - if (execution.finished || execution.data.resultData.error) { + if (execution.data?.resultData?.error) { throw new ConflictError(`The execution "${executionId} has finished already.`); } + if (execution.finished) { + const { workflowData } = execution; + const { nodes } = this.getWorkflow(workflowData); + if (this.isSendAndWaitRequest(nodes, suffix)) { + res.render('send-and-wait-no-action-required', { isTestWebhook: false }); + return { noWebhookResponse: true }; + } else { + throw new ConflictError(`The execution "${executionId} has finished already.`); + } + } + const lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string; // Set the node as disabled so that the data does not get executed again as it would result @@ -83,17 +123,7 @@ export class WaitingWebhooks implements IWebhookManager { execution.data.resultData.runData[lastNodeExecuted].pop(); const { workflowData } = execution; - - const workflow = new Workflow({ - id: workflowData.id, - name: workflowData.name, - nodes: workflowData.nodes, - connections: workflowData.connections, - active: workflowData.active, - nodeTypes: this.nodeTypes, - staticData: workflowData.staticData, - settings: workflowData.settings, - }); + const workflow = this.getWorkflow(workflowData); const workflowStartNode = workflow.getNode(lastNodeExecuted); if (workflowStartNode === null) { @@ -116,8 +146,13 @@ export class WaitingWebhooks implements IWebhookManager { if (webhookData === undefined) { // If no data got found it means that the execution can not be started via a webhook. // Return 404 because we do not want to give any data if the execution exists or not. - const errorMessage = `The workflow for execution "${executionId}" does not contain a waiting webhook with a matching path/method.`; - throw new NotFoundError(errorMessage); + if (this.isSendAndWaitRequest(workflow.nodes, suffix)) { + res.render('send-and-wait-no-action-required', { isTestWebhook: false }); + return { noWebhookResponse: true }; + } else { + const errorMessage = `The workflow for execution "${executionId}" does not contain a waiting webhook with a matching path/method.`; + throw new NotFoundError(errorMessage); + } } const runExecutionData = execution.data; diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index f357bbc018..2deae842fc 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -6,6 +6,13 @@ import type { PushType } from '@n8n/api-types'; import { GlobalConfig } from '@n8n/config'; import { WorkflowExecute } from 'n8n-core'; +import { + ApplicationError, + ErrorReporterProxy as ErrorReporter, + NodeOperationError, + Workflow, + WorkflowHooks, +} from 'n8n-workflow'; import type { IDataObject, IExecuteData, @@ -28,13 +35,7 @@ import type { ITaskDataConnections, ExecuteWorkflowOptions, IWorkflowExecutionDataProcess, -} from 'n8n-workflow'; -import { - ApplicationError, - ErrorReporterProxy as ErrorReporter, - NodeOperationError, - Workflow, - WorkflowHooks, + EnvProviderState, } from 'n8n-workflow'; import { Container } from 'typedi'; @@ -1008,6 +1009,7 @@ export async function getBase( connectionInputData: INodeExecutionData[], siblingParameters: INodeParameters, mode: WorkflowExecuteMode, + envProviderState: EnvProviderState, executeData?: IExecuteData, defaultReturnRunIndex?: number, selfData?: IDataObject, @@ -1028,6 +1030,7 @@ export async function getBase( connectionInputData, siblingParameters, mode, + envProviderState, executeData, defaultReturnRunIndex, selfData, diff --git a/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.ee.test.ts b/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.ee.test.ts index cb1b3952ad..a2a48587f0 100644 --- a/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.ee.test.ts +++ b/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.ee.test.ts @@ -3,13 +3,12 @@ import { mockClear } from 'jest-mock-extended'; import { User } from '@/databases/entities/user'; import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import { WorkflowHistoryRepository } from '@/databases/repositories/workflow-history.repository'; -import { Logger } from '@/logging/logger.service'; import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service.ee'; -import { mockInstance } from '@test/mocking'; +import { mockInstance, mockLogger } from '@test/mocking'; import { getWorkflow } from '@test-integration/workflow'; const workflowHistoryRepository = mockInstance(WorkflowHistoryRepository); -const logger = mockInstance(Logger); +const logger = mockLogger(); const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository); const workflowHistoryService = new WorkflowHistoryService( logger, @@ -106,10 +105,6 @@ describe('WorkflowHistoryService', () => { // Assert expect(workflowHistoryRepository.insert).toHaveBeenCalled(); - expect(logger.error).toHaveBeenCalledWith( - 'Failed to save workflow history version for workflow 123', - expect.any(Error), - ); }); }); }); diff --git a/packages/cli/src/workflows/workflow-history/workflow-history.service.ee.ts b/packages/cli/src/workflows/workflow-history/workflow-history.service.ee.ts index eddb8bf7e6..3b171e3422 100644 --- a/packages/cli/src/workflows/workflow-history/workflow-history.service.ee.ts +++ b/packages/cli/src/workflows/workflow-history/workflow-history.service.ee.ts @@ -1,3 +1,4 @@ +import { ensureError } from 'n8n-workflow'; import { Service } from 'typedi'; import type { User } from '@/databases/entities/user'; @@ -79,10 +80,10 @@ export class WorkflowHistoryService { workflowId, }); } catch (e) { - this.logger.error( - `Failed to save workflow history version for workflow ${workflowId}`, - e as Error, - ); + const error = ensureError(e); + this.logger.error(`Failed to save workflow history version for workflow ${workflowId}`, { + error, + }); } } } diff --git a/packages/cli/templates/send-and-wait-no-action-required.handlebars b/packages/cli/templates/send-and-wait-no-action-required.handlebars new file mode 100644 index 0000000000..7dcf99f10b --- /dev/null +++ b/packages/cli/templates/send-and-wait-no-action-required.handlebars @@ -0,0 +1,73 @@ + + + + + + + + No action required + + + + +
+
+
+
+

No action required

+
+
+ +
+
+ + + \ No newline at end of file diff --git a/packages/cli/test/integration/commands/worker.cmd.test.ts b/packages/cli/test/integration/commands/worker.cmd.test.ts index 1ff05181b2..585d64cfb4 100644 --- a/packages/cli/test/integration/commands/worker.cmd.test.ts +++ b/packages/cli/test/integration/commands/worker.cmd.test.ts @@ -11,8 +11,8 @@ import { ExternalSecretsManager } from '@/external-secrets/external-secrets-mana import { License } from '@/license'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { Publisher } from '@/scaling/pubsub/publisher.service'; +import { Subscriber } from '@/scaling/pubsub/subscriber.service'; import { ScalingService } from '@/scaling/scaling.service'; -import { OrchestrationHandlerWorkerService } from '@/services/orchestration/worker/orchestration.handler.worker.service'; import { OrchestrationWorkerService } from '@/services/orchestration/worker/orchestration.worker.service'; import { setupTestCommand } from '@test-integration/utils/test-command'; @@ -27,10 +27,10 @@ const externalSecretsManager = mockInstance(ExternalSecretsManager); const license = mockInstance(License, { loadCertStr: async () => '' }); const messageEventBus = mockInstance(MessageEventBus); const logStreamingEventRelay = mockInstance(LogStreamingEventRelay); -const orchestrationHandlerWorkerService = mockInstance(OrchestrationHandlerWorkerService); const scalingService = mockInstance(ScalingService); const orchestrationWorkerService = mockInstance(OrchestrationWorkerService); mockInstance(Publisher); +mockInstance(Subscriber); const command = setupTestCommand(Worker); @@ -48,6 +48,5 @@ test('worker initializes all its components', async () => { expect(scalingService.setupWorker).toHaveBeenCalledTimes(1); expect(logStreamingEventRelay.init).toHaveBeenCalledTimes(1); expect(orchestrationWorkerService.init).toHaveBeenCalledTimes(1); - expect(orchestrationHandlerWorkerService.initWithOptions).toHaveBeenCalledTimes(1); expect(messageEventBus.send).toHaveBeenCalledTimes(1); }); diff --git a/packages/cli/test/shared/mocking.ts b/packages/cli/test/shared/mocking.ts index 60b712b115..099988a896 100644 --- a/packages/cli/test/shared/mocking.ts +++ b/packages/cli/test/shared/mocking.ts @@ -4,6 +4,8 @@ import type { Class } from 'n8n-core'; import type { DeepPartial } from 'ts-essentials'; import { Container } from 'typedi'; +import type { Logger } from '@/logging/logger.service'; + export const mockInstance = ( serviceClass: Class, data: DeepPartial | undefined = undefined, @@ -22,3 +24,6 @@ export const mockEntityManager = (entityClass: Class) => { Object.assign(entityManager, { connection: dataSource }); return entityManager; }; + +export const mockLogger = () => + mock({ withScope: jest.fn().mockReturnValue(mock()) }); diff --git a/packages/core/package.json b/packages/core/package.json index 50762d8c14..95cf23efa6 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "n8n-core", - "version": "1.62.1", + "version": "1.63.0", "description": "Core functionality of n8n", "main": "dist/index", "types": "dist/index.d.ts", diff --git a/packages/core/src/Agent/index.ts b/packages/core/src/Agent/index.ts index 75195b6acf..1d4e916d33 100644 --- a/packages/core/src/Agent/index.ts +++ b/packages/core/src/Agent/index.ts @@ -11,6 +11,7 @@ import type { IExecuteData, IDataObject, } from 'n8n-workflow'; +import { createEnvProviderState } from 'n8n-workflow'; export const createAgentStartJob = ( additionalData: IWorkflowExecuteAdditionalData, @@ -49,6 +50,7 @@ export const createAgentStartJob = ( connectionInputData, siblingParameters, mode, + createEnvProviderState(), executeData, defaultReturnRunIndex, selfData, diff --git a/packages/core/src/PartialExecutionUtils/DirectedGraph.ts b/packages/core/src/PartialExecutionUtils/DirectedGraph.ts index 33cc114698..606f624d02 100644 --- a/packages/core/src/PartialExecutionUtils/DirectedGraph.ts +++ b/packages/core/src/PartialExecutionUtils/DirectedGraph.ts @@ -12,6 +12,11 @@ export type GraphConnection = { // fromName-outputType-outputIndex-inputIndex-toName type DirectedGraphKey = `${string}-${NodeConnectionType}-${number}-${number}-${string}`; +type RemoveNodeBaseOptions = { + reconnectConnections: boolean; + skipConnectionFn?: (connection: GraphConnection) => boolean; +}; + /** * Represents a directed graph as an adjacency list, e.g. one list for the * vertices and one list for the edges. @@ -77,17 +82,34 @@ export class DirectedGraph { * connections making sure all parent nodes are connected to all child nodes * and return the new connections. */ - removeNode(node: INode, options?: { reconnectConnections: true }): GraphConnection[]; - removeNode(node: INode, options?: { reconnectConnections: false }): undefined; - removeNode(node: INode, { reconnectConnections = false } = {}): undefined | GraphConnection[] { - if (reconnectConnections) { - const incomingConnections = this.getDirectParents(node); - const outgoingConnections = this.getDirectChildren(node); + removeNode( + node: INode, + options?: { reconnectConnections: true } & RemoveNodeBaseOptions, + ): GraphConnection[]; + removeNode( + node: INode, + options?: { reconnectConnections: false } & RemoveNodeBaseOptions, + ): undefined; + removeNode( + node: INode, + options: RemoveNodeBaseOptions = { reconnectConnections: false }, + ): undefined | GraphConnection[] { + if (options.reconnectConnections) { + const incomingConnections = this.getDirectParentConnections(node); + const outgoingConnections = this.getDirectChildConnections(node); const newConnections: GraphConnection[] = []; for (const incomingConnection of incomingConnections) { + if (options.skipConnectionFn && options.skipConnectionFn(incomingConnection)) { + continue; + } + for (const outgoingConnection of outgoingConnections) { + if (options.skipConnectionFn && options.skipConnectionFn(outgoingConnection)) { + continue; + } + const newConnection = { ...incomingConnection, to: outgoingConnection.to, @@ -165,7 +187,7 @@ export class DirectedGraph { return this; } - getDirectChildren(node: INode) { + getDirectChildConnections(node: INode) { const nodeExists = this.nodes.get(node.name) === node; a.ok(nodeExists); @@ -183,7 +205,7 @@ export class DirectedGraph { } private getChildrenRecursive(node: INode, children: Set) { - const directChildren = this.getDirectChildren(node); + const directChildren = this.getDirectChildConnections(node); for (const directChild of directChildren) { // Break out if we found a cycle. @@ -202,13 +224,13 @@ export class DirectedGraph { * argument. * * If the node being passed in is a child of itself (e.g. is part of a - * cylce), the return set will contain it as well. + * cycle), the return set will contain it as well. */ getChildren(node: INode) { return this.getChildrenRecursive(node, new Set()); } - getDirectParents(node: INode) { + getDirectParentConnections(node: INode) { const nodeExists = this.nodes.get(node.name) === node; a.ok(nodeExists); @@ -225,6 +247,27 @@ export class DirectedGraph { return directParents; } + private getParentConnectionsRecursive(node: INode, connections: Set) { + const parentConnections = this.getDirectParentConnections(node); + + for (const connection of parentConnections) { + // break out of cycles + if (connections.has(connection)) { + continue; + } + + connections.add(connection); + + this.getParentConnectionsRecursive(connection.from, connections); + } + + return connections; + } + + getParentConnections(node: INode) { + return this.getParentConnectionsRecursive(node, new Set()); + } + getConnection( from: INode, outputIndex: number, diff --git a/packages/core/src/PartialExecutionUtils/__tests__/DirectedGraph.test.ts b/packages/core/src/PartialExecutionUtils/__tests__/DirectedGraph.test.ts index 426a5405c7..d6eedf416d 100644 --- a/packages/core/src/PartialExecutionUtils/__tests__/DirectedGraph.test.ts +++ b/packages/core/src/PartialExecutionUtils/__tests__/DirectedGraph.test.ts @@ -89,6 +89,60 @@ describe('DirectedGraph', () => { }); }); + describe('getParentConnections', () => { + // ┌─────┐ ┌─────┐ ┌─────┐ ┌─────┐ + // │node1├──►│node2├──►│node3│──►│node4│ + // └─────┘ └─────┘ └─────┘ └─────┘ + test('returns all parent connections', () => { + // ARRANGE + const node1 = createNodeData({ name: 'Node1' }); + const node2 = createNodeData({ name: 'Node2' }); + const node3 = createNodeData({ name: 'Node3' }); + const node4 = createNodeData({ name: 'Node4' }); + const graph = new DirectedGraph() + .addNodes(node1, node2, node3, node4) + .addConnections( + { from: node1, to: node2 }, + { from: node2, to: node3 }, + { from: node3, to: node4 }, + ); + + // ACT + const connections = graph.getParentConnections(node3); + + // ASSERT + const expectedConnections = graph.getConnections().filter((c) => c.to !== node4); + expect(connections.size).toBe(2); + expect(connections).toEqual(new Set(expectedConnections)); + }); + + // ┌─────┐ ┌─────┐ ┌─────┐ + // ┌─►│node1├───►│node2├──►│node3├─┐ + // │ └─────┘ └─────┘ └─────┘ │ + // │ │ + // └───────────────────────────────┘ + test('terminates when finding a cycle', () => { + // ARRANGE + const node1 = createNodeData({ name: 'Node1' }); + const node2 = createNodeData({ name: 'Node2' }); + const node3 = createNodeData({ name: 'Node3' }); + const graph = new DirectedGraph() + .addNodes(node1, node2, node3) + .addConnections( + { from: node1, to: node2 }, + { from: node2, to: node3 }, + { from: node3, to: node1 }, + ); + + // ACT + const connections = graph.getParentConnections(node3); + + // ASSERT + expect(connections.size).toBe(3); + expect(connections).toEqual(new Set(graph.getConnections())); + }); + }); + describe('removeNode', () => { // XX // ┌─────┐ ┌─────┐ ┌─────┐ diff --git a/packages/core/src/PartialExecutionUtils/__tests__/findSubgraph.test.ts b/packages/core/src/PartialExecutionUtils/__tests__/findSubgraph.test.ts index a5187eacf4..76a1afcc69 100644 --- a/packages/core/src/PartialExecutionUtils/__tests__/findSubgraph.test.ts +++ b/packages/core/src/PartialExecutionUtils/__tests__/findSubgraph.test.ts @@ -9,6 +9,8 @@ // XX denotes that the node is disabled // PD denotes that the node has pinned data +import { NodeConnectionType } from 'n8n-workflow'; + import { createNodeData } from './helpers'; import { DirectedGraph } from '../DirectedGraph'; import { findSubgraph } from '../findSubgraph'; @@ -222,4 +224,110 @@ describe('findSubgraph', () => { .addConnections({ from: trigger, to: destination }), ); }); + + describe('root nodes', () => { + // ►► + // ┌───────┐ ┌───────────┐ + // │trigger├─────►│destination│ + // └───────┘ └──▲────────┘ + // │AiLanguageModel + // ┌┴──────┐ + // │aiModel│ + // └───────┘ + test('always retain connections that have a different type than `NodeConnectionType.Main`', () => { + // ARRANGE + const trigger = createNodeData({ name: 'trigger' }); + const destination = createNodeData({ name: 'destination' }); + const aiModel = createNodeData({ name: 'ai_model' }); + + const graph = new DirectedGraph() + .addNodes(trigger, destination, aiModel) + .addConnections( + { from: trigger, to: destination }, + { from: aiModel, type: NodeConnectionType.AiLanguageModel, to: destination }, + ); + + // ACT + const subgraph = findSubgraph(graph, destination, trigger); + + // ASSERT + expect(subgraph).toEqual(graph); + }); + + // This graph is not possible, it's only here to make sure `findSubgraph` + // does not follow non-Main connections. + // + // ┌────┐ ┌───────────┐ + // │root┼───►destination│ + // └──▲─┘ └───────────┘ + // │AiLanguageModel + // ┌┴──────┐ + // │aiModel│ + // └▲──────┘ + // ┌┴──────┐ + // │trigger│ + // └───────┘ + // turns into an empty graph, because there is no `Main` typed connection + // connecting destination and trigger. + test('skip non-Main connection types', () => { + // ARRANGE + const trigger = createNodeData({ name: 'trigger' }); + const root = createNodeData({ name: 'root' }); + const aiModel = createNodeData({ name: 'aiModel' }); + const destination = createNodeData({ name: 'destination' }); + const graph = new DirectedGraph() + .addNodes(trigger, root, aiModel, destination) + .addConnections( + { from: trigger, to: aiModel }, + { from: aiModel, type: NodeConnectionType.AiLanguageModel, to: root }, + { from: root, to: destination }, + ); + + // ACT + const subgraph = findSubgraph(graph, destination, trigger); + + // ASSERT + expect(subgraph.getConnections()).toHaveLength(0); + expect(subgraph.getNodes().size).toBe(0); + }); + + // + // XX + // ┌───────┐ ┌────┐ ┌───────────┐ + // │trigger├───►root├───►destination│ + // └───────┘ └──▲─┘ └───────────┘ + // │AiLanguageModel + // ┌┴──────┐ + // │aiModel│ + // └───────┘ + // turns into + // ┌───────┐ ┌───────────┐ + // │trigger├────────────►destination│ + // └───────┘ └───────────┘ + test('skip disabled root nodes', () => { + // ARRANGE + const trigger = createNodeData({ name: 'trigger' }); + const root = createNodeData({ name: 'root', disabled: true }); + const aiModel = createNodeData({ name: 'ai_model' }); + const destination = createNodeData({ name: 'destination' }); + + const graph = new DirectedGraph() + .addNodes(trigger, root, aiModel, destination) + .addConnections( + { from: trigger, to: root }, + { from: aiModel, type: NodeConnectionType.AiLanguageModel, to: root }, + { from: root, to: destination }, + ); + + // ACT + const subgraph = findSubgraph(graph, root, trigger); + + // ASSERT + expect(subgraph).toEqual( + new DirectedGraph() + .addNodes(trigger, destination) + .addConnections({ from: trigger, to: destination }), + ); + }); + }); }); diff --git a/packages/core/src/PartialExecutionUtils/findStartNodes.ts b/packages/core/src/PartialExecutionUtils/findStartNodes.ts index 12a9688c1c..28772bfc9a 100644 --- a/packages/core/src/PartialExecutionUtils/findStartNodes.ts +++ b/packages/core/src/PartialExecutionUtils/findStartNodes.ts @@ -80,7 +80,7 @@ function findStartNodesRecursive( } // Recurse with every direct child that is part of the sub graph. - const outGoingConnections = graph.getDirectChildren(current); + const outGoingConnections = graph.getDirectChildConnections(current); for (const outGoingConnection of outGoingConnections) { const nodeRunData = getIncomingData( runData, diff --git a/packages/core/src/PartialExecutionUtils/findSubgraph.ts b/packages/core/src/PartialExecutionUtils/findSubgraph.ts index ea1df91840..d05561e31a 100644 --- a/packages/core/src/PartialExecutionUtils/findSubgraph.ts +++ b/packages/core/src/PartialExecutionUtils/findSubgraph.ts @@ -1,4 +1,4 @@ -import type { INode } from 'n8n-workflow'; +import { NodeConnectionType, type INode } from 'n8n-workflow'; import type { GraphConnection } from './DirectedGraph'; import { DirectedGraph } from './DirectedGraph'; @@ -21,7 +21,7 @@ function findSubgraphRecursive( return; } - let parentConnections = graph.getDirectParents(current); + let parentConnections = graph.getDirectParentConnections(current); // If the current node has no parents, don’t keep this branch. if (parentConnections.length === 0) { @@ -58,11 +58,24 @@ function findSubgraphRecursive( // The node is replaced by a set of new connections, connecting the parents // and children of it directly. In the recursive call below we'll follow // them further. - parentConnections = graph.removeNode(current, { reconnectConnections: true }); + parentConnections = graph.removeNode(current, { + reconnectConnections: true, + // If the node has non-Main connections we don't want to rewire those. + // Otherwise we'd end up connecting AI utilities to nodes that don't + // support them. + skipConnectionFn: (c) => c.type !== NodeConnectionType.Main, + }); } // Recurse on each parent. for (const parentConnection of parentConnections) { + // Skip parents that are connected via non-Main connection types. They are + // only utility nodes for AI and are not part of the data or control flow + // and can never lead too the trigger. + if (parentConnection.type !== NodeConnectionType.Main) { + continue; + } + findSubgraphRecursive(graph, destinationNode, parentConnection.from, trigger, newGraph, [ ...currentBranch, parentConnection, @@ -87,15 +100,38 @@ function findSubgraphRecursive( * - take every incoming connection and connect it to every node that is * connected to the current node’s first output * 6. Recurse on each parent + * 7. Re-add all connections that don't use the `Main` connections type. + * Theses are used by nodes called root nodes and they are not part of the + * dataflow in the graph they are utility nodes, like the AI model used in a + * lang chain node. */ export function findSubgraph( graph: DirectedGraph, destinationNode: INode, trigger: INode, ): DirectedGraph { - const newGraph = new DirectedGraph(); + const subgraph = new DirectedGraph(); - findSubgraphRecursive(graph, destinationNode, destinationNode, trigger, newGraph, []); + findSubgraphRecursive(graph, destinationNode, destinationNode, trigger, subgraph, []); - return newGraph; + // For each node in the subgraph, if it has parent connections of a type that + // is not `Main` in the input graph, add the connections and the nodes + // connected to it to the subgraph + // + // Without this all AI related workflows would not work when executed + // partially, because all utility nodes would be missing. + for (const node of subgraph.getNodes().values()) { + const parentConnections = graph.getParentConnections(node); + + for (const connection of parentConnections) { + if (connection.type === NodeConnectionType.Main) { + continue; + } + + subgraph.addNodes(connection.from, connection.to); + subgraph.addConnection(connection); + } + } + + return subgraph; } diff --git a/packages/core/src/PartialExecutionUtils/recreateNodeExecutionStack.ts b/packages/core/src/PartialExecutionUtils/recreateNodeExecutionStack.ts index f2f1f4af68..4926becb79 100644 --- a/packages/core/src/PartialExecutionUtils/recreateNodeExecutionStack.ts +++ b/packages/core/src/PartialExecutionUtils/recreateNodeExecutionStack.ts @@ -64,7 +64,7 @@ export function recreateNodeExecutionStack( for (const startNode of startNodes) { const incomingStartNodeConnections = graph - .getDirectParents(startNode) + .getDirectParentConnections(startNode) .filter((c) => c.type === NodeConnectionType.Main); let incomingData: INodeExecutionData[][] = []; @@ -135,7 +135,7 @@ export function recreateNodeExecutionStack( // Check if the destinationNode has to be added as waiting // because some input data is already fully available const incomingDestinationNodeConnections = graph - .getDirectParents(destinationNode) + .getDirectParentConnections(destinationNode) .filter((c) => c.type === NodeConnectionType.Main); if (incomingDestinationNodeConnections !== undefined) { for (const connection of incomingDestinationNodeConnections) { diff --git a/packages/core/src/ScheduledTaskManager.ts b/packages/core/src/ScheduledTaskManager.ts index fd2bb525a9..00396903a5 100644 --- a/packages/core/src/ScheduledTaskManager.ts +++ b/packages/core/src/ScheduledTaskManager.ts @@ -30,8 +30,9 @@ export class ScheduledTaskManager { deregisterCrons(workflowId: string) { const cronJobs = this.cronJobs.get(workflowId) ?? []; - for (const cronJob of cronJobs) { - cronJob.stop(); + while (cronJobs.length) { + const cronJob = cronJobs.pop(); + if (cronJob) cronJob.stop(); } } diff --git a/packages/core/test/ScheduledTaskManager.test.ts b/packages/core/test/ScheduledTaskManager.test.ts index 3ff8837ca9..5166240856 100644 --- a/packages/core/test/ScheduledTaskManager.test.ts +++ b/packages/core/test/ScheduledTaskManager.test.ts @@ -56,8 +56,13 @@ describe('ScheduledTaskManager', () => { scheduledTaskManager.registerCron(workflow, everyMinute, onTick); scheduledTaskManager.registerCron(workflow, everyMinute, onTick); scheduledTaskManager.registerCron(workflow, everyMinute, onTick); + + expect(scheduledTaskManager.cronJobs.get(workflow.id)?.length).toBe(3); + scheduledTaskManager.deregisterCrons(workflow.id); + expect(scheduledTaskManager.cronJobs.get(workflow.id)?.length).toBe(0); + expect(onTick).not.toHaveBeenCalled(); jest.advanceTimersByTime(10 * 60 * 1000); // 10 minutes expect(onTick).not.toHaveBeenCalled(); diff --git a/packages/design-system/package.json b/packages/design-system/package.json index 42f2f75111..36fcf31528 100644 --- a/packages/design-system/package.json +++ b/packages/design-system/package.json @@ -1,6 +1,6 @@ { "name": "n8n-design-system", - "version": "1.52.1", + "version": "1.53.0", "main": "src/main.ts", "import": "src/main.ts", "scripts": { diff --git a/packages/design-system/src/components/N8nInputLabel/InputLabel.vue b/packages/design-system/src/components/N8nInputLabel/InputLabel.vue index 4a89233bf1..e522579713 100644 --- a/packages/design-system/src/components/N8nInputLabel/InputLabel.vue +++ b/packages/design-system/src/components/N8nInputLabel/InputLabel.vue @@ -33,7 +33,14 @@ const addTargetBlank = (html: string) =>