Circular/Cyclic Graph Nodes immediately overwhelm the call stack about flume HOT 6 CLOSED

Thanks for bringing this up, this is something I've thought about but haven't settled on the best way to handle it. The problem is that you technically could set up cyclical graphs that are valid, as long as the function for one of the nodes in the loop knows to break out it at some point. It would probably be trivial to just track the number of loops in the engine and bail out if it ever gets to some very high number. Maybe that would be a sensible default to have it bailout after like 5000 loops or something, but make it overridable so you can allow infinite loops if you want.

from flume.

That's what I'm thinking. The ability to do cyclical graphs would be useful in many cases, but having the JS engine choke seems a bad idea. Having Flume handle it makes a lot of sense, especially if you add the iteration configuration option.

from flume.

Yeah I think you're right. Do you have an opinion on the details of how this exception should be handled? My instinct is to not bail out of the entire resolveRootNode function, and instead only bail out of the processing of the current root node input. I have a pending PR that detects circular dependencies per root node input, and throws a descriptive warning, but doesn't crash the engine. That way if you have 5 inputs to the root node, and only one has a circular dependency problem, the values of the remaining 4 ports will still correctly resolve, and the value of the circular port will resolve to undefined and log an error.

from flume.

I'm toying with the idea of also adding this check to the node editor itself as an opt-in validation. That way users could be warned if their node setup would create an infinite loop, or better yet, do the check when connecting a port and prevent the connection if it would result in a cyclical graph. (Also with a warning toast of some kind potentially).

from flume.

All of this sounds great. Doing the check when making the connection makes the most sense, since it will only happen at that moment instead of multiple times. Doing a recursive check through a large graph could get pretty expensive, so doing it once would keep it fast.

from flume.

Agreed, I'll merge the engine bailout part of this and then open and link a new issue for doing this check in the editor. Thanks again for bringing this up.

from flume.