support maxConnections event on net server #31337
Conversation
| @@ -1535,6 +1535,7 @@ function onconnection(err, clientHandle) { | |||
| } | |||
|
|
|||
| if (self.maxConnections && self._connections >= self.maxConnections) { | |||
| self.emit('maxConnections'); | |||
There was a problem hiding this comment.
I think this is totally fine as is.
| @@ -1535,6 +1535,7 @@ function onconnection(err, clientHandle) { | |||
| } | |||
|
|
|||
| if (self.maxConnections && self._connections >= self.maxConnections) { | |||
| self.emit('maxConnections'); | |||
There was a problem hiding this comment.
I think this is totally fine as is.
|
I'm not going to block this, but I think following the API pattern of https://nodejs.org/api/http.html#http_event_clienterror would make sense for this if, as in the HTTP case, emitting an I'd imagine something like a Also, the docs don't make it clear when the event in this PR is emitted. Level-triggered? Every time a client is rejected? An event name like Also, the |
There was a problem hiding this comment.
This is a temporary block to give more time to discuss @sam-github's suggestions. I know they said their comments are not blocking, but there's no rush here, this is a new feature, and we should aim to get it right the first time (and introduce it as Experimental if we think it's likely to have significant changes as we iterate).
Trott
added
the
semver-minor
|
ping @Trott |
|
Closing |
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesI think this could help with the developer experience, when the server connection limit is reached there is not feedback.
At this point, the event is emitted every new connection over the limit, I mean, if the limit is 10, and we get 12 connections, the event will be emitted twice.