`Socket.prototype._handle is deprecated` warning is caused by `MDNS.ts`

[CMCDragonkai]

Describe the bug

Inside MDNS.ts, it's using _handle in a few places. In particular:

        if (platform === 'linux' && disableLinuxMulticastAllOption) {
          socketUtils.disableSocketMulticastAll(
            (unicastSocket as any)._handle.fd,
          );
        }

          if (platform === 'linux' && disableLinuxMulticastAllOption) {
            socketUtils.disableSocketMulticastAll((socket as any)._handle.fd);
          }

The usage of _handle in the Socket prototype causes warnings.

It looks like this in PK CLI:

(node:500471) [DEP0112] DeprecationWarning: Socket.prototype._handle is deprecated
(Use `node --trace-deprecation ...` to show where the warning was created)

Right after starting the agent.

Find an alternative way to do the above, or we need some way of disabling the warning? This is some sort of internal property right?

Apparently Node allows disabling warnings on startup.

To Reproduce

1. Run Polykey-CLI polykey agent start
2. Observe warning in the terminal

Expected behavior

No warnings should occur.

Additional context

• https://nodejs.org/api/deprecations.html#DEP0112

Notify maintainers

@amydevs

[linear]

ENG-394 `Socket.prototype._handle` is deprecated` warning is caused by `MDNS.ts`

[CMCDragonkai]

One way to get around this warning is potentially doing all the work in the C++ level? One way to do this is pass the reference to the JS socket object to the native code side, and get the native code to do the necessary referencing of the underlying FD.

Also can we start changing all our native code to Rust to align with js-quic and js-exec? I know js-db is still pending, but it's usually better.

Look at node native code docs as to how to directly interact with socket objects.

[aryanjassal]

If we are only using the net.Socket._handle to extract the fd of the socket, can't we just create the socket with a predetermined fd and reference that wherever we need to?

The new net.Socket() constructor has an option to specify the fd. We can use the C++ socketUtils to find the closest unoccupied file descriptor, and use that while construction, and keep track of that in a Map or a Record. We can then later reference that when we need to call disableSocketMulticastAll.

Doing it this way would be simpler than directly interacting with the actual Socket object.

https://nodejs.org/api/net.html#new-netsocketoptions

@CMCDragonkai

[CMCDragonkai]

I'm not really sure what that means. You'd have to try it. But my understanding is that we are creating sockets for this purpose and so using the rust native code is better anyway to align all of our native libraries.

[aryanjassal]

After looking over the source code a bit more, my solution feels more like a temporary patch than a permanent solution. As I couldn't find much information about this online, I asked ChatGPT, and it agreed with you that creating a native binding which manages the entire socket lifetime seems to be the best solution, as no API currently exists which allows us to do the fine-grained multicast manipulation or get the fd without relying on _handle.

[amydevs]

@aryanjassal i've tried to implement the solution you had suggested originally when working on it in the https://github.com/MatrixAI/js-mdns/tree/feature-remove-warning branch. However, I was not able to successfully get the node socket to correctly attach to the fd of the existing socket. There should be a test written in that repo that demonstrates this behaviour.

[tegefaulkes]

If I recall, the socket modification was to avoid some extra logic in the js-mdns library to handle it. We may just have to do that in the end to resolve this. @amy I don't recall the exact details of this. Can you give a summary of why the socket was modified leading to this warning and what it was trying to avoid in the code. So basically how need to fix this.

[CMCDragonkai]

It's cause disableSocketMulticastAll needs to access to the fd number which is hidden/encapsulated within the socket APIs.

Therefore I believe it's better to go down one layer into the C++ and do it via rust plugin like we do with other native code instead.

Plus doing this would be required to create a proper native MDNS stack that works well with the OS stack anyway (right now our MDNS stack doesn't co-operate with the OS native stack).