Skip to content

Commit

Permalink
Properly log initial connection attempt failure
Browse files Browse the repository at this point in the history
Also log disconnect after explicit call to stop().
  • Loading branch information
ungive committed Jul 19, 2024
1 parent 75fcf85 commit 6f1183a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 28 deletions.
39 changes: 25 additions & 14 deletions clients/cpp/src/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ ClientImpl::ClientImpl(std::string const& address, ClientOptions options)

ClientImpl::~ClientImpl()
{
stop();
// Stop the client
{
std::unique_lock<std::mutex> lock(m_mutex);
internal_stop(lock);
}
// Stop the manager loop thread
{
const std::lock_guard<std::mutex> lock(m_mutex);
Expand All @@ -84,12 +88,14 @@ ClientImpl::~ClientImpl()
void ClientImpl::start()
{
const std::lock_guard<std::mutex> lock(m_mutex);
m_was_explicitly_started = true;
internal_start();
}

void ClientImpl::stop()
{
std::unique_lock<std::mutex> lock(m_mutex);
m_was_explicitly_stopped = true;
internal_stop(lock);
}

Expand Down Expand Up @@ -428,15 +434,30 @@ void ClientImpl::on_websocket_open()
{
const std::lock_guard<std::mutex> lock(m_mutex);
update_connected(true);
log(Debug) << "connected";
m_was_explicitly_started = false;
}

void ClientImpl::on_websocket_close()
{
const std::lock_guard<std::mutex> lock(m_mutex);
update_connected(false);
reset_connection_state();
log(Info) << "disconnected";
if (m_was_explicitly_started) {
m_was_explicitly_started = false;
auto retrying = m_options.websocket.reconnect_delay.has_value();
log(Error) << (retrying ? "initial connection attempt failed"
: "connection failed")
<< var("retrying", retrying)
<< var("address", m_conn->address())
<< var("conn_timeout", m_options.websocket.connect_timeout)
<< var("reconn_delay", m_options.websocket.reconnect_delay)
<< var("max_reconn_delay",
m_options.websocket.max_reconnect_delay);
}
if (m_was_explicitly_stopped) {
m_was_explicitly_stopped = false;
log(Info) << "client stopped";
}
}

void ClientImpl::on_websocket_message(std::string const& message)
Expand Down Expand Up @@ -529,17 +550,7 @@ void ClientImpl::internal_start()
return;
}
m_started = true;
if (!m_conn->start()) {
auto retrying = m_options.websocket.reconnect_delay.has_value();
log(Error) << (retrying ? "initial connection attempt failed"
: "connection failed")
<< var("retrying", retrying)
<< var("address", m_conn->address())
<< var("conn_timeout", m_options.websocket.connect_timeout)
<< var("reconn_delay", m_options.websocket.reconnect_delay)
<< var("max_reconn_delay",
m_options.websocket.max_reconnect_delay);
}
m_conn->start();
}

void ClientImpl::reset_connection_state()
Expand Down
2 changes: 2 additions & 0 deletions clients/cpp/src/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ class ClientImpl : public IClient

bool m_started{ false };
bool m_connected{ false };
bool m_was_explicitly_started{ false };
bool m_was_explicitly_stopped{ false };
// Use a recursive mutex, since many methods could trigger a reconnect,
// which would call close and would trigger the close callback,
// which locks this mutex again.
Expand Down
6 changes: 2 additions & 4 deletions clients/cpp/src/websocket/backend_libhv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ClientImpl : public BaseClient
int64_t send_text(const char* data, size_t length) override;

protected:
bool internal_start() override;
void internal_start() override;
void internal_stop() override;

std::unique_ptr<hv::WebSocketClient> create_conn();
Expand Down Expand Up @@ -90,7 +90,7 @@ std::unique_ptr<hv::WebSocketClient> ClientImpl::create_conn()
return conn;
}

bool ClientImpl::internal_start()
void ClientImpl::internal_start()
{
// Reconnect
if (m_options.reconnect_delay.has_value()) {
Expand Down Expand Up @@ -123,8 +123,6 @@ bool ClientImpl::internal_start()
}
// Open the connection
m_conn->open(m_address.c_str(), headers);
perror("internal_start() after");
return m_conn->isConnected();
}

void ClientImpl::internal_stop()
Expand Down
6 changes: 3 additions & 3 deletions clients/cpp/src/websocket/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ void BaseClient::on_message(
m_message_callback = callback;
}

bool BaseClient::start()
void BaseClient::start()
{
const std::lock_guard<std::mutex> lock(m_mutex);
if (m_started) {
return true;
return;
}
m_started = true;
return internal_start();
internal_start();
}

void BaseClient::stop()
Expand Down
11 changes: 4 additions & 7 deletions clients/cpp/src/websocket/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,8 @@ class IClient
* have been called and returned.
* If a mutex is held while start() is called,
* it can safely be locked from any of the event handlers.
*
* @returns Whether the client has
* successfully connected the first time or not.
*/
virtual bool start() = 0;
virtual void start() = 0;

/**
* @brief Stops the websocket client.
Expand Down Expand Up @@ -145,7 +142,7 @@ class Client : public IClient
return m_impl->send_text(data, length);
}

inline bool start() override { return m_impl->start(); }
inline void start() override { return m_impl->start(); }

inline void stop() override { return m_impl->stop(); }

Expand All @@ -171,12 +168,12 @@ class BaseClient : public IClient

virtual int64_t send_text(const char* data, size_t length) = 0;

bool start() override;
void start() override;

void stop() override;

protected:
virtual bool internal_start() = 0;
virtual void internal_start() = 0;
virtual void internal_stop() = 0;

/**
Expand Down

0 comments on commit 6f1183a

Please sign in to comment.