Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

msg transport - by reference issue #50

Open
scargill opened this issue Jun 26, 2017 · 2 comments
Open

msg transport - by reference issue #50

scargill opened this issue Jun 26, 2017 · 2 comments

Comments

@scargill
Copy link

Following discussions elsewhere I made a node called "node-red-contrib-diode" after discovering something I should have known already - that messages are passed by referrence, not by value. This can cause issues as someone may use a message twice in a function - sending it out with node-send and then re-using it. Should the receiving function or node alter the message in any way, because it was sent by reference - this can actually damage the original message. "diode" stops this - but I think this should be clearly pointed out in the documentation.

I found this...

ttp://nodered.org/docs/creating-nodes/node-js
Section "receiving messages"

Suggest you add: "Please note that a reference to the original message is passed here. Altering that message could, in edge cases, affect the calling function."

http://nodered.org/docs/writing-functions
Section "Writing functions"
Subsection "Writing a function"...

You say:

"The returned message object does not need to be same object as was passed in; the function can construct a completely new object before returning it. For example:
var newMsg = { payload: msg.payload.length };
return newMsg;
Note: constructing a new message object will lose any message properties of the received message. This will break some flows, for example the HTTP In/Response flow requires the msg.req and msg.res properties to be preserved end-to-end. In general, function nodes should return the message object they were passed having made any changes to its properties."

So firstly - why would construction a new message lose properties of the old. Using the code DaveCJ provided:

var newMsg = RED.util.cloneMessage(msg);

does not lose any of the original message - I tested it... so you seem to be saying NOT to make a new object - but you don't warn of the consequences. Why not change that example to use the code above which will ensure everything is copied across - scrap the warning that data could be lost and instead tell readers that this might slightly increase overhead - but will eliminate any chance of interaction with the original message.

In my diode node - here you see the unprocessed output - and the output through the node - they are identical down to the msgID - except for the payload which I deliberately altered to show the feedback effect.

msg : Object
{ _msgid: "e5a704f0.1a58f8", topic: "", payload: "goodbye", blah: "fred" }
6/26/2017, 2:31:23 PMnode: d9f3747f.7ef978
msg : Object
{ _msgid: "e5a704f0.1a58f8", topic: "", payload: "hello", blah: "fred" }

Hope this is helpful.

@DeanCording
Copy link
Contributor

DeanCording commented Jan 1, 2018

Hi Pete,

I'm rummaging around this piece of code at the moment and it's a bit more complex. In fact, I ran up against a problem recently where I couldn't send a message because it contained an unclonable object.

The bit of code of interest is:

   if (msgSent) {
        var clonedmsg = redUtil.cloneMessage(m);
        sendEvents.push({n:node,m:clonedmsg});
    } else {
        sendEvents.push({n:node,m:m});
        msgSent = true;
    }

This is in the middle of handling sending messages to all connected nodes. So, the first message sent is the original object and all others are clones but only on each output.

So if I do the following:

var msg = {topic: 'msg1'};
var msgs = [msg, msg, msg];
node.send(msgs);

the same original msg object is sent to the first node on each output.

Also, as a msg is passed down a flow, the original msg object will be passed to the first node on each output down the chain if it isn't explicitly cloned. So if I reuse the original msg object in any of those nodes, I could affect the msg object received by other nodes. In particular, if a node saves a non-primitive msg property, other nodes could change that property.

Dean

@DeanCording
Copy link
Contributor

Here's a flow that demonstrates what happens:

[{"id":"98f350a7.0414e","type":"function","z":"e316ac4b.c85a2","name":"Flow 1 Node 1","func":"var savedObject = context.get('saved')|| {};\n\n\nif (msg.payload == \"output\") {\n \n node.warn(savedObject);\n \n} else if (msg.payload == \"alter\") {\n \n savedObject.added = \"xxxxxx\";\n\n} else {\n \n context.set('saved', msg.payload);\n \n}\n\n\nreturn msg;","outputs":1,"noerr":0,"x":289,"y":781,"wires":[["8f076630.0c0d98"]]},{"id":"8f076630.0c0d98","type":"function","z":"e316ac4b.c85a2","name":"Flow 1 Node 2","func":"var savedObject = context.get('saved')|| {};\n\n\nif (msg.payload == \"output\") {\n \n node.warn(savedObject);\n \n} else if (msg.payload == \"alter\") {\n \n savedObject.added = \"xxxxxx\";\n\n} else {\n \n context.set('saved', msg.payload);\n \n}\n\n\nreturn msg;","outputs":1,"noerr":0,"x":486,"y":777,"wires":[[]]},{"id":"2564e3d2.21e26c","type":"inject","z":"e316ac4b.c85a2","name":"Output","topic":"","payload":"output","payloadType":"str","repeat":"","crontab":"","once":false,"x":78,"y":887,"wires":[["98f350a7.0414e","ca8b8882.cf13e8"]]},{"id":"eef8b5dd.9e61b8","type":"inject","z":"e316ac4b.c85a2","name":"Set property","topic":"","payload":"{\"prop1\":2,\"prop2\":3}","payloadType":"json","repeat":"","crontab":"","once":false,"x":99,"y":729,"wires":[["98f350a7.0414e","ca8b8882.cf13e8"]]},{"id":"3269dfbf.27978","type":"inject","z":"e316ac4b.c85a2","name":"Alter","topic":"","payload":"alter","payloadType":"str","repeat":"","crontab":"","once":false,"x":313,"y":700,"wires":[["8f076630.0c0d98"]]},{"id":"ca8b8882.cf13e8","type":"function","z":"e316ac4b.c85a2","name":"Flow 2 Node 1","func":"var savedObject = context.get('saved')|| {};\n\n\nif (msg.payload == \"output\") {\n \n node.warn(savedObject);\n \n} else if (msg.payload == \"alter\") {\n \n savedObject.added = \"xxxxxx\";\n\n} else {\n \n context.set('saved', msg.payload);\n \n}\n\n\nreturn msg;","outputs":1,"noerr":0,"x":290,"y":875,"wires":[["c2eb7c87.6c63a"]]},{"id":"c2eb7c87.6c63a","type":"function","z":"e316ac4b.c85a2","name":"Flow 2 Node 2","func":"var savedObject = context.get('saved')|| {};\n\n\nif (msg.payload == \"output\") {\n \n node.warn(savedObject);\n \n} else if (msg.payload == \"alter\") {\n \n savedObject.added = \"xxxxxx\";\n\n} else {\n \n context.set('saved', msg.payload);\n \n}\n\n\nreturn msg;","outputs":1,"noerr":0,"x":508,"y":880,"wires":[[]]}]

Click on 'Set Property' and then on 'Output' - all nodes will set a node context property from the msg.payload.

Now click on 'Alter' which only goes to the second node, and then click on 'Output' - notice how the outputs for nodes on the first flow are the same even though only the second node made the change. Also notice how the second flow is not affected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants