Make TCP server websocket-aware #408
Merged
+1,575
−678
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
karlseguin
changed the title
karlseguin
force-pushed
the
websocket_server
branch
from
2981a50 to
5bfebb6
Compare
Open
|
@karlseguin The change LGTM. thank you! |
Adding HTTP & websocket awareness to the TCP server. HTTP server handles `GET /json/version` and websocket upgrade requests. Conceptually, websocket handling is the same code as before, but receiving data will parse the websocket frames and writing data will wrap it in a websocket frame. The previous `Ctx` was split into a `Server` and a `Client`. This was largely done to make it easy to write unit tests, since the `Client` is a generic, all its dependencies (i.e. the server) can be mocked out. This also makes it a bit nicer to know if there is or isn't a client (via the server's client optional). Added a MemoryPool for the Send object (I thought that was a nice touch!) Removed MacOS hack on accept/conn completion usage. Known issues: - When framing an outgoing message, the entire message has to be duped. This is no worse than how it was before, but it should be possible to eliminate this in the future. Probably not part of this PR. - Websocket parsing will reject continuation frames. I don't know of a single client that will send a fragmented message (websocket has its own message fragmentation), but we should probably still support this just in case. - I don't think the receive, timeout and close completions can safely be re-used like we're doing. I believe they need to be associated with a specific client socket. - A new connection creates a new browser session. I think this is right (??), but for the very first, we're throwing out a perfectly usable session. I'm thinking this might be a change to how Browser/Sessions work. - zig build test won't compile. This branch reproduces the issue with none of these changes: https://github.com/karlseguin/browser/tree/broken_test_build (or, as a diff to main): lightpanda-io/browser@main...karlseguin:broken_test_build
Move more logic into the reader. Avoid copying partial messages in cases where we know that the buffer is large enough. This is mostly groundwork for trying to add support for continuation frames.
karlseguin
force-pushed
the
websocket_server
branch
from
5bfebb6 to
701e827
Compare
done |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding HTTP & websocket awareness to the TCP server.
HTTP server handles
GET /json/versionand websocket upgrade requests.Conceptually, websocket handling is the same code as before, but receiving
data will parse the websocket frames and writing data will wrap it in
a websocket frame.
The previous
Ctxwas split into aServerand aClient. This waslargely done to make it easy to write unit tests, since the
Clientisa generic, all its dependencies (i.e. the server) can be mocked out. This
also makes it a bit nicer to know if there is or isn't a client (via the
server's client optional).
Added a MemoryPool for the Send object (I thought that was a nice touch!)
Removed MacOS hack on accept/conn completion usage.