Topic getting lost/overridden

Have a strange issue. Broke it down to a fairly basic example. Below.

Node red version Using v0.20.5.

Now when It comes out of first node I get 2 msgs as expected in the debug with the 2 topics. But when it goes through Template node the first topic is overritten and both messages that come out the end have the same topic.

Am I doing something wrong or missing something or is this a bug?

[
    {
        "id": "9d62f710.7f7fe",
        "type": "tab",
        "label": "Flow 3",
        "disabled": false,
        "info": ""
    },
    {
        "id": "826ff31b.5cd5e",
        "type": "function",
        "z": "9d62f710.7f7fe",
        "name": "Processor",
        "func": "msg.source = {};\nmsg.source.topic = msg.topic;\nmsg.source.payload = msg.payload;\n\nvar topics = [];\n\nswitch(msg.source.topic) {\n  case \"zigbee2mqtt/Office Switch\":\n    // code block\n    topics.push(\"zigbee2mqtt/Office RGB/set\");\n    break;\n  case \"zigbee2mqtt/Spare Bedroom Switch\":\n    // code block\n    topics.push(\"zigbee2mqtt/Spare Bedroom RGB/set\");\n    break;\n  case \"zigbee2mqtt/Downstairs Switch\":\n    // code block\n    topics.push(\"zigbee2mqtt/Hall/set\");\n    topics.push(\"zigbee2mqtt/Landing/set\");\n    break;\n  case \"zigbee2mqtt/Landing Switch\":\n    // code block\n    topics.push(\"zigbee2mqtt/Hall/set\");\n    topics.push(\"zigbee2mqtt/Landing/set\");\n    break;\n  case \"zigbee2mqtt/Loo PIR\":\n    // code block\n    topics.push(\"zigbee2mqtt/Loo White/set\");\n    break;\n  case \"zigbee2mqtt/Grey Switch\":\n    // code block\n    topics.push(\"zigbee2mqtt/Hall/set\");\n    //,\"zigbee2mqtt/Landing/set\"};\n    break;     \n  default:\n    topics = [];\n    // code block\n}\nif (msg.payload.action) {\n    switch(msg.payload.action) {\n      case \"up\":\n        // code block\n        msg.state = \"ON\";\n        break;\n      case \"down\":\n        // code block\n        msg.state = \"OFF\";\n        break;\n      case \"hold_up\":\n        // code block\n        msg.state = \"ON\";\n        break;\n      case \"hold_down\":\n        // code block\n        msg.state = \"OFF\";\n        break;\n      case \"circle_click\":\n        // code block\n        msg.state = \"ON\";\n        break;\n      default:\n        msg.state = null;\n        // code block\n    }\n}\n\nif (msg.payload.occupancy !== null) {\n    switch(msg.payload.occupancy) {\n      case true:\n        // code block\n        msg.state = \"ON\";\n        break;\n        case false:\n        // code block\n        msg.state = \"OFF\";\n        break;        \n    }\n}\n\nif (msg.state && topics.length > 0) {\n    topics.forEach(function(element) {\n        msg.topic = element;\n        node.send([ msg , null ]);\n    })\n    \n}else{\n    return [null,msg];\n}    ",
        "outputs": 2,
        "noerr": 0,
        "x": 250,
        "y": 240,
        "wires": [
            [
                "df6f0cd0.2354f",
                "8fd756c4.3556a"
            ],
            [
                "541cc3eb.e55dac"
            ]
        ]
    },
    {
        "id": "df6f0cd0.2354f",
        "type": "template",
        "z": "9d62f710.7f7fe",
        "name": "Set State",
        "field": "payload",
        "fieldType": "msg",
        "format": "handlebars",
        "syntax": "mustache",
        "template": "{\"state\":\"{{state}}\"}",
        "output": "str",
        "x": 440,
        "y": 100,
        "wires": [
            [
                "aacb50fc.5354a"
            ]
        ]
    },
    {
        "id": "aacb50fc.5354a",
        "type": "debug",
        "z": "9d62f710.7f7fe",
        "name": "Post State Output",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "true",
        "targetType": "full",
        "x": 630,
        "y": 100,
        "wires": []
    },
    {
        "id": "541cc3eb.e55dac",
        "type": "debug",
        "z": "9d62f710.7f7fe",
        "name": "UnProcessed",
        "active": false,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "true",
        "targetType": "full",
        "x": 440,
        "y": 300,
        "wires": []
    },
    {
        "id": "5de67808.9aba9",
        "type": "inject",
        "z": "9d62f710.7f7fe",
        "name": "",
        "topic": "zigbee2mqtt/Downstairs Switch",
        "payload": "{\"battery\":\"100.00\",\"voltage\":3000,\"linkquality\":5,\"brightness\":254,\"action\":\"up\"}",
        "payloadType": "json",
        "repeat": "",
        "crontab": "",
        "once": false,
        "onceDelay": 0.1,
        "x": 110,
        "y": 300,
        "wires": [
            [
                "826ff31b.5cd5e"
            ]
        ]
    },
    {
        "id": "8fd756c4.3556a",
        "type": "debug",
        "z": "9d62f710.7f7fe",
        "name": "Processor Output",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "true",
        "targetType": "full",
        "x": 510,
        "y": 220,
        "wires": []
    }
]

