From 76c04815f7f53bf6b4c06bbe5afa52f51f28750d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 20 Oct 2023 18:26:33 +0200 Subject: [PATCH 1/2] fix(core): Reduce logging overhead for levels that do not output (#7479) all current logging calls execute `callsites()` to figure out what code tried to log. This happens even for logging methods that aren't supposed to create any output. Under moderate load, this can take up quite a lot of resources. This PR changes the logger to make all ignorable logging methods a No-Op. In a small benchmark with a simple webhook, with log-level set to `warn`, and using `ab -c 50 -n 500 http://localhost:5678/webhook/testing`, these were the response times: ### Before ![Before](https://github.com/n8n-io/n8n/assets/196144/01680fd9-3d2a-4f7f-bb1c-5b03bd7d5bc3) ### After ![After](https://github.com/n8n-io/n8n/assets/196144/ccacb20a-48ca-455a-a8cb-098c9c0e352e) --- packages/cli/src/Logger.ts | 20 +++++++++++++++----- packages/core/bin/generate-known | 5 +---- packages/core/bin/generate-ui-types | 5 +---- packages/workflow/src/LoggerProxy.ts | 10 +++++----- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/Logger.ts b/packages/cli/src/Logger.ts index 2a113cb8cc892..0395946f62b59 100644 --- a/packages/cli/src/Logger.ts +++ b/packages/cli/src/Logger.ts @@ -9,22 +9,32 @@ import callsites from 'callsites'; import { basename } from 'path'; import config from '@/config'; +const noOp = () => {}; +const levelNames = ['debug', 'verbose', 'info', 'warn', 'error'] as const; + export class Logger implements ILogger { private logger: winston.Logger; constructor() { const level = config.getEnv('logs.level'); - const output = config - .getEnv('logs.output') - .split(',') - .map((output) => output.trim()); - this.logger = winston.createLogger({ level, silent: level === 'silent', }); + // Change all methods with higher log-level to no-op + for (const levelName of levelNames) { + if (this.logger.levels[levelName] > this.logger.levels[level]) { + Object.defineProperty(this, levelName, { value: noOp }); + } + } + + const output = config + .getEnv('logs.output') + .split(',') + .map((output) => output.trim()); + if (output.includes('console')) { let format: winston.Logform.Format; if (['debug', 'verbose'].includes(level)) { diff --git a/packages/core/bin/generate-known b/packages/core/bin/generate-known index 5cabfa9b801d9..df38a7ecc6d13 100755 --- a/packages/core/bin/generate-known +++ b/packages/core/bin/generate-known @@ -6,10 +6,7 @@ const { LoggerProxy } = require('n8n-workflow'); const { packageDir, writeJSON } = require('./common'); const { loadClassInIsolation } = require('../dist/ClassLoader'); -LoggerProxy.init({ - log: console.log.bind(console), - warn: console.warn.bind(console), -}); +LoggerProxy.init(console); const loadClass = (sourcePath) => { try { diff --git a/packages/core/bin/generate-ui-types b/packages/core/bin/generate-ui-types index f320dea3904f2..317c0e121edc2 100755 --- a/packages/core/bin/generate-ui-types +++ b/packages/core/bin/generate-ui-types @@ -4,10 +4,7 @@ const { LoggerProxy, NodeHelpers } = require('n8n-workflow'); const { PackageDirectoryLoader } = require('../dist/DirectoryLoader'); const { packageDir, writeJSON } = require('./common'); -LoggerProxy.init({ - log: console.log.bind(console), - warn: console.warn.bind(console), -}); +LoggerProxy.init(console); function findReferencedMethods(obj, refs = {}, latestName = '') { for (const key in obj) { diff --git a/packages/workflow/src/LoggerProxy.ts b/packages/workflow/src/LoggerProxy.ts index 24df176fff8e0..875e1e50a0ec1 100644 --- a/packages/workflow/src/LoggerProxy.ts +++ b/packages/workflow/src/LoggerProxy.ts @@ -22,21 +22,21 @@ export function log(type: LogTypes, message: string, meta: object = {}) { // Convenience methods below export function debug(message: string, meta: object = {}) { - getInstance().log('debug', message, meta); + getInstance().debug(message, meta); } export function info(message: string, meta: object = {}) { - getInstance().log('info', message, meta); + getInstance().info(message, meta); } export function error(message: string, meta: object = {}) { - getInstance().log('error', message, meta); + getInstance().error(message, meta); } export function verbose(message: string, meta: object = {}) { - getInstance().log('verbose', message, meta); + getInstance().verbose(message, meta); } export function warn(message: string, meta: object = {}) { - getInstance().log('warn', message, meta); + getInstance().warn(message, meta); } From 450e0cc66abbe57697f66835a837e53b5eb883a3 Mon Sep 17 00:00:00 2001 From: OlegIvaniv Date: Mon, 23 Oct 2023 10:07:33 +0200 Subject: [PATCH 2/2] fix(editor): Fix connections disappearing after reactivating canvas and renaming a node (#7483) Github issue / Community forum post (link here to close automatically): - https://community.n8n.io/t/1-11-1-possible-bug-all-nodes-randomly-losing-their-connectors/31856 - https://community.n8n.io/t/lines-between-nodes-have-disappeared/31846 --------- Signed-off-by: Oleg Ivaniv --- cypress/e2e/12-canvas.cy.ts | 33 +++++++++++++++++++++++ packages/editor-ui/src/views/NodeView.vue | 3 ++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/cypress/e2e/12-canvas.cy.ts b/cypress/e2e/12-canvas.cy.ts index 75e05fc712ff6..9e09d27089b6a 100644 --- a/cypress/e2e/12-canvas.cy.ts +++ b/cypress/e2e/12-canvas.cy.ts @@ -8,8 +8,10 @@ import { MERGE_NODE_NAME, } from './../constants'; import { WorkflowPage as WorkflowPageClass } from '../pages/workflow'; +import { WorkflowExecutionsTab } from '../pages'; const WorkflowPage = new WorkflowPageClass(); +const ExecutionsTab = new WorkflowExecutionsTab(); const DEFAULT_ZOOM_FACTOR = 1; const ZOOM_IN_X1_FACTOR = 1.25; // Zoom in factor after one click @@ -306,4 +308,35 @@ describe('Canvas Node Manipulation and Navigation', () => { WorkflowPage.getters.canvasNodes().should('have.length', 3); WorkflowPage.getters.nodeConnections().should('have.length', 1); }); + + // ADO-1240: Connections would get deleted after activating and deactivating NodeView + it('should preserve connections after rename & node-view switch', () => { + WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); + WorkflowPage.actions.executeWorkflow(); + + ExecutionsTab.actions.switchToExecutionsTab(); + ExecutionsTab.getters.successfulExecutionListItems().should('have.length', 1); + + ExecutionsTab.actions.switchToEditorTab(); + + ExecutionsTab.actions.switchToExecutionsTab(); + ExecutionsTab.getters.successfulExecutionListItems().should('have.length', 1); + + ExecutionsTab.actions.switchToEditorTab(); + WorkflowPage.getters.canvasNodes().should('have.length', 2); + + WorkflowPage.getters.canvasNodes().last().click(); + cy.get('body').trigger('keydown', { key: 'F2' }); + cy.get('.rename-prompt').should('be.visible'); + cy.get('body').type(RENAME_NODE_NAME); + cy.get('body').type('{enter}'); + WorkflowPage.getters.canvasNodeByName(RENAME_NODE_NAME).should('exist'); + // Make sure all connections are there after save & reload + WorkflowPage.actions.saveWorkflowOnButtonClick(); + cy.reload(); + cy.waitForLoad(); + WorkflowPage.getters.canvasNodes().should('have.length', 2); + cy.get('.rect-input-endpoint.jtk-endpoint-connected').should('have.length', 1); + }) }); diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 75ad41b5b417e..028537a67cdd0 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -223,6 +223,7 @@ import { WORKFLOW_LM_CHAT_MODAL_KEY, AI_NODE_CREATOR_VIEW, DRAG_EVENT_DATA_KEY, + UPDATE_WEBHOOK_ID_NODE_TYPES, } from '@/constants'; import { copyPaste } from '@/mixins/copyPaste'; import { externalHooks } from '@/mixins/externalHooks'; @@ -345,7 +346,6 @@ import { EVENT_ADD_INPUT_ENDPOINT_CLICK } from '@/plugins/jsplumb/N8nAddInputEnd import { sourceControlEventBus } from '@/event-bus/source-control'; import { getConnectorPaintStyleData, OVERLAY_ENDPOINT_ARROW_ID } from '@/utils/nodeViewUtils'; import { useViewStacks } from '@/components/Node/NodeCreator/composables/useViewStacks'; -import { UPDATE_WEBHOOK_ID_NODE_TYPES } from '@/constants'; interface AddNodeOptions { position?: XYPosition; @@ -2894,6 +2894,7 @@ export default defineComponent({ this.instance.unbind(EVENT_CONNECTION_DETACHED, this.onConnectionDragAbortDetached); this.instance.unbind(EVENT_PLUS_ENDPOINT_CLICK, this.onPlusEndpointClick); this.instance.unbind(EVENT_ADD_INPUT_ENDPOINT_CLICK, this.onAddInputEndpointClick); + this.eventsAttached = false; }, unbindEndpointEventListeners(bind = true) { if (this.instance) {