WebSocket

[ronag]

https://fetch.spec.whatwg.org/#websocket-protocol

[mcollina]

cc @lpinca

[szmarczak]

This should be quite easy. Reference: https://github.com/szmarczak/http2-wrapper/blob/master/examples/ws/index.js

[lpinca]

@szmarczak I know it is a demo but that example basically skips the opening handshake. There is no validation and no subprotocol/extension negotiation.

[WenheLI]

Interested in this one. Would try to implement if it is availabe!

[joshxyzhimself]

Are there any advantages in building websockets here on top of undici compared to what exists in https://github.com/websockets/ws? (maybe performance?)

[mcollina]

I'm totally ok in using ws for this. It's a great module by @lpinca and I hope we will emebed it in core one day.

[jodevsa]

@mcollina this would be really a great opportunity for me to learn more about WS. Can I start working on this?

[mcollina]

I don't think we should do another implementation, I think the ws module is great, we might just pull it in as a dep.

cc @lpinca @KhafraDev

[KhafraDev]

ws is great but I think the implementation is too different from the spec WebSocket. There's also outstanding issues where it uses a custom EventTarget (https://github.com/websockets/ws/issues/1818). Then there's a bunch of webidl issues I could spot with a cursory glance (Websocket extends EventEmitter, underscore indexed properties) and the fact that WPTs aren't being run.

Undici (or maybe the nodejs org) should fork or add ws as a git submodule where we could make breaking changes from the ws api & run WPTs, to ensure cross compatibility.

---

Basically I see this as another spec fetch vs. node-fetch issue. Both might share a similar API but are different enough that there isn't a cross-compatibility guarantee.

[lpinca]

Following the https://websockets.spec.whatwg.org/ spec in every single detail is a non-goal for ws. It is limiting and there is no real advantage in my opinion.

I think jsdom runs the websockets web-platform-tests. They use ws under the hood with some minor adjustments.

[mcollina]

I think we should have our own implementation then. Maybe reusing ws or some parts of it under the hood.

[jodevsa]

@mcollina i will give it a try and formulate a plan before implementing so that we don't waste time

[jodevsa]

I think we should have our own implementation then. Maybe reusing ws or some parts of it under the hood.

I'm still not quite sure about how we want to implement it. Do we wanna use ws as a dependency inside undici or do we want to implement WS from scratch and maybe get some ideas from ws?

[mcollina]

Wdyt @KhafraDev?

[KhafraDev]

Using ws:

• uses http(s) under the hood
• no undici Dispatcher
• less security risk

Creating our own:

• faster websockets (probably)
• can use undici dispatchers (do websockets work with proxies?)
• more potential security risks

If possible, having our own implementation that borrows from ws would be the ideal solution. We would need to implement the entirety of the WebSocket class in either case, for the reasons mentioned above ("Following the ... spec ... is a non-goal for ws").

[lpinca]

• faster websockets (probably)

The WebSocket protocol has nothing to do with HTTP apart from the initial handshake. Once the initial connection is established the work is done against a {net,tls}.Socket. I'm sure a new implementation would be faster but it won't be faster just because you can reuse something in undici. You have to write a faster parser and message writer. Also, the parser and message writer are not the bottlenecks in ws. The bottleneck is net.Socket. Of course, ws adds some overhead over a plain net.Socket, that is inevitable. A new implementation can try to reduce that overhead, for example by corking writes, but I think it will not be much faster.

• can use undici dispatchers (do websockets work with proxies?)

Yes, they do, but not all HTTP proxies can proxy WebSocket.

---

ws was not designed to be used in fetch. This part of the WHATWG spec https://websockets.spec.whatwg.org/#websocket-protocol alters RFC 6455 and refers to browser related things (CSP, HSTS) that do not make sense in a server side context. It should be implemented from scratch.

Other ws deviations from the WHATWG spec that I can think of are:

• The WebSocket class inherits from EventEmitter instead of EventTarget. Inheriting from EventTarget negatively affects performance and is limiting.
• No direct Blob supports. There is no Blob support for Writable.prototype.write() in Node.js.
• Custom additions and events.

[KhafraDev]

I took a shot at it and got close (I think) to a working initial handshake. If anyone wants a place to start, it might be worth checking my branch out https://github.com/KhafraDev/undici/tree/ws/lib/websocket. ~~It doesn't currently work btw (the onUpgrade callback is never called).~~

[jimmywarting]

👍 for rolling our own that is more spec compatible with wpt tests and webidl stuff would like things like EventTarget instead of EventEmitter and blob support.

I would really much like it if it where possible to get a blob from the file system, send it with socket.send(blob) and for it to be no data passing via the main thread. Data would instead just be directly read from the disc to the socket with c and uses a faster system call (Like Bun.js blobs) same goes for other fs method like writing/copying/moving blobs could just use fast paths instead of passing data via the js stack

[KhafraDev]

I took another stab at the initial handshake and got it working! The mistake was really dumb on my part (sha1 is a supported hash, not SHA-1 and some error swallowing).

import { WebSocket } from './lib/websocket/websocket.js'
import { getGlobalDispatcher } from './index.js'

new WebSocket('wss://echo.websocket.events/')

Anyone is free to work/contribute to my branch - help would be appreciated(!!). I'm making some progress, but there is still a lot left to do.

---

Fetch is now integrated with the websocket spec & the initial handshake has been rewritten to follow the websocket spec where applicable.

[jodevsa]

Hi @KhafraDev,

great start! I'm really interested in working with you on this. I hope I can make some progress today; I'll try to leave some handovers in the PR comments

[jodevsa]

Hi @KhafraDev,

I was able to implement sending TEXT data and do the masking logic; we are currently able to send and receive text messages through WS:

const { WebSocket } = require('./lib/websocket/websocket.js')

const ws = new WebSocket('wss://echo.websocket.events/')

ws.addEventListener('message', (event) => {
  console.log('received:', event.data) // "echo.websocket.events sponsored by Lob.com"
})

ws.addEventListener('open', () => {
  let i = 0
  setInterval(() => {
    const text = `hello. This is message number #${i}`
    console.log('sending:', text)
    ws.send(text)
    i++
  }, 1000)
})

please have a look at my PR: https://github.com/KhafraDev/undici/pull/30