I am trying to reverse engineer your flow, dissecting to understand why you are using the template node, and why pushing the topic into an array that you later on are going to loop through ?

You receive single messages that have to be processed correct ? Is it to combine certain actions ?

The actual use case is as part of a zigbee2mqtt project. So I want a switch to be able to control multiple zigbee devices. So in the example I have a switch and it should be turning on Both a light and the top and bottom of the stairs. Issue is that the topic for the second devices is overridden. As for the template I could potentially direct generate the output message in the function but the template just seemed the simplist solution and allows me to extend things like brightness etc that may want to do latter.

The ultimate goal is have 5-6 switches that all have 3 buttons that support both long and shortpresses as well as long release. So 9 different inputs and want to be able to map them 1:1 or 1:many output mqtt messages to control things.

Although I am a big fan of function nodes, I am not sure if it makes it easier, nor clearer.

I tried it in a different way:

[{"id":"a34b1f24.10b778","type":"inject","z":"67dcd8a2.7278d","name":"","topic":"zigbee2mqtt/Downstairs Switch","payload":"{\"battery\":\"100.00\",\"voltage\":3000,\"linkquality\":5,\"brightness\":254,\"action\":\"up\"}","payloadType":"json","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":116,"y":572,"wires":[["449817d6.d2eac"]]},{"id":"6825f548.d19c7c","type":"switch","z":"67dcd8a2.7278d","name":"set Topic","property":"topic","propertyType":"msg","rules":[{"t":"regex","v":"Downstairs Switch","vt":"str","case":false},{"t":"regex","v":"Office Switch","vt":"str","case":false},{"t":"regex","v":"Spare Bedroom Switch","vt":"str","case":false},{"t":"regex","v":"Loo PIR","vt":"str","case":false},{"t":"regex","v":"Grey Switch","vt":"str","case":false}],"checkall":"true","repair":false,"outputs":5,"x":279,"y":572,"wires":[["aaeefd70.f48ee","44226384.340394"],["281bf8f5.b9028"],["5597980c.d2c458"],["fb5b06f4.1a4648"],["aaeefd70.f48ee"]],"l":false},{"id":"ab95d2e1.f4678","type":"debug","z":"67dcd8a2.7278d","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","x":622,"y":572,"wires":[]},{"id":"aaeefd70.f48ee","type":"change","z":"67dcd8a2.7278d","name":"zigbee2mqtt/Hall/set","rules":[{"t":"set","p":"topic","pt":"msg","to":"zigbee2mqtt/Hall/set","tot":"str"}],"action":"","property":"","from":"","to":"","reg":false,"x":411,"y":484,"wires":[["ab95d2e1.f4678"]],"l":false},{"id":"44226384.340394","type":"change","z":"67dcd8a2.7278d","name":"zigbee2mqtt/Landing/set","rules":[{"t":"set","p":"topic","pt":"msg","to":"zigbee2mqtt/Landing/set","tot":"str"}],"action":"","property":"","from":"","to":"","reg":false,"x":411,"y":528,"wires":[["ab95d2e1.f4678"]],"l":false},{"id":"fb5b06f4.1a4648","type":"change","z":"67dcd8a2.7278d","name":"zigbee2mqtt/Loo White/set","rules":[{"t":"set","p":"topic","pt":"msg","to":"zigbee2mqtt/Loo White/set","tot":"str"}],"action":"","property":"","from":"","to":"","reg":false,"x":411,"y":660,"wires":[["ab95d2e1.f4678"]],"l":false},{"id":"5597980c.d2c458","type":"change","z":"67dcd8a2.7278d","name":"zigbee2mqtt/Loo White/set","rules":[{"t":"set","p":"topic","pt":"msg","to":"zigbee2mqtt/Loo White/set","tot":"str"}],"action":"","property":"","from":"","to":"","reg":false,"x":411,"y":616,"wires":[["ab95d2e1.f4678"]],"l":false},{"id":"281bf8f5.b9028","type":"change","z":"67dcd8a2.7278d","name":"zigbee2mqtt/Office RGB/set","rules":[{"t":"set","p":"topic","pt":"msg","to":"zigbee2mqtt/Office RGB/set","tot":"str"}],"action":"","property":"","from":"","to":"","reg":false,"x":411,"y":572,"wires":[["ab95d2e1.f4678"]],"l":false},{"id":"449817d6.d2eac","type":"change","z":"67dcd8a2.7278d","name":"Action ?","rules":[{"t":"change","p":"payload.action","pt":"msg","from":"(hold_up|circle_click|up)","fromt":"re","to":"ON","tot":"str"},{"t":"change","p":"payload.action","pt":"msg","from":"(hold_down|down)","fromt":"re","to":"OFF","tot":"str"}],"action":"","property":"","from":"","to":"","reg":false,"x":213,"y":572,"wires":[["6825f548.d19c7c"]],"l":false}]

