search

mirage / irmin-server

A high-performance server for Irmin
ISC License
24 stars 7 forks source link

Feat: implementation of websocket with updated examples #47

Closed dinakajoy closed 2 years ago

dinakajoy commented 2 years ago

This PR implements a cli and browser communication between a client and a server on irmin-server via websocket. This solution impose a stream-like structure layered on top of the message-oriented websocket connection. More details on the issue #46

zshipko commented 2 years ago

Thanks, this is great :smile:

I will need a little more time to do a full review, but based on CI it looks like the .opam files need to be updated to match the changes to the dune files. I think you will need to add pin-depends for irmin and graphql since this depends on unreleased changes.

zshipko commented 2 years ago

Nice, CI is passing :partying_face: I will look everything over once more before I merge.

@dinakajoy: Would you please remove the duplicate CI commits?

dinakajoy commented 2 years ago

This is great! I have a few suggestions, please feel free to ignore them. Great work :)

Thank you for taking time to go through it. I will effect the suggested changes, thank you. Regarding the ws and wss question, it's just like http and https but for websocket (@zshipko @patricoferris)

zshipko commented 2 years ago

What's the purpose of base64 encoding the messages? Is that for the browser?

patricoferris commented 2 years ago

The Base64 encoding is needed because OCaml strings are just bytes whereas Websockets use UTF-8 [1] to send and receive messages. With the unix clients we don't hit any problems because ocaml-websocket doesn't do any checks or conversions. In the browser, however, this is not the case and things fail if the message is not UTF-8. Base64 is valid UTF-8 so this seemed the simplest workaround.

Websockets do support binary mode but even when I tried this I couldn't get it to work, the encoding of strings in js_of_ocaml/brr is a little confusing to me. See [1] for that also.

[1] https://datatracker.ietf.org/doc/html/rfc6455#section-5.6