Problem with node-red-contrib-actionflows since upgrade to 2.2.0

General
21

Hi @SandeepA

the alternative is someone raises an issue against node-red-contrib-actionflows so it gets updated not to use an API it shouldn't.

Looking at the repo I can see no one has reported it yet. It isn't immediately clear from looking at the code why it is storing internal state in global context at all. But that would need the author of the node to comment.

22

Thanks for the response @knolleary.
I just checked on the issues section on GitHub (new there) . I see an issue was opened to the developer few hours back.

Looking at the code, from what i could make out from the code - the code does below in runtimeMap

var af = RED.settings.functionGlobalContext.get("actionflows");
 and then down the line ...
  RED.settings.functionGlobalContext.set("actionflows", af);

to get/set global context variables .

What i understand from the code - mainly when flows are deployed - then to create and set up an initiate global map
RED.events.on("flows:started", runtimeMap);

Unfortunately i dont have enough knowledge on node-red backend to know if intended if i can replace functionGlobalContext with something else which can give intended functionality.
Else i can give it a shot to create a local copy to see if it works.

Any pointers can help. Else we wait and hope that Developer fixes it.

23

Well that's the thing. At a glance, I don't know if its using global context for a good reason or not. As all of the code is in the one file, it it simply needs to share data between the nodes, then it could just be a global variable in the source code and not touch context at all.

But if its use of global context serves a bigger purpose (so that unrelated nodes can access the data for some reason) then it needs something else.

From the node readme, I don't see any mention of context - so it could be an internal implementation detail that can be modified as I described. It really does need Steve to have a say on it.

24

Hi all.

In the interest of node-red and what to do about this situation, I have forked this repo to better understand what is going on.

After reviewing and fixing it in my branch I have come to the opinion that global context is not necessary and should not be used in this manner. I wont go into details here but will say I have invited the developer to discuss the PR and my thoughts.

I have created a PR - hopefully the developer picks this up soon.

If there an absolute need, you can install and test the PR by following the instructions in the issue raised by on the repository however the best bet is to wait until the developer has merged and released a fixed version.

Update - an alterative approach

If you are simply using the actionflows to call subroutines, then I would like to suggest this capability is already built in to node-red as of 2.1.0 in the form of the Link Call node.

See the Link Call node here and in action here

2 Likes
25

Based on the code - even my guess is its not needed and global may suffice. I don't see any unrelated nodes using this and needing this info. They will probably not even be aware that this was implemented by this developer.

Yes if the developer himself planned it for other non related nodes - not sure. But then the fix still should not impact this nodes for sure.

I see Steve also replied and created a fork on this. Will wait for a few days for the developer.

26

Thank You so much Steve. I will have a look. Will wait if a formal release comes from the developer. Else need to see how to take it your patch in. Thank You for the super swift response.

In my case link node doesn't suffice. What i have is as below :

  1. I have defined a flow framework is defined . And the plug ins to that framework are defined external to NR (DB table) like below (pseudo).

    {
    "flow1" : "RunX",
    "flow2" : "RunY",
    "flow3" : "RunZ"
    }

Each of these RunX, RunY , RunZ are action flows. And they can be overridden by user by writing his own plugs in and replacing them here. So essentially user can use the same framework and only needs to write his plugs in if some out of box is not supported by framework.

Example : Some uses may not need flow3 at all. So they wont have that attrbute. So some users may replace flow2 value with RunABC keeping flow 1 and flow 3 attributes the same.

White writing this note i realized why the developer has done it like this. Cos he wanted to allow this dynamism. When calling it in the manner i do - i actually do the below (taken from the original solution by developer).

image
image1668Ɨ660 154 KB

Link Nodes do not give this option.
I don't know if i could explain .

27

Ok, I understand now why the developer used global context however, I suspect this kind of hook-in from a function node should be more formally considered.

As it happens, the PR I raised did not discard global context so should still work - lets see how the developer responds.

28

I am incorrect here. I understood (well I think) why the developer uses functionGlobalContext.

He wants to allow the below dynamism.
And to allow this he needs to have "actionflows" object in global context .
And this is the strength of my framework. I can keep my plug in names outside and allow a user to decide how he wants to control his different flows using the same framework.

image
image1668Ɨ660 154 KB

It means we are back to square one. How can i get this work ?
I don't know yet if Steve's suggestion would handle it.