The statement node.send(msg) is useful inside loops but there is a right way to use it (and possibly many wrong ways).

When I build a totally new object inside the loop then it is ok to use node.send(msg). In such case, I don´t care to preserve the existing properties. It is a brand new object.

Now there are cases where you want to preserve properties from the previous msg. Perhaps you just want to add or modify a single property (seems to be your case). For that purpose, you have to clone your msg first and then you add/modify the property of the new object, and finally, use node.send.

Playing with this excerpt of code may help to understand:

msg.source = {};
msg.source.topic = msg.topic;
msg.source.payload = msg.payload;

var topics = [];
topics.push("zigbee2mqtt/Hall/set");
topics.push("zigbee2mqtt/Landing/set");

topics.forEach(function(element) {
    var newmsg = RED.util.cloneMessage(msg);
    newmsg.topic = element;
    node.send(newmsg)});

I have shown you the root cause of the odd behavior but I missed to explain another important thing.

You are probably wondering: How come that the topmost debug node shows no change in topics (or topics overridden in your words) but the middle debug node shows different topics?

Apparently, it makes no sense after all both debug nodes are connected to the very same output of the function node. As there is a template node in between the function node and the topmost debug node you suspected the template node is messing with the topics, right?

Here comes the missing point. You have to be aware that Node-RED will clone the message object whenever you branch your flow (two wires in the same output). One of the branches will have the original msg object. The others will have cloned msg objects.

In your flow the middle debug node is receiving cloned msgs).

Thanks for the replies. And the great explanations as well. So would i be correct in thinking that with a single branch the msg is actually passed my ref vs than by value. So ultimately this is why the template node gets the duplicate topic vs the different ones. As the in memory version has been replaced and both "threads" are using the same object in memory.

Now I understand the situation then it easy to fix and to be honest the issue was always about "WHY" rather than getting it fixed.

But it does beg the question as to if it is good behaviour of Node-Red in general for example as it wouldn't be unusual to have a debug and code branch from a single output. With the current behaviour the act of deleting the the debug branch could adversely change the behaviour of downstream blocks. As such would it not be better to always clone the msg and GC the original or worse case somehow visually show on the branch via shading or a thicker outline the primary and cloned paths perhaps.

cloning objects comes at a performance cost - especially for large objects... and some (eg with circular references) are painful - so if at all possible it's better not to clone - BUT... does have the gotcha as described.

As soon as you branch like that it does clone one of them. The reason you had the problem was because you were sending the same message mulitple times one after another down the same branch.

Exacty, you got the point. As a matter of fact JavaScrip always pass an object by reference. Your loop is sending several times the same reference and before the next node downstream has the chance to do its job your loop already ended. The msg.topic value that the next node in line will use is the last value left by the code inside the loop. When you clone an object it will still be passed by reference but it is a reference to another object.