From 0d1a0b54318008305dd5ce508f68675b8f24a015 Mon Sep 17 00:00:00 2001 From: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:44:28 +0100 Subject: [PATCH] fix(Rename Keys Node): Add a warning for regex that affect performance (#18911) --- .../core/nodes-testing/node-test-harness.ts | 14 ++++ .../DebugHelper/test/DebugHelper.test.ts | 5 ++ .../test/ThrowAnError.workflow.json | 64 ++++++++++++++++ .../nodes/RenameKeys/RenameKeys.node.ts | 75 ++++++++++++------- 4 files changed, 131 insertions(+), 27 deletions(-) create mode 100644 packages/nodes-base/nodes/DebugHelper/test/DebugHelper.test.ts create mode 100644 packages/nodes-base/nodes/DebugHelper/test/ThrowAnError.workflow.json diff --git a/packages/core/nodes-testing/node-test-harness.ts b/packages/core/nodes-testing/node-test-harness.ts index 616d7785e0..c265f461ab 100644 --- a/packages/core/nodes-testing/node-test-harness.ts +++ b/packages/core/nodes-testing/node-test-harness.ts @@ -331,6 +331,20 @@ export class NodeTestHarness { }); const msg = `Equality failed for "${testData.description}" at node "${nodeName}"`; + // When continue on fail is on the json wrapper is removed for some reason + const runs = output.nodeData[nodeName]; + if (Array.isArray(runs)) { + for (let runIndex = 0; runIndex < runs.length; runIndex++) { + const run = runs[runIndex]; + if (!Array.isArray(run)) continue; + for (let itemIndex = 0; itemIndex < run.length; itemIndex++) { + const original = run[itemIndex]; + if (original && !original.json) { + run[itemIndex] = { json: { ...original } }; + } + } + } + } expect(resultData, msg).toEqual(output.nodeData[nodeName]); }); diff --git a/packages/nodes-base/nodes/DebugHelper/test/DebugHelper.test.ts b/packages/nodes-base/nodes/DebugHelper/test/DebugHelper.test.ts new file mode 100644 index 0000000000..bd245b289f --- /dev/null +++ b/packages/nodes-base/nodes/DebugHelper/test/DebugHelper.test.ts @@ -0,0 +1,5 @@ +import { NodeTestHarness } from '@nodes-testing/node-test-harness'; + +describe('Test DebugHelper Node', () => { + new NodeTestHarness().setupTests(); +}); diff --git a/packages/nodes-base/nodes/DebugHelper/test/ThrowAnError.workflow.json b/packages/nodes-base/nodes/DebugHelper/test/ThrowAnError.workflow.json new file mode 100644 index 0000000000..7087a326fd --- /dev/null +++ b/packages/nodes-base/nodes/DebugHelper/test/ThrowAnError.workflow.json @@ -0,0 +1,64 @@ +{ + "nodes": [ + { + "parameters": {}, + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [-448, 192], + "id": "9f62b838-ecbe-4012-8026-9e3d48669c49", + "name": "When clicking ‘Execute workflow’" + }, + { + "parameters": {}, + "type": "n8n-nodes-base.debugHelper", + "typeVersion": 1, + "position": [-224, 192], + "id": "7d381110-e62e-4911-8527-e6265fe4be11", + "name": "DebugHelper", + "onError": "continueErrorOutput" + }, + { + "parameters": {}, + "type": "n8n-nodes-base.noOp", + "typeVersion": 1, + "position": [0, 192], + "id": "0a6ef4ec-3876-4d99-998e-9ba26ca474b5", + "name": "No Operation, do nothing" + } + ], + "connections": { + "When clicking ‘Execute workflow’": { + "main": [ + [ + { + "node": "DebugHelper", + "type": "main", + "index": 0 + } + ] + ] + }, + "DebugHelper": { + "main": [ + [], + [ + { + "node": "No Operation, do nothing", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": { + "No Operation, do nothing": [ + { + "error": "Node has thrown an error" + } + ] + }, + "meta": { + "instanceId": "27cc9b56542ad45b38725555722c50a1c3fee1670bbb67980558314ee08517c4" + } +} diff --git a/packages/nodes-base/nodes/RenameKeys/RenameKeys.node.ts b/packages/nodes-base/nodes/RenameKeys/RenameKeys.node.ts index 7b853a2b0c..557429330e 100644 --- a/packages/nodes-base/nodes/RenameKeys/RenameKeys.node.ts +++ b/packages/nodes-base/nodes/RenameKeys/RenameKeys.node.ts @@ -9,7 +9,6 @@ import type { INodeType, INodeTypeDescription, } from 'n8n-workflow'; - interface IRenameKey { currentKey: string; newKey: string; @@ -81,7 +80,7 @@ export class RenameKeys implements INodeType { displayName: 'Regex', name: 'regexReplacement', placeholder: 'Add new regular expression', - description: 'Adds a regular expressiond', + description: 'Adds a regular expression', type: 'fixedCollection', typeOptions: { multipleValues: true, @@ -149,11 +148,21 @@ export class RenameKeys implements INodeType { ], }, ], + hints: [ + { + type: 'warning', + message: + 'Complex regex patterns like nested quantifiers .*+, ()*+, or multiple wildcards may cause performance issues. Consider using simpler patterns like [a-z]+ or \\w+ for better performance.', + displayCondition: + '={{ $parameter.additionalOptions.regexReplacement.replacements && $parameter.additionalOptions.regexReplacement.replacements.some(r => r.searchRegex && /(\\.\\*\\+|\\)\\*\\+|\\+\\*|\\*.*\\*|\\+.*\\+|\\?.*\\?|\\{[0-9]+,\\}|\\*{2,}|\\+{2,}|\\?{2,}|[a-zA-Z0-9]{4,}[\\*\\+]|\\([^)]*\\|[^)]*\\)[\\*\\+])/.test(r.searchRegex)) }}', + whenToDisplay: 'always', + location: 'outputPane', + }, + ], }; async execute(this: IExecuteFunctions): Promise { const items = this.getInputData(); - const returnData: INodeExecutionData[] = []; let item: INodeExecutionData; @@ -208,34 +217,46 @@ export class RenameKeys implements INodeType { newItem.json = renameObjectKeys(newItem.json, depth as number); }; for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { - renameKeys = this.getNodeParameter('keys.key', itemIndex, []) as IRenameKey[]; - const regexReplacements = this.getNodeParameter( - 'additionalOptions.regexReplacement.replacements', - itemIndex, - [], - ) as IDataObject[]; + try { + renameKeys = this.getNodeParameter('keys.key', itemIndex, []) as IRenameKey[]; + const regexReplacements = this.getNodeParameter( + 'additionalOptions.regexReplacement.replacements', + itemIndex, + [], + ) as IDataObject[]; - item = items[itemIndex]; + item = items[itemIndex]; - // Copy the whole JSON data as data on any level can be renamed - newItem = { - json: deepCopy(item.json), - pairedItem: { - item: itemIndex, - }, - }; + // Copy the whole JSON data as data on any level can be renamed + newItem = { + json: deepCopy(item.json), + pairedItem: { + item: itemIndex, + }, + }; - if (item.binary !== undefined) { - // Reference binary data if any exists. We can reference it - // as this nodes does not change it - newItem.binary = item.binary; + if (item.binary !== undefined) { + // Reference binary data if any exists. We can reference it + // as this nodes does not change it + newItem.binary = item.binary; + } + + renameKeys.forEach(renameKey); + + regexReplacements.forEach(regexReplaceKey); + + returnData.push(newItem); + } catch (error) { + if (this.continueOnFail()) { + const executionErrorData = this.helpers.constructExecutionMetaData( + this.helpers.returnJsonArray({ error: error.message }), + { itemData: { item: itemIndex } }, + ); + returnData.push(...executionErrorData); + continue; + } + throw error; } - - renameKeys.forEach(renameKey); - - regexReplacements.forEach(regexReplaceKey); - - returnData.push(newItem); } return [returnData];