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

IO accepts dht-rpc options but never uses socket #39

Open
smoyer64 opened this issue Mar 21, 2022 · 3 comments
Open

IO accepts dht-rpc options but never uses socket #39

smoyer64 opened this issue Mar 21, 2022 · 3 comments

Comments

@smoyer64
Copy link

Since IO isn't supposed to be part of the public API, it used to be possible to provide an already established socket via the constructor. As IO is currently written, the option can be passed but it's never used as both clientsocket and serversocket are connected without considering its presence. See https://github.com/mafintosh/dht-rpc/blob/7527e8f83e7457750a0ece5469ed3b579c5799de/lib/io.js#L136.

There's a further bit of confusion in this function - it appears that you can provide either a port number or a function to provide a socket using the bind option but this parameter is also simply passed-through from the dht-rpc options. This dual use of the bind option isn't currently documented in the dht-rpc API. I understand that this is considered the RI for use in hyperswarm and that implementations in other languages are encourage to follow that language's conventions but I'm trying to write an implementation with the same public API. Replicating a duck-typed option in a strongly typed language is going to encourage awkward code.

@mafintosh
Copy link
Contributor

Can you simplify what you mean here? Do you want to pass in a socket in the constructor and listen on that?

@smoyer64
Copy link
Author

There are two facets above (and maybe should have been two issues):

  1. If a socket is provided as a dht-rpc option, that socket should be used to create the IO.

  2. Bind being either a port number OR a function to return a socket is confusing for users and hard for implementations in strongly-typed languages (even if the language provides sum types, it's still confusing in the API).

@mafintosh
Copy link
Contributor

dht-rpc needs two sockets - one for when it's persistent and one for when it's used ephemerally (it auto switches between the modes). It needs two for firewall session issues. For the second issue, you don't have to mirror this api in an above implementation if there is another paradigme that makes more sense for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants