websocket example incorrect comment?

[ronag]

https://github.com/uNetworking/uWebSockets.js/blob/master/examples/WebSockets.js#L21

  message: (ws, message, isBinary) => {
    /* Ok is false if backpressure was built up, wait for drain */
    let ok = ws.send(message, isBinary);
  },

Not sure if this comment is correct? Looking at https://github.com/uNetworking/uWebSockets/blob/master/src/WebSocket.h#L60 it seems like it returns false on failure (not related to backpressure). Also is a bit strange that it returns true if the message is dropped?

Maybe should be?

  message: (ws, message, isBinary) => {
    let ok = ws.send(message, isBinary);
    let needDrain = ws.amountBuffered() >= maxBackpressure;
  },

[dragonbane0]

No, in fact all the "failure" states you see in the code only occur when there was backpressure built up during writing to the socket. So the example is correct in that way. Failure basically means it failed to be written in one go.

But I agree returning true is probably a bit unintuitive when the send was completely dropped due too high backpressure, but this is more an issue with the bool only having 2 states.

Usually you should avoid even running into this case by checking bufferedAmount and implementing your own backpressure handling. It was actually a bit more severe of an issue when the documentation was out of date regarding maxBackpressure being used for ws.send (this is now fixed). I had a lot of messages dropped and didn't realize it, since I configured that value pretty low since I thought it only applied to publish and not send as the doc used to say.

[hst-m]

When using Publish you can't check the backpressure with ws.getBufferedAmount() so this is why maxBackpressure was added. Then shortly later it was updated to also apply to ws.send() with the docs lagging. It is probably faster to not call ws.getBufferedAmount() in JS and let it be auto handled in native. But as discussed here #414 in some cases it is preferable to just drop the connection rather than randomly drop messages. So some type of update should be coming: "add an option to make it close the connection instead of skipping sending"

[dragonbane0]

Yeah that would be good. In my own ws.send handling I close the connection when the backpressure gets too bad, which was only circumvented by not realizing maxBackpressure was silently dropping messages on ws.send. But for convenience and better Publish support it would be nice if this behaviour could be replicated by the library as well to auto close the connection once maxBackpressure is exceeded.