erlangpl icon indicating copy to clipboard operation
erlangpl copied to clipboard

Make supervision tree fetching more error proof

Open gomoripeti opened this issue 7 years ago • 5 comments

If a child is wrongly specified as supervisor, but it does not implement this behaviour, the supervisor:which_children/0 call can return unpredictable values. (Especially if the worker is a gen_server which can return any sort of funny value in its handle_call).

This issue has been seen in more than one open-source library.

Hopefully closes #37

gomoripeti avatar Jun 01 '17 09:06 gomoripeti

So if some branch of sup-tree is not OTP correct for example it would skip whole branch?

baransu avatar Jun 01 '17 12:06 baransu

I think in most cases a worker child is incorrectly declared as a supervisor child so there are no grandchildren and it is safe to handle this process as a leaf. If this process is a custom parent which has children but does not implement the which_children api then, yes, the subtree under it would be ignored. But in this case epl would have a hard time to figure out grandchildren anyway. (I only know of rabbitmq which has its own supervisor implementation, but it also supports the which_children call)

gomoripeti avatar Jun 01 '17 20:06 gomoripeti

We want to enforce correct OTP structure so rather throw error than ignore something incorrect.

/ cc @michalslaski

baransu avatar Jun 01 '17 22:06 baransu

Or instead of throwing an error we could show an information in the UI that this branch of the tree cannot be resolved.

arkgil avatar Jun 04 '17 09:06 arkgil

It makes total sense not to hide the discrepancy. The easiest way would be to just crash with a very descriptive error message. But it would be more user-friendly to either print an error message (only once and not every second when the sup-tree is updated) or indicate this in the UI somehow. If it is not just a crash I would like to leave the implementation to you guys.

gomoripeti avatar Jun 05 '17 22:06 gomoripeti