Error importing a flow

General
1

OK, so this code is AI generated so take with a generous pinch of salt.

However, on import, it generates a node marked as invalid but with no way to fix the issue! It is the Wait node that has the error. Nothing is shown in the log.

I've never seen this before and I don't think it should ever happen, importing a node with no way to fix an error.

Incidentally, I can manually re-create the delay node with exactly the same settings with no error.

[
    {
        "id": "tab_downloads",
        "type": "tab",
        "label": "Downloads Manager",
        "disabled": false
    },
    {
        "id": "watch",
        "type": "watch",
        "z": "tab_downloads",
        "name": "Watch Downloads",
        "files": "C:\\Users\\YOUR_USERNAME\\Downloads",
        "recursive": false,
        "x": 160,
        "y": 80,
        "wires": [
            [
                "filter"
            ]
        ]
    },
    {
        "id": "filter",
        "type": "function",
        "z": "tab_downloads",
        "name": "Ignore temp files",
        "func": "const path = require('path');\n\nconst file = msg.payload;\nconst ext = path.extname(file).toLowerCase();\n\nif (ext === '.crdownload' || ext === '.tmp') {\n    return null;\n}\n\nreturn msg;",
        "outputs": 1,
        "x": 390,
        "y": 80,
        "wires": [
            [
                "delay"
            ]
        ]
    },
    {
        "id": "delay",
        "type": "delay",
        "z": "tab_downloads",
        "name": "Wait 5 seconds",
        "pauseType": "delay",
        "timeout": "5",
        "timeoutUnits": "seconds",
        "x": 610,
        "y": 80,
        "wires": [
            [
                "moveImage"
            ]
        ]
    },
    {
        "id": "moveImage",
        "type": "function",
        "z": "tab_downloads",
        "name": "Move images",
        "func": "const fs = require('fs');\nconst path = require('path');\n\nconst file = msg.payload;\nconst ext = path.extname(file).toLowerCase();\n\nconst images = [\n'.jpg','.jpeg','.png','.gif','.bmp','.webp','.tif','.tiff','.svg','.heic','.avif'\n];\n\nif (!images.includes(ext)) {\n    return null;\n}\n\nconst dir = path.dirname(file);\nconst targetDir = path.join(dir,'images');\n\nif (!fs.existsSync(targetDir)) {\n    fs.mkdirSync(targetDir,{recursive:true});\n}\n\nconst dest = path.join(targetDir,path.basename(file));\n\ntry {\n    fs.renameSync(file,dest);\n    node.status({fill:'green',shape:'dot',text:path.basename(file)});\n}\ncatch(err){\n    node.error(err,msg);\n}\n\nreturn null;",
        "outputs": 0,
        "x": 820,
        "y": 80,
        "wires": []
    },
    {
        "id": "injectHourly",
        "type": "inject",
        "z": "tab_downloads",
        "name": "Hourly",
        "props": [],
        "repeat": "3600",
        "once": true,
        "onceDelay": "5",
        "x": 140,
        "y": 220,
        "wires": [
            [
                "archiveOld"
            ]
        ]
    },
    {
        "id": "archiveOld",
        "type": "function",
        "z": "tab_downloads",
        "name": "Archive files >6 months",
        "func": "const fs = require('fs');\nconst path = require('path');\n\nconst downloadDir='C:\\\\Users\\\\YOUR_USERNAME\\\\Downloads';\nconst archiveDir=path.join(downloadDir,'archive');\n\nif(!fs.existsSync(archiveDir))\n    fs.mkdirSync(archiveDir,{recursive:true});\n\nconst cutoff=Date.now()-183*24*60*60*1000;\n\nfor(const item of fs.readdirSync(downloadDir)){\n\n    if(item==='archive' || item==='images')\n        continue;\n\n    const full=path.join(downloadDir,item);\n\n    let stat;\n\n    try{\n        stat=fs.statSync(full);\n    }\n    catch(e){\n        continue;\n    }\n\n    if(stat.isDirectory())\n        continue;\n\n    if(stat.mtimeMs<cutoff){\n        try{\n            fs.renameSync(full,path.join(archiveDir,item));\n        }\n        catch(err){\n            node.warn(err.message);\n        }\n    }\n}\n\nreturn null;",
        "outputs": 0,
        "x": 410,
        "y": 220,
        "wires": []
    }
]
2

All three of your function nodes contain require which as I understand it is incorrect in a Node-red function, so an iffy delay seems to be the least of your problems :laughing:

I wonder if these lines are somehow messing up the editor's error checking.

3

Did I not imply that the code wasn't to be trusted? That isn't the issue, the flow certainly cannot run as-is because of several faults in all of the function nodes. These issues are not impacting the delay node error, I've checked and already corrected all of the function nodes.

The issue is being able to import a flow containing a node that cannot be fixed.

4

Yes you did and I apologise if you thought I had not noticed.

I just wondered if the delay node would still be flagged invalid if there were no issues with the functions.
Or putting it another way, if the delay node's flows.json entry is in any way different from a manually created delay node.

5

Here they are:

[
    {
        "id": "delay",
        "type": "delay",
        "z": "tab_downloads",
        "name": "Wait 5 seconds.",
        "pauseType": "delay",
        "timeout": "5",
        "timeoutUnits": "seconds",
        "rate": "",
        "nbRateUnits": "",
        "randomFirst": "",
        "randomLast": "",
        "randomUnits": "seconds",
        "allowrate": false,
        "outputs": 1,
        "x": 610,
        "y": 80,
        "wires": [
            []
        ]
    },
    {
        "id": "64d8bfea635c913e",
        "type": "delay",
        "z": "tab_downloads",
        "name": "Wait 5 seconds",
        "pauseType": "delay",
        "timeout": "5",
        "timeoutUnits": "seconds",
        "rate": "1",
        "nbRateUnits": "1",
        "rateUnits": "second",
        "randomFirst": "1",
        "randomLast": "5",
        "randomUnits": "seconds",
        "drop": false,
        "allowrate": false,
        "outputs": 1,
        "x": 620,
        "y": 140,
        "wires": [
            [
                "2ebc425b3dc5c288"
            ]
        ]
    }
]

THe 2nd entry is the manually added correct one.

Any differences should be dealt with by the node so there is possible a small bug in the delay node.

6

Quite a few differences there.

It looks like you either need a value for nbRateUnits or for randomFirst and randomLast.
I wonder what those attributes do.

It's hardly up to the node to handle any bad imports. If only we could educate chatgpt about the ways it borks Node-red code. :thinking:

7

I think the point is that it should be possible, by opening it and entering valid data into all the fields, to get it into a good state.

8

I’m afraid I disagree. I imported something that cannot be fixed without manually hacking the flow file. That is totally against the concepts of node-red.

The node is absolutely responsible for ensuring that it can be made valid.

9

It is possible to correct the error. The problem is that different modes of operation bring up different fields. If you go through every combination of options and fill in the empty fields then it sorts it.

It is difficult to know what the node should do about that. Should the node check for invalid fields in an import and correct them? There are probably many node types where this could be a problem.

10

Or perhaps the node should ignore invalid values on fields that are not relevant for the current mode of operation.

11

This is exactly the risk of using LLMs for Node-RED flow generation.

In the case of the delay node, you were lucky enough to get a validation error

image
image278Ă—193 3.26 KB

In many cases, a nodes properties may not be validated at all and the issue only surfaces in the runtime!

Imagine a flow delivered with a node property "target": "all" but target was supposed to be a number or an enum of "one|some|every". The LLM has no idea (lack of schema - one of the things we tried to ratify for NR5) that "all" is not valid! Best case scenario, nothing bad happens, middle case scenario it crashes node-red. worst case - well, you can use your imagination!

It is super easy to state differences should be dealt with by the node however one could also argue that golden path (drag a node to the canvas & change a property) it works perfectly fine and that the issue is you used some randomly predicted next word generator to generate some json & they would technically also be correct.

The truth is, in an ideal world EVERY node would provide a SCHEMA that the LLM ingests and validates input before generating the flow. Additionally, all nodes would have the ability to self heal given invalid or missing properties. Unfortunately, that is not the case and not likely to happen any time soon.

FWIW, FlowFuse does flow AI building but it uses a combination of strategies/MCP tools/schemas etc to build the nodes (and validate them) to create flows (i.e. it does NOT generate a JSON blob then hope for the best)

PS. If you feel strongly enough about this being a bug, you can always raise a PR (and issue of course).

12

OK, not very friendly though. :frowning:

I don't think it is. Both the Editor part and the runtime part should both supply sensible defaults for all properties. Indeed, if I remember correctly, a lot of the core nodes do indeed do that. Certainly I try to make mine do that as I've had problems in the past with undefined or invalid properties being passed.

Or simply provide defaults that are then written to the runtime so that the error mark disappears.

Yes, I realise this of course. But .. people WILL do that won't they? Indeed, beginners are more likely to do this and will be the least likely to understand what is happening.

But surely this is why the Editor has had property validation from the very start! The Editor config code is supposed to validate inputs. In fact ANY and EVERY application that takes user input MUST validate those inputs otherwise it is wide open to security issues and bugs. That is programming 102.

But - that pandora's box is well and truly not only open but smashed into little pieces. It can be ignored - but it will massively undermine the usefulness of Node-RED.

After all, even FlowFuse has added AI hasn't it? Might be somewhat restricted just now - at least for me, I'm only seeing the function node helper in Node-RED of course - but the implication is that AI is, in fact, useful for Node-RED development.

If I find time I may do this. But I am a little disappointed that this issue is being swept away so easily. I've not even actually asked for a fix, just that it is taken into account as nodes are being looked at. This will surely re-surface at some point. It doesn't even need any changes to core, just applying the existing features consistently.

I will continue to design my nodes to deal with this issue as I think I have done since the early days.

13

They probably do, but here the issue is not that the property is not included in the import, but that the import is providing illegal values, such as "nbRateUnits": "",. The node validates that and correctly flags it as an illegal value.

As I said, the import is providing an illegal value, it is not that the properties are missing.

It is validating them and marking them as invalid. The issue is that because many of the fields are only optionally shown it is not clear from the editor which fields are in error.

What exactly is the fix that you propose? The node is, as far as we know, correctly validating the import and marking the fields as invalid.