net: setKeepAlive() ignores the error returned by the handle
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 122k
- Forks
- 37.3k
- Avg merge
- 4d 2h
- Merged PRs (30d)
- 283
Description
Version
v27.0.0-pre (main)
Platform
Darwin 25.4.0 arm64
Subsystem
net
What steps will reproduce the bug?
const net = require('net');
const server = net.createServer().listen(0, () => {
const socket = net.connect(server.address().port, () => {
// Looks like it succeeded
const returned = socket.setKeepAlive(true, -5000);
console.log('returns socket:', returned === socket);
// The handle actually returned EINVAL
const err = 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:
const err = this._handle.setTypeOfService(tos);
if (err && !isWindows) {
throw new ErrnoException(err, 'setTypeOfService');
}
What do you see instead?
setKeepAlive() discards the return value of the handle call:
this._handle.setKeepAlive(enable, initialDelay, interval, count);
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the socket.setKeepAlive() path and TCPWrap::SetKeepAlive in src/tcp_wrap.cc, then reproduce the negative-delay case from the issue. Compare the handling with setTypeOfService() and investigate the platform behavior noted for macOS and other systems. Done means underlying failures are no longer silently discarded, using the team-approved error behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, javascript, nodejs
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100