ws: use transport similar to the socket ones

[belochub]

Previously every JSTP message was sent in a separate WebSocket frame. This commit changes WebSocket usage so that JSTP message is not limited to a single WebSocket frame. This is accomplished by making the WebSocket JSTP transport behave in a way similar to the other available transports, which is after every message, there is a separator. This change leads to a WebSocket frame being able to hold multiple JSTP messages at once or a part of a big JSTP message, that cannot be transmitted in a single frame. This is a breaking change since it completely changes the way the messages are transmitted over WebSockets.

[belochub]

@lundibundi the solution you proposed wouldn't allow to send multiple JSTP messages in one WebSocket frame except for some really rare cases where smaller JSTP messages (that can fit into one WS frame) are buffered instead of being sent immediately due to a bigger JSTP message (that can't fit into one WS frame) being transmitted at that time.

[lundibundi]

Why is it important to send them in one frame then? I thought the idea was to avoid flooding the WS with messages so it won't drop the connection upon sending lots of messages.

Basically I don't like the idea of running 50ms interval for each WS connection the server has, i.e. if we have only 1k connections that don't send anything that would still result in 1k callbacks being evaluated every 50ms and is basically wasting resources. Therefore I'd suggest to either:

• use the scheme I suggested and only run interval when needed
• use your scheme but run the interval only when there is something to send and remove it otherwise, this will at least somehow offload the unneded work

Maybe @nechaido or @aqrln has any other ideas?

[belochub]

I thought the idea was to avoid flooding the WS with messages so it won't drop the connection upon sending lots of messages.

WebSocket does not drop the connection when you are sending lots of messages, and this PR was not created to avoid flooding it with messages. The problem that exists is that, according to the W3C standard for WebSockets, send(data) method operates in such way:

if the data cannot be sent, e.g. because it would need to be buffered but the buffer is full, the user agent must flag the WebSocket as full and then close the WebSocket connection

This makes it impossible to send JSTP message bigger than browser's buffer for WebSocket frames (which is usually 64KiB) since every JSTP message is sent in a single frame. This PR solves this problem by sending bigger messages in multiple WebSocket frames sequentially.

The way of handling the bufferedAmount is based on this basic example from the W3C standard for WebSockets: https://html.spec.whatwg.org/multipage/web-sockets.html#dom-websocket-bufferedamount

As a positive side effect, this way makes it possible to transmit multiple JSTP messages in a single WebSocket frame.

[belochub]

@lundibundi Also, about

running 50ms interval for each WS connection the server has

Take a closer look at the code, the setInterval function is only used in browser, not on the server.

[belochub]

@lundibundi end(data) method in Transport declared in ws-browser.js calls this.send() if the data argument is being passed. The only issue I can think of, is that this method also closes the underlying socket immediately, which means it will happen before all of the buffered messages are sent.

[belochub]

Unfortunately, the implementation had one important problem before, that of bufferedAmount attribute returning the number of bytes of the UTF-8 text being sent, not UTF-16 that is used in strings in JavaScript, so I've updated the implementation to account for that.

[belochub]

@lundibundi I think it can be simplified even more if we use a string buffer instead of an array of strings, what do you think about that?

[lundibundi]

I'm not sure about the performance implications of that, though if you think it'll have better performance than an Array feel free to implement it that way.

:thinking: What do you think of storing strings of maxLength right in the send method: upon send you check the length of the last element in the array and if it's less than maxLength you append to it the remaining amount and then push everything else as the next element of an array. This may lessen the burden of 'interval' of going through elements and appending/slicing them manually in one place instead making it as user sends messages, wdyt?

Edit: I think we can land this as is and just continue working on it in a separate PR, wdyt? Edit2: By 'land' above I meant to not require/block this PR to have any of the optimizations we were talking about and instead land it as soon as main functionality/tests is ready.

[belochub]

@lundibundi I don't think we can land it now, since this is missing some tests, and we should also wait for a review from @aqrln.

[lundibundi]

Well, I meant to postpone optimization we concluded to a separate PR and not block this PR on them. I'll clarify my comment.

[belochub]

Closing in favor of https://github.com/metarhia/jstp/pull/383.