heroiclabs / nakama-cpp

Generic C/C++ client for Nakama server.
https://heroiclabs.com/docs/cpp-client-guide
Apache License 2.0
68 stars 25 forks source link

Trigger onDisconnect callback on wslay errors #75

Closed redbaron closed 1 year ago

redbaron commented 1 year ago

disconnect's remote argument actually means "transport initiated" when true and "client initiated" when false. wslay error causes disconnect from the transport side (even though remote side didn't sent us close frame), therefore should be calling this->disconnect(true)

lugehorsam commented 1 year ago

I'm not sure I understand, here's the comment for remote in the disconnect info:

        /// true if close was initiated by server.
        bool remote = false;
redbaron commented 1 year ago

comment is inaccurate. it was true at the time when wslay IO errors were not processed , so only reason of transport initiated disconnect was if remote server closes connection. Now that IO errors also cause disconnect, more appropriate name for this variable will be transport_initiated or something like this.

We want to call onDisconnect when transport initiates disconnect and we don't want to call it (from transport level), when RtClient initiates disconnect, because higher level RtClient does it already.

lugehorsam commented 1 year ago

We don't use the remote field, do we? It's only intended for external users as far as I know. I think we should keep remote and add transport_initiated instead although I'm not sure in what situations that would be useful for them to know.

redbaron commented 1 year ago

We do use it: https://github.com/heroiclabs/nakama-cpp/blob/master/impl/wsWslay/NWebsocketWslay.cpp#L309

lugehorsam commented 1 year ago

We don't read the value.

redbaron commented 1 year ago

If remote is true we fire disconnect callback: https://github.com/heroiclabs/nakama-cpp/blob/master/impl/wsWslay/NWebsocketWslay.cpp#L243