Fix client. Add server. Add Lwt support.
Hi @seliopou,
I've been looking into using httpaf for ocaml-graphql-server, but that requires a supporting websocket library. After stumbling on websocketaf, I went down the rabbit hole of making websocketaf actually work (though there are still some deficiencies that need to be addressed!).
The initial state of the repo appeared to outline the direction you wanted to take the library in. I've tried to continue in that direction, for the most part keeping the signatures that were already in place, and further taking inspiration from httpaf. I've attempted to match the code style too (though you be the judge of that).
Broadly speaking, there are four areas of changes listed below.
Core
I've switched from jbuilder to dune, and introduced bigstringaf as a dependency (4efc11a, a5e591c).
The frame parsing and serialization logic in Websocket.Frame had a number of subtle bugs, primarily around the bit fiddling (e3d7a1e, bba1257). I've added a couple of tests which helped sort that out, though even more tests would be helpful indeed (8382d0e).
I've fixed up Reader, taking inspiration from httpaf, though it does not validate the frame yet, i.e. it operates on a parser of type unit Angstrom.t rather than (unit, 'error) result Angstrom.t (
d45840f). This could be a future improvement.
I've updated Wsd in a couple of ways (e0cf9bb):
- Added a
mode, which determines whetherWsdis acting as a client or server. This takes care of masking payloads appropriately, and means the optional argument?maskis no longer required forWsd.scheduleandWsd.send_bytes. - Added a function
Wsd.when_ready_to_write, similar toHttpaf.Body.when_ready_to_write, which is used when a server connection yields writing (Server_connection.yield_writer). - Added functions for interacting with the "I/O loop" (
next,report_result).
Client
I tried to complete the client code following the approach you had already set up: transitioning from Uninitialized to Handshake to Websocket (ed99164).
The most tricky part was transitioning from Handshake to Websocket: you need change the state to Websocket and then close the Httpaf.Client_connection.t in this particular order to trigger the Httpaf.Body.when_ready_to_write-hook which will continue the IO loop. This was quite the puzzle for me 😅
Server
Server support was needed for my own purposes, so I added that (65185b0). It follows the same approach as the client connection code by transitioning from Uninitialized to Handshake to Websocket:
Server_handshakeis the initial websocket handshake, and is essentially aHttpaf.Server_connection.t. It could be arguably be omitted since it does not add additional functionality, however further improvements could change that.Server_websockethandles the connection once its been upgraded to the websocket protocol. It usesReaderto read frames, andWsdto send frames.Server_connectionis responsible for composing the behavior ofServer_handshakeandServer_websocket.
As with the client code, the trickiest part is transitioning from Handshake to Websocket in Server_connection. This is challenging because this transition should happen after the handshake response has been written to the wire. There is currently no hook in httpaf for this without writing a non-empty response, in which case Httpaf.Body.flush can be used. The current code writes a single space in the response body, but a better solution should replace this. My ideas are:
- Add a
flushfunction toHttpaf.Server_connection. - Count the number of bytes written in
Server_handshaketo determine when the response has been fully transferred.
What do you think? Maybe you have better ideas.
Lastly I should mention that the HTTP handshake request is currently not validated by the server, but obviously it should be (Server_connection.passes_scrutiny) -- I've considered it future work.
Lwt support
To actually test the client and server, I've implemented Lwt support as a new OPAM package, websocketaf-lwt (db0c615). It's basically copy/paste of httpaf-lwt. Error reporting is missing though (future work).
Finally, two examples are included (9ada40d):
-
examples/lwt/echo_serverruns a websocket server and echoes frames back to clients. To run:dune exec examples/lwt/echo_server.mlYou can then use a client, e.g.
wscatfrom NPM, as follows:wscat localhost:8080 -
examples/lwt/wscatreads lines from stdin as text frames and sends to a websocket server. Received frames are written to stdout.dune exec examples/lwt/wscat.exe echo.websocket.org
Note that the two examples, wscat and echo_server, does not work together, since httpaf strictly disallows a body for 101 responses (other clients typically are more lenient).
Summary
I realise this is a massive PR, which is always unfortunate. I felt like I had to get to a working example -- interacting with external websocket clients and servers -- to validate that the code actually worked. If you would consider it helpful, I can split this PR into multiple PRs. Some of the changes are probably less contentious than others. I know @anmonteiro has done further work to address some of the limitations listed above. I expect he'll follow up with another PR, or we can combine our changes if you prefer.
Let me know how to best proceed. Thanks! 🙏
Thanks for sending this over. Integrating this with httpaf properly requires some support for upgrading connections, which I hope to implement in next couple weeks. Once I get something working, I'll ping this issue again to see how easy it will be to adapt to it.
Thanks again!
Np, thanks for the update 👍
This is great, thanks for the clear description of the changes and the organized patches. It's making it very easy to evaluate your contributions and get them merged in.
I thought a good way to proceed was to first get master to actually build with the obvious fixes, and then start looking at the other parts. So to that end I've been cherry-picking some of the commits these commits in separate PRs to start moving master to that state.
Glad that it's helpful 😄 Let me know if I can help in any way.
Hello! I'm looking at using websockets in a project and this PR looks like a promising contribution. What's the status here?