upgradeWebSocket could return a WebSocketStream

[MarkTiedemann]

Currently, Deno.upgradeWebSocket returns a WebSocket. It could return a WebSocketStream, too, for example:

const conn = await Deno.listen({ port: 80 });
const httpConn = Deno.serveHttp(await conn.accept());
const e = await httpConn.nextRequest();
if (e) {
  const { stream, response } = Deno.upgradeWebSocket(e.request);
  const [_, { writable, readable }] = await Promise.all([
    e.respondWith(response),
    stream.connection,
  ]);
  // Use writable and/or readable
}

[crowlKats]

Yes, this is the plan longterm, and was actually the plan originally to some degree (for both statements, its rather not alongside, but instead of), however, the idea is that the server side and client side use the same API for a smoother experience, but since browsers do not have WebSocketStream yet, this isnt exactly the case

[danopia]

I'm looking forward to this feature as well 😃 I have Deno programs as both the client and the server, so I get to do the nice W3C streams stuff on the client side, just to deal with the classic WebSocket callbacks on the server side.

[bartlomieju]

I discussed it with @crowlKats recently and the fact that there's still no spec makes it quite hard to ship in Deno.

@crowlKats anything changed since we spoke about it?

[crowlKats]

Sadly no, there has been no progress from what I am aware

[korkje]

Could it make sense to, at least while WebSocketStream is getting all spec'ed and ready, provide an experimental/unstable upgradeWebSocketStream alternative alongside upgradeWebSocket? Would prevent any breaking changes for users, while also letting users choose their preferred API.

[korkje]

Tried my hand at a PR taking some inspiration from, and trying to improve upon, the earlier PR (#16732) by @crowlKats.

[masx200]

export class WebSocketStream {
    public socket: WebSocket;
    public readable: ReadableStream<Uint8Array>;
    public writable: WritableStream<Uint8Array>;

    constructor(socket: WebSocket) {
        this.socket = socket;
        this.readable = new ReadableStream({
            start(controller) {
                socket.onmessage = function ({ data }) {
                    // console.log(data);
                    return controller.enqueue(new Uint8Array(data));
                };
                socket.onerror = (e) => controller.error(e);
                socket.onclose = (/* e */) => controller.close(/* e */);
            },
            cancel(/* reason */) {
                socket.close();
            },
        });
        this.writable = new WritableStream({
            start(controller) {
                socket.onerror = (e) => controller.error(e);
            },
            write(chunk) {
                // console.log(chunk);
                socket.send(chunk);
            },
            close() {
                socket.close();
            },
            abort(e) {
                console.error(e);
                socket.close();
            },
        });
    }
}

[rikka0w0]

export class WebSocketStream {

One important feature of the WebSocketStream is the backpressure handling, and this simple wrapper does not support backpressure at all. It is possible to implement backpressure on the outgoing stream via WebSocket: bufferedAmount, but not possible on the incoming stream. Missing the backpressure support can overwhelm the receiver and potentially cause out-of-memory.