From 1e4c167b370c76ac74908ede46833a9044631f94 Mon Sep 17 00:00:00 2001 From: Avocado Date: Tue, 25 Aug 2026 14:40:54 +0900 Subject: [PATCH] net: warn on keep-alive delays truncated to zero The keep-alive delays are given in milliseconds but the underlying socket options are configured in whole seconds, so a positive value below 1000 ms rounds down to 0. That leaves the system default in place instead of applying the requested timing, and there is nothing to indicate that the value had no effect. Emit a KeepAliveWarning when a positive initialDelay or interval is truncated to zero, and document the behaviour. A value of 0 keeps its documented meaning of leaving the current setting unchanged and does not warn. Two existing tests passed values below 1000 ms that did not match what their comments described; they now use 1000 ms. Refs: https://github.com/nodejs/node/issues/57712 Signed-off-by: Avocado --- doc/api/net.md | 7 +- lib/net.js | 19 +++ .../test-net-keepalive-truncation-warning.js | 108 ++++++++++++++++++ test/parallel/test-net-keepalive.js | 5 +- .../parallel/test-net-persistent-keepalive.js | 2 +- 5 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-net-keepalive-truncation-warning.js diff --git a/doc/api/net.md b/doc/api/net.md index ab569a7b9c6f..5bfa20adbb1d 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -1669,7 +1669,12 @@ corresponding system default unchanged. `initialDelay` and `interval` are specified in milliseconds but the underlying socket options are configured in whole seconds; the values are -divided by `1000` and rounded down before being applied. +divided by `1000` and rounded down before being applied. A positive value +below `1000` therefore rounds down to `0`, which leaves the corresponding +system default unchanged rather than applying the requested timing. Since +this is rarely intended, a `KeepAliveWarning` process warning is emitted in +that case. Sub-second timings cannot be expressed: use a value of at least +`1000` milliseconds. Enabling the keep-alive functionality will set the following socket options: diff --git a/lib/net.js b/lib/net.js index d2b510c64bbb..dc9eb39b3639 100644 --- a/lib/net.js +++ b/lib/net.js @@ -847,6 +847,20 @@ Socket.prototype.setNoDelay = function(enable) { }; +// The underlying socket options are configured in whole seconds, so a positive +// value below 1000 ms is truncated to 0, which leaves the system default in +// place instead of applying the requested timing. Warn so that this is not +// silently ignored. +function warnOnTruncatedKeepAlive(msecs, seconds, name) { + if (seconds === 0 && msecs > 0) { + process.emitWarning( + `The keep-alive ${name} of ${msecs} ms was truncated to 0 seconds and ` + + 'has no effect. Use a value of at least 1000 ms.', + 'KeepAliveWarning', + ); + } +} + Socket.prototype.setKeepAlive = function(enable, initialDelayMsecs, intervalMsecs, count) { if (enable !== null && typeof enable === 'object') { @@ -861,6 +875,11 @@ Socket.prototype.setKeepAlive = function(enable, initialDelayMsecs, const interval = intervalMsecs === undefined ? undefined : ~~(intervalMsecs / 1000); + if (enable) { + warnOnTruncatedKeepAlive(initialDelayMsecs, initialDelay, 'initialDelay'); + warnOnTruncatedKeepAlive(intervalMsecs, interval, 'interval'); + } + if (!this._handle) { this[kSetKeepAlive] = enable; this[kSetKeepAliveInitialDelay] = initialDelay; diff --git a/test/parallel/test-net-keepalive-truncation-warning.js b/test/parallel/test-net-keepalive-truncation-warning.js new file mode 100644 index 000000000000..363adebabe5b --- /dev/null +++ b/test/parallel/test-net-keepalive-truncation-warning.js @@ -0,0 +1,108 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +// The keep-alive delays are given in milliseconds but the underlying socket +// options are configured in whole seconds. A positive value below 1000 ms is +// truncated to 0 seconds, which leaves the system default in place instead of +// applying the requested timing. Verifies that this emits a warning rather +// than being silently ignored. + +// Warnings are emitted on the process, so the cases run one at a time to keep +// each one's warnings from being observed by the others. +const cases = [ + // A delay below 1000 ms is truncated to 0 and warns. + { + configure: (client) => client.setKeepAlive(true, 400), + check: (messages) => { + assert.strictEqual(messages.length, 1); + assert.match(messages[0], /initialDelay of 400 ms/); + assert.match(messages[0], /at least 1000 ms/); + }, + }, + // The interval is truncated the same way and warns independently. + { + configure: (client) => client.setKeepAlive(true, 5000, 500), + check: (messages) => { + assert.strictEqual(messages.length, 1); + assert.match(messages[0], /interval of 500 ms/); + }, + }, + // Both delays can be truncated by the same call. + { + configure: (client) => client.setKeepAlive(true, 400, 500), + check: (messages) => { + assert.strictEqual(messages.length, 2); + assert.match(messages[0], /initialDelay of 400 ms/); + assert.match(messages[1], /interval of 500 ms/); + }, + }, + // The options object form warns as well. + { + configure: (client) => client.setKeepAlive({ + enable: true, + initialDelay: 999, + }), + check: (messages) => { + assert.strictEqual(messages.length, 1); + assert.match(messages[0], /initialDelay of 999 ms/); + }, + }, + // A delay of at least 1000 ms is applied as requested and does not warn. + { + configure: (client) => client.setKeepAlive(true, 1000), + check: (messages) => assert.deepStrictEqual(messages, []), + }, + // 0 means "leave the current value unchanged" and is not a truncation. + { + configure: (client) => client.setKeepAlive(true, 0), + check: (messages) => assert.deepStrictEqual(messages, []), + }, + // Omitting the delay does not warn. + { + configure: (client) => client.setKeepAlive(true), + check: (messages) => assert.deepStrictEqual(messages, []), + }, + // Nothing is configured when keep-alive is disabled, so there is nothing to + // warn about. + { + configure: (client) => client.setKeepAlive(false, 400), + check: (messages) => assert.deepStrictEqual(messages, []), + }, +]; + +function runCase({ configure, check }, done) { + const messages = []; + const onWarning = (warning) => { + if (warning.name === 'KeepAliveWarning') messages.push(warning.message); + }; + process.on('warning', onWarning); + + const server = net.createServer(); + server.listen(0, common.mustCall(() => { + const client = net.connect( + { port: server.address().port }, + common.mustCall(() => { + configure(client); + client.end(); + })); + + client.on('end', common.mustCall(() => { + server.close(common.mustCall(() => { + // Warnings are emitted on the next tick. + setImmediate(() => { + process.removeListener('warning', onWarning); + check(messages); + done(); + }); + })); + })); + })); +} + +(function next(i) { + if (i === cases.length) return; + runCase(cases[i], common.mustCall(() => next(i + 1))); +})(0); diff --git a/test/parallel/test-net-keepalive.js b/test/parallel/test-net-keepalive.js index 26a4c5b3d648..86d1fb617940 100644 --- a/test/parallel/test-net-keepalive.js +++ b/test/parallel/test-net-keepalive.js @@ -38,8 +38,9 @@ const echoServer = net.createServer(common.mustCall((connection) => { }, 1), common.platformTimeout(100)); connection.setTimeout(0); assert.notStrictEqual(connection.setKeepAlive, undefined); - // Send a keepalive packet after 50 ms - connection.setKeepAlive(true, common.platformTimeout(50)); + // Send a keepalive packet after 1 second. Values below 1000 ms are + // truncated to 0 seconds and would leave keep-alive unconfigured. + connection.setKeepAlive(true, 1000); connection.on('end', function() { connection.end(); }); diff --git a/test/parallel/test-net-persistent-keepalive.js b/test/parallel/test-net-persistent-keepalive.js index 8caf640a4bbe..5dbc8560b35b 100644 --- a/test/parallel/test-net-persistent-keepalive.js +++ b/test/parallel/test-net-persistent-keepalive.js @@ -27,7 +27,7 @@ echoServer.on('listening', common.mustCall(function() { clientConnection = new net.Socket(); // Send a keepalive packet after 1000 ms // and make sure it persists - const s = clientConnection.setKeepAlive(true, 400); + const s = clientConnection.setKeepAlive(true, 1000); assert.ok(s instanceof net.Socket); clientConnection.connect(this.address().port); clientConnection.setTimeout(0);