use TryInto<SocketAddress> bound for bind/listen functions in rama

[fa993]

Closes #400

[fa993]

I actually wanted to ask a bit about the error types, I've marked them as io::Error in the bounds since the functions originally returned a result type with an io::Error but I can see that this is causing issues with the callers as not all TryInto are implemented with that.

[GlenDC]

I actually wanted to ask a bit about the error types, I've marked them as io::Error in the bounds since the functions originally returned a result type with an io::Error but I can see that this is causing issues with the callers as not all TryInto are implemented with that.

So in this case we do not control all the different possible types that can be turned into a SocketAddress, given we use the non-sealed TryInto (std) trait. As such it can also be really any error. If you wanted to go very statically typed you could make an enum like enum Error<E> { AddrError(E), BindError(io::Error) }, but only because you expect people to handle it separately. However, there's no reason for that given people can also downcast a BoxError into an io::Error in case they want to catch something specific there.

As such I would just turn these errors into a BoxError and get it over with. This makes it also a lot easier to add more underlying errors in future. Another approach could be in future to split up the address injection from the actual network socket binding. But for now this is probably ok as-is.

Hope this helps you out together with my comments on your code.

Thank you for the swift resolution by te way, greatly appreciated.

[fa993]

LMK if anything else is required

[GlenDC]

LMK if anything else is required

Is it possible that you forget to push that "last" commit mentioned in the comments? I still see io::Error being used?

[fa993]

LMK if anything else is required

Is it possible that you forget to push that "last" commit mentioned in the comments? I still see io::Error being used?

Yeah, sorry about that 😅

[GlenDC]

Great job, seems flawless. Thank you very much for this swift and great first contribution!

[GlenDC]

@fa993 there are some errors, not your fault, but more showing that the work is sadly still incomplete. Like described before, the goal was to keep this change non-breaking.

Atm this fails:

 ^^^^ the trait `From<&std::string::String>` is not implemented for `SocketAddress`

Meaning that TryFrom<&String> is not implemented for SocketAddress while I guess it is for ToSocketAddrs.

Problem is that it implements this: impl<T: ToSocketAddrs + ?Sized> ToSocketAddrs for &T

We cannot implement that. But perhaps you can atleast implement TryFrom<&String> for SocketAddress. Given we anyway parse the string and no own it.

[GlenDC]

Once the tests pass regarding &String -> SocketAddress I'll re-evaluate if we can merge, which I think should be possible.

[GlenDC]

Thanks a lot for this extra mile. Great work. Take care!