adamdruppe / arsd

This is a collection of modules that I've released over the years. Most of them stand alone, or have just one or two dependencies in here, so you don't have to download this whole repo.
http://arsd-official.dpldocs.info/arsd.html
530 stars 127 forks source link

http2.d: WebSocketClient throws wtf #249

Closed andre2007 closed 1 year ago

andre2007 commented 4 years ago

I try to write a websocket client server echo scenario.

The server part is working fine now and looks like this

/+ dub.sdl:
    name "server"
    dependency "arsd-official:cgi" version="6.0.2"
+/

import std;
import arsd.cgi;

void main() 
{
    writeln("Listening on port 5000");
    processPoolSize = 1;
    cgiMainImpl!((cgi) { websocketEcho(cgi); }, Cgi, defaultMaxContentLength)(["--port", "5000"]);
}

void websocketEcho(Cgi cgi) {
    assert(cgi.websocketRequested(), "i want a web socket!");
    auto websocket = cgi.acceptWebsocket();

    while(websocket.recvAvailable(300.seconds)) 
    {
        auto msg = websocket.recv();
        if (msg.opcode == WebSocketOpcode.close) break;
        websocket.send(msg.data);

    }
    websocket.close();
}

The client looks like this:

/+ dub.sdl:
    name "client"
    dependency "arsd-official:http" version="6.0.2"
    subConfiguration "arsd-official:http" "without_openssl"
+/

import std;
import arsd.http2;

void main() 
{
    ubyte[] data = [0, 0, 0];
    Uri uri = "ws://localhost:5000";

    foreach(i; 0..50)
    {
        auto ws = new WebSocket(uri);
        ws.send(data);
        WebSocketFrame frame = ws.waitForNextMessage();
        ubyte[] recvBuffer = frame.data;
        assert(recvBuffer == data);
    }
}

I am not 100% whether waitForNextMessage is the correct function. What I want is to receive (blocking) the whole answer as ubyte[]. Maybe convenient methods receiveBinary and receiveString are missing.

The client throws this exception:

object.Exception@/home/user/.dub/packages/arsd-official-6.0.2/arsd-official/http2.d(2433): wtf

/home/user/.dub/packages/arsd-official-6.0.2/arsd-official/http2.d:2433 void arsd.http2.WebSocket.llsend(ubyte[]) [0x7f861954f614] /home/user/.dub/packages/arsd-official-6.0.2/arsd-official/http2.d:2920 void arsd.http2.WebSocketFrame.send(scope void delegate(ubyte[])) [0x7f8619550ba6] /home/user/.dub/packages/arsd-official-6.0.2/arsd-official/http2.d:2575 void arsd.http2.WebSocket.send(const(ubyte[])) [0x7f861954fbf8] client.d:18 _Dmain [0x7f8619547fbe]

adamdruppe commented 4 years ago

The exception there is super simple: you never called ws.connect(); before attempting to send.

In that loop, it will also be important to explicitly ws.close() especially since it is single-process on the receiving side or else they will very, very slowly timeout.

But the ideal interface for the client is actually to use its little event loop which combines things for you

        auto ws = new WebSocket(Uri("wss://echo.websocket.org/"));

        bool first = true;

        ws.onopen = () {
                writeln("opened!");
                ws.send("Hello, world!");
        };

        ws.onclose = () {
                writeln("bye");
        };

        ws.onmessage = (in char[] message) {
                writeln("received: ", message);
                if(first) {
                        ws.send("thank you");
                        first = false;
                } else
                        ws.close();
        };

        ws.connect();

 WebSocket.eventLoop();

something like that so you set callbacks then let it internally loop.

though the upper level function call the same lower level one it doesn't combine except in the callback section. maybe i can change that but for now that's the easiest way to make it work moe

andre2007 commented 4 years ago

Thanks a lot. I translated my example and made following observations:

Maybe a ConnectionError exception could be thrown in case there was never a ws.connect executed or the connection was lost in the meantime.

/+ dub.sdl:
    name "client"
    dependency "arsd-official:http" version="6.0.2"
    subConfiguration "arsd-official:http" "without_openssl"
+/

import std;
import arsd.http2;

void main()
{
    ubyte[] data = [0, 0, 0];
    Uri uri = "ws://localhost:5000";

    foreach(i; 0..50)
    {
        auto ws = new WebSocket(uri);

        ws.onopen = () {
            writeln("opened!");
            ws.send(data);
        };

        ws.onclose = () {
            writeln("bye");
        };

        ws.onbinarymessage = (in ubyte[] message) {
            writeln("binary message");
            assert(message == data);
            ws.close();
            writeln("foo");
        };

        ws.connect();
        WebSocket.eventLoop();
        writeln("event loop stopped");
    }
}
adamdruppe commented 3 years ago

so onclose is supposed to be called when the other side sends their final close packet... I used to have it pretty bugged, fixed I think now.

But I'm looking through my old issues and the connect thing is still real. I could make it throw an exception if you try to send before it is connected, or just go ahead and connect it automatically... I'm leaning toward the latter.

andre2007 commented 3 years ago

Happy new year Adam.

Both ways are fine for me.

Just one remark, if possible could you please avoid terms like "wtf" in the source code? This makes it a lot more easier to use your library in my day to day job because it looks a lot more professional.

adamdruppe commented 3 years ago

On Fri, Jan 01, 2021 at 08:19:10AM -0800, andre2007 wrote:

Just one remark, if possible could you please avoid terms like "wtf" in the source code?

I actually changed that recently anyway, now all those cases return an error code in the response object instead so they can be dealt with asynchronously and independently.

This makes it a lot more easier to use your library in my day to day job because it looks a lot more professional.

Can't promise that, I hate coding and type what I think and that's often cuss words.

If it really bothers you maybe just do a grep ahead of time. I'm not opposed to PRs.

adamdruppe commented 3 years ago

$ cgrep 'wtf' | grep '"' | wc 45 388 3903

lol quite a few.

andre2007 commented 3 years ago

Actually it isn't an issue that the source code contains these words. The issue is, the exceptions message contains the bad word. If the customers of the company I am working for would see it, e.g. while looking at logs, I would get in trouble.

adamdruppe commented 3 years ago

yeah that's why i searched for the quote too, these are strings. most of them are assert(0) so not supposed to happen but still

andre2007 commented 1 year ago

This issue is also fixed.