๐ŸŽ‰ Node-RED 1.0.0-beta.3 released

In principle I think you are right, only drawback could be for those users that have used "return msg" heavily in huge flows instead of "node.send". So for some, they will be lucky, for others, well,,,

PS I thought about using a text editor, like UltraEdit, for the maintenance and open the flow, search "return msg" and replace with "node.send(msg)", maybe that could work

Why? Thats the last thing you'd want to do. (edit: by which I mean, don't do that).

Once you call return msg we know you can't send another message from the function, so it is 'safe' for us not to clone that msg.

OK, then I misunderstood

Nick, you are of course CORRECT and this has SOLVED the problem - though this means I have a LOT of code to go through as I've done that a lot of times over the years. Thank you so much and thank you to Walter Krambring who basically said the same thing (I read Nick's first). I hope all of this helps others - I'll update my blog accordingly.

Now if you could just top that by getting my now screwed DARK them working... (just kidding, please do sometime add a dark theme, I've been using a third party dark theme for the editor for ages and it is now bust).

Cymplecy - that WOULD work but see Nick's solution - it is down to the async nature of NR - debug works but by the time the the slower MQTT function gets there, msg.topic has been altered. I ended up with 2 message objects to solve it. This is going to catch lots of people out - my problem is however, for me, history thanks to you guys.

If the change is made to clone messages on node.send then you won't need to make any changes.

So yes - @krambriw and @scargill - if you do have "old" unfixed backups around - that may be useful to test if the (proposed/not yet existing) fix works... so please hang onto them. Thanks !

Don't worry Dave, I'm back in the UK in October and my separate setup there will be upgraded to 1.0 when officially released. No doubt I'll have more questions :slight_smile: A "fix" WOULD be good guys as that one really caught me out. I'm not th best programmer but not the worst either. I tried putting

runtimeSyncDelivery: true;

at the start of settings.js but that had no obvious effect. I left a note elsewhere on that one.

Pete

We should thank you! Node-RED has become so great and useful, that said already by so many, I can just agree and say the same!
In some way, problems on the way helps to improve things, as usual
Have a nice evening

Edit: Just wanted to add that all "strange" behavior I have seen in my flows is originating from code I have written in function nodes. All the rest has been working just perfect and in the same way as before, I have not been able to find anything "outside" the function nodes that misbehaved

Personally, I would rather the function node change didn't happen. We really don't want function nodes to be less performant.

I think that Dave's suggestion would be best here. As send is async anyway, I don't think there would be much impact in performance and actually I think it would avoid some relatively common errors.

1 Like

Maybe it would be possible to get a new function (e.g. node.sendNoClone ()), which works without cloning the message or (if possible) to add to node.send an additional parameter that prevents cloning.

The standard node.send() should clone the message to allow working existing code and for future functions it is possible the developer can decide to avoid the cloning and the performance break-in.

For example, I often already put node.send into places where I have no problem, but instead I handle huge objects the performance is more important (for example, in setTimeout / setInterval).

3 Likes

We just have to move on with the hw exchange, I assume performance loss can be compensated upgrading to Pi4 and later, nothing strange, iPhone 4 is also very slow compared to the latest

A new function with a new name would be best I think, as you propose. In my opinion, this makes it easy to upgrade a flow using a text editor, search and replace node.send with, node.sendNC or whatever. Adding it as a parameter is not so good from this perspective unless it is placed in the first position,,,like node.send(nc=true, msg)

A search & replace could then be "node.send(" ----> "node.send(nc=true,"

But I guess using a parameter would require that NR can handle all "standard" cases where there is no parameter given

I am very much against this and Node-RED is not iOS. The whole point of using something like a Pi with Linux is the ability to use REALLY limited hardware for powerful purposes.

We should be looking to improve performance not degrade it.

2 Likes

Sure, I agree but so far this is what I have seen happening in all areas involving sw. Unfortunately. Or can you give an example of the opposite?

I agree with @TotallyInformation and @Hypnos here. I'd rather not enforce cloning each and every message a function sends.

If that behavior gets implemented, there should be at least a possibility to send an uncloned message (like the mentioned node.sendNoClone()).

As a professional developer who understands the underlying concepts (...and sometimes knows what he is doing :grin:) I'd rather have the choice to do "the right thing" myself. For example, this will be useful for cases where you node.send() new message objects anyway, so no need to clone here.

I understand the concerns, however we must factor in the impact it will have if we don't make this change.

There have already been two cases on this thread of flows that were broken from users who are relatively active on this forum. That suggests there is a much wider community of users who will hit this and their flows will be broken.

We can't ignore that - we need to find a way to minimise the impact.

The choice is either, for one specific use of the Function node:

  1. a potential performance drop - one that will be negligible unless the message is particularly large.
  2. flows that no longer do what the user expects. This could range from very obvious breakage, to something very subtle and hard to detect.

I'd argue #2 is a worse outcome then #1 - particular if we can find a way to reclaim the performance drop.

1 Like

Valid points, of course.

But sending the same message object (meaning the reference) multiple times is a (user) mistake in the first place. That is something a user should be made aware of.
It is just a coincidence that - until now - this has been working in most of the cases because the flow processing was different in <v1.0. Now the impact will be much greater.

I see your point, that the "greater good" would be to take care of this for the user.
But the possibility for the advanced user to send an uncloned message should be retained.

If the thought is to change the default implementation of node.send is to clone by default, then this would surely also have implications? For example, perhaps a user has something not clonable in msg that breaks it in a different way?

If it is the intention regardless, then can I suggest an additional optional parameter sendClone (default TBD) is used for node.send ?

Or perhaps there is a way to determine that the user is sending the original msg and have node-red issue a console warning and help on how to disable the warning or how to work around it.

Perhaps this warning could be generated if the additional optional parameter is not present (and is defaulting to "don't clone)?

This would keep node.send backwards compatible AND alert the user to potential issues?

Example....

node.send(msg); // warning generated - you are sending a reference. See http://blah blah for Info

node.send(msg, true); // no warning generated, clone sent

node.send(msg, false); // no warning generated, reference sent

Yes, I understand that. It would certainly help many users to change the default node.send function to clone the message by default. So first it can be ensured that everything continues to work on migrate to node-red 1.0.

However, if you in addition will provide an additional way to disable the cloning for node.send (maybe by another node.sendNoClone() method) as i note above, anyone can go afterwards about the existing functional nodes and adapt them accordingly.