From b780436a6b445dc5951217b5a1f2c61b34961757 Mon Sep 17 00:00:00 2001 From: oleg Date: Thu, 14 Dec 2023 12:01:00 +0100 Subject: [PATCH 1/8] perf(editor): Improve canvas rendering performance (#8022) ## Summary - Refactor usage of `setSuspendDrawing`, removing it from loops and only calling it after batch operations are done - Batch adding of nodes to improve copy/paste and workflow load performance - Cache i18n calls - Debounce connections dragging handler if there are more than 20 nodes ## Related tickets and issues > Include links to **Linear ticket** or Github issue or Community forum post. Important in order to close *automatically* and provide context to reviewers. - https://community.n8n.io/t/slow-ui-in-big-scenarios/33830/8 --------- Signed-off-by: Oleg Ivaniv --- packages/editor-ui/src/components/Node.vue | 3 +- packages/editor-ui/src/plugins/i18n/index.ts | 20 ++- .../jsplumb/N8nAddInputEndpointRenderer.ts | 1 - .../jsplumb/N8nAddInputEndpointType.ts | 5 - .../plugins/jsplumb/N8nPlusEndpointType.ts | 6 - .../editor-ui/src/stores/workflows.store.ts | 3 - packages/editor-ui/src/views/NodeView.vue | 123 +++++++++++------- 7 files changed, 94 insertions(+), 67 deletions(-) diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index 8fe9b64379e3f..07a0b273e2410 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -637,8 +637,7 @@ export default defineComponent({ // so we only update it when necessary (when node is mounted and when it's opened and closed (isActive)) try { const nodeSubtitle = - this.nodeHelpers.getNodeSubtitle(this.data, this.nodeType, this.getCurrentWorkflow()) || - ''; + this.nodeHelpers.getNodeSubtitle(this.data, this.nodeType, this.workflow) || ''; this.nodeSubtitle = nodeSubtitle.includes(CUSTOM_API_CALL_KEY) ? '' : nodeSubtitle; } catch (e) { diff --git a/packages/editor-ui/src/plugins/i18n/index.ts b/packages/editor-ui/src/plugins/i18n/index.ts index 28b27ea3383c0..712add0cd2361 100644 --- a/packages/editor-ui/src/plugins/i18n/index.ts +++ b/packages/editor-ui/src/plugins/i18n/index.ts @@ -23,6 +23,8 @@ export const i18nInstance = createI18n({ }); export class I18nClass { + private baseTextCache = new Map(); + private get i18n() { return i18nInstance.global; } @@ -50,11 +52,25 @@ export class I18nClass { key: BaseTextKey, options?: { adjustToNumber?: number; interpolate?: { [key: string]: string } }, ): string { + // Create a unique cache key + const cacheKey = `${key}-${JSON.stringify(options)}`; + + // Check if the result is already cached + if (this.baseTextCache.has(cacheKey)) { + return this.baseTextCache.get(cacheKey) ?? key; + } + + let result: string; if (options?.adjustToNumber !== undefined) { - return this.i18n.tc(key, options.adjustToNumber, options?.interpolate).toString(); + result = this.i18n.tc(key, options.adjustToNumber, options?.interpolate ?? {}).toString(); + } else { + result = this.i18n.t(key, options?.interpolate ?? {}).toString(); } - return this.i18n.t(key, options?.interpolate).toString(); + // Store the result in the cache + this.baseTextCache.set(cacheKey, result); + + return result; } /** diff --git a/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointRenderer.ts b/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointRenderer.ts index c7102d755219a..54d4d98695598 100644 --- a/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointRenderer.ts +++ b/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointRenderer.ts @@ -70,7 +70,6 @@ export const register = () => { container.appendChild(unconnectedGroup); container.appendChild(defaultGroup); - endpointInstance.setupOverlays(); endpointInstance.setVisible(false); return container; diff --git a/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointType.ts b/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointType.ts index d80697fe8d21a..4bd54ae486b58 100644 --- a/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointType.ts +++ b/packages/editor-ui/src/plugins/jsplumb/N8nAddInputEndpointType.ts @@ -33,11 +33,6 @@ export class N8nAddInputEndpoint extends EndpointRepresentation { this.endpoint.getOverlays()[overlay].setVisible(visible); }); - this.setVisible(visible); - // Re-trigger the success state if label is set if (visible && this.label) { this.setSuccessOutput(this.label); } - this.instance.setSuspendDrawing(false); } setSuccessOutput(label: string) { diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index 5818dd4f5098a..a217641863410 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -894,8 +894,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, { }, }; } - - this.workflowExecutionPairedItemMappings = getPairedItemsMapping(this.workflowExecutionData); }, resetAllNodesIssues(): boolean { @@ -1130,7 +1128,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, { }; } this.workflowExecutionData.data!.resultData.runData[pushData.nodeName].push(pushData.data); - this.workflowExecutionPairedItemMappings = getPairedItemsMapping(this.workflowExecutionData); }, clearNodeExecutionData(nodeName: string): void { if (!this.workflowExecutionData?.data) { diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 88058420803d0..62a587743e1b0 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -75,6 +75,7 @@ @removeNode="(name) => removeNode(name, true)" :key="`${stickyData.id}_sticky`" :name="stickyData.name" + :workflow="currentWorkflowObject" :isReadOnly="isReadOnlyRoute || readOnlyEnv" :instance="instance" :isActive="!!activeNode && activeNode.name === stickyData.name" @@ -732,6 +733,7 @@ export default defineComponent({ showTriggerMissingTooltip: false, workflowData: null as INewWorkflowData | null, activeConnection: null as null | Connection, + isInsertingNodes: false, isProductionExecutionPreview: false, enterTimer: undefined as undefined | ReturnType, exitTimer: undefined as undefined | ReturnType, @@ -2735,15 +2737,6 @@ export default defineComponent({ }); }, ); - setTimeout(() => { - NodeViewUtils.addConnectionTestData( - info.source, - info.target, - info.connection?.connector?.hasOwnProperty('canvas') - ? (info.connection.connector.canvas as HTMLElement) - : undefined, - ); - }, 0); const endpointArrow = NodeViewUtils.getOverlay( info.connection, @@ -2763,14 +2756,44 @@ export default defineComponent({ if (!this.suspendRecordingDetachedConnections) { this.historyStore.pushCommandToUndo(new AddConnectionCommand(connectionData)); } - this.nodeHelpers.updateNodesInputIssues(); - this.resetEndpointsErrors(); + // When we add multiple nodes, this event could be fired hundreds of times for large workflows. + // And because the updateNodesInputIssues() method is quite expensive, we only call it if not in insert mode + if (!this.isInsertingNodes) { + this.nodeHelpers.updateNodesInputIssues(); + this.resetEndpointsErrors(); + setTimeout(() => { + NodeViewUtils.addConnectionTestData( + info.source, + info.target, + info.connection?.connector?.hasOwnProperty('canvas') + ? (info.connection.connector.canvas as HTMLElement) + : undefined, + ); + }, 0); + } } } catch (e) { console.error(e); } }, + addConectionsTestData() { + this.instance.connections.forEach((connection) => { + NodeViewUtils.addConnectionTestData( + connection.source, + connection.target, + connection?.connector?.hasOwnProperty('canvas') + ? (connection?.connector.canvas as HTMLElement) + : undefined, + ); + }); + }, onDragMove() { + const totalNodes = this.nodes.length; + void this.callDebounced('updateConnectionsOverlays', { + debounceTime: totalNodes > 20 ? 200 : 0, + }); + }, + updateConnectionsOverlays() { this.instance?.connections.forEach((connection) => { NodeViewUtils.showOrHideItemsLabel(connection); NodeViewUtils.showOrHideMidpointArrow(connection); @@ -3899,7 +3922,7 @@ export default defineComponent({ if (!nodes?.length) { return; } - + this.isInsertingNodes = true; // Before proceeding we must check if all nodes contain the `properties` attribute. // Nodes are loaded without this information so we must make sure that all nodes // being added have this information. @@ -3957,60 +3980,64 @@ export default defineComponent({ // check and match credentials, apply new format if old is used this.matchCredentials(node); - this.workflowsStore.addNode(node); if (trackHistory) { this.historyStore.pushCommandToUndo(new AddNodeCommand(node)); } }); - // Wait for the node to be rendered + // Wait for the nodes to be rendered await this.$nextTick(); - // Suspend drawing this.instance?.setSuspendDrawing(true); - // Load the connections - if (connections !== undefined) { - let connectionData; - for (const sourceNode of Object.keys(connections)) { - for (const type of Object.keys(connections[sourceNode])) { - for ( - let sourceIndex = 0; - sourceIndex < connections[sourceNode][type].length; - sourceIndex++ - ) { - const outwardConnections = connections[sourceNode][type][sourceIndex]; - if (!outwardConnections) { - continue; - } + if (connections) { + await this.addConnections(connections); + } + // Add the node issues at the end as the node-connections are required + this.nodeHelpers.refreshNodeIssues(); + this.nodeHelpers.updateNodesInputIssues(); + this.resetEndpointsErrors(); + this.isInsertingNodes = false; + + // Now it can draw again + this.instance?.setSuspendDrawing(false, true); + }, + async addConnections(connections: IConnections) { + const batchedConnectionData: Array<[IConnection, IConnection]> = []; + + for (const sourceNode in connections) { + for (const type in connections[sourceNode]) { + connections[sourceNode][type].forEach((outwardConnections, sourceIndex) => { + if (outwardConnections) { outwardConnections.forEach((targetData) => { - connectionData = [ - { - node: sourceNode, - type, - index: sourceIndex, - }, - { - node: targetData.node, - type: targetData.type, - index: targetData.index, - }, - ] as [IConnection, IConnection]; - - this.__addConnection(connectionData); + batchedConnectionData.push([ + { node: sourceNode, type, index: sourceIndex }, + { node: targetData.node, type: targetData.type, index: targetData.index }, + ]); }); } - } + }); } } - // Add the node issues at the end as the node-connections are required - void this.nodeHelpers.refreshNodeIssues(); + // Process the connections in batches + await this.processConnectionBatch(batchedConnectionData); + setTimeout(this.addConectionsTestData, 0); + }, - // Now it can draw again - this.instance?.setSuspendDrawing(false, true); + async processConnectionBatch(batchedConnectionData: Array<[IConnection, IConnection]>) { + const batchSize = 100; + + for (let i = 0; i < batchedConnectionData.length; i += batchSize) { + const batch = batchedConnectionData.slice(i, i + batchSize); + + batch.forEach((connectionData) => { + this.__addConnection(connectionData); + }); + } }, + async addNodesToWorkflow(data: IWorkflowDataUpdate): Promise { // Because nodes with the same name maybe already exist, it could // be needed that they have to be renamed. Also could it be possible From 3436f15e8292de9598793b667213fa6e57b72535 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Thu, 14 Dec 2023 13:13:02 +0200 Subject: [PATCH 2/8] fix(editor): Fix dialogVisibleChanged hooks event when meta.value is undefined (no-changelog) (#7986) ## Summary Provide details about your pull request and what it adds, fixes, or changes. Photos and videos are recommended. In specific scenarios `meta.value` can come as `undefined`, causing the hook to fail. #### How to test the change: 1. Close Expression editor modal dialog with empty value ## Issues fixed Include links to Github issue or Community forum post or **Linear ticket**: > Important in order to close automatically and provide context to reviewers https://linear.app/n8n/issue/N8N-7084/typeerror-evalueslice-is-not-a-function-in-evalueslice1-evalueslice-is https://n8nio.sentry.io/issues/4715787194/ ## Review / Merge checklist - [x] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md)) - [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. - [ ] Tests included. > A bug is not considered fixed, unless a test is added to prevent it from happening again. A feature is not complete without tests. > > *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e). --- packages/editor-ui/src/hooks/cloud.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/editor-ui/src/hooks/cloud.ts b/packages/editor-ui/src/hooks/cloud.ts index f4990cba6338e..4c68bc4d284fd 100644 --- a/packages/editor-ui/src/hooks/cloud.ts +++ b/packages/editor-ui/src/hooks/cloud.ts @@ -268,7 +268,7 @@ export const n8nCloudHooks: PartialDeep = { dialogVisibleChanged: [ (_, meta) => { const segmentStore = useSegment(); - const currentValue = meta.value.slice(1); + const currentValue = meta.value?.slice(1) ?? ''; let isValueDefault = false; switch (typeof meta.parameter.default) { From fcb8b91f37e1fb0ef42f411c84390180e1ed7bbe Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Thu, 14 Dec 2023 12:37:03 +0100 Subject: [PATCH 3/8] fix(editor): Disable auto scroll and list size check when clicking on executions (#7983) ## Summary Auto scrolling to the active execution is unnecessary as well as checking if more items can be loaded. #### How to test the change: 1. Open a workflow with a long list of executions (preferably with more executions that fits in the list when first rendered and need to use infinite scroll to load more items) 2. Go to executions tabs 3. Scroll to the bottom 4. Click on an item that is visible only if the list is scrolled down. The list should not be scrolled to the active item automatically --- .../src/components/ExecutionsView/ExecutionsSidebar.vue | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/editor-ui/src/components/ExecutionsView/ExecutionsSidebar.vue b/packages/editor-ui/src/components/ExecutionsView/ExecutionsSidebar.vue index d8c26cd5dcd85..2f6b482b73d60 100644 --- a/packages/editor-ui/src/components/ExecutionsView/ExecutionsSidebar.vue +++ b/packages/editor-ui/src/components/ExecutionsView/ExecutionsSidebar.vue @@ -120,10 +120,6 @@ export default defineComponent({ this.$router.go(-1); } }, - 'workflowsStore.activeWorkflowExecution'() { - this.checkListSize(); - this.scrollToActiveCard(); - }, }, mounted() { // On larger screens, we need to load more then first page of executions From 329e5bf9eed8556aba2bbd50bad9dbd6d3b373ad Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Thu, 14 Dec 2023 13:36:36 +0100 Subject: [PATCH 4/8] fix(editor): Add back credential `use` permission (#8023) ## Summary A shared credential is not selectable in NDV Aim If a credential is shared with a user they should be able to select it in node details view --- cypress/e2e/17-sharing.cy.ts | 20 +++++++++++++++++++ .../src/composables/useNodeHelpers.ts | 2 +- packages/editor-ui/src/permissions.ts | 4 ++++ packages/editor-ui/src/stores/users.store.ts | 2 +- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/17-sharing.cy.ts b/cypress/e2e/17-sharing.cy.ts index 77d9fd92cfc4a..71f41250eca77 100644 --- a/cypress/e2e/17-sharing.cy.ts +++ b/cypress/e2e/17-sharing.cy.ts @@ -98,6 +98,26 @@ describe('Sharing', { disableAutoLogin: true }, () => { ndv.actions.close(); }); + it('should open W1, add node using C2 as U2', () => { + cy.signin(INSTANCE_MEMBERS[0]); + + cy.visit(workflowsPage.url); + workflowsPage.getters.workflowCards().should('have.length', 2); + workflowsPage.getters.workflowCard('Workflow W1').click(); + workflowPage.actions.addNodeToCanvas('Airtable', true, true); + ndv.getters.credentialInput().find('input').should('have.value', 'Credential C2'); + ndv.actions.close(); + workflowPage.actions.saveWorkflowOnButtonClick(); + + workflowPage.actions.openNode('Notion'); + ndv.getters + .credentialInput() + .find('input') + .should('have.value', 'Credential C1') + .should('be.enabled'); + ndv.actions.close(); + }); + it('should not have access to W2, as U3', () => { cy.signin(INSTANCE_MEMBERS[1]); diff --git a/packages/editor-ui/src/composables/useNodeHelpers.ts b/packages/editor-ui/src/composables/useNodeHelpers.ts index fda8a2fa35bc1..93b262381f047 100644 --- a/packages/editor-ui/src/composables/useNodeHelpers.ts +++ b/packages/editor-ui/src/composables/useNodeHelpers.ts @@ -420,7 +420,7 @@ export function useNodeHelpers() { .getCredentialsByType(credentialTypeDescription.name) .filter((credential: ICredentialsResponse) => { const permissions = getCredentialPermissions(currentUser, credential); - return permissions.read; + return permissions.use; }); if (userCredentials === null) { diff --git a/packages/editor-ui/src/permissions.ts b/packages/editor-ui/src/permissions.ts index dfbec10fb4530..639ced565978f 100644 --- a/packages/editor-ui/src/permissions.ts +++ b/packages/editor-ui/src/permissions.ts @@ -101,6 +101,10 @@ export const getCredentialPermissions = (user: IUser | null, credential: ICreden test: (permissions) => hasPermission(['rbac'], { rbac: { scope: 'credential:delete' } }) || !!permissions.isOwner, }, + { + name: 'use', + test: (permissions) => !!permissions.isOwner || !!permissions.isSharee, + }, ]; return parsePermissionsTable(user, table); diff --git a/packages/editor-ui/src/stores/users.store.ts b/packages/editor-ui/src/stores/users.store.ts index 312a5ec0db83e..6d20dda8e4611 100644 --- a/packages/editor-ui/src/stores/users.store.ts +++ b/packages/editor-ui/src/stores/users.store.ts @@ -95,7 +95,7 @@ export const useUsersStore = defineStore(STORES.USERS, { return (resource: ICredentialsResponse): boolean => { const permissions = getCredentialPermissions(this.currentUser, resource); - return permissions.read; + return permissions.use; }; }, }, From 53c0b49d15047461e3b65baed65c9d76dff99539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 14 Dec 2023 16:16:12 +0100 Subject: [PATCH 5/8] fix(core): Initialize queue once in queue mode (#8025) We're initializing the queue twice because of a [bad merge](https://github.com/n8n-io/n8n/pull/7303/commits/2c6347453806f69b9941389773c14cd0afdd7173). No associated known bugs but no need to init the queue twice. We should follow up by investigating if any pending bugs can be associated to this. --- packages/cli/src/commands/worker.ts | 1 - .../integration/commands/worker.cmd.test.ts | 18 +++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index 856b4fa43b39b..97a41988ec4a4 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -292,7 +292,6 @@ export class Worker extends BaseCommand { this.logger.debug('Queue init complete'); await this.initOrchestration(); this.logger.debug('Orchestration init complete'); - await this.initQueue(); await Container.get(OrchestrationWorkerService).publishToEventLog( new EventMessageGeneric({ diff --git a/packages/cli/test/integration/commands/worker.cmd.test.ts b/packages/cli/test/integration/commands/worker.cmd.test.ts index 3092e80f6eac2..ee5ac646d2d02 100644 --- a/packages/cli/test/integration/commands/worker.cmd.test.ts +++ b/packages/cli/test/integration/commands/worker.cmd.test.ts @@ -65,15 +65,15 @@ test('worker initializes all its components', async () => { expect(worker.queueModeId).toBeDefined(); expect(worker.queueModeId).toContain('worker'); expect(worker.queueModeId.length).toBeGreaterThan(15); - expect(worker.initLicense).toHaveBeenCalled(); - expect(worker.initBinaryDataService).toHaveBeenCalled(); - expect(worker.initExternalHooks).toHaveBeenCalled(); - expect(worker.initExternalSecrets).toHaveBeenCalled(); - expect(worker.initEventBus).toHaveBeenCalled(); - expect(worker.initOrchestration).toHaveBeenCalled(); - expect(OrchestrationHandlerWorkerService.prototype.initSubscriber).toHaveBeenCalled(); - expect(OrchestrationWorkerService.prototype.publishToEventLog).toHaveBeenCalled(); - expect(worker.initQueue).toHaveBeenCalled(); + expect(worker.initLicense).toHaveBeenCalledTimes(1); + expect(worker.initBinaryDataService).toHaveBeenCalledTimes(1); + expect(worker.initExternalHooks).toHaveBeenCalledTimes(1); + expect(worker.initExternalSecrets).toHaveBeenCalledTimes(1); + expect(worker.initEventBus).toHaveBeenCalledTimes(1); + expect(worker.initOrchestration).toHaveBeenCalledTimes(1); + expect(OrchestrationHandlerWorkerService.prototype.initSubscriber).toHaveBeenCalledTimes(1); + expect(OrchestrationWorkerService.prototype.publishToEventLog).toHaveBeenCalledTimes(1); + expect(worker.initQueue).toHaveBeenCalledTimes(1); jest.restoreAllMocks(); }); From c5e6ba8cdd4a8f117ccc2e89e55497117156d8af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 14 Dec 2023 18:13:12 +0100 Subject: [PATCH 6/8] fix(core): Restore workflow ID during execution creation (#8031) ## Summary Restore workflow ID during execution creation removed by [this PR](https://github.com/n8n-io/n8n/pull/8002/files#diff-c8cbb62ca9ab2ae45e5f565cd8c63fff6475809a6241ea0b90acc575615224af). The missing workflow ID, and more generally the fact that `workflow.id` is optional when it should not be, causes `PermissionChecker.check` to misreport a credential as inaccessible when it should be accessible. More generally, start reporting ID-less workflows so we can root them out and prevent this at type level. ## Related tickets and issues https://n8nio.slack.com/archives/C035KBDA917/p1702539465555529 --- packages/cli/src/commands/worker.ts | 18 ++++++++++++++++-- .../repositories/execution.repository.ts | 2 +- .../repositories/execution.repository.test.ts | 1 + packages/workflow/src/Workflow.ts | 10 +++++++++- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index 97a41988ec4a4..07b4ce98fe491 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -13,7 +13,13 @@ import type { INodeTypes, IRun, } from 'n8n-workflow'; -import { Workflow, NodeOperationError, sleep, ApplicationError } from 'n8n-workflow'; +import { + Workflow, + NodeOperationError, + sleep, + ApplicationError, + ErrorReporterProxy as EventReporter, +} from 'n8n-workflow'; import * as Db from '@/Db'; import * as ResponseHelper from '@/ResponseHelper'; @@ -130,7 +136,15 @@ export class Worker extends BaseCommand { { extra: { executionId } }, ); } - const workflowId = fullExecutionData.workflowData.id!; + const workflowId = fullExecutionData.workflowData.id!; // @tech_debt Ensure this is not optional + + if (!workflowId) { + EventReporter.report('Detected ID-less workflow', { + level: 'info', + extra: { execution: fullExecutionData }, + }); + } + this.logger.info( `Start job: ${job.id} (Workflow ID: ${workflowId} | Execution: ${executionId})`, ); diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 7598ff788f922..093430326763d 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -220,7 +220,7 @@ export class ExecutionRepository extends Repository { const { connections, nodes, name } = workflowData ?? {}; await this.executionDataRepository.insert({ executionId, - workflowData: { connections, nodes, name }, + workflowData: { connections, nodes, name, id: workflowData?.id }, data: stringify(data), }); return String(executionId); diff --git a/packages/cli/test/integration/database/repositories/execution.repository.test.ts b/packages/cli/test/integration/database/repositories/execution.repository.test.ts index d16645367f66b..62cb57d2b8ac5 100644 --- a/packages/cli/test/integration/database/repositories/execution.repository.test.ts +++ b/packages/cli/test/integration/database/repositories/execution.repository.test.ts @@ -43,6 +43,7 @@ describe('ExecutionRepository', () => { const executionDataRepo = Container.get(ExecutionDataRepository); const executionData = await executionDataRepo.findOneBy({ executionId }); expect(executionData?.workflowData).toEqual({ + id: workflow.id, connections: workflow.connections, nodes: workflow.nodes, name: workflow.name, diff --git a/packages/workflow/src/Workflow.ts b/packages/workflow/src/Workflow.ts index 6ec06db20cadd..f09256d8ca1be 100644 --- a/packages/workflow/src/Workflow.ts +++ b/packages/workflow/src/Workflow.ts @@ -52,6 +52,7 @@ import { RoutingNode } from './RoutingNode'; import { Expression } from './Expression'; import { NODES_WITH_RENAMABLE_CONTENT } from './Constants'; import { ApplicationError } from './errors/application.error'; +import * as EventReporter from './ErrorReporterProxy'; function dedupe(arr: T[]): T[] { return [...new Set(arr)]; @@ -94,7 +95,14 @@ export class Workflow { settings?: IWorkflowSettings; pinData?: IPinData; }) { - this.id = parameters.id as string; + if (!parameters.id) { + EventReporter.report('Detected ID-less workflow', { + level: 'info', + extra: { parameters }, + }); + } + + this.id = parameters.id as string; // @tech_debt Ensure this is not optional this.name = parameters.name; this.nodeTypes = parameters.nodeTypes; this.pinData = parameters.pinData; From f432d865243dee3d6ce803004fa51e92936df9e8 Mon Sep 17 00:00:00 2001 From: Deborah Date: Thu, 14 Dec 2023 19:15:38 +0000 Subject: [PATCH 7/8] docs: Two small UI copy fixes in sub-nodes (#8032) --- .../OutputParserItemList/OutputParserItemList.node.ts | 2 +- .../nodes-langchain/nodes/tools/ToolSerpApi/ToolSerpApi.node.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserItemList/OutputParserItemList.node.ts b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserItemList/OutputParserItemList.node.ts index 57896165c8eb4..24327b2970e2f 100644 --- a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserItemList/OutputParserItemList.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserItemList/OutputParserItemList.node.ts @@ -55,7 +55,7 @@ export class OutputParserItemList implements INodeType { type: 'number', default: -1, description: - 'Defines many many items should be returned maximally. If set to -1, there is no limit.', + 'Defines how many items should be returned maximally. If set to -1, there is no limit.', }, // For that to be easily possible the metadata would have to be returned and be able to be read. // Would also be possible with a wrapper but that would be even more hacky and the output types diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolSerpApi/ToolSerpApi.node.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolSerpApi/ToolSerpApi.node.ts index 44e75881e589f..77147abdc2868 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolSerpApi/ToolSerpApi.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolSerpApi/ToolSerpApi.node.ts @@ -97,7 +97,7 @@ export class ToolSerpApi implements INodeType { type: 'string', default: 'google.com', description: - 'Defines the country to use for search. Head to Google countries page for a full list of supported countries.', + 'Defines the domain to use for search. Head to Google domains page for a full list of supported domains.', }, { displayName: 'Language', From f18bc5f4b7779ab49e44f223c3f2c0f7e4341db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 15 Dec 2023 10:56:35 +0100 Subject: [PATCH 8/8] refactor(core): Warn on sqlite DB detected during init on queue mode (#8034) When setting up queue mode, it is easy to overlook that not exporting Postgres env vars will default the worker to use sqlite, which will fail during execution with a non-obvious error. Hence add warnings when starting a worker with an incompatible DB type. --- packages/cli/src/commands/BaseCommand.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/cli/src/commands/BaseCommand.ts b/packages/cli/src/commands/BaseCommand.ts index 413c2e53072e7..fc8b28c0df4fb 100644 --- a/packages/cli/src/commands/BaseCommand.ts +++ b/packages/cli/src/commands/BaseCommand.ts @@ -80,6 +80,12 @@ export abstract class BaseCommand extends Command { ); } + if (config.getEnv('executions.mode') === 'queue' && dbType === 'sqlite') { + this.logger.warn( + 'Queue mode is not officially supported with sqlite. Please switch to PostgreSQL.', + ); + } + if ( process.env.N8N_BINARY_DATA_TTL ?? process.env.N8N_PERSISTED_BINARY_DATA_TTL ??