nodejs / nodejs/node

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

Đang mở
#65,529 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Ngôn ngữ chính
JavaScript
Star
122k
Fork
37.3k
Merge trung bình
4 ngày 2 giờ
Pull request đã merge (30 ngày)
283

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với đường đi socket.setKeepAlive() và TCPWrap::SetKeepAlive trong src/tcp_wrap.cc, sau đó tái hiện trường hợp độ trễ âm từ issue. So sánh cách xử lý này với setTypeOfService() và điều tra hành vi của nền tảng được ghi nhận trên macOS và các hệ thống khác. Được xem là hoàn tất khi các lỗi nền tảng không còn bị âm thầm loại bỏ, bằng cách sử dụng hành vi lỗi đã được nhóm phê duyệt.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
cpp, javascript, nodejs
Lĩnh vực
networking
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
48/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.