inhabitedtype
Support upgrades via I/O operations
Add Reqd.respond_with_upgrade, which will create response with a with status code 101, and the provided headers. On the next write operation, this response, along with the inspiring request will be provided to the runtime. It is up to the runtime to serialize the resposne and send it on the wire before handing off the socket to an upgrade handler.
The runtime code almost certainly has bugs in it, so this isn't going to be merged immediately. Specifically, I'm pretty sure that both in async and lwt the sockets will be closed right after the upgrade handlers are called. Also, neither currently serialize the response on the wire.
Subsumes #134.
There's been renewed interest in using httpaf with ocaml-graphql-server, so I've spent some time revisiting the idea. The primary blocker is websocket support, which this PR is a crucial part of. As such, I've played around with integrating this PR with websocketaf for the sake of understanding it better. Here are my thoughts based on that attempt, though I might be trying to apply respond_with_upgrade wrongly given the details have not been fleshed out yet.
On the next write operation, this response, along with the inspiring request will be provided to the runtime. It is up to the runtime to serialize the resposne and send it on the wire before handing off the socket to an upgrade handler.
It seems to me that the runtime is not particularly well suited for writing the response to the wire. The code for serialising a response is currently kept in Writer, which is not exposed by Httpaf. I assume the runtime would thus need to duplicate the function write_response — or maybe you have something else in mind 🤔
I'm wondering whether a better invariant would be for `Upgrade to signal to the runtime that the upgrade response has already been written to the wire, and that the socket can be handed off to an upgrade handler. I've experimentally implemented that here on top of this branch.
Based on those semantics of `Upgrade, I've revisited the existing PR for websocket server connections and updated it to use respond_with_upgrade. For the sake of completeness, the PR also includes a Lwt runtime and example programs (echo_server.exe and wscat.exe are now compatible because of respond_with_upgrade! 🥳).
It could easily be the case I'm missing something, but on the other hand maybe this feedback is useful. Regardless, I'd be happy to see upgrade support in httpaf. Let me know if I can help 🙏
Hey @andreas, this has been on our minds as well recently. My plan was to spend some time working on this this weekend, and I'll definitely take a look at your PR. The example programs are especially appreciated!
@andreas I force-pushed to this branch so you may want to update your fork. I'm going to try and add some more tests so I can merge #172 and then get this in.
Do you think this approach is workable in general? I'm hoping that something like this can also be adapted for use with sendfile, and other such things.
@andreas I force-pushed to this branch so you may want to update your fork. I'm going to try and add some more tests so I can merge #172 and then get this in.
Thanks for the heads up. I've updated my branch to be based on the latest version of this branch (https://github.com/inhabitedtype/httpaf/compare/upgrade...andreas:upgrade2).
Do you think this approach is workable in general? I'm hoping that something like this can also be adapted for use with sendfile, and other such things.
I like the approach of signalling an upgrade to the runtime. To recap, these are the two different approaches, as I understand them:
A. The semantics suggested in this PR, where `Upgrade means the runtime is responsible for writing the 101 Switching Protocols to the wire.
B. The semantics suggested in my branch, where `Upgrade means 101 Switching Protocols has already been written to the wire.
Do I understand correctly, that you prefer (A) because it would more easily support something like sendfile?
I personally found (B) to be easiest to work with, as I was adapting websocketaf to make use of respond_with_upgrade.
Yeah, my question was regarding how the upgrade is signaled to the runtime. I don't quite know which of A or B to do just yet. We're changing a bunch of stuff related to the server connection module and underlying machinery. Once that work's complete I'll rebase this and make a final determination.
But I'd say as long as the state machine changes for flushing the write and then surfacing the upgrade is simple and straightforward, I wouldn't be opposed to that. Probably when I first wrote this I either thought it was more trouble than it's worth or actually bumped into some issue that made it problematic. We'll see once more once the dust settles.
Hi folks. This PR no longer applies at all cleanly after #172. However, I have tried to recreate a PR with basically the same design in #203.
My PR takes Option B as described by @andreas. This seemed like the much easier method to me, especially as, as was pointed out, Httpaf provides no other way for the client application to write the response to the wire.
