Kylegoetz udp

[kylegoetz]

Overview

There were no UDP types or terms. Now there are:

• UDPSocket
• ListenSocket
• ClientSockAddr
• IO.UDP.clientSocket.impl.v1 : Text -> Text -> Either Failure UDPSocket -- first param is hostname, second is port to connect to
• IO.UDP.ClientSockAddr.toText.v1 :: ClientSockAddr -> Text
• IO.UDP.UDPSocket.toText.impl.v1 :: UDPSocket -> Text
• IO.UDP.UDPSocket.close.impl.v1 :: UDPSocket -> Either Failure ()
• IO.UDP.UDPSocket.socket.impl.v1 :: UDPSocket -> Socket removed, see discussion below
• IO.UDP.serverSocket.impl.v1 :: Text -> Text -> Either Failure ListenSocket -- first param is the IP address, second is port
• IO.UDP.ListenSocket.recvFrom.impl.v1 :: ListenSocket -> Either Failure (Bytes, ClientSockAddr)
• IO.UDP.ListenSocket.sendTo.impl.v1 :: ListenSocket -> Bytes -> ClientSockAddr -> Either Failure ()
• IO.UDP.ListenSocket.toText.impl.v1 :: ListenSocket -> Text
• IO.UDP.ListenSocket.close.impl.v1 :: ListenSocket -> Either Failure ()
• IO.UDP.ListenSocket.socket.impl.v1 :: ListenSocket -> Socket removed, see discussion below
• IO.UDP.UDPSocket.recv.impl.v1 :: UDPSocket -> Either Failure Bytes
• IO.UDP.UDPSocket.send.impl.v1 :: UDPSocket -> Bytes -> Either Failure ()

Closes #4757

Implementation notes

1. Adds Network.UDP (Haskell) and racket/udp (JIT) as dependencies.
2. exposes the above functions as builtins (with a small bit of mapping for Text-wrapping newtypes + Bytes/ByteString mapping)

Interesting/controversial decisions

I chose to use the existing Network.UDP Haskell library because it appeared to be the easiest way to add UDP built-ins to Unison.

The Haskell library tries to parallel a TCP connection-based library even though UDP is a connection-less protocol.

It might have been better to write more Haskell to create our own sockets configurable with UDP. Maybe a Unison Socket builtin that takes the same parameters as the Haskell Socket rather than a pre-baked TCP socket. Then implement everything at a lower-level and expose that to Unison, where Unison could have something like:

type Socket = <builtin>
type UDPSocket = UDPSocket Socket
type TCPSocket = TCPSocket Socket

createUdpClient :: HostName -> Port -> Either Failure UDPSocket
createUdpClient = cases HostName h, Port p -> builtInCreateSocket h p "DATAGRAM"

createTcpClient :: HostName -> Port -> Either Failure TCPSocket
createTcpClient = cases HostName h, Port p -> builtInCreateSocket h p "STREAM"

This might actually be a good improvement in the future because it's my understanding that you might need alternative configuration for the underlying Socket in order to support UDP-based multicast, e.g. Don't quote me on that.

Edit The JIT code mirrors the Haskell, which turns out to be similar under the hood.

Test coverage

There are tests found at @unison/runtime-tests/@kylegoetz/udp

Loose ends

Maybe could use more testing to verify the Racket/JIT is actually spitting out () when it's supposed to (on send-y functions and close).

One important thing to note is that when creating a client, the parameters are hostname and port, but when creating a server, the parameters are IP address and port. Because there is no IPAddress type in base currently, (contrast with HostName), I've relied on readMaybe to cast the Text coming from Unison to Maybe IP and then failing on Nothing but invoking the Haskell server creation on Just ip.

[aryairani]

Hi @kylegoetz this is exciting! You could try try taking a look at tcp-tests.u and see if that provides some inspiration for testing the UDP functionality. The more we can test, the better. Especially with a [email protected] 😅

@runarorama Could you take a look when you have a chance and weigh in on Kyle's Decisions / Tests / and IPAddress loose end? Is there anything you'd change about the API before we locked in some builtins?

