fix: add auto retry to failed connect, fixes failed unit tests

include/dpp/httpsclient.h

@@ -137,10 +137,6 @@ using https_client_completion_event = std::function<void(class https_client*)>;
  * @note plaintext HTTP without SSL is also supported via a "downgrade" setting
  */
 class DPP_EXPORT https_client : public ssl_client {
-	/**
-	 * @brief Current connection state
-	 */
-	http_state state;
 
 	/**
 	 * @brief The type of the request, e.g. GET, POST
@@ -241,7 +237,12 @@ class DPP_EXPORT https_client : public ssl_client {
 	 * @brief Function to call when HTTP request is completed
 	 */
 	https_client_completion_event completed;
-
+
+	/**
+	 * @brief Current connection state
+	 */
+	http_state state;
+
 	/**
 	 * @brief Connect to a specific HTTP(S) server and complete a request.
 	 * 
@@ -361,7 +362,6 @@ class DPP_EXPORT https_client : public ssl_client {
 	 * @return Split URL
 	 */
 	static http_connect_info get_host_info(std::string url);
-
 };
 
 }

include/dpp/sslclient.h

@@ -147,6 +147,16 @@ class DPP_EXPORT ssl_client
 	 */
 	time_t last_tick;
 
+	/**
+	 * @brief Start time of connection
+	 */
+	time_t start;
+
+	/**
+	 * @brief How many times we retried connect()
+	 */
+	uint8_t connect_retries{0};
+
 	/**
 	 * @brief Hostname connected to
 	 */
@@ -177,11 +187,24 @@ class DPP_EXPORT ssl_client
 	 */
 	bool connected{false};
 
+	/**
+	 * @brief True if tcp connect() succeeded
+	 */
+	bool tcp_connect_done{false};
+
 	/**
 	 * @brief Timer handle for one second timer
 	 */
 	timer timer_handle;
 
+	/**
+	 * @brief Unique ID of socket used as a nonce
+	 * You can use this to identify requests vs reply
+	 * if you want. D++ itself only sets this, and does
+	 * not use it in any logic. It starts at 1 and increments
+	 * for each request made.
+	 */
+	uint64_t unique_id;
 
 	/**
 	 * @brief Called every second
@@ -207,6 +230,14 @@ class DPP_EXPORT ssl_client
 	 */
 	uint64_t get_bytes_in();
 
+	/**
+	 * @brief Every request made has a unique ID. This increments
+	 * for every request, starting at 1. You can use this for statistics,
+	 * or to associate requests and replies in external event loops.
+	 * @return Unique ID
+	 */
+	uint64_t get_unique_id() const;
+
 	/**
 	 * @brief Get SSL cipher name
 	 * @return std::string ssl cipher name

src/dpp/events/message_create.cpp

@@ -22,7 +22,6 @@
 #include <dpp/discordevents.h>
 #include <dpp/cluster.h>
 #include <dpp/message.h>
-#include <dpp/stringops.h>
 #include <dpp/json.h>
 
 
@@ -39,11 +38,12 @@ namespace dpp::events {
 void message_create::handle(discord_client* client, json &j, const std::string &raw) {
 
 	if (!client->creator->on_message_create.empty()) {
-		json d = j["d"];
-		dpp::message_create_t msg(client, raw);
-		msg.msg.fill_from_json(&d, client->creator->cache_policy);
-		msg.msg.owner = client->creator;
-		client->creator->queue_work(1, [client, msg]() {
+		json js = j;
+		client->creator->queue_work(1, [client, js, raw]() {
+			json d = js["d"];
+			dpp::message_create_t msg(client, raw);
+			msg.msg = message(client->owner).fill_from_json(&d, client->creator->cache_policy);
+			msg.msg.owner = client->creator;
 			client->creator->on_message_create.call(msg);
 		});
 	}

src/dpp/httpsclient.cpp

@@ -32,7 +32,6 @@ namespace dpp {
 
 https_client::https_client(cluster* creator, const std::string &hostname, uint16_t port,  const std::string &urlpath, const std::string &verb, const std::string &req_body, const http_headers& extra_headers, bool plaintext_connection, uint16_t request_timeout, const std::string &protocol, https_client_completion_event done)
 	: ssl_client(creator, hostname, std::to_string(port), plaintext_connection, false),
-	state(HTTPS_HEADERS),
 	request_type(verb),
 	path(urlpath),
 	request_body(req_body),
@@ -42,7 +41,8 @@ https_client::https_client(cluster* creator, const std::string &hostname, uint16
 	http_protocol(protocol),
 	timeout(time(nullptr) + request_timeout),
 	timed_out(false),
-	completed(done)
+	completed(done),
+	state(HTTPS_HEADERS)
 {
 	nonblocking = false;
 	https_client::connect();
@@ -157,6 +157,10 @@ bool https_client::handle_buffer(std::string &buffer)
 		switch (state) {
 			case HTTPS_HEADERS:
 				if (buffer.find("\r\n\r\n") != std::string::npos) {
+
+					/* Add 10 seconds to retrieve body */
+					timeout += 10;
+
 					/* Got all headers, proceed to new state */
 
 					std::string unparsed = buffer;
