Moddable-OpenSource / moddable

Tools for developers to create truly open IoT products using standard JavaScript on low cost microcontrollers.
http://www.moddable.com
1.32k stars 236 forks source link

WebSocket / Socket failing with "nothing to read" #908

Closed cmidgley closed 2 years ago

cmidgley commented 2 years ago

Build environment: Linux Target device: ESP32

Description Using WebSocket in a "load test" fails on a regular basis with "nothing to read". Upon further debugging (and fixing WebSocket) it appears that the issue is in Socket or perhaps in IDF. Everything works for a while (could be 10's of messages, or thousands) and then a data available message is received with a tiny (perhaps 1-5 byte) size (technically, that is fine) but then stalls out afterwards with no more data even though it is clearly incomplete.

Note that WebSocket's implementation can not handle this fractional message. See below for a modified WebSocket (with extra debug crap as well, which will eventually need to be removed) that should address this limitation. It should not be necessary to see the deeper socket issue.

Steps to Reproduce I simply took the websocket examples - client and server - and merged them into a single test on loopback localhost (gist).

Running this, and typically after perhaps a few hundred to many a few thousand messages it will fail with an exception "nothing to read".

Other information I think this is a multi-level problem:

1) The main problem is that Socket appears to provide a read message with only a few bytes of data when the write that was sent is much larger. I have verified that it isn't just a fragmentation problem - I modified WebSocket to detect the issue and wait for another message to be received but none arrives. 2) Socket.read(String, '\n') call will fail with an exception when no data is available, which seems wrong to me as I would expect just an empty string. The WebSocket implementation assumes that it would return without an exception as well (which is corrected in my version). 3) In general, WebSocket assumes that full packets are always delivered on the Socket interface (with some exceptions), otherwise it fails with the read error from socket. I have modified WebSocket, adding a new set of states, which helped isolate this problem to Socket. If this code is of interest (I know it's the old WebSocket, but it's the only server...), I'm glad to do a pull with a clean version of the refactored code. It does have a couple extra features as well, such as getting the remote IP address and passing through the ability to look at the write buffer size.

phoddie commented 2 years ago

I'm not surprised. The original WebSocket parser doesn't know how to handle a message that arrives in pieces. The new Ecma-419 WebSocket implementation handles this correctly. I'm not inclined to spend much time on the original implementation as the goal is to deprecate that.

The main problem is that Socket appears to provide a read message with only a few bytes of data when the write that was sent is much larger

That's not a bug. That's lwip. Until the current buffer is full consumed, it may not be able able to receive another buffer. That's eventually true of any network stack, (none can buffer forever) it is just that lwip has the most extreme limit. The Socket API doesn't guarantee any amount of buffering (it cannot, because different stacks have different behavior).

If this code is of interest (I know it's the old WebSocket, but it's the only server...), I'm glad to do a pull with a clean version of the refactored code

That would be great. If it is reasonably straightforward, we can merge that. If not, having it as a PR will allow others to use it.

cmidgley commented 2 years ago

Totally aware of that socket doesn't guarantee anything on sizing - hence why I updated WebSocket to handle partials everywhere. What I didn't know is that LWIP requires I drain all reads before it sends more data! That one is new to me ... and will add to the complexity of WebSocket.

If only 419 had server support... (hint, hint, wink, wink....)

phoddie commented 2 years ago

If only 419 had server support... (hint, hint, wink, wink....)

In fact, it was discussed at our May meeting. The consensus is that we should try to do some server work for 2nd Edition of Ecma-419 (the same edition that the current client work target). The current thinking is to get started on HTTP Server in about a month. WebSocket Server also seems in reach. MQTT Server probably isn't for this round. We'll see.

What I didn't know is that LWIP requires I drain all reads before it sends more data! That one is new to me ... and will add to the complexity of WebSocket.

The details are more subtle, but since the implementation can do what it wants, your assumption here is the right one for implementing against. (lwip frees buffer space by packet, so until you've read all the data in the packet, it can't free that buffer)