Skip to content

net: add new options to net.Socket and net.Server#41310

Merged
nodejs-github-bot merged 2 commits intonodejs:masterfrom
ShogunPanda:net-socket-options
Feb 22, 2022
Merged

net: add new options to net.Socket and net.Server#41310
nodejs-github-bot merged 2 commits intonodejs:masterfrom
ShogunPanda:net-socket-options

Conversation

@ShogunPanda
Copy link
Contributor

Hello!
This PR adds the new following options to net.Socket.connect:

  • noDelay (boolean): If set to true, calls uv_tcp_nodelay on the socket after connection. By default it is disabled.
  • keepAlive (boolean) and keepAliveDelay (number): If set to true calls uv_tcp_nodelay on the socket after connection. By default it is disabled.

This changes are needed as preliminary work on #34185 to avoid excessive JS to C++ bridging.
As I'm not a C++ expert, I encountered the following problems:

  1. Not sure how to test changes (even though test suite is passing so apparently nothing is broken)
  2. Error handling in TCPWrap::AfterConnect was tentative so some C++ expert should take a look. Moreover, what shall we do if uv_tcp_connect succeeds but any of uv_tcp_nodelay or uv_tcp_keepalive fails? Shall we close the socket or let user handle it? Currently I don't do anything.

Once this is initially reviewed, I will also add (either here or in a new PR) the analogous on inbound connections.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants