-
Notifications
You must be signed in to change notification settings - Fork 572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement websockets #1795
Merged
Merged
Changes from 1 commit
Commits
Show all changes
82 commits
Select commit
Hold shift + click to select a range
bb688ac
initial handshake
KhafraDev e6e2686
minor fixes
KhafraDev 9aea277
feat: working initial handshake!
KhafraDev f1ae69a
feat(ws): initial WebSocket class implementation
KhafraDev bec82fd
fix: allow http: and ws: urls
KhafraDev 7e6725e
fix(ws): use websocket spec
KhafraDev 5030683
fix(ws): use websocket spec
KhafraDev a928c58
feat: implement url getter
KhafraDev ecff3d7
feat: implement some of `WebSocket.close` and ready state
KhafraDev 6ec2701
fix: body is null for websockets & pass socket to response
KhafraDev 39c7b43
fix: store the fetch controller & response on ws
KhafraDev fb84cb8
fix: remove invalid tests
KhafraDev 85a4046
feat: implement readyState getter
KhafraDev 190cabe
feat: implement `protocol` and `extensions` getters
KhafraDev 454a5c3
feat: implement event listeners
KhafraDev 5ae04ee
feat: implement binaryType attribute
KhafraDev c9ff83a
fix: add argument length checks
KhafraDev 484e732
feat: basic unfragmented message parsing
KhafraDev 8619653
fix: always remove previous listener
KhafraDev c70bd8b
feat: add in idlharness WPT
KhafraDev 34758e6
implement sending a message for WS and add a websocketFrame class
jodevsa 6a6b1b3
Merge pull request #30 from jodevsa/ws
KhafraDev a87e53b
feat: allow sending ArrayBuffer/Views & Blob
KhafraDev d983115
fix: remove duplicate `upgrade` and `connection` headers
KhafraDev feee358
feat: add in WebSocket.close() and handle closing frames
KhafraDev 5e61a84
refactor WebsocketFrame and support receiving frames in multiple chunks
jodevsa 0e07468
fixes
jodevsa e288ea5
Merge pull request #31 from jodevsa/ws
KhafraDev cbad426
move WebsocketFrame to its own file
KhafraDev d2462a1
feat: export WebSocket & add types
KhafraDev 1340dd4
fix: tsd
KhafraDev 8d98382
feat(wpt): use WebSocketServer & run test
KhafraDev 3cae2ec
fix: properly set/read close code & close reason
KhafraDev 73c8389
fix: flakiness in websocket test runner
KhafraDev 8b7bc44
fix: receive message with arraybuffer binary type
KhafraDev eb09dc6
feat: split WebsocketFrame into 2 classes (sent & received)
KhafraDev 174c030
fix: parse fragmented frames more efficiently & close frame
KhafraDev 7ee907b
fix: add types for MessageEvent and CloseEvent
KhafraDev 6985bc3
fix: subprotocol validation & add wpts
KhafraDev eae7f76
fix: protocol validation & protocol webidl & add wpts
KhafraDev 682d880
fix: correct bufferedAmount calc. & message event w/ blob
KhafraDev 052a804
fix: don't truncate typedarrays
KhafraDev 5f2fdab
feat: add remaining wpts
KhafraDev 6caaa47
fix: allow sending payloads > 65k bytes
KhafraDev 2d5b9ae
fix: mask data > 125 bytes properly
KhafraDev 0880cef
revert changes to core
KhafraDev 02805e8
fix: decrement bufferedAmount after write
KhafraDev 58114ab
fix: handle ping and pong frames
KhafraDev a099471
fix: simplify receiving frame logic
KhafraDev 346bdb8
fix: disable extensions & validate frames
KhafraDev 5113fb2
fix: send close frame upon receiving
KhafraDev dee913c
lint
KhafraDev b3c4314
fix: validate status code & utf-8
KhafraDev 7741277
fix: add hooks
KhafraDev b1339fb
fix: check if frame is unfragmented correctly
KhafraDev 61c921c
fix: send ping app data in pong frames
KhafraDev c7249d3
export websocket on node >= 18 & add diagnostic_channels
KhafraDev aadf25a
mark test as flaky
KhafraDev 30b1e23
fix: couple bug fixes
KhafraDev 6a7134b
fix: fragmented frame end detection
KhafraDev b6411a6
fix: use TextDecoder for utf-8 validation
KhafraDev 4fbfbf4
fix: handle incomplete chunks
KhafraDev e43fa5b
revert: handle incomplete chunks
KhafraDev 553a0f2
mark WebSockets as experimental
KhafraDev e0289f7
fix: sending 65k bytes is still flaky on linux
KhafraDev dcc3801
fix: apply suggestions
KhafraDev 797acc3
fix: apply some suggestions
KhafraDev dfc57ab
add basic docs
KhafraDev a0094a4
feat: use streaming parser for frames
KhafraDev 883d9d9
feat: validate some frames & remove WebsocketFrame class
KhafraDev 71848d9
fix: parse close frame & move failWebsocketConnection
KhafraDev b468029
fix: read close reason and read entire close body
KhafraDev 8839538
fix: echo close frame if one hasn't been sent
KhafraDev 78b77a8
fix: emit message event on message receive
KhafraDev 6cfc42e
fix: minor fixes
KhafraDev 7f1b5d4
fix: ci
KhafraDev 24ae13b
fix: set was clean exit after server receives close frame
KhafraDev 5086ce8
fix: check if received close frame for clean close
KhafraDev 6441dc3
fix: set sent close after writing frame
KhafraDev 476a192
feat: implement error messages
KhafraDev 28c7ff6
fix: add error event handler to socket
KhafraDev 73128fd
fix: address reviews
KhafraDev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have concurrent writes this might change the order of messages:
The string message is sent before the blob.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know of any good way of fixing this without needing a Promise for every send.. Preferably node would allow writing a blob to a writable/duplex stream.
But then there would be issues masking the body/creating a frame. I guess the only other solution is adding a queue, but that's better for another time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the reason why there is no
Blob
support inws
. This and because the user can already doI agree with this 1
Footnotes
/~https://github.com/ricea/websocketstream-explainer/tree/832f88d7f8eaecbe51da318d802951a3f8aadb2f#non-goals ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, when I was writing out my comment is kinda just clicked - "oh... this is why ws doesn't support blobs", lol. I still think undici should implement it for the sake of complying with the spec.
When I get around to implementing websocketstream this shouldn't be a problem at least!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well,
ws
already uses a queue (only whenpermessage-deflate
is enabled and negotiated since compression is asynchronous) so support forBlob
could be added, but I think it does not worth the effort. I think thatBlob
is of little use in Node.js if not natively supported by streams.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, but
send(await blob.arrayBuffer())
would require reading the hole blob into memory before sending...sure you could do something like:
But then you would get multiple messages and you would have to assemble them yourself on the receiving side as well 😞
if you can accept blob response types then it could as well be piped to a tmp location on the disc and wouldn't hold up any memory when receiving blobs
are there any limit on how large a message can be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, just went into the github repo and learned about a new
WebSocketStream
forgot that it existed, only head about it when it was in early proposal. Well, then you could instead maybe do:blob.stream().pipeTo(websocket)
?Should we implement a
globalThis.WebSocketStream
also?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's up to you. With
ws
you could send each chunk as a message fragment and receive the whole thing as a single message on the receiving end.AFAIK the whole message is buffered in memory before the
'message'
event is emitted. That message is then returned to the user as aBlob
.