Improve ZeroTier protocol error messages

Source/dvlnet/base_protocol.h

@@ -75,7 +75,7 @@ private:
 	tl::expected<void, PacketError> recv_ingame(packet &pkt, endpoint_t sender);
 	bool is_recognized(endpoint_t sender);
 
-	tl::expected<bool, PacketError> wait_network();
+	tl::expected<void, PacketError> wait_network();
 	bool wait_firstpeer();
 	tl::expected<void, PacketError> wait_join();
 };
@@ -91,18 +91,18 @@ plr_t base_protocol<P>::get_master()
 }
 
 template <class P>
-tl::expected<bool, PacketError> base_protocol<P>::wait_network()
+tl::expected<void, PacketError> base_protocol<P>::wait_network()
 {
 	// wait for ZeroTier for 5 seconds
 	for (auto i = 0; i < 500; ++i) {
 		tl::expected<bool, PacketError> status = proto.network_online();
 		if (!status.has_value())
-			return status;
+			return tl::make_unexpected(std::move(status).error());
 		if (*status)
-			return true;
+			return {};
 		SDL_Delay(10);
 	}
-	return false;
+	return tl::make_unexpected("Timeout waiting for ZeroTier network initialization");
 }
 
 template <class P>
@@ -178,20 +178,20 @@ int base_protocol<P>::create(std::string_view addrstr)
 	gamename = addrstr;
 	isGameHost_ = true;
 
-	tl::expected<bool, PacketError> isReady = wait_network();
+	tl::expected<void, PacketError> isReady = wait_network();
 	if (!isReady.has_value()) {
-		LogError("wait_network: {}", isReady.error().what());
+		const std::string_view message = isReady.error().what();
+		SDL_SetError("%.*s", static_cast<int>(message.size()), message.data());
 		return -1;
 	}
-	if (*isReady) {
-		plr_self = 0;
-		if (tl::expected<void, PacketError> result = Connect(plr_self);
-		    !result.has_value()) {
-			LogError("Connect: {}", result.error().what());
-			return -1;
-		}
+	plr_self = 0;
+	if (tl::expected<void, PacketError> result = Connect(plr_self);
+	    !result.has_value()) {
+		const std::string_view message = result.error().what();
+		SDL_SetError("%.*s", static_cast<int>(message.size()), message.data());
+		return -1;
 	}
-	return (plr_self == PLR_BROADCAST ? -1 : plr_self);
+	return plr_self;
 }
 
 template <class P>
@@ -200,20 +200,18 @@ int base_protocol<P>::join(std::string_view addrstr)
 	gamename = addrstr;
 	isGameHost_ = false;
 
-	tl::expected<bool, PacketError> isReady = wait_network();
+	tl::expected<void, PacketError> isReady = wait_network();
 	if (!isReady.has_value()) {
 		const std::string_view message = isReady.error().what();
-		SDL_SetError("wait_join: %.*s", static_cast<int>(message.size()), message.data());
+		SDL_SetError("%.*s", static_cast<int>(message.size()), message.data());
 		return -1;
 	}
-	if (*isReady) {
-		if (wait_firstpeer()) {
-			tl::expected<void, PacketError> result = wait_join();
-			if (!result.has_value()) {
-				const std::string_view message = result.error().what();
-				SDL_SetError("wait_join: %.*s", static_cast<int>(message.size()), message.data());
-				return -1;
-			}
+	if (wait_firstpeer()) {
+		tl::expected<void, PacketError> result = wait_join();
+		if (!result.has_value()) {
+			const std::string_view message = result.error().what();
+			SDL_SetError("%.*s", static_cast<int>(message.size()), message.data());
+			return -1;
 		}
 	}
 	return (plr_self == PLR_BROADCAST ? -1 : plr_self);

Source/dvlnet/protocol_zt.cpp

@@ -93,9 +93,9 @@ tl::expected<bool, PacketError> protocol_zt::network_online()
 		set_reuseaddr(fd_udp);
 		auto ret = lwip_bind(fd_udp, (struct sockaddr *)&in6, sizeof(in6));
 		if (ret < 0) {
-			Log("lwip, (udp) bind: {}", strerror(errno));
-			SDL_SetError("lwip, (udp) bind: %s", strerror(errno));
-			return tl::make_unexpected(ProtocolError());
+			std::string_view format = "Error binding to ZeroTier UDP socket: {}";
+			PacketError error = ProtocolError(format, strerror(errno));
+			return tl::make_unexpected(std::move(error));
 		}
 		set_nonblock(fd_udp);
 	}
@@ -104,15 +104,15 @@ tl::expected<bool, PacketError> protocol_zt::network_online()
 		set_reuseaddr(fd_tcp);
 		auto r1 = lwip_bind(fd_tcp, (struct sockaddr *)&in6, sizeof(in6));
 		if (r1 < 0) {
-			Log("lwip, (tcp) bind: {}", strerror(errno));
-			SDL_SetError("lwip, (udp) bind: %s", strerror(errno));
-			return tl::make_unexpected(ProtocolError());
+			std::string_view format = "Error binding to ZeroTier TCP socket: {}";
+			PacketError error = ProtocolError(format, strerror(errno));
+			return tl::make_unexpected(std::move(error));
 		}
 		auto r2 = lwip_listen(fd_tcp, 10);
 		if (r2 < 0) {
-			Log("lwip, listen: {}", strerror(errno));
-			SDL_SetError("lwip, listen: %s", strerror(errno));
-			return tl::make_unexpected(ProtocolError());
+			std::string_view format = "Error listening on ZeroTier TCP socket: {}";
+			PacketError error = ProtocolError(format, strerror(errno));
+			return tl::make_unexpected(std::move(error));
 		}
 		set_nonblock(fd_tcp);
 		set_nodelay(fd_tcp);
@@ -183,7 +183,9 @@ tl::expected<bool, PacketError> protocol_zt::send_queued_peer(const endpoint &pe
 		if (decltype(len)(r) == len) {
 			state.send_queue.pop_front();
 		} else {
-			return tl::make_unexpected(ProtocolError());
+			std::string_view format = "Impossible number of bytes sent: {} available, {} sent";
+			PacketError error = ProtocolError(format, len, decltype(len)(r));
+			return tl::make_unexpected(std::move(error));
 		}
 	}
 	return true;

Source/dvlnet/protocol_zt.h

@@ -13,13 +13,16 @@
 
 #include "dvlnet/frame_queue.h"
 #include "dvlnet/packet.h"
+#include "utils/log.hpp"
 
 namespace devilution {
 namespace net {
 
-inline PacketError ProtocolError()
+template <typename... Args>
+PacketError ProtocolError(std::string_view fmt, Args &&...args)
 {
-	return PacketError("Protocol error");
+	auto str = detail::format(fmt, std::forward<Args>(args)...);
+	return PacketError(str);
 }
 
 class protocol_zt {
@@ -56,8 +59,11 @@ public:
 
 		tl::expected<void, PacketError> unserialize(const buffer_t &buf)
 		{
-			if (buf.size() != 16)
-				return tl::make_unexpected(ProtocolError());
+			if (buf.size() != 16) {
+				std::string_view format = "Endpoint deserialization expected 16 bytes, got {}";
+				PacketError error = ProtocolError(format, buf.size());
+				return tl::make_unexpected(std::move(error));
+			}
 			std::copy(buf.begin(), buf.end(), addr.begin());
 			return {};
 		}