refactor and fix std::promise usage in RtClient (#151)

core/core-rt/NRtClient.cpp

@@ -33,7 +33,7 @@ NRtClient::NRtClient(NRtTransportPtr transport, const std::string& host, int32_t
     , _port(port)
     , _ssl(ssl)
     , _transport(transport)
-    , _connectPromise(std::make_unique<std::promise<void>>())
+    , _connectPromise(nullptr)
 {
     NLOG_INFO("Created");
 
@@ -121,19 +121,11 @@ std::future<void> NRtClient::connectAsync(NSessionPtr session, bool createStatus
         return emptyPromise.get_future();
     }
 
-    // stomp the old promise
     _connectPromise = std::make_unique<std::promise<void>>();
-
-    // already connected
-    if (_transport->isConnected()) {
-
-        _connectPromise->set_value();
-        return _connectPromise->get_future();
-    }
-
+    // get future before connecting in case callbacks fire and modify promise (e.g., multithreading).
+    std::future<void> connectFuture = _connectPromise->get_future();
     connect(session, createStatus, protocol);
-
-    return _connectPromise->get_future();
+    return connectFuture;
 }
 
 bool NRtClient::isConnecting() const
@@ -204,13 +196,19 @@ void NRtClient::onTransportConnected()
 
     try
     {
-        // signal to the user's future that the connection has completed.
-        _connectPromise->set_value();
+        if (_connectPromise)
+        {
+            // signal to the user's future that the connection has completed.
+            _connectPromise->set_value();
+            _connectPromise.reset(nullptr);
+        }
     }
-    catch (const std::future_error&)
+    catch (const std::future_error& e)
     {
-        // if we get an exception here, it means the connect promise has completed already from a previous connect.
-        // this can happen if the transport double fires or some other unexpected cases, like the user disconnecting while a connection is being made.
+        //  std::future_error on the following conditions:
+        //  *this has no shared state. The error code is set to no_state.
+        //  The shared state already stores a value or exception. The error code is set to promise_already_satisfied.
+        NLOG_WARN("Unexpected exception caught on transport connect: " + std::string(e.what()));
     }
 }
 
@@ -227,12 +225,19 @@ void NRtClient::onTransportDisconnected(const NRtClientDisconnectInfo& info)
 
     try
     {
-        // assume we are disconnecting mid-connect
-       _connectPromise->set_exception(std::make_exception_ptr<NRtException>(NRtException(NRtError(RtErrorCode::CONNECT_ERROR, "Disconnected while connecting."))));
+        if (_connectPromise)
+        {
+            // assume we are disconnecting mid-connect
+            _connectPromise->set_exception(std::make_exception_ptr<NRtException>(NRtException(NRtError(RtErrorCode::CONNECT_ERROR, "Disconnected while connecting."))));
+            _connectPromise.reset(nullptr);
+        }
     }
-    catch(const std::future_error& e)
+    catch (const std::future_error& e)
     {
-        // we've already set the state on this, so we've already connected, so nothing else to do.
+        // std::future_error on the following conditions:
+        // *this has no shared state. The error code is set to no_state.
+        // The shared state already stores a value or exception. The error code is set to promise_already_satisfied.
+        NLOG_WARN("Unexpected exception caught on transport disconnect: " + std::string(e.what()));
     }
 
 
@@ -253,10 +258,20 @@ void NRtClient::onTransportError(const std::string& description)
         _listener->onError(error);
     }
 
-    bool futureCompleted = _connectPromise->get_future().wait_for(std::chrono::seconds(0)) == std::future_status::ready;
-    if (!futureCompleted)
+    try
+    {
+        if (_connectPromise)
+        {
+            _connectPromise->set_exception(std::make_exception_ptr<NRtException>(NRtException(NRtError(RtErrorCode::CONNECT_ERROR, "An error occurred while connecting."))));
+            _connectPromise.reset(nullptr);
+        }
+    }
+    catch (const std::future_error& e)
     {
-        _connectPromise->set_exception(std::make_exception_ptr<NRtException>(NRtException(NRtError(RtErrorCode::CONNECT_ERROR, "An error occurred while connecting."))));
+        // std::future_error on the following conditions:
+        // *this has no shared state. The error code is set to no_state.
+        // The shared state already stores a value or exception. The error code is set to promise_already_satisfied.
+        NLOG_WARN("Unexpected exception caught on transport error: " + std::string(e.what()));
     }
 }
 

impl/wsWslay/NWebsocketWslay.cpp

@@ -337,6 +337,7 @@ namespace Nakama {
             NLOG(NLogLevel::Debug, "Wslay result: ERROR %s", errMessage.c_str());
             _state = State::Disconnected;
             this->_io->close();
+            _ctx.reset(nullptr);
             fireOnError(errMessage);
             return;
         }

test/src/realtime/test_lifecycle.cpp

@@ -170,5 +170,15 @@ namespace Nakama {
 
             test.stopTest(connected);
         }
+
+        void test_connectivity_loss()
+        {
+            bool threadedTick = true;
+            NTest test(__func__, threadedTick);
+            test.setTestTimeoutMs(60 * 1000);
+            test.runTest();
+            NSessionPtr session = test.client->authenticateDeviceAsync("mytestdevice0001", opt::nullopt, opt::nullopt, {}).get();
+            test.rtClient->connect(session, true);
+        }
     }
 }

test/src/realtime/test_realtime.cpp

@@ -37,6 +37,7 @@ void test_rt_reconnect();
 void test_rt_connect_callback();
 void test_rt_double_connect();
 void test_rt_double_connect_async();
+void test_connectivity_loss();
 
 void run_realtime_tests()
 {
@@ -58,6 +59,9 @@ void test_realtime()
     test_rt_double_connect();
     test_rt_double_connect_async();
 
+    // optional "test". run websocket for a full minute. useful for testing connection loss with network link conditioner.
+    // test_connectivity_loss();
+
     /// change to 10 iterations to trigger https://github.com/microsoft/libHttpClient/issues/698 bug
     for (int i = 0; i < 1; i++) {
         test_rt_reconnect();