Extension-websocket

[GreenArrow]

I did a few more tests, and if I do a case-insensitive compare:

strcmpi(value, "Upgrade") == 0

in handshake.cpp the websocket seems to be upgraded successfully. For some reason, my server setup appears to be sending a header “upgrade” instead of “Upgrade” during the handshake (even if my nginx config also lists “Upgrade”).

With above change, I am able to connect successfully to the server and send/receive data properly. I have a new issue of “lost” messages that only happens in the desktop build as well, but this looks like colyseus related, so I will just wait till @endel releases a new colyseus client update to test things again.

[Mathias_Westerdahl]

Ok, that’s good to know. I’ll update the extension with a case insensitive test then.

[endel]

Hi there, sorry the delay to provide more info here.

So, the example project from colyseus-defold worked fine, but after trying to use it on a real project (Raft Wars) I was losing my hair because some messages seemed to arrive inconsistently, I thought the problem was on my end, then today I’ve tried to reproduce the issue using a plain WebSocket server and the new Defold extension.

Apparently, the new defold extension is not able to dequeue messages arriving real fast one after another. I’ve prepared an example repo where you can see the issue happening, here: https://github.com/endel/defold-extension-websocket-issue

There are a few things to observe in this example:

1. Right after the connection is established, the server is going to send 4 short text messages and 4 short binary messages - this is to test if the client is going to receive them properly

2. Sometimes, this warning is displayed in the client-side, and no messages are received (need to keep closing & compiling the Defold project again until it works)

WARNING:WEBSOCKET: Waiting for socket to be available for reading

3. Most of the times, only the very last message is received by the client:

DEBUG:SCRIPT: received message, (RAW) =>
DEBUG:SCRIPT: 
DEBUG:SCRIPT: (BINARY VERSION) =>
DEBUG:SCRIPT: 
{ --[[0x115b5f870]]
  1 = 4,
  2 = 4,
  3 = 4,
  4 = 4
}

If we use setTimeout() for each message being sent by the server, none if this happens. The problem really seems to be when we have a bunch of messages enqueued one after the other.

Let me know if I can help with this! Cheers!

[britzl]

Let me know if I can help with this! Cheers!

Love it! Many thanks for providing this feedback.

I created an issue to track this: https://github.com/defold/extension-websocket/issues/6

[Mathias_Westerdahl]

Thanks for the excellent repro!
I’m not really sure why the extension cannot read the message, I can clearly see that the server sends the “Upgrade” handshake, but when the extension wants to read that, there’s no data left to read, thus producing the “Waiting for socket to be available for reading”.
I’m looking further into this…

[Mathias_Westerdahl]

@endel We now have a PR up waiting for review.
In the meantime, you can test it with your scenario, by using this dependency:
https://github.com/defold/extension-websocket/archive/issue-6-multiple-messages.zip

[GreenArrow]

Using Windows 10. Defold 1.2.174 beta.

html5 build gives me these errors:

desktop build gives me these errors:

For the desktop build, if I comment out DebugLog (just to test), like:

build will run fine. The good news is that, initial tests in my own project seem to work ok now. I previously reported “lost” messages as well in my desktop build tests, which I thought was colyseus related, but your fix also solved my issues.

Big thanks to @endel as well for the repro case!

I can help test again when you have a new version fixing the html5 build. Thanks!

[GreenArrow]

Proposed changes:

Inside DebugLog:

#if defined(__EMSCRIPTEN__)
    char* buffer = (char*)alloca(buffer_size);
#else
    char* buffer = (char*)_alloca(buffer_size);
#endif 

Then replace:

PushMessage(conn, conn->m_Buffer, recv_bytes);

with:

PushMessage(conn, recv_bytes);

These changes fixed the build errors for me in both the desktop and html5 builds. And so far, my tests seem fine.

[Mathias_Westerdahl]

Yes, I was a bit hasty in my testing before lunch

Update:
I’ve fixed the compilation errors now, please try it again.

[GreenArrow]

Can confirm the compile errors are fixed now, for both builds.

I’ve been testing this PR the last few hours, and it looks stable in my test projects, along with @endel’s test branch. Hopefully he finds this stable too.

[endel]

Awesome @Mathias_Westerdahl, thanks!

Thanks for testing as well @GreenArrow, indeed it’s way better now!

One last thing I’d like to note, regarding server-initiated disconnects:

When the server closes the WebSocket connection - thus sending a “close” code - the client-side is receiving both EVENT_ERROR and EVENT_DISCONNECTED. Only EVENT_DISCONNECTED should’ve been called there since no actual error has happened. I tried to fix this by not calling CLOSE_CONN() here but don’t know why I couldn’t compile by using my branch (?? )

Based on the request described above, I’ve updated my test scenario to accept both a "close" and "terminate" message from the client. When the client sends a “close” message, the server closes the connection with code 1005. If the client sends "terminate", the server closes the connection with code 1006. None of these close codes should result in an error (I don’t have a reference for this, but using the Node.js WebSocket client, the error callback is not triggered in any of them: https://github.com/endel/defold-extension-websocket-issue/blob/master/Server/client.js)

It would be really great if we can expose the “close code” when the WebSocket connection is closed. This was not supported by the previous extension made by @britzl, but it can be a low hanging fruit to add on this new one. What do you think @Mathias_Westerdahl?

(I haven’t yet given a shot for Android and iOS, by Murphy’s Law something probably awaits us there )

[Mathias_Westerdahl]

The issue #6 was merged to master, and is ready for use, and I have now a pull request for issue #8, for receiving the “close” event.

Note that I didn’t get the exact same behavior as you when the server sent a the abnormal close event (1006). In my case the socket closed and we’d just continue polling that socket, never receiving anything.

In this case I will still generate two error+disconnect events.
(I’ll happily take ideas on why this is the case and/or how to solve it properly)

If the case we do receive the proper close event, we’ll just send the disconnect event to the user.

I also changed the error message to also be called data.message since it’s easier to remember (plus less code).

I also added the game.project setting websocket.debug = 2 for outputting verbose info about the sent/received packages etc. Set to 0 (default) to disable debugging.

[Mathias_Westerdahl]

I’ve just now released the version 1.5.0 of the extension-websocket.

• Added protocol to connection parameters
• Fix header issue for discrepancy between paths (e.g. “/” vs “/subpath”)
• Documentation fix for websocket.EVENT_ERROR

Our plan is to announced this extension as “released” when we release Defold 1.2.175 (in about a week).

[Mathias_Westerdahl]

I’ve now removed the “Beta” tag on this extension, and we are now actively promoting this extension over the old “defold-luasocket”.