Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node: add dgram.Socket APIs #18324

Open
kt3k opened this issue Apr 14, 2022 · 7 comments
Open

node: add dgram.Socket APIs #18324

kt3k opened this issue Apr 14, 2022 · 7 comments
Labels
feat new feature (which has been agreed to/accepted) node compat

Comments

@kt3k
Copy link
Member

kt3k commented Apr 14, 2022

dgram.Socket is needed to run multicast-dns in compat mode. (feature request from community)

@cmorten
Copy link
Contributor

cmorten commented May 9, 2022

Will take a look if no-one is already on this?

@cmorten
Copy link
Contributor

cmorten commented May 18, 2022

TODO:

  • socket.addMembership()
  • socket.addSourceSpecificMembership()
  • socket.dropMembership()
  • socket.dropSourceSpecificMembership()
  • socket.setBroadcast()
  • socket.setMulticastInterface()
  • socket.setMulticastLoopback()
  • socket.setMulticastTtl()
  • socket.setTtl()
  • socket.ref()
  • socket.unref()

@kt3k kt3k changed the title compat: add dgram.Socket node: add dgram.Socket APIs Mar 21, 2023
@kt3k kt3k transferred this issue from denoland/std Mar 21, 2023
@bukhalo
Copy link

bukhalo commented Aug 3, 2023

I'm sorely missing the implementation of the rest of dgram's methods, someone please!

@bukhalo
Copy link

bukhalo commented Sep 27, 2023

I looked at the source code and I see that the methods in the list above, have already been released. But I still can't use the libraries that depend on dgram.

Here is for an example code that works in Node but doesn't work in Deno:

import dgram from "node:dgram";

const socket = dgram.createSocket({
  type: "udp4",
  reuseAddr: true,
});

socket.bind(5353)

socket.on("error", (err) => {
  console.error(err);
});

When running in Deno, I get this error:

Error: bind EADDRINUSE 0.0.0.0:5353
    at __node_internal_captureLargerStackTrace (ext:deno_node/internal/errors.ts:91:9)
    at __node_internal_exceptionWithHostPort (ext:deno_node/internal/errors.ts:215:10)
    at node:dgram:273:20
    at Array.processTicksAndRejections (ext:deno_node/_next_tick.ts:36:15)
    at eventLoopTick (ext:core/01_core.js:180:29) {
  errno: -48,
  code: "EADDRINUSE",
  syscall: "bind",
  address: "0.0.0.0",
  port: 5353
}

@cmorten maybe you can help me figure out the problem?

@cmorten
Copy link
Contributor

cmorten commented Sep 27, 2023

Hi @hey-ab 👋

Trying to refresh mem as this was a while ago! From what I can see the TODO list I posted still stands - much of the dgram module internals is yet to be implemented (requires porting C code to Deno APIs).

R.E. your specific issue, I think it’s most likely due to the socket address reuse capability never being implemented, see https://github.com/denoland/deno/blob/main/ext/node/polyfills/internal_binding/udp_wrap.ts#L322

@bukhalo
Copy link

bukhalo commented Sep 27, 2023

Hmm... That's sad, unfortunately I've never written in Rust and won't be able to help out. 😢
Am I correct in assuming that my PR is still valid in this case? denoland/docs#150

@cmorten
Copy link
Contributor

cmorten commented Sep 27, 2023

Hmm... That's sad, unfortunately I've never written in Rust and won't be able to help out. 😢 Am I correct in assuming that my PR is still valid in this case? denoland/deno-docs#150

When I say Deno APIs I meant JS/TS using the Deno.* namespace (e.g. https://deno.land/api@v1.37.0?unstable=&s=Deno.listenDatagram) - it might be that Rust isn’t needed here, often it’s hooking up the appropriate calls. Worth doing the analysis to see if possible or if there’s a feature gap on APIs or their options.

That said if there’s functionality that is required which Deno doesn’t support then yes, some Rust needed.

If this is documented that the UDP/dgram layer is fully supported then yes would agree that is misleading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat new feature (which has been agreed to/accepted) node compat
Projects
None yet
Development

No branches or pull requests

4 participants