diff --git a/packages/frontend/editor-ui/src/composables/useCanvasOperations.test.ts b/packages/frontend/editor-ui/src/composables/useCanvasOperations.test.ts index 651daa3e91..39ebd8a320 100644 --- a/packages/frontend/editor-ui/src/composables/useCanvasOperations.test.ts +++ b/packages/frontend/editor-ui/src/composables/useCanvasOperations.test.ts @@ -51,11 +51,15 @@ import { useTelemetry } from './useTelemetry'; import { useToast } from '@/composables/useToast'; import * as nodeHelpers from '@/composables/useNodeHelpers'; +import { TelemetryHelpers } from 'n8n-workflow'; + vi.mock('n8n-workflow', async (importOriginal) => { - const actual = await importOriginal<{}>(); + // eslint-disable-next-line @typescript-eslint/consistent-type-imports + const actual = await importOriginal(); return { ...actual, TelemetryHelpers: { + ...actual.TelemetryHelpers, generateNodesGraph: vi.fn().mockReturnValue({ nodeGraph: { nodes: [], @@ -2726,34 +2730,6 @@ describe('useCanvasOperations', () => { }); }); - describe('duplicateNodes', () => { - it('should duplicate nodes', async () => { - const workflowsStore = mockedStore(useWorkflowsStore); - const nodeTypesStore = useNodeTypesStore(); - const nodeTypeDescription = mockNodeTypeDescription({ name: SET_NODE_TYPE }); - - nodeTypesStore.nodeTypes = { - [SET_NODE_TYPE]: { 1: nodeTypeDescription }, - }; - - const nodes = buildImportNodes(); - workflowsStore.setNodes(nodes); - workflowsStore.getNodesByIds.mockReturnValue(nodes); - workflowsStore.outgoingConnectionsByNodeName.mockReturnValue({}); - - const workflowObject = createTestWorkflowObject(workflowsStore.workflow); - workflowsStore.workflowObject = workflowObject; - workflowsStore.createWorkflowObject.mockReturnValue(workflowObject); - - const canvasOperations = useCanvasOperations(); - const duplicatedNodeIds = await canvasOperations.duplicateNodes(['1', '2']); - - expect(duplicatedNodeIds.length).toBe(2); - expect(duplicatedNodeIds).not.toContain('1'); - expect(duplicatedNodeIds).not.toContain('2'); - }); - }); - describe('copyNodes', () => { it('should copy nodes', async () => { const workflowsStore = mockedStore(useWorkflowsStore); @@ -3420,6 +3396,69 @@ describe('useCanvasOperations', () => { }); }); + describe('duplicateNodes', () => { + it('should duplicate nodes', async () => { + const workflowsStore = mockedStore(useWorkflowsStore); + const nodeTypesStore = useNodeTypesStore(); + const nodeTypeDescription = mockNodeTypeDescription({ name: SET_NODE_TYPE }); + + nodeTypesStore.nodeTypes = { + [SET_NODE_TYPE]: { 1: nodeTypeDescription }, + }; + + const nodes = buildImportNodes(); + workflowsStore.setNodes(nodes); + workflowsStore.getNodesByIds.mockReturnValue(nodes); + workflowsStore.outgoingConnectionsByNodeName.mockReturnValue({}); + + const workflowObject = createTestWorkflowObject(workflowsStore.workflow); + workflowsStore.workflowObject = workflowObject; + workflowsStore.createWorkflowObject.mockReturnValue(workflowObject); + + const canvasOperations = useCanvasOperations(); + const duplicatedNodeIds = await canvasOperations.duplicateNodes(['1', '2']); + + expect(duplicatedNodeIds.length).toBe(2); + expect(duplicatedNodeIds).not.toContain('1'); + expect(duplicatedNodeIds).not.toContain('2'); + }); + + it('should not crash when TelemetryHelpers.generateNodesGraph throws error', async () => { + const workflowsStore = mockedStore(useWorkflowsStore); + const telemetry = useTelemetry(); + const nodeTypesStore = useNodeTypesStore(); + const nodeTypeDescription = mockNodeTypeDescription({ name: SET_NODE_TYPE }); + + nodeTypesStore.nodeTypes = { + [SET_NODE_TYPE]: { 1: nodeTypeDescription }, + }; + + const nodes = buildImportNodes(); + workflowsStore.setNodes(nodes); + workflowsStore.getNodesByIds.mockReturnValue(nodes); + workflowsStore.outgoingConnectionsByNodeName.mockReturnValue({}); + + const workflowObject = createTestWorkflowObject(workflowsStore.workflow); + workflowsStore.workflowObject = workflowObject; + workflowsStore.createWorkflowObject.mockReturnValue(workflowObject); + + // Mock TelemetryHelpers.generateNodesGraph to throw an error for this test + vi.mocked(TelemetryHelpers.generateNodesGraph).mockImplementationOnce(() => { + throw new Error('Telemetry generation failed'); + }); + + // This should not throw even when telemetry fails + const canvasOperations = useCanvasOperations(); + await expect(canvasOperations.duplicateNodes(['1', '2'])).resolves.not.toThrow(); + + // Telemetry should not be tracked when generation fails + expect(telemetry.track).not.toHaveBeenCalledWith( + 'User imported workflow', + expect.any(Object), + ); + }); + }); + describe('importTemplate', () => { it('should import template to canvas', async () => { const projectsStore = mockedStore(useProjectsStore); diff --git a/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts index 0ffec15e53..f1713391ee 100644 --- a/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts @@ -1916,37 +1916,41 @@ export function useCanvasOperations() { removeUnknownCredentials(workflowData); - const nodeGraph = JSON.stringify( - TelemetryHelpers.generateNodesGraph( - workflowData as IWorkflowBase, - workflowHelpers.getNodeTypes(), - { - nodeIdMap, - sourceInstanceId: - workflowData.meta && workflowData.meta.instanceId !== rootStore.instanceId - ? workflowData.meta.instanceId - : '', - isCloudDeployment: settingsStore.isCloudDeployment, - }, - ).nodeGraph, - ); + try { + const nodeGraph = JSON.stringify( + TelemetryHelpers.generateNodesGraph( + workflowData as IWorkflowBase, + workflowHelpers.getNodeTypes(), + { + nodeIdMap, + sourceInstanceId: + workflowData.meta && workflowData.meta.instanceId !== rootStore.instanceId + ? workflowData.meta.instanceId + : '', + isCloudDeployment: settingsStore.isCloudDeployment, + }, + ).nodeGraph, + ); - if (source === 'paste') { - telemetry.track('User pasted nodes', { - workflow_id: workflowsStore.workflowId, - node_graph_string: nodeGraph, - }); - } else if (source === 'duplicate') { - telemetry.track('User duplicated nodes', { - workflow_id: workflowsStore.workflowId, - node_graph_string: nodeGraph, - }); - } else { - telemetry.track('User imported workflow', { - source, - workflow_id: workflowsStore.workflowId, - node_graph_string: nodeGraph, - }); + if (source === 'paste') { + telemetry.track('User pasted nodes', { + workflow_id: workflowsStore.workflowId, + node_graph_string: nodeGraph, + }); + } else if (source === 'duplicate') { + telemetry.track('User duplicated nodes', { + workflow_id: workflowsStore.workflowId, + node_graph_string: nodeGraph, + }); + } else { + telemetry.track('User imported workflow', { + source, + workflow_id: workflowsStore.workflowId, + node_graph_string: nodeGraph, + }); + } + } catch { + // If telemetry fails, don't throw an error } // Fix the node position as it could be totally offscreen diff --git a/packages/workflow/src/telemetry-helpers.ts b/packages/workflow/src/telemetry-helpers.ts index d59b01438a..5bfae8f8dc 100644 --- a/packages/workflow/src/telemetry-helpers.ts +++ b/packages/workflow/src/telemetry-helpers.ts @@ -42,6 +42,7 @@ import type { import { NodeConnectionTypes } from './interfaces'; import { getNodeParameters } from './node-helpers'; import { jsonParse } from './utils'; +import { DEFAULT_EVALUATION_METRIC } from './evaluation-helpers'; const isNodeApiError = (error: unknown): error is NodeApiError => typeof error === 'object' && error !== null && 'name' in error && error?.name === 'NodeApiError'; @@ -414,11 +415,18 @@ export function generateNodesGraph( options?.isCloudDeployment && node.parameters?.operation === 'setMetrics' ) { - const metrics = node.parameters?.metrics as IDataObject; + const metrics = node.parameters?.metrics as IDataObject | undefined; - nodeItem.metric_names = (metrics.assignments as Array<{ name: string }> | undefined)?.map( - (metric: { name: string }) => metric.name, - ); + // If metrics are not defined, it means the node is using preconfigured metric + if (!metrics) { + const predefinedMetricKey = + (node.parameters?.metric as string | undefined) ?? DEFAULT_EVALUATION_METRIC; + nodeItem.metric_names = [predefinedMetricKey]; + } else { + nodeItem.metric_names = (metrics.assignments as Array<{ name: string }> | undefined)?.map( + (metric: { name: string }) => metric.name, + ); + } } else if (node.type === CODE_NODE_TYPE) { const { language } = node.parameters; nodeItem.language = diff --git a/packages/workflow/test/telemetry-helpers.test.ts b/packages/workflow/test/telemetry-helpers.test.ts index b2f48d5db0..a0296b9e73 100644 --- a/packages/workflow/test/telemetry-helpers.test.ts +++ b/packages/workflow/test/telemetry-helpers.test.ts @@ -28,6 +28,7 @@ import { userInInstanceRanOutOfFreeAiCredits, } from '../src/telemetry-helpers'; import { randomInt } from '../src/utils'; +import { DEFAULT_EVALUATION_METRIC } from '../src/evaluation-helpers'; describe('getDomainBase should return protocol plus domain', () => { test('in valid URLs', () => { @@ -1418,6 +1419,136 @@ describe('generateNodesGraph', () => { evaluationTriggerNodeNames: [], }); }); + + test('should handle Evaluation node with undefined metrics - uses default predefined metric', () => { + const workflow: Partial = { + nodes: [ + { + parameters: { + operation: 'setMetrics', + // metrics is undefined - should fall back to default metric + }, + id: 'eval-node-id', + name: 'Evaluation Node', + type: 'n8n-nodes-base.evaluation', + typeVersion: 1, + position: [100, 100], + }, + ], + connections: {}, + pinData: {}, + }; + + expect(generateNodesGraph(workflow, nodeTypes, { isCloudDeployment: true })).toEqual({ + nodeGraph: { + node_types: ['n8n-nodes-base.evaluation'], + node_connections: [], + nodes: { + '0': { + id: 'eval-node-id', + type: 'n8n-nodes-base.evaluation', + version: 1, + position: [100, 100], + metric_names: [DEFAULT_EVALUATION_METRIC], // Default metric + }, + }, + notes: {}, + is_pinned: false, + }, + nameIndices: { 'Evaluation Node': '0' }, + webhookNodeNames: [], + evaluationTriggerNodeNames: [], + }); + }); + + test('should handle Evaluation node with custom metric parameter', () => { + const workflow: Partial = { + nodes: [ + { + parameters: { + operation: 'setMetrics', + metric: 'helpfulness', + // metrics is undefined but metric parameter is set + }, + id: 'eval-node-id', + name: 'Evaluation Node', + type: 'n8n-nodes-base.evaluation', + typeVersion: 1, + position: [100, 100], + }, + ], + connections: {}, + pinData: {}, + }; + + expect(generateNodesGraph(workflow, nodeTypes, { isCloudDeployment: true })).toEqual({ + nodeGraph: { + node_types: ['n8n-nodes-base.evaluation'], + node_connections: [], + nodes: { + '0': { + id: 'eval-node-id', + type: 'n8n-nodes-base.evaluation', + version: 1, + position: [100, 100], + metric_names: ['helpfulness'], // Custom metric from parameter + }, + }, + notes: {}, + is_pinned: false, + }, + nameIndices: { 'Evaluation Node': '0' }, + webhookNodeNames: [], + evaluationTriggerNodeNames: [], + }); + }); + + test('should handle Evaluation node with valid metrics assignments', () => { + const workflow: Partial = { + nodes: [ + { + parameters: { + operation: 'setMetrics', + metrics: { + assignments: [ + { name: 'accuracy', value: 0.95 }, + { name: 'precision', value: 0.87 }, + { name: 'recall', value: 0.92 }, + ], + }, + }, + id: 'eval-node-id', + name: 'Evaluation Node', + type: 'n8n-nodes-base.evaluation', + typeVersion: 1, + position: [100, 100], + }, + ], + connections: {}, + pinData: {}, + }; + + expect(generateNodesGraph(workflow, nodeTypes, { isCloudDeployment: true })).toEqual({ + nodeGraph: { + node_types: ['n8n-nodes-base.evaluation'], + node_connections: [], + nodes: { + '0': { + id: 'eval-node-id', + type: 'n8n-nodes-base.evaluation', + version: 1, + position: [100, 100], + metric_names: ['accuracy', 'precision', 'recall'], + }, + }, + notes: {}, + is_pinned: false, + }, + nameIndices: { 'Evaluation Node': '0' }, + webhookNodeNames: [], + evaluationTriggerNodeNames: [], + }); + }); }); describe('extractLastExecutedNodeCredentialData', () => {