node-red-dashboard icon indicating copy to clipboard operation
node-red-dashboard copied to clipboard

MaxEventListener Warning

Open sumitshinde-84 opened this issue 9 months ago • 6 comments

Current Behavior

When more than 10 UI-event widgets are added to the Node-RED instance, a warning appears in the Node-RED logs, as shown in the image below:

Image

Expected Behavior

It should not prevent users from using the UI-event widget as many times as they want.

Steps To Reproduce

  1. Add 10 more UI-event widgets to the Node-RED instance
  2. Deploy the flow

Environment

  • Dashboard version: 1.22.1
  • Node-RED version: 4.0.9
  • Node.js version: 20.18.3
  • npm version:
  • Platform/OS:
  • Browser: Safari

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

sumitshinde-84 avatar Feb 21 '25 11:02 sumitshinde-84

Just adding here that the error in the above screenshot is about the MaxListenersExceededWarning (which occurs on the server-side), in case somebody is searching for that keyword.

When anybody wants to have a look at this, it might be useful to have also a look at this issue. Seems that both the ui-event and ui-control node have a similar issue.

Seems indeed that both nodes indeed have an onSocket function in their js file:

  • In the ui-control code.
  • In the ui-event code.

So both nodes needs to be refactored to add those event listeners only once for all the ui-event nodes, and once for all the ui-control nodes.

I think that the event handlers themself don't need to be refactored, because (in both nodes) the event handler uses wNode instead of the node variable itself:

Image

I have always been wondering why that wNode was being searched inside the list of available nodes, while the node was already available. Because both variables point currently to the same node object, i.e. this node. But now it is useful, because the node id arrives as an input parameter. Since the node id arrives as an input parameter at the handler function, the handler can be used globally for all the node instances.

However the onSocket is passed via the widgetEvents to the ui-base (see here), where it is registered for every ui-event or ui-control node instance. Not sure how you can determine over there whether it has already been registered. If I remember correctly you cannot simple ask NodeJs whether a specific event handler has already been registered. If that is indeed the case, perhaps we can workaround that by keeping a list of event names in a list. If the event name is already in that list, I doesn't need to be registered again...

Anyway contributions from the community are welcome...

bartbutenaers avatar Mar 11 '25 21:03 bartbutenaers

perhaps we can workaround that by keeping a list of event names in a list. If the event name is already in that list, I doesn't need to be registered again...

On second thought that was not my best proposal ever. The ui-base has no idea which stuff needs to be added once, or twice or always. It is the ui nodes themselves that tell the ui-base which listeners it need to register.

Therefore imho the ui-control and ui-event need to pass their onSocket listener only once to ui-base.

Anyway contributions from the community are welcome...

Since the similar issue for the ui-control node is already 6 months open, I don't expect any volunteers anymore to show up... Will have a look at this myself.

bartbutenaers avatar Mar 11 '25 22:03 bartbutenaers

@sumitshinde-84 I have added 11 ui-event nodes to my flow, but I don't see that message in my server logs. Did you do something extra to trigger the message? I have not checked if the max listener default value is 10 on my Windows portable, but I assume it is cross platform that value... It seems that this value 10 is a default value set by NodeJs, not by socketio...

bartbutenaers avatar Mar 12 '25 06:03 bartbutenaers

Ten is indeed the default max listener count before it starts logging the warning about a potential leak. There is nothing inherently wrong with having lots of listeners - the main thing to verify is the listeners get cleaned up when nodes are removed.

So I don't think this need lots of refactoring. We just need to verify the listeners are being removed, and then we can use setMaxListeners to increase the limit (or disable the warning entirely).

knolleary avatar Mar 20 '25 09:03 knolleary

Customer raised this today

https://app-eu1.hubspot.com/help-desk/26586079/view/233410279/ticket/141649819838/thread/11872092373#live-chat

hardillb avatar May 15 '25 12:05 hardillb

For reference - User experiencing similar issue: https://discourse.nodered.org/t/ui-template-with-chart-js-echarts-vue-lifecycle-hooks/98971

Steve-Mcl avatar Sep 06 '25 20:09 Steve-Mcl