@@ -211,6 +215,10 @@ bool https_client::handle_buffer(std::string &buffer)
 								state_changed = true;
 								continue;
 							}
+							if (!buffer.empty()) {
+								/* Got a bit of body content in the same read as the headers */
+								continue;
+							}
 							return true;
 						} else {
 							/* Non-HTTP-like response with invalid headers. Go no further. */
@@ -242,11 +250,11 @@ bool https_client::handle_buffer(std::string &buffer)
 			case HTTPS_CHUNK_TRAILER:
 				if (buffer.length() >= 2 && buffer.substr(0, 2) == "\r\n") {
 					if (state == HTTPS_CHUNK_LAST) {
-						state = HTTPS_DONE;
 						if (completed) {
 							completed(this);
 							completed = {};
 						}
+						state = HTTPS_DONE;
 						this->close();
 						return false;
 					} else {
@@ -281,11 +289,11 @@ bool https_client::handle_buffer(std::string &buffer)
 				body += buffer;
 				buffer.clear();
 				if (content_length == ULLONG_MAX || body.length() >= content_length) {
-					state = HTTPS_DONE;
 					if (completed) {
 						completed(this);
 						completed = {};
 					}
+					state = HTTPS_DONE;
 					this->close();
 					return false;
 				}
@@ -317,24 +325,26 @@ http_state https_client::get_state() {
 }
 
 void https_client::one_second_timer() {
-	if ((this->sfd == SOCKET_ERROR || time(nullptr) >= timeout) && this->state != HTTPS_DONE) {
-		/* if and only if response is timed out */
-		if (this->sfd != SOCKET_ERROR) {
-			timed_out = true;
-		}
-		keepalive = false;
+	if (!tcp_connect_done && time(nullptr) >= timeout) {
+		timed_out = true;
+		this->close();
+	} else if (tcp_connect_done && !connected && time(nullptr) >= timeout && this->state != HTTPS_DONE) {
 		this->close();
+		timed_out = true;
+	} else if (time(nullptr) >= timeout && this->state != HTTPS_DONE) {
+		this->close();
+		timed_out = true;
 	}
 }
 
 void https_client::close() {
 	if (state != HTTPS_DONE) {
-		state = HTTPS_DONE;
-		ssl_client::close();
 		if (completed) {
 			completed(this);
 			completed = {};
 		}
+		state = HTTPS_DONE;
+		ssl_client::close();
 	}
 }
 

src/dpp/message.cpp

@@ -663,8 +663,9 @@ embed::embed() : timestamp(0) {
 }
 
 message::message() : managed(0), channel_id(0), guild_id(0), sent(0), edited(0), webhook_id(0), interaction_metadata{},
-	owner(nullptr), type(mt_default), flags(0), pinned(false), tts(false), mention_everyone(false)
+	type(mt_default), flags(0), pinned(false), tts(false), mention_everyone(false)
 {
+	owner = nullptr;
 	message_reference.channel_id = 0;
 	message_reference.guild_id = 0;
 	message_reference.message_id = 0;
@@ -1059,8 +1060,10 @@ attachment::attachment(struct message* o, json *j) : attachment(o) {
 
 void attachment::download(http_completion_event callback) const {
 	/* Download attachment if there is one attached to this object */
-	if (owner == nullptr || owner->owner == nullptr) {
-		throw dpp::logic_exception(err_no_owning_message, "attachment has no owning message/cluster");
+	if (owner == nullptr) {
+		throw dpp::logic_exception(err_no_owning_message, "attachment has no owning message");
+	} else if (owner->owner == nullptr) {
+		throw dpp::logic_exception(err_no_owning_message, "attachment has no owning cluster");
 	}
 	if (callback && this->id && !this->url.empty()) {
 		owner->owner->request(this->url, dpp::m_get, callback);