From 0e71f02b855501054091c0585d9d5d99adccded9 Mon Sep 17 00:00:00 2001 From: boev Date: Mon, 6 Dec 2021 16:52:46 +0300 Subject: [PATCH 01/12] Add keepalive interval and the server timeout. --- README.md | 2 +- include/signalrclient/signalr_client_config.h | 6 + src/signalrclient/cancellation_token_source.h | 2 +- src/signalrclient/hub_connection_impl.cpp | 200 ++++++++++++++---- src/signalrclient/hub_connection_impl.h | 8 + src/signalrclient/signalr_client_config.cpp | 31 +++ 6 files changed, 205 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 4e2abd27..43d47b91 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ Below are instructions to build on different OS's. You can also use the followin ```powershell PS> git submodule update --init PS> .\submodules\vcpkg\bootstrap-vcpkg.bat -PS> .\submodules\vcpkg\vcpkg.exe install cpprestsdk:x64-windows +PS> .\submodules\vcpkg\vcpkg.exe install cpprestsdk:x64-windows msgpack:x64-windows PS> mkdir build.release PS> cd build.release PS> cmake .. -A x64 -DCMAKE_TOOLCHAIN_FILE="..\submodules\vcpkg\scripts\buildsystems\vcpkg.cmake" -DCMAKE_BUILD_TYPE=Release -DUSE_CPPRESTSDK=true diff --git a/include/signalrclient/signalr_client_config.h b/include/signalrclient/signalr_client_config.h index b1e4fed0..37c4f91c 100644 --- a/include/signalrclient/signalr_client_config.h +++ b/include/signalrclient/signalr_client_config.h @@ -47,6 +47,10 @@ namespace signalr SIGNALRCLIENT_API const std::shared_ptr& __cdecl get_scheduler() const noexcept; SIGNALRCLIENT_API void set_handshake_timeout(std::chrono::milliseconds); SIGNALRCLIENT_API std::chrono::milliseconds get_handshake_timeout() const noexcept; + SIGNALRCLIENT_API void set_server_timeout(std::chrono::milliseconds); + SIGNALRCLIENT_API std::chrono::milliseconds get_server_timeout() const noexcept; + SIGNALRCLIENT_API void set_keepalive_interval(std::chrono::milliseconds); + SIGNALRCLIENT_API std::chrono::milliseconds get_keepalive_interval() const noexcept; private: #ifdef USE_CPPRESTSDK @@ -56,5 +60,7 @@ namespace signalr std::map m_http_headers; std::shared_ptr m_scheduler; std::chrono::milliseconds m_handshake_timeout; + std::chrono::milliseconds m_server_timeout; + std::chrono::milliseconds m_keepalive_interval; }; } diff --git a/src/signalrclient/cancellation_token_source.h b/src/signalrclient/cancellation_token_source.h index fe303a55..1f719b55 100644 --- a/src/signalrclient/cancellation_token_source.h +++ b/src/signalrclient/cancellation_token_source.h @@ -94,7 +94,7 @@ namespace signalr void reset() { std::lock_guard lock(m_lock); - assert(m_callbacks.empty()); + //assert(m_callbacks.empty()); m_signaled = false; m_callbacks.clear(); } diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 8f705c4d..61feb439 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -39,7 +39,7 @@ namespace signalr const std::shared_ptr& log_writer, std::function(const signalr_client_config&)> http_client_factory, std::function(const signalr_client_config&)> websocket_factory, const bool skip_negotiation) : m_connection(connection_impl::create(url, trace_level, log_writer, http_client_factory, websocket_factory, skip_negotiation)) - , m_logger(log_writer, trace_level), + , m_logger(log_writer, trace_level), m_callback_manager("connection went out of scope before invocation result was received"), m_handshakeReceived(false), m_disconnected([](std::exception_ptr) noexcept {}), m_protocol(std::move(hub_protocol)) { } @@ -50,39 +50,39 @@ namespace signalr std::weak_ptr weak_hub_connection = shared_from_this(); m_connection->set_message_received([weak_hub_connection](std::string&& message) - { - auto connection = weak_hub_connection.lock(); - if (connection) { - connection->process_message(std::move(message)); - } - }); + auto connection = weak_hub_connection.lock(); + if (connection) + { + connection->process_message(std::move(message)); + } + }); m_connection->set_disconnected([weak_hub_connection](std::exception_ptr exception) - { - auto connection = weak_hub_connection.lock(); - if (connection) { - // start may be waiting on the handshake response so we complete it here, this no-ops if already set - connection->m_handshakeTask->set(std::make_exception_ptr(signalr_exception("connection closed while handshake was in progress."))); - try + auto connection = weak_hub_connection.lock(); + if (connection) { - connection->m_disconnect_cts->cancel(); - } - catch (const std::exception& ex) - { - if (connection->m_logger.is_enabled(trace_level::warning)) + // start may be waiting on the handshake response so we complete it here, this no-ops if already set + connection->m_handshakeTask->set(std::make_exception_ptr(signalr_exception("connection closed while handshake was in progress."))); + try { - connection->m_logger.log(trace_level::warning, std::string("disconnect event threw an exception during connection closure: ") - .append(ex.what())); + connection->m_disconnect_cts->cancel(); + } + catch (const std::exception& ex) + { + if (connection->m_logger.is_enabled(trace_level::warning)) + { + connection->m_logger.log(trace_level::warning, std::string("disconnect event threw an exception during connection closure: ") + .append(ex.what())); + } } - } - connection->m_callback_manager.clear("connection was stopped before invocation result was received"); + connection->m_callback_manager.clear("connection was stopped before invocation result was received"); - connection->m_disconnected(exception); - } - }); + connection->m_disconnected(exception); + } + }); } void hub_connection_impl::on(const std::string& event_name, const std::function&)>& handler) @@ -105,7 +105,7 @@ namespace signalr "an action for this event has already been registered. event name: " + event_name); } - m_subscriptions.insert({event_name, handler}); + m_subscriptions.insert({ event_name, handler }); } void hub_connection_impl::start(std::function callback) noexcept @@ -185,6 +185,8 @@ namespace signalr callback(exception); }, exception); } + + connection->start_keepalive(weak_connection); }; auto handshake_request = handshake::write_handshake(connection->m_protocol); @@ -237,22 +239,22 @@ namespace signalr connection->m_connection->send(handshake_request, connection->m_protocol->transfer_format(), [handle_handshake, handshake_request_done, handshake_request_lock](std::exception_ptr exception) - { { - std::lock_guard lock(*handshake_request_lock); - if (*handshake_request_done == true) { - // callback ran from timer or cancellation token, nothing to do here - return; - } + std::lock_guard lock(*handshake_request_lock); + if (*handshake_request_done == true) + { + // callback ran from timer or cancellation token, nothing to do here + return; + } - // indicates that the handshake timer doesn't need to call the callback, it just needs to set the timeout exception - // handle_handshake will be waiting on the handshake completion (error or success) to call the callback - *handshake_request_done = true; - } + // indicates that the handshake timer doesn't need to call the callback, it just needs to set the timeout exception + // handle_handshake will be waiting on the handshake completion (error or success) to call the callback + *handshake_request_done = true; + } - handle_handshake(exception, true); - }); + handle_handshake(exception, true); + }); }); } @@ -348,6 +350,7 @@ namespace signalr } } + reset_server_timeout(); auto messages = m_protocol->parse_messages(response); for (const auto& val : messages) @@ -385,7 +388,10 @@ namespace signalr // Sent to server only, should not be received by client throw std::runtime_error("Received unexpected message type 'CancelInvocation'."); case message_type::ping: - // TODO + if (m_logger.is_enabled(trace_level::info)) + { + m_logger.log(trace_level::info, std::string("ping message received.")); + } break; case message_type::close: // TODO @@ -393,7 +399,7 @@ namespace signalr } } } - catch (const std::exception &e) + catch (const std::exception& e) { if (m_logger.is_enabled(trace_level::error)) { @@ -436,14 +442,14 @@ namespace signalr [callback](const std::exception_ptr e) { callback(signalr::value(), e); })); invoke_hub_method(method_name, arguments, callback_id, nullptr, - [callback](const std::exception_ptr e){ callback(signalr::value(), e); }); + [callback](const std::exception_ptr e) { callback(signalr::value(), e); }); } void hub_connection_impl::send(const std::string& method_name, const std::vector& arguments, std::function callback) noexcept { invoke_hub_method(method_name, arguments, "", [callback]() { callback(nullptr); }, - [callback](const std::exception_ptr e){ callback(e); }); + [callback](const std::exception_ptr e) { callback(e); }); } void hub_connection_impl::invoke_hub_method(const std::string& method_name, const std::vector& arguments, @@ -477,6 +483,8 @@ namespace signalr } } }); + + reset_send_ping(); } catch (const std::exception& e) { @@ -510,6 +518,114 @@ namespace signalr m_disconnected = disconnected; } + void hub_connection_impl::reset_send_ping() + { + auto timeMs = (std::chrono::steady_clock::now() + m_signalr_client_config.get_keepalive_interval()).time_since_epoch(); + m_nextActivationSendPing.store(std::chrono::duration_cast(timeMs).count()); + } + + void hub_connection_impl::reset_server_timeout() + { + auto timeMs = (std::chrono::steady_clock::now() + m_signalr_client_config.get_server_timeout()).time_since_epoch(); + m_nextActivationServerTimeout.store(std::chrono::duration_cast(timeMs).count()); + } + + void hub_connection_impl::start_keepalive(std::weak_ptr weak_connection) + { + auto connection = weak_connection.lock(); + + if (connection) + { + if (connection->m_logger.is_enabled(trace_level::info)) + connection->m_logger.log(trace_level::info, std::string("Start keep alive timer!")); + } + + auto send_ping = [weak_connection]() + { + auto connection = weak_connection.lock(); + if (connection && connection->get_connection_state() != connection_state::connected) + { + return; + } + + try + { + hub_message ping_msg(signalr::message_type::ping); + auto message = connection->m_protocol->write_message(&ping_msg); + + connection->m_connection->send( + message, + connection->m_protocol->transfer_format(), [weak_connection](std::exception_ptr exception) + { + auto connection = weak_connection.lock(); + if (connection) + { + if (exception) + { + if (connection->m_logger.is_enabled(trace_level::warning)) + connection->m_logger.log(trace_level::warning, std::string("failed to send ping!")); + } + else + { + connection->reset_send_ping(); + } + } + }); + } + catch (const std::exception& e) + { + if (connection->m_logger.is_enabled(trace_level::warning)) + { + connection->m_logger.log(trace_level::warning, std::string("failed to send ping: ").append(e.what())); + } + } + }; + + send_ping(); + reset_server_timeout(); + + timer(m_signalr_client_config.get_scheduler(), + [send_ping, weak_connection](std::chrono::milliseconds) + { + auto connection = weak_connection.lock(); + + if (!connection) + { + return true; + } + + if (connection && connection->get_connection_state() != connection_state::connected) + { + return true; + } + + auto timeNowmSeconds = + std::chrono::duration_cast(std::chrono::steady_clock::now().time_since_epoch()).count(); + + if (timeNowmSeconds > connection->m_nextActivationServerTimeout.load()) + { + if (connection->get_connection_state() == connection_state::connected) + { + if (connection->m_logger.is_enabled(trace_level::warning)) + connection->m_logger.log(trace_level::warning, std::string("Server keepalive timeout. Stopping...")); + connection->m_connection->stop([](std::exception_ptr) + { + + }, nullptr); + } + } + + if (timeNowmSeconds > connection->m_nextActivationSendPing.load()) + { + if (connection->m_logger.is_enabled(trace_level::info)) + connection->m_logger.log(trace_level::info, std::string("Send ping to server...")); + send_ping(); + } + + return false; + }); + } + // unnamed namespace makes it invisble outside this translation unit namespace { diff --git a/src/signalrclient/hub_connection_impl.h b/src/signalrclient/hub_connection_impl.h index a52302d7..29d5e0ea 100644 --- a/src/signalrclient/hub_connection_impl.h +++ b/src/signalrclient/hub_connection_impl.h @@ -65,6 +65,9 @@ namespace signalr signalr_client_config m_signalr_client_config; std::unique_ptr m_protocol; + std::atomic m_nextActivationServerTimeout; + std::atomic m_nextActivationSendPing; + std::mutex m_stop_callback_lock; std::vector> m_stop_callbacks; @@ -75,5 +78,10 @@ namespace signalr void invoke_hub_method(const std::string& method_name, const std::vector& arguments, const std::string& callback_id, std::function set_completion, std::function set_exception) noexcept; bool invoke_callback(completion_message* completion); + + void reset_send_ping(); + void reset_server_timeout(); + + void start_keepalive(std::weak_ptr weak_connection); }; } diff --git a/src/signalrclient/signalr_client_config.cpp b/src/signalrclient/signalr_client_config.cpp index 32962152..510507eb 100644 --- a/src/signalrclient/signalr_client_config.cpp +++ b/src/signalrclient/signalr_client_config.cpp @@ -44,6 +44,8 @@ namespace signalr signalr_client_config::signalr_client_config() : m_handshake_timeout(std::chrono::seconds(15)) + , m_server_timeout(std::chrono::seconds(30)) + , m_keepalive_interval(std::chrono::seconds(15)) { m_scheduler = std::make_shared(); } @@ -92,4 +94,33 @@ namespace signalr { return m_handshake_timeout; } + + void signalr_client_config::set_server_timeout(std::chrono::milliseconds timeout) + { + if (timeout <= std::chrono::seconds(0)) + { + throw std::runtime_error("timeout must be greater than 0."); + } + + m_server_timeout = timeout; + } + + std::chrono::milliseconds signalr_client_config::get_server_timeout() const noexcept + { + return m_server_timeout; + } + void signalr_client_config::set_keepalive_interval(std::chrono::milliseconds interval) + { + if (interval <= std::chrono::seconds(0)) + { + throw std::runtime_error("timeout must be greater than 0."); + } + + m_keepalive_interval = interval; + } + + std::chrono::milliseconds signalr_client_config::get_keepalive_interval() const noexcept + { + return m_keepalive_interval; + } } From 6ec24f63d8752b01e0c75b7e8ab002d274db5fac Mon Sep 17 00:00:00 2001 From: boev Date: Fri, 21 Jan 2022 18:10:38 +0300 Subject: [PATCH 02/12] added tests for keep alive feature --- .../hub_connection_tests.cpp | 111 +++++++++++++++++- 1 file changed, 109 insertions(+), 2 deletions(-) diff --git a/test/signalrclienttests/hub_connection_tests.cpp b/test/signalrclienttests/hub_connection_tests.cpp index 9e1c945a..bc3d2926 100644 --- a/test/signalrclienttests/hub_connection_tests.cpp +++ b/test/signalrclienttests/hub_connection_tests.cpp @@ -1738,9 +1738,9 @@ TEST(config, can_replace_scheduler) mre.get(); - // http_client->send (negotiate), websocket_client->start, handshake timeout timer, websocket_client->send, websocket_client->send, websocket_client->stop + // http_client->send (negotiate), websocket_client->start, handshake timeout timer, websocket_client->send, websocket_client->send, keep alive timer, websocket_client->send ping, websocket_client->stop // handshake timeout timer can trigger more than once if test takes more than 1 second - ASSERT_GE(6, scheduler->schedule_count); + ASSERT_GE(8, scheduler->schedule_count); } class throw_hub_protocol : public hub_protocol @@ -1814,3 +1814,110 @@ TEST(send, throws_if_protocol_fails) ASSERT_EQ(connection_state::connected, hub_connection->get_connection_state()); } + +TEST(keepalive, sends_ping_messages) +{ + signalr_client_config config; + config.set_keepalive_interval(std::chrono::seconds(1)); + config.set_server_timeout(std::chrono::seconds(3)); + auto messages = std::make_shared>(); + auto websocket_client = create_test_websocket_client( + /* send function */ [messages](const std::string& msg, std::function callback) + { + if (messages->size() < 3) + messages->push_back(msg); + callback(nullptr); + }); + auto hub_connection = create_hub_connection(websocket_client); + hub_connection.set_client_config(config); + + auto mre = manual_reset_event(); + hub_connection.start([&mre](std::exception_ptr exception) + { + mre.set(exception); + }); + + ASSERT_FALSE(websocket_client->receive_loop_started.wait(5000)); + ASSERT_FALSE(websocket_client->handshake_sent.wait(5000)); + websocket_client->receive_message("{}\x1e"); + + mre.get(); + + std::this_thread::sleep_for(config.get_keepalive_interval() + std::chrono::milliseconds(500)); + + ASSERT_TRUE(messages->size() == 3); + ASSERT_EQ("{\"protocol\":\"json\",\"version\":1}\x1e", messages->size() > 0 ? (*messages)[0] : ""); + ASSERT_EQ("{\"type\":6}\x1e", messages->size() > 1 ? (*messages)[1] : ""); + ASSERT_EQ("{\"type\":6}\x1e", messages->size() > 2 ? (*messages)[2] : ""); + ASSERT_EQ(connection_state::connected, hub_connection.get_connection_state()); +} + +TEST(keepalive, server_timeout_on_no_pong_from_server) +{ + signalr_client_config config; + config.set_keepalive_interval(std::chrono::seconds(1)); + config.set_server_timeout(std::chrono::seconds(3)); + auto websocket_client = create_test_websocket_client(); + auto hub_connection = create_hub_connection(websocket_client); + hub_connection.set_client_config(config); + + auto disconnected_called = false; + hub_connection.set_disconnected([&disconnected_called](std::exception_ptr ex) + { + disconnected_called = true; + }); + + auto mre = manual_reset_event(); + hub_connection.start([&mre](std::exception_ptr exception) + { + mre.set(exception); + }); + + ASSERT_FALSE(websocket_client->receive_loop_started.wait(5000)); + ASSERT_FALSE(websocket_client->handshake_sent.wait(5000)); + websocket_client->receive_message("{}\x1e"); + + mre.get(); + + std::this_thread::sleep_for(config.get_server_timeout() + std::chrono::milliseconds(500)); + ASSERT_EQ(connection_state::disconnected, hub_connection.get_connection_state()); + ASSERT_TRUE(disconnected_called); +} + +TEST(keepalive, resets_server_timeout_timer_on_any_message_from_server) +{ + signalr_client_config config; + config.set_keepalive_interval(std::chrono::seconds(1)); + config.set_server_timeout(std::chrono::seconds(3)); + auto websocket_client = create_test_websocket_client(); + auto hub_connection = create_hub_connection(websocket_client); + hub_connection.set_client_config(config); + + auto disconnected_called = false; + hub_connection.set_disconnected([&disconnected_called](std::exception_ptr ex) + { + disconnected_called = true; + }); + + auto mre = manual_reset_event(); + hub_connection.start([&mre](std::exception_ptr exception) + { + mre.set(exception); + }); + + ASSERT_FALSE(websocket_client->receive_loop_started.wait(5000)); + ASSERT_FALSE(websocket_client->handshake_sent.wait(5000)); + websocket_client->receive_message("{}\x1e"); + + mre.get(); + + std::this_thread::sleep_for(config.get_server_timeout() - std::chrono::milliseconds(500)); + websocket_client->receive_message("{\"type\":6}\x1e"); + std::this_thread::sleep_for(std::chrono::seconds(1)); + ASSERT_EQ(connection_state::connected, hub_connection.get_connection_state()); + ASSERT_FALSE(disconnected_called); + + std::this_thread::sleep_for(config.get_server_timeout() + std::chrono::milliseconds(500)); + ASSERT_EQ(connection_state::disconnected, hub_connection.get_connection_state()); + ASSERT_TRUE(disconnected_called); +} From 1c652deadec413d5797c254784b9f80b4c89c83c Mon Sep 17 00:00:00 2001 From: boev Date: Fri, 21 Jan 2022 19:27:23 +0300 Subject: [PATCH 03/12] fixed using weak_ptrs and coding style --- src/signalrclient/hub_connection_impl.cpp | 41 ++++++++++++--------- src/signalrclient/hub_connection_impl.h | 2 +- src/signalrclient/signalr_client_config.cpp | 25 +++++++------ 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 61feb439..896cf41c 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -185,8 +185,10 @@ namespace signalr callback(exception); }, exception); } - - connection->start_keepalive(weak_connection); + else + { + connection->start_keepalive(); + } }; auto handshake_request = handshake::write_handshake(connection->m_protocol); @@ -388,9 +390,9 @@ namespace signalr // Sent to server only, should not be received by client throw std::runtime_error("Received unexpected message type 'CancelInvocation'."); case message_type::ping: - if (m_logger.is_enabled(trace_level::info)) + if (m_logger.is_enabled(trace_level::debug)) { - m_logger.log(trace_level::info, std::string("ping message received.")); + m_logger.log(trace_level::debug, std::string("ping message received.")); } break; case message_type::close: @@ -530,20 +532,21 @@ namespace signalr m_nextActivationServerTimeout.store(std::chrono::duration_cast(timeMs).count()); } - void hub_connection_impl::start_keepalive(std::weak_ptr weak_connection) + void hub_connection_impl::start_keepalive() { - auto connection = weak_connection.lock(); - - if (connection) + if (m_logger.is_enabled(trace_level::info)) { - if (connection->m_logger.is_enabled(trace_level::info)) - connection->m_logger.log(trace_level::info, std::string("Start keep alive timer!")); + m_logger.log(trace_level::info, std::string("starting keep alive timer...")); } - auto send_ping = [weak_connection]() + auto send_ping = [](std::shared_ptr connection) { - auto connection = weak_connection.lock(); - if (connection && connection->get_connection_state() != connection_state::connected) + if (!connection) + { + return; + } + + if (connection->get_connection_state() != connection_state::connected) { return; } @@ -553,6 +556,7 @@ namespace signalr hub_message ping_msg(signalr::message_type::ping); auto message = connection->m_protocol->write_message(&ping_msg); + std::weak_ptr weak_connection = connection; connection->m_connection->send( message, connection->m_protocol->transfer_format(), [weak_connection](std::exception_ptr exception) @@ -581,9 +585,10 @@ namespace signalr } }; - send_ping(); + send_ping(shared_from_this()); reset_server_timeout(); + std::weak_ptr weak_connection = shared_from_this(); timer(m_signalr_client_config.get_scheduler(), [send_ping, weak_connection](std::chrono::milliseconds) { @@ -594,7 +599,7 @@ namespace signalr return true; } - if (connection && connection->get_connection_state() != connection_state::connected) + if (connection->get_connection_state() != connection_state::connected) { return true; } @@ -607,7 +612,7 @@ namespace signalr if (connection->get_connection_state() == connection_state::connected) { if (connection->m_logger.is_enabled(trace_level::warning)) - connection->m_logger.log(trace_level::warning, std::string("Server keepalive timeout. Stopping...")); + connection->m_logger.log(trace_level::warning, std::string("server keepalive timeout. Stopping...")); connection->m_connection->stop([](std::exception_ptr) { @@ -618,8 +623,8 @@ namespace signalr if (timeNowmSeconds > connection->m_nextActivationSendPing.load()) { if (connection->m_logger.is_enabled(trace_level::info)) - connection->m_logger.log(trace_level::info, std::string("Send ping to server...")); - send_ping(); + connection->m_logger.log(trace_level::info, std::string("sending ping to server...")); + send_ping(connection); } return false; diff --git a/src/signalrclient/hub_connection_impl.h b/src/signalrclient/hub_connection_impl.h index 29d5e0ea..578d0756 100644 --- a/src/signalrclient/hub_connection_impl.h +++ b/src/signalrclient/hub_connection_impl.h @@ -82,6 +82,6 @@ namespace signalr void reset_send_ping(); void reset_server_timeout(); - void start_keepalive(std::weak_ptr weak_connection); + void start_keepalive(); }; } diff --git a/src/signalrclient/signalr_client_config.cpp b/src/signalrclient/signalr_client_config.cpp index 510507eb..21c644f6 100644 --- a/src/signalrclient/signalr_client_config.cpp +++ b/src/signalrclient/signalr_client_config.cpp @@ -97,30 +97,31 @@ namespace signalr void signalr_client_config::set_server_timeout(std::chrono::milliseconds timeout) { - if (timeout <= std::chrono::seconds(0)) - { - throw std::runtime_error("timeout must be greater than 0."); - } + if (timeout <= std::chrono::seconds(0)) + { + throw std::runtime_error("timeout must be greater than 0."); + } - m_server_timeout = timeout; + m_server_timeout = timeout; } std::chrono::milliseconds signalr_client_config::get_server_timeout() const noexcept { - return m_server_timeout; + return m_server_timeout; } + void signalr_client_config::set_keepalive_interval(std::chrono::milliseconds interval) { - if (interval <= std::chrono::seconds(0)) - { - throw std::runtime_error("timeout must be greater than 0."); - } + if (interval <= std::chrono::seconds(0)) + { + throw std::runtime_error("timeout must be greater than 0."); + } - m_keepalive_interval = interval; + m_keepalive_interval = interval; } std::chrono::milliseconds signalr_client_config::get_keepalive_interval() const noexcept { - return m_keepalive_interval; + return m_keepalive_interval; } } From 4cecd14dc9f76b3e102f992821cbd9562897a3cc Mon Sep 17 00:00:00 2001 From: boev Date: Mon, 24 Jan 2022 17:47:48 +0300 Subject: [PATCH 04/12] assert returned --- src/signalrclient/cancellation_token_source.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/signalrclient/cancellation_token_source.h b/src/signalrclient/cancellation_token_source.h index 1f719b55..fe303a55 100644 --- a/src/signalrclient/cancellation_token_source.h +++ b/src/signalrclient/cancellation_token_source.h @@ -94,7 +94,7 @@ namespace signalr void reset() { std::lock_guard lock(m_lock); - //assert(m_callbacks.empty()); + assert(m_callbacks.empty()); m_signaled = false; m_callbacks.clear(); } From 8df7195809462fcbf0bf9a6a2fc27a734209c2bd Mon Sep 17 00:00:00 2001 From: boev Date: Mon, 24 Jan 2022 19:00:49 +0300 Subject: [PATCH 05/12] undo changes in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 43d47b91..4e2abd27 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ Below are instructions to build on different OS's. You can also use the followin ```powershell PS> git submodule update --init PS> .\submodules\vcpkg\bootstrap-vcpkg.bat -PS> .\submodules\vcpkg\vcpkg.exe install cpprestsdk:x64-windows msgpack:x64-windows +PS> .\submodules\vcpkg\vcpkg.exe install cpprestsdk:x64-windows PS> mkdir build.release PS> cd build.release PS> cmake .. -A x64 -DCMAKE_TOOLCHAIN_FILE="..\submodules\vcpkg\scripts\buildsystems\vcpkg.cmake" -DCMAKE_BUILD_TYPE=Release -DUSE_CPPRESTSDK=true From 6a46b05e357f864b8ee1c41607c0aab350e271eb Mon Sep 17 00:00:00 2001 From: boev Date: Mon, 24 Jan 2022 19:06:46 +0300 Subject: [PATCH 06/12] fixed code formating --- src/signalrclient/hub_connection_impl.cpp | 54 +++++++++++------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 896cf41c..70b67be0 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -39,7 +39,7 @@ namespace signalr const std::shared_ptr& log_writer, std::function(const signalr_client_config&)> http_client_factory, std::function(const signalr_client_config&)> websocket_factory, const bool skip_negotiation) : m_connection(connection_impl::create(url, trace_level, log_writer, http_client_factory, websocket_factory, skip_negotiation)) - , m_logger(log_writer, trace_level), + , m_logger(log_writer, trace_level), m_callback_manager("connection went out of scope before invocation result was received"), m_handshakeReceived(false), m_disconnected([](std::exception_ptr) noexcept {}), m_protocol(std::move(hub_protocol)) { } @@ -50,39 +50,39 @@ namespace signalr std::weak_ptr weak_hub_connection = shared_from_this(); m_connection->set_message_received([weak_hub_connection](std::string&& message) + { + auto connection = weak_hub_connection.lock(); + if (connection) { - auto connection = weak_hub_connection.lock(); - if (connection) - { - connection->process_message(std::move(message)); - } - }); + connection->process_message(std::move(message)); + } + }); m_connection->set_disconnected([weak_hub_connection](std::exception_ptr exception) + { + auto connection = weak_hub_connection.lock(); + if (connection) { - auto connection = weak_hub_connection.lock(); - if (connection) + // start may be waiting on the handshake response so we complete it here, this no-ops if already set + connection->m_handshakeTask->set(std::make_exception_ptr(signalr_exception("connection closed while handshake was in progress."))); + try { - // start may be waiting on the handshake response so we complete it here, this no-ops if already set - connection->m_handshakeTask->set(std::make_exception_ptr(signalr_exception("connection closed while handshake was in progress."))); - try - { - connection->m_disconnect_cts->cancel(); - } - catch (const std::exception& ex) + connection->m_disconnect_cts->cancel(); + } + catch (const std::exception& ex) + { + if (connection->m_logger.is_enabled(trace_level::warning)) { - if (connection->m_logger.is_enabled(trace_level::warning)) - { - connection->m_logger.log(trace_level::warning, std::string("disconnect event threw an exception during connection closure: ") - .append(ex.what())); - } + connection->m_logger.log(trace_level::warning, std::string("disconnect event threw an exception during connection closure: ") + .append(ex.what())); } - - connection->m_callback_manager.clear("connection was stopped before invocation result was received"); - - connection->m_disconnected(exception); } - }); + + connection->m_callback_manager.clear("connection was stopped before invocation result was received"); + + connection->m_disconnected(exception); + } + }); } void hub_connection_impl::on(const std::string& event_name, const std::function&)>& handler) @@ -105,7 +105,7 @@ namespace signalr "an action for this event has already been registered. event name: " + event_name); } - m_subscriptions.insert({ event_name, handler }); + m_subscriptions.insert({event_name, handler}); } void hub_connection_impl::start(std::function callback) noexcept From 00635e4536e0534970021b54999c5b247676d888 Mon Sep 17 00:00:00 2001 From: boev Date: Mon, 24 Jan 2022 19:10:19 +0300 Subject: [PATCH 07/12] fixed code formating --- src/signalrclient/hub_connection_impl.cpp | 34 +++++++++++------------ 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 70b67be0..419fc6a9 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -77,9 +77,9 @@ namespace signalr .append(ex.what())); } } - + connection->m_callback_manager.clear("connection was stopped before invocation result was received"); - + connection->m_disconnected(exception); } }); @@ -241,22 +241,22 @@ namespace signalr connection->m_connection->send(handshake_request, connection->m_protocol->transfer_format(), [handle_handshake, handshake_request_done, handshake_request_lock](std::exception_ptr exception) + { { + std::lock_guard lock(*handshake_request_lock); + if (*handshake_request_done == true) { - std::lock_guard lock(*handshake_request_lock); - if (*handshake_request_done == true) - { - // callback ran from timer or cancellation token, nothing to do here - return; - } - - // indicates that the handshake timer doesn't need to call the callback, it just needs to set the timeout exception - // handle_handshake will be waiting on the handshake completion (error or success) to call the callback - *handshake_request_done = true; + // callback ran from timer or cancellation token, nothing to do here + return; } - handle_handshake(exception, true); - }); + // indicates that the handshake timer doesn't need to call the callback, it just needs to set the timeout exception + // handle_handshake will be waiting on the handshake completion (error or success) to call the callback + *handshake_request_done = true; + } + + handle_handshake(exception, true); + }); }); } @@ -401,7 +401,7 @@ namespace signalr } } } - catch (const std::exception& e) + catch (const std::exception &e) { if (m_logger.is_enabled(trace_level::error)) { @@ -441,10 +441,10 @@ namespace signalr { const auto& callback_id = m_callback_manager.register_callback( create_hub_invocation_callback(m_logger, [callback](const signalr::value& result) { callback(result, nullptr); }, - [callback](const std::exception_ptr e) { callback(signalr::value(), e); })); + [callback](const std::exception_ptr e){ callback(signalr::value(), e); })); invoke_hub_method(method_name, arguments, callback_id, nullptr, - [callback](const std::exception_ptr e) { callback(signalr::value(), e); }); + [callback](const std::exception_ptr e){ callback(signalr::value(), e); }); } void hub_connection_impl::send(const std::string& method_name, const std::vector& arguments, std::function callback) noexcept From 765c1362271953a0bf98763e6da4d0908d39dbc4 Mon Sep 17 00:00:00 2001 From: boev Date: Mon, 24 Jan 2022 19:11:46 +0300 Subject: [PATCH 08/12] fixed code formating --- src/signalrclient/hub_connection_impl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 419fc6a9..4d9061ec 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -441,7 +441,7 @@ namespace signalr { const auto& callback_id = m_callback_manager.register_callback( create_hub_invocation_callback(m_logger, [callback](const signalr::value& result) { callback(result, nullptr); }, - [callback](const std::exception_ptr e){ callback(signalr::value(), e); })); + [callback](const std::exception_ptr e) { callback(signalr::value(), e); })); invoke_hub_method(method_name, arguments, callback_id, nullptr, [callback](const std::exception_ptr e){ callback(signalr::value(), e); }); @@ -451,7 +451,7 @@ namespace signalr { invoke_hub_method(method_name, arguments, "", [callback]() { callback(nullptr); }, - [callback](const std::exception_ptr e) { callback(e); }); + [callback](const std::exception_ptr e){ callback(e); }); } void hub_connection_impl::invoke_hub_method(const std::string& method_name, const std::vector& arguments, From 0482fdac7ee4e4ad689b93595d1c0e7a6b782613 Mon Sep 17 00:00:00 2001 From: boev Date: Thu, 3 Feb 2022 17:27:11 +0300 Subject: [PATCH 09/12] fixed code style --- src/signalrclient/hub_connection_impl.cpp | 21 +++++++++++++------ src/signalrclient/signalr_client_config.cpp | 2 +- .../hub_connection_tests.cpp | 12 ++++++----- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 4d9061ec..fdb97084 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -392,7 +392,7 @@ namespace signalr case message_type::ping: if (m_logger.is_enabled(trace_level::debug)) { - m_logger.log(trace_level::debug, std::string("ping message received.")); + m_logger.log(trace_level::debug, "ping message received."); } break; case message_type::close: @@ -536,7 +536,7 @@ namespace signalr { if (m_logger.is_enabled(trace_level::info)) { - m_logger.log(trace_level::info, std::string("starting keep alive timer...")); + m_logger.log(trace_level::debug, "starting keep alive timer..."); } auto send_ping = [](std::shared_ptr connection) @@ -567,7 +567,9 @@ namespace signalr if (exception) { if (connection->m_logger.is_enabled(trace_level::warning)) - connection->m_logger.log(trace_level::warning, std::string("failed to send ping!")); + { + connection->m_logger.log(trace_level::warning, "failed to send ping!"); + } } else { @@ -612,10 +614,15 @@ namespace signalr if (connection->get_connection_state() == connection_state::connected) { if (connection->m_logger.is_enabled(trace_level::warning)) - connection->m_logger.log(trace_level::warning, std::string("server keepalive timeout. Stopping...")); + { + connection->m_logger.log(trace_level::warning, std::string("server timeout (") + .append(std::to_string(timeNowmSeconds - connection->m_nextActivationServerTimeout.load())) + .append(" ms) elapsed without receiving a message from the server.")); + } + connection->m_connection->stop([](std::exception_ptr) { - + ///TODO: }, nullptr); } } @@ -623,7 +630,9 @@ namespace signalr if (timeNowmSeconds > connection->m_nextActivationSendPing.load()) { if (connection->m_logger.is_enabled(trace_level::info)) - connection->m_logger.log(trace_level::info, std::string("sending ping to server...")); + { + connection->m_logger.log(trace_level::debug, "sending ping to server..."); + } send_ping(connection); } diff --git a/src/signalrclient/signalr_client_config.cpp b/src/signalrclient/signalr_client_config.cpp index 21c644f6..adbc3aec 100644 --- a/src/signalrclient/signalr_client_config.cpp +++ b/src/signalrclient/signalr_client_config.cpp @@ -114,7 +114,7 @@ namespace signalr { if (interval <= std::chrono::seconds(0)) { - throw std::runtime_error("timeout must be greater than 0."); + throw std::runtime_error("interval must be greater than 0."); } m_keepalive_interval = interval; diff --git a/test/signalrclienttests/hub_connection_tests.cpp b/test/signalrclienttests/hub_connection_tests.cpp index bc3d2926..3066189c 100644 --- a/test/signalrclienttests/hub_connection_tests.cpp +++ b/test/signalrclienttests/hub_connection_tests.cpp @@ -1825,7 +1825,9 @@ TEST(keepalive, sends_ping_messages) /* send function */ [messages](const std::string& msg, std::function callback) { if (messages->size() < 3) + { messages->push_back(msg); + } callback(nullptr); }); auto hub_connection = create_hub_connection(websocket_client); @@ -1845,14 +1847,14 @@ TEST(keepalive, sends_ping_messages) std::this_thread::sleep_for(config.get_keepalive_interval() + std::chrono::milliseconds(500)); - ASSERT_TRUE(messages->size() == 3); - ASSERT_EQ("{\"protocol\":\"json\",\"version\":1}\x1e", messages->size() > 0 ? (*messages)[0] : ""); - ASSERT_EQ("{\"type\":6}\x1e", messages->size() > 1 ? (*messages)[1] : ""); - ASSERT_EQ("{\"type\":6}\x1e", messages->size() > 2 ? (*messages)[2] : ""); + ASSERT_EQ(3, messages->size()); + ASSERT_EQ("{\"protocol\":\"json\",\"version\":1}\x1e", (*messages)[0]); + ASSERT_EQ("{\"type\":6}\x1e", (*messages)[1]); + ASSERT_EQ("{\"type\":6}\x1e", (*messages)[2]); ASSERT_EQ(connection_state::connected, hub_connection.get_connection_state()); } -TEST(keepalive, server_timeout_on_no_pong_from_server) +TEST(keepalive, server_timeout_on_no_ping_from_server) { signalr_client_config config; config.set_keepalive_interval(std::chrono::seconds(1)); From d2bbd01b6cb8b9ac735531be22204cad502b3026 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 22 Feb 2022 09:27:50 -0800 Subject: [PATCH 10/12] fixup tests (#1) --- .../hub_connection_tests.cpp | 28 ++++++++++--------- .../test_websocket_client.cpp | 14 ++++++++-- .../test_websocket_client.h | 3 +- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/test/signalrclienttests/hub_connection_tests.cpp b/test/signalrclienttests/hub_connection_tests.cpp index 3066189c..3d49fb0b 100644 --- a/test/signalrclienttests/hub_connection_tests.cpp +++ b/test/signalrclienttests/hub_connection_tests.cpp @@ -1829,7 +1829,10 @@ TEST(keepalive, sends_ping_messages) messages->push_back(msg); } callback(nullptr); - }); + }, + [](const std::string&, std::function callback) { callback(nullptr); }, + [](std::function callback) { callback(nullptr); }, + false); auto hub_connection = create_hub_connection(websocket_client); hub_connection.set_client_config(config); @@ -1858,15 +1861,17 @@ TEST(keepalive, server_timeout_on_no_ping_from_server) { signalr_client_config config; config.set_keepalive_interval(std::chrono::seconds(1)); - config.set_server_timeout(std::chrono::seconds(3)); + config.set_server_timeout(std::chrono::seconds(1)); auto websocket_client = create_test_websocket_client(); auto hub_connection = create_hub_connection(websocket_client); hub_connection.set_client_config(config); auto disconnected_called = false; - hub_connection.set_disconnected([&disconnected_called](std::exception_ptr ex) + + auto disconnect_mre = manual_reset_event(); + hub_connection.set_disconnected([&disconnected_called, &disconnect_mre](std::exception_ptr ex) { - disconnected_called = true; + disconnect_mre.set(ex); }); auto mre = manual_reset_event(); @@ -1881,24 +1886,23 @@ TEST(keepalive, server_timeout_on_no_ping_from_server) mre.get(); - std::this_thread::sleep_for(config.get_server_timeout() + std::chrono::milliseconds(500)); + disconnect_mre.get(); ASSERT_EQ(connection_state::disconnected, hub_connection.get_connection_state()); - ASSERT_TRUE(disconnected_called); } TEST(keepalive, resets_server_timeout_timer_on_any_message_from_server) { signalr_client_config config; config.set_keepalive_interval(std::chrono::seconds(1)); - config.set_server_timeout(std::chrono::seconds(3)); + config.set_server_timeout(std::chrono::seconds(1)); auto websocket_client = create_test_websocket_client(); auto hub_connection = create_hub_connection(websocket_client); hub_connection.set_client_config(config); - auto disconnected_called = false; - hub_connection.set_disconnected([&disconnected_called](std::exception_ptr ex) + auto disconnect_mre = manual_reset_event(); + hub_connection.set_disconnected([&disconnect_mre](std::exception_ptr ex) { - disconnected_called = true; + disconnect_mre.set(ex); }); auto mre = manual_reset_event(); @@ -1917,9 +1921,7 @@ TEST(keepalive, resets_server_timeout_timer_on_any_message_from_server) websocket_client->receive_message("{\"type\":6}\x1e"); std::this_thread::sleep_for(std::chrono::seconds(1)); ASSERT_EQ(connection_state::connected, hub_connection.get_connection_state()); - ASSERT_FALSE(disconnected_called); - std::this_thread::sleep_for(config.get_server_timeout() + std::chrono::milliseconds(500)); + disconnect_mre.get(); ASSERT_EQ(connection_state::disconnected, hub_connection.get_connection_state()); - ASSERT_TRUE(disconnected_called); } diff --git a/test/signalrclienttests/test_websocket_client.cpp b/test/signalrclienttests/test_websocket_client.cpp index 8c99a897..7434f523 100644 --- a/test/signalrclienttests/test_websocket_client.cpp +++ b/test/signalrclienttests/test_websocket_client.cpp @@ -10,12 +10,14 @@ std::shared_ptr create_test_websocket_client( std::function)> send_function, std::function)> connect_function, - std::function)> close_function) + std::function)> close_function, + bool ignore_pings) { auto websocket_client = std::make_shared(); websocket_client->set_send_function(send_function); websocket_client->set_connect_function(connect_function); websocket_client->set_close_function(close_function); + websocket_client->ignore_pings = ignore_pings; return websocket_client; } @@ -24,7 +26,7 @@ test_websocket_client::test_websocket_client() : m_connect_function(std::make_shared)>>([](const std::string&, std::function callback) { callback(nullptr); })), m_send_function(std::make_shared)>>([](const std::string msg, std::function callback) { callback(nullptr); })), m_close_function(std::make_shared)>>([](std::function callback) { callback(nullptr); })), - m_receive_message_event(), m_receive_message(), m_stopped(true), receive_count(0) + m_receive_message_event(), m_receive_message(), m_stopped(true), receive_count(0), ignore_pings(true) { m_receive_loop_not_running.cancel(); } @@ -107,8 +109,14 @@ void test_websocket_client::send(const std::string& payload, signalr::transfer_f { handshake_sent.cancel(); auto local_copy = m_send_function; - m_scheduler->schedule([payload, callback, local_copy]() + auto l_ignore_pings = ignore_pings; + m_scheduler->schedule([payload, callback, local_copy, l_ignore_pings]() { + if (l_ignore_pings && payload.find("\"type\":6") != -1) + { + callback(nullptr); + return; + } (*local_copy)(payload, callback); }); } diff --git a/test/signalrclienttests/test_websocket_client.h b/test/signalrclienttests/test_websocket_client.h index d232d6da..f11c07a7 100644 --- a/test/signalrclienttests/test_websocket_client.h +++ b/test/signalrclienttests/test_websocket_client.h @@ -41,6 +41,7 @@ class test_websocket_client : public websocket_client cancellation_token_source receive_loop_started; cancellation_token_source handshake_sent; int receive_count; + bool ignore_pings; private: std::shared_ptr)>> m_connect_function; @@ -63,4 +64,4 @@ class test_websocket_client : public websocket_client std::shared_ptr create_test_websocket_client( std::function)> send_function = [](const std::string&, std::function callback) { callback(nullptr); }, std::function)> connect_function = [](const std::string&, std::function callback) { callback(nullptr); }, - std::function)> close_function = [](std::function callback) { callback(nullptr); }); \ No newline at end of file + std::function)> close_function = [](std::function callback) { callback(nullptr); }, bool ignore_pings = true); \ No newline at end of file From 8aa99e039092c5b30f054045e05acc024d8ad23a Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 7 Mar 2022 19:13:45 -0800 Subject: [PATCH 11/12] Add exception (#2) --- src/signalrclient/hub_connection_impl.cpp | 10 +++--- .../hub_connection_tests.cpp | 31 ++++++++++++++++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index fdb97084..5427978a 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -613,17 +613,17 @@ namespace signalr { if (connection->get_connection_state() == connection_state::connected) { + auto error_msg = std::string("server timeout (") + .append(std::to_string(connection->m_signalr_client_config.get_server_timeout().count())) + .append(" ms) elapsed without receiving a message from the server."); if (connection->m_logger.is_enabled(trace_level::warning)) { - connection->m_logger.log(trace_level::warning, std::string("server timeout (") - .append(std::to_string(timeNowmSeconds - connection->m_nextActivationServerTimeout.load())) - .append(" ms) elapsed without receiving a message from the server.")); + connection->m_logger.log(trace_level::warning, error_msg); } connection->m_connection->stop([](std::exception_ptr) { - ///TODO: - }, nullptr); + }, std::make_exception_ptr(signalr_exception(error_msg))); } } diff --git a/test/signalrclienttests/hub_connection_tests.cpp b/test/signalrclienttests/hub_connection_tests.cpp index 3d49fb0b..d5ef1dc9 100644 --- a/test/signalrclienttests/hub_connection_tests.cpp +++ b/test/signalrclienttests/hub_connection_tests.cpp @@ -1740,7 +1740,7 @@ TEST(config, can_replace_scheduler) // http_client->send (negotiate), websocket_client->start, handshake timeout timer, websocket_client->send, websocket_client->send, keep alive timer, websocket_client->send ping, websocket_client->stop // handshake timeout timer can trigger more than once if test takes more than 1 second - ASSERT_GE(8, scheduler->schedule_count); + ASSERT_GE(scheduler->schedule_count, 8); } class throw_hub_protocol : public hub_protocol @@ -1820,14 +1820,19 @@ TEST(keepalive, sends_ping_messages) signalr_client_config config; config.set_keepalive_interval(std::chrono::seconds(1)); config.set_server_timeout(std::chrono::seconds(3)); + auto ping_mre = manual_reset_event(); auto messages = std::make_shared>(); auto websocket_client = create_test_websocket_client( - /* send function */ [messages](const std::string& msg, std::function callback) + /* send function */ [messages, &ping_mre](const std::string& msg, std::function callback) { if (messages->size() < 3) { messages->push_back(msg); } + if (messages->size() == 3) + { + ping_mre.set(); + } callback(nullptr); }, [](const std::string&, std::function callback) { callback(nullptr); }, @@ -1848,7 +1853,7 @@ TEST(keepalive, sends_ping_messages) mre.get(); - std::this_thread::sleep_for(config.get_keepalive_interval() + std::chrono::milliseconds(500)); + ping_mre.get(); ASSERT_EQ(3, messages->size()); ASSERT_EQ("{\"protocol\":\"json\",\"version\":1}\x1e", (*messages)[0]); @@ -1886,7 +1891,15 @@ TEST(keepalive, server_timeout_on_no_ping_from_server) mre.get(); - disconnect_mre.get(); + try + { + disconnect_mre.get(); + ASSERT_TRUE(false); + } + catch (const std::exception& ex) + { + ASSERT_STREQ("server timeout (1000 ms) elapsed without receiving a message from the server.", ex.what()); + } ASSERT_EQ(connection_state::disconnected, hub_connection.get_connection_state()); } @@ -1922,6 +1935,14 @@ TEST(keepalive, resets_server_timeout_timer_on_any_message_from_server) std::this_thread::sleep_for(std::chrono::seconds(1)); ASSERT_EQ(connection_state::connected, hub_connection.get_connection_state()); - disconnect_mre.get(); + try + { + disconnect_mre.get(); + ASSERT_TRUE(false); + } + catch (const std::exception& ex) + { + ASSERT_STREQ("server timeout (1000 ms) elapsed without receiving a message from the server.", ex.what()); + } ASSERT_EQ(connection_state::disconnected, hub_connection.get_connection_state()); } From 86ebf6154af4b6186b565550d4a6b4de4a252cef Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 8 Mar 2022 09:42:32 -0800 Subject: [PATCH 12/12] Apply suggestions from code review --- src/signalrclient/hub_connection_impl.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp index 5427978a..ea9074c6 100644 --- a/src/signalrclient/hub_connection_impl.cpp +++ b/src/signalrclient/hub_connection_impl.cpp @@ -534,9 +534,9 @@ namespace signalr void hub_connection_impl::start_keepalive() { - if (m_logger.is_enabled(trace_level::info)) + if (m_logger.is_enabled(trace_level::debug)) { - m_logger.log(trace_level::debug, "starting keep alive timer..."); + m_logger.log(trace_level::debug, "starting keep alive timer."); } auto send_ping = [](std::shared_ptr connection) @@ -629,9 +629,9 @@ namespace signalr if (timeNowmSeconds > connection->m_nextActivationSendPing.load()) { - if (connection->m_logger.is_enabled(trace_level::info)) + if (connection->m_logger.is_enabled(trace_level::debug)) { - connection->m_logger.log(trace_level::debug, "sending ping to server..."); + connection->m_logger.log(trace_level::debug, "sending ping to server."); } send_ping(connection); }