nodejs / nodejs/node

setkeepalive < 1000 silently ignored

Open
#57,712 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

doc net
Dominant language
JavaScript
Stars
122k
Forks
37.3k
Avg merge
4d 2h
Merged PRs (30d)
283

Description

Version

22.14.0

Platform
N/A
Subsystem

No response

What steps will reproduce the bug?

Create a TCP socket and call socket.setKeepAlive(true, 400).

How often does it reproduce? Is there a required condition?

Any keepalive less than 1 second.

What is the expected behavior? Why is that the expected behavior?

This should send keepalive packets every 400 ms.

The documentation says as much:

keepAliveInitialDelay {number} If set to a positive number, it sets the
initial delay before the first keepalive probe is sent on an idle socket.
Default: 0.

What do you see instead?

The code truncates to the nearest 1000ms, so the socket doesn't send keepalive packets and no error is issued.

Additional information

It seems this parameter is even broken in the tests.

https://github.com/nodejs/node/blob/3db54912fa2e4a696d4e2ec27dfb2aeecfc65191/test/parallel/test-net-keepalive.js#L42
https://github.com/nodejs/node/blob/3db54912fa2e4a696d4e2ec27dfb2aeecfc65191/test/parallel/test-net-persistent-keepalive.js#L30

Ideally, the keepalive interval would be respected in milliseconds. Assuming that's infeasible,

  1. a runtime warning should be issued if setKeepAlive is given a value <1000.
  2. the documentation should be updated to reflect that, even though the value is given in milliseconds, it will only be respected to the second.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the socket.setKeepAlive(true, 400) entry point and inspect the behavior covered by test/parallel/test-net-keepalive.js and test/parallel/test-net-persistent-keepalive.js. Determine whether subsecond delays should be supported or rejected, then update the relevant tests and documentation so the chosen behavior is explicit and verified.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.