nodejs / nodejs/node

net: setKeepAlive() ignores the error returned by the handle

Abierto
#65,529 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
JavaScript
Estrellas
122k
Forks
37.3k
Merge medio
4 d 2 h
PR fusionados (30 d)
283

Descripción

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.

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza con la ruta socket.setKeepAlive() y TCPWrap::SetKeepAlive en src/tcp_wrap.cc; después, reproduce el caso de retardo negativo del issue. Compara el tratamiento con setTypeOfService() e investiga el comportamiento de la plataforma indicado para macOS y otros sistemas. Se considera terminado cuando los fallos subyacentes ya no se descartan silenciosamente, usando el comportamiento de errores aprobado por el equipo.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
cpp, javascript, nodejs
Área
networking
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Bastante claro
Aptitud para principiantes
48/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.