One immediate not so clean hack i can think is - if for every new message i have an action dummy call (non dynamic) and then internally the "global" object is attached to the message when action out is called. I hate to make every msg tagged with this object. Not yet sure if it will even work.

Is there other property setting of NR - i have this "actionflows" object in memory and available via. some existing NR API - which i can use in function node ? Then instead of globalContext this "actionflows" object can be put there - not sure.

29

Read the code on the PR#20.
What i could make out is - Yes - You access the global context - but in a different way.

initRuntimeMapper(node.context().global);
That allows "actionflows" to still sit in globalContext.

Interesting to note is - the developer did intend to use this -
line 1 in map function
"//var af = node.context().global.get('actionflows');"

The commented it.

30

Yes & i understand why they left it - because context isn't available at that point in the code. I simply deferred the logic using the new initRuntimeMapper function.

31

Aah Ok. I am yet to get into internal backend core details.
Thanks.

32

For what it's worth I am having this happen an I am not even using actionflow's. I'm merely calling get's from a function node in a subroutine and it bombs on deployment.

33

That is due to way the actionflows node had been written.

If you are not using actionflows, uninstall it.

34

Hi Steve,

FYI , have updated github with my testing.
The fix works well in terms of backward compatibility. Both dynamic invocation and non dynamic parts work as usual.

(Tested on 2.1.4 not on 2.2.2).
Let me know if you foresee any further tests.

Additionally , need to understand what your comment on *global context not available" means.
What can be such scenarios ?

35

It was a guarded assumption on my side - in that if someone were to set global context to file storage but didnt have file storage (i.e. cloud instance) would global be empty?I now have a feeling there would be other issues so may be moot.

My last comment on the issue (along the lines of not using global context at all and adding the ability to invoke via a msg property) still stands as my recommendation - if the developer wishes to proceed along those lines great - but - if they just want to get it fixed, the PR I added will work.

Not sure I have anything more to say TBH

The real reason i became involved was from a node-red perspective - understanding if this issue was something node-red should fix. It has been looked at and discussed behind the scenes & deemed so far that the hidden API will not return (at least not with the same signature) so the dev must make a decision and publish a fix otherwise this node will not be compatible with any newer versions of node-red.

Hope that helps?

2 Likes
36

Yes. Noted and makes sense. Will check with dev. Thanks a ton Steve for your help on this piece.

37

Hi @Steve-Mcl , @knolleary

This is more of a feature request - but connected to this issue of action flows which is being seen across other areas (HA etc.)
It connects to the suggestion provided by Steve on Link Call Node in conjunction with Link in & Link Out.

Link Call covers the basic scenario of action flows . The thing it doesn't cover is the dynamic invocation.

Is there a possibility to provide such an ability to Link Call Node ?
i.e. while defining the Link Call node , user has an option to indicate which msg attribute would have the name of the Link In node ? The user can either define msg attribute or select the Link IN nodes like today. Not both.

So if i have msg.dbflowName = "xyz" & I configure msg.dbflowName on Link Call Node.
Link Call Node would connect to the Link In node with the name "xyz".
In the scenario where such Link In node doesn't exist - the Link Call Node will become a passthrough and simply continue.

  1. This adds a dynamic nature to Link Call - thereby making flows significantly dynamic (aka action dynamism) where user can define the path of a flow being decided dynamically at run time based on msg attributes - which user can control via. external configurations.
  2. Core support comes & need for action flows - as can be seen by its wide use is no longer required.

Cons :-

  1. I don't know if this is a simple or very complicate piece to implement.
  2. Wider issue - if you see such use of Link Node something useful.
  3. This stems from a case where a custom node broke - i have reservation if changing core due to such needs is a good idea. (Benefit being it enhances the core).
38

I think a simple switch before a set of link call nodes would give same result e.g

image

39

And be more "readable" imho

1 Like
40

That approach doesn't scale if you want to do this in multiple places through your flows - which is the reason for the Link Call node after all. If you add a new callable flow, you then have to update all of your switch nodes to add it as an option. Doable, but not very usable.

I think this is worth giving proper consideration.

Given we don't visualise where a Link Call goes to, then I'm not sure having the switch node in front of lots of link call nodes adds a lot of readability versus a lot of extra nodes.

And of course, if you don't like it, you don't have to use it - you can stick to the Switch node approach.