From 9d3fbb2cc2f690df59a4be5fd297cd9d0862e579 Mon Sep 17 00:00:00 2001 From: Craig Edwards Date: Mon, 9 Dec 2024 17:42:29 +0000 Subject: [PATCH] fix reconnect on timeout --- include/dpp/sslclient.h | 2 +- include/dpp/wsclient.h | 20 +++++++++++++++++--- src/dpp/cluster.cpp | 21 ++++++++++++++++----- src/dpp/discordclient.cpp | 13 +++++++++++-- src/dpp/wsclient.cpp | 25 +++++++++++++++++++++++-- 5 files changed, 68 insertions(+), 13 deletions(-) diff --git a/include/dpp/sslclient.h b/include/dpp/sslclient.h index 9d237172d3..3fe1c4c0db 100644 --- a/include/dpp/sslclient.h +++ b/include/dpp/sslclient.h @@ -314,7 +314,7 @@ class DPP_EXPORT ssl_client /** * @brief Called when the TCP socket has data to read - * @param fd Fild descriptor + * @param fd File descriptor * @param ev Socket events */ void on_read(dpp::socket fd, const struct dpp::socket_events& ev); diff --git a/include/dpp/wsclient.h b/include/dpp/wsclient.h index da2a6c6fdf..11251f2a1c 100644 --- a/include/dpp/wsclient.h +++ b/include/dpp/wsclient.h @@ -153,7 +153,7 @@ class DPP_EXPORT websocket_client : public ssl_client { protected: /** - * @brief (Re)connect + * @brief Connect to websocket server */ virtual void connect(); @@ -163,6 +163,18 @@ class DPP_EXPORT websocket_client : public ssl_client { */ [[nodiscard]] ws_state get_state() const; + /** + * @brief If true the connection timed out while waiting, + * when waiting for SSL negotiation, TCP connect(), or HTTP. + */ + bool timed_out; + + /** + * @brief Time at which the connection should be abandoned, + * if we are still connecting or negotiating with a HTTP server + */ + time_t timeout; + public: /** @@ -171,8 +183,10 @@ class DPP_EXPORT websocket_client : public ssl_client { * @param port Port to connect to * @param urlpath The URL path components of the HTTP request to send * @param opcode The encoding type to use, either OP_BINARY or OP_TEXT - * @note Voice websockets only support OP_TEXT, and other websockets must be - * OP_BINARY if you are going to send ETF. + * @note This just indicates the default for frames sent. Certain sockets, + * such as voice websockets, may send a combination of OP_TEXT and OP_BINARY + * frames, whereas shard websockets will only ever send OP_BINARY for ETF and + * OP_TEXT for JSON. */ websocket_client(cluster* creator, const std::string& hostname, const std::string& port = "443", const std::string& urlpath = "", ws_opcode opcode = OP_BINARY); diff --git a/src/dpp/cluster.cpp b/src/dpp/cluster.cpp index 489ac17851..eee7436bd9 100644 --- a/src/dpp/cluster.cpp +++ b/src/dpp/cluster.cpp @@ -221,11 +221,22 @@ void cluster::start(start_type return_after) { auto session_id = old->sessionid; log(ll_info, "Reconnecting shard " + std::to_string(shard_id)); /* Make a new resumed connection based off the old one */ - shards[shard_id] = new discord_client(*old, seq_no, session_id); - /* Delete the old one */ - delete old; - /* Set up the new shard's IO events */ - shards[shard_id]->run(); + try { + shards[shard_id] = nullptr; + shards[shard_id] = new discord_client(*old, seq_no, session_id); + /* Delete the old one */ + delete old; + old = nullptr; + /* Set up the new shard's IO events */ + shards[shard_id]->run(); + } + catch (const std::exception& e) { + log(ll_info, "Exception when reconnecting shard " + std::to_string(shard_id) + ": " + std::string(e.what())); + delete shards[shard_id]; + delete old; + old = nullptr; + add_reconnect(shard_id); + } /* It is not possible to reconnect another shard within the same 5-second window, * due to discords strict rate limiting on shard connections, so we bail out here * and only try another reconnect in the next timer interval. Do not try and make diff --git a/src/dpp/discordclient.cpp b/src/dpp/discordclient.cpp index c7c8407d72..a59f76e325 100644 --- a/src/dpp/discordclient.cpp +++ b/src/dpp/discordclient.cpp @@ -274,12 +274,12 @@ bool discord_client::handle_frame(const std::string &buffer, ws_opcode opcode) auto o = j.find("op"); if (o != j.end() && !o->is_null()) { - uint32_t op = o->get(); + shard_frame_type op = o->get(); switch (op) { case ft_invalid_session: /* Reset session state and fall through to 10 */ - op = 10; + op = ft_hello; log(dpp::ll_debug, "Failed to resume session " + sessionid + ", will reidentify"); this->sessionid.clear(); this->last_seq = 0; @@ -359,6 +359,15 @@ bool discord_client::handle_frame(const std::string &buffer, ws_opcode opcode) this->last_heartbeat_ack = time(nullptr); websocket_ping = utility::time_f() - ping_start; break; + case ft_heartbeat: + case ft_identify: + case ft_presence: + case ft_voice_state_update: + case ft_resume: + case ft_request_guild_members: + case ft_request_soundboard_sounds: + log(dpp::ll_error, "Received invalid opcode on websocket for session " + sessionid); + break; } } return true; diff --git a/src/dpp/wsclient.cpp b/src/dpp/wsclient.cpp index ffad79ea3f..8f455d085c 100644 --- a/src/dpp/wsclient.cpp +++ b/src/dpp/wsclient.cpp @@ -42,7 +42,9 @@ websocket_client::websocket_client(cluster* creator, const std::string& hostname : ssl_client(creator, hostname, port), state(HTTP_HEADERS), path(urlpath), - data_opcode(opcode) + data_opcode(opcode), + timed_out(false), + timeout(time(nullptr) + 5) { uint64_t k = (time(nullptr) * time(nullptr)); /* A 64 bit value as hex with leading zeroes is always 16 chars. @@ -303,7 +305,9 @@ bool websocket_client::parseheader(std::string& data) void websocket_client::one_second_timer() { - if (((time(nullptr) % 20) == 0) && (state == CONNECTED)) { + time_t now = time(nullptr); + + if (((now % 20) == 0) && (state == CONNECTED)) { /* For sending pings, we send with payload */ unsigned char out[MAXHEADERSIZE]; std::string payload = "keepalive"; @@ -312,6 +316,23 @@ void websocket_client::one_second_timer() ssl_client::socket_write(header); ssl_client::socket_write(payload); } + + /* Handle timeouts for connect(), SSL negotiation and HTTP negotiation */ + if (!timed_out && sfd != INVALID_SOCKET) { + if (!tcp_connect_done && now >= timeout) { + log(ll_warning, "Websocket connection timed out: connect()"); + timed_out = true; + this->close(); + } else if (tcp_connect_done && !connected && now >= timeout && this->state != CONNECTED) { + log(ll_warning, "Websocket connection timed out: SSL handshake"); + timed_out = true; + this->close(); + } else if (now >= timeout && this->state != CONNECTED) { + log(ll_warning, "Websocket connection timed out: HTTP negotiation"); + timed_out = true; + this->close(); + } + } } void websocket_client::handle_ping(const std::string &payload)