node icon indicating copy to clipboard operation
node copied to clipboard

Provide an easy way to get all net.Server connections

Open qiulang opened this issue 1 year ago • 5 comments

What is the problem this feature will solve?

Currently, there seems no way to get all net.Server connections with the public API.

net.server.getConnections((err, count) => {...}) only returns the count of the connection, I can't get each connection within its callback. I tried for (let socket of server.connections) {...} inside the callback, and got the error TypeError: server.connections is not iterable.

I have to store each socket myself on Event: 'connection' callback, keep an array of those socket objects.

What is the feature you are proposing to solve the problem?

Inside net.server.getConnections((err, count) => {...}) callback, in addition to returning connection count, also return all the connections.

What alternatives have you considered?

No response

qiulang avatar Jun 12 '23 09:06 qiulang

A similar feature was originally proposed in https://github.com/nodejs/node/pull/42812, but I consider it an anti-pattern in Node.js (see https://github.com/nodejs/node/pull/42812#pullrequestreview-952580864):

Constructing potentially large lists of resources generally feels like an anti-pattern in Node.js, and unless the application code is entirely synchronous, it is prone to race conditions.

tniessen avatar Jun 13 '23 11:06 tniessen

Another reason not to do this is that it would be incongruent with server.getConnections()1. That method reports connections handled by worker processes but the feature being requested here can't do that because such connections are gone.

1 Which, for the record, I think is of questionable design too.

bnoordhuis avatar Jun 14 '23 19:06 bnoordhuis

Honestly I can't imagine what current getConnections() could be useful for at all. Anyway everyone has to implement the list of connected sockets manually to be able to close the server fully. If an app serves millions of clients, the list would only occupy several MBs in memory which is nothing compared to all the allocated socket objects. Probably it's the lookups in containers with millions of items that are problem?

Antonius-S avatar Jun 15 '23 09:06 Antonius-S

@tniessen @bnoordhuis If it weren't for net.getConnections() I won't ask this feature request. As @Antonius-S said

I can't imagine what current getConnections() could be useful for at all.

Especially @bnoordhuis said "such connections are gone", although I don't get why. If some connections are gone, what is the point of using count in its callback?

qiulang avatar Jun 17 '23 14:06 qiulang

I assume getConnections() to count connections is mostly used as a metric in practice, e.g., with Prometheus or so.

And I believe what @bnoordhuis meant is that the primary process in a cluster does not have access to all connection objects because some exist in the address space / JS heap of other processes.

tniessen avatar Jun 18 '23 09:06 tniessen

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Dec 16 '23 01:12 github-actions[bot]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Jan 15 '24 01:01 github-actions[bot]