Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an easy way to get all net.Server connections #48433

Closed
qiulang opened this issue Jun 12, 2023 · 7 comments
Closed

Provide an easy way to get all net.Server connections #48433

qiulang opened this issue Jun 12, 2023 · 7 comments
Labels
feature request Issues that request new features to be added to Node.js. net Issues and PRs related to the net subsystem. stale

Comments

@qiulang
Copy link

qiulang commented Jun 12, 2023

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 qiulang added the feature request Issues that request new features to be added to Node.js. label Jun 12, 2023
@tniessen
Copy link
Member

A similar feature was originally proposed in #42812, but I consider it an anti-pattern in Node.js (see #42812 (review)):

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 tniessen added the net Issues and PRs related to the net subsystem. label Jun 13, 2023
@bnoordhuis
Copy link
Member

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.

@Antonius-S
Copy link

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?

@qiulang
Copy link
Author

qiulang commented Jun 17, 2023

@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?

@tniessen
Copy link
Member

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.

Copy link
Contributor

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 github-actions bot added the stale label Dec 16, 2023
Copy link
Contributor

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 github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. net Issues and PRs related to the net subsystem. stale
Projects
None yet
Development

No branches or pull requests

4 participants