You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
net: setKeepAlive() ignores the error returned by the handle #65529
constnet=require('net');constserver=net.createServer().listen(0,()=>{constsocket=net.connect(server.address().port,()=>{// Looks like it succeededconstreturned=socket.setKeepAlive(true,-5000);console.log('returns socket:',returned===socket);// The handle actually returned EINVALconsterr=socket._handle.setKeepAlive(true,-5,1,10);console.log('handle returns:',err);socket.destroy();server.close();});});
Output:
returns socket: true
handle returns: -22
How often does it reproduce? Is there a required condition?
Every time, for any value the platform rejects.
What is the expected behavior? Why is that the expected behavior?
socket.setKeepAlive() should not report success when the underlying
operation failed. setTypeOfService(), right next to it in the same file,
already does this:
TCPWrap::SetKeepAlive does propagate the error to JS
(args.GetReturnValue().Set(err) in src/tcp_wrap.cc), and uv_tcp_keepalive_ex() returns UV_EINVAL for a negative delay, so the
information is available. It is simply dropped.
The caller has no way to tell: the return value is the socket either way,
and no error is thrown or emitted.
Additional information
Found while working on #57712 / #65528, which adds a warning for delays
below 1000 ms that are truncated to 0. That change is about values which
are silently reduced; this one is about an error which is silently
discarded, so it seemed better to report it separately.
I have not checked how a value above the platform limit behaves on other
systems. On macOS setKeepAlive(true, 40000000) (40000 s) is accepted by
the handle, so any range check would need per-platform investigation first.
I am happy to open a PR if the direction is agreed. Whether a failure
should throw, like setTypeOfService(), or only emit a warning is a
decision I would rather leave to the team, since throwing would be a
breaking change.
Activity