dmonad/lib0

Websocket does not support binary messages

somebody1234 opened this issue · 6 comments

Reproduction steps:

websocket.send(new Uint8Array([0, 1, 2, 3]).buffer)

Cause:

lib0/websocket.js

Lines 128 to 132 in 1c3bc44

send (message) {
if (this.ws) {
this.ws.send(JSON.stringify(message))
}
}

it would also be nice to be able to disable ping/pong (since ping and pong frames are already built into the protocol)

dmonad commented

Hi @somebody1234,

You are right about that. The websocket module lacks features and is not optimal. Although I want correct you, ping/pong is not handled by the browser and must be done manually.

Nevertheless, I can't make these kinds of major changes at the moment because it would break existing projects. If I were to reimplement this module (and I eventually will), I would do many things differently. You'll see that none of my newer ws projects depend on this module anymore.

I have to wait for the next major release to remove this module. I suggest that you copy paste the things that are useful to you.

I will add a deprecation notice to lib0/websocket.

re: pings and pongs, aren't they built into the protocol itself? (MDN, javascript.info explaining that ping/pong is inaccessible by the browser WebSocket api)

but more importantly it'd be nice to have the option, in case the server does not understand the {"type":"ping"} payload - for example, i'm not sure if it's a valid message for a JSON-RPC websocket

dmonad commented

Yeah, they are built-in, but you can't use them in the browser, which is just a hilarious design decision. If you use a node WebSocket library, you can send pings. But not in the browser..

The backend needs to understand {type:"ping"}, because we can't send native control frames from the browser.

pings sent from the backend are answered automatically.

yeah, but the thing is, doesn't the browser handle ping+pong and disconnection logic already? so it shouldn't be strictly necessary to reimplement it in JS?

dmonad commented

As I said, no it doesn't. But maybe you can figure this out yourself.

ah, browsers never send pings. but i do still think it would be useful to be able to disable it, in case:

  • the client-side code is connecting to a third-party server that doesn't understand {"type":"ping"}, or
  • the server is using a higher level protocol that disallows it, or
  • the server sends pings and kills the connection from its side instead (of course, this isn't guaranteed to be sufficient), or
  • the client-side code perhaps just doesn't need a keepalive

i guess there isn't really a downside to just stick with the copy-pasted implementation though, especially if it already exists (assuming upstream isn't going to change that much anyway)