@ChrisPenner @dolio Could you give a spot check as well?

[ChrisPenner]

I'll defer to Dan for calling conventions and Rúnar for interface design, so aside from that I'm not seeing any issues 👍🏼

[dolio]

I think this eventually won't pass ormolu.

[aryairani]

I changed the workflows a bit to be more flexible, we'll see what happens.

[dolio]

Looks okay to me. I was a little skeptical about network-udp-0.0.0, but it looks pretty simple, mostly wrapping network and other stuff, and I don't see any other udp implementations that just provide normal socket APIs (like, not some kind of streaming framework).

[kylegoetz]

@aryairani I've pushed a udp-tests.u and updated the tests.u to reference it.

A couple notes:

1. putting the UDP tests into a scratch file alongside the Tests ability + supporting terms, I was able to main = do Tests.run udp.tests and all the tests pass (and when I intentionally broke some to make sure the tests were written correctly, those tests would fail)
2. putting the code into udp-tests.u. and then updating tests.u with a line !udp.tests below !tcp.tests causes the integration tests to fail.

```ucm
    .> load unison-src/builtin-tests/tests.u.> add
    ```
    🛑
    The transcript failed due to an error in the stanza above. The error is:
      I couldn't figure out what udp.tests refers to here:
         11 |   !udp.tests
      I think its type should be:
          -- [snip]      
          Unit -> Unit

If I copy the udp.tests term into tests.u so there can be no doubt that it will find the tests, then I get the error

I couldn't find a term for ##IO.UDP.clientSocket.impl.v1.
         69 | UDP.client h p = Either.toException (##IO.UDP.clientSocket.impl.v1 h p) 
      Make sure it's spelled correctly.

But in UCM I can view ##IO.UDP.clientSocket.impl.v1 and see

test/main> view ##IO.UDP.clientSocket.impl.v1
  builtin UDP.client.impl :
    lib.base_2_17_0.Text
    -> lib.base_2_17_0.Text
    ->{lib.base_2_17_0.IO} Either Failure UDP.UDPSocket

I don't know if the integration tests uses a pre-built UCM binary rather than my custom built one with the new UDP built-ins using stack build and then stack exec unison, but that would explain the failure. And obviously I'm using the hashes for these built-ins since they're not part of base yet. I've created utility functions for the integration tests

UDP.client h p = Either.toException (##IO.UDP.clientSocket.impl.v1 h p)
UDP.server i p = Either.toException (##IO.UDP.serverSocket.impl.v1 i p)
closeClient = ##IO.UDP.UDPSocket.close.impl.v1 >> Either.toException
send s b = Either.toException (##IO.UDP.UDPSocket.send.impl.v1 s b)
closeServer = ##IO.UDP.ListenSocket.close.impl.v1 >> Either.toException
sendTo s b a = Either.toException (##IO.UDP.ListenSocket.sendTo.impl.v1 s b a)

and Unison doesn't complain about these hashes when part of a scratch file and run as part of the main I pasted above.

[kylegoetz]

@runarorama if IpAddress as a data type that doesn't exist in base currently will be addressed, the underlying Haskell for the IP type used here is (as one would expect) a sum type of IPv4 and IPv6 data constructors. So both v4 and v6 (but no other formats—if there are any) are supported by the UDP library when constructing a UDP "server."

Also, for all involved, if y'all want the interface to be different in some way, I can rewrite or add built-ins. I just tried to parallel the TCP terms/types as closely as possible while taking into account the different natures of UDP vs TCP (i.e., connection-less vs connection-present).

[aryairani]

I merged trunk so that CI will start running again.

[aryairani]

I haven't had a chance to try to reproduce the testing issue you mentioned, but I will try soon.

[aryairani]

I don't know if the integration tests uses a pre-built UCM binary rather than my custom built one with the new UDP built-ins using stack build and then stack exec unison

No, it shouldn't be; if it is, then that's a bug in the CI setup. I think there might be one, I'm looking into it now.

[aryairani]

looks good I think, will merge after one more test pass