refactor: replace fatal log with exceptions for error handling and si…

…mplify Client destructor
MasterLaplace · Nov 11, 2024 · a5c911a · a5c911a
1 parent c643f76
commit a5c911a
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 84 deletions.
diff --git a/Flakkari/Network/IOMultiplexer.cpp b/Flakkari/Network/IOMultiplexer.cpp
@@ -16,10 +16,7 @@ namespace Flakkari::Network {
 PSELECT::PSELECT(FileDescriptor fileDescriptor, long int seconds, long int microseconds)
 {
     if (fileDescriptor == -1)
-    {
-        FLAKKARI_LOG_ERROR("Socket is -1");
         throw std::runtime_error("Socket is -1");
-    }
 
     FD_ZERO(&_fds);
     _maxFd = fileDescriptor;
@@ -43,6 +40,7 @@ void PSELECT::addSocket(FileDescriptor socket)
         throw std::runtime_error("Socket is -1");
     if (socket > FD_SETSIZE)
         throw std::runtime_error("Index out of range");
+
     FD_SET(socket, &_fds);
     if (socket > _maxFd)
         _maxFd = socket;
@@ -55,6 +53,7 @@ void PSELECT::removeSocket(FileDescriptor socket)
         throw std::runtime_error("Socket is -1");
     if (socket > _maxFd)
         throw std::runtime_error("Index out of range");
+
     FD_CLR(socket, &_fds);
     _sockets.erase(std::remove(_sockets.begin(), _sockets.end(), socket), _sockets.end());
     _maxFd = *std::max_element(_sockets.begin(), _sockets.end());
@@ -78,6 +77,7 @@ bool PSELECT::isReady(FileDescriptor socket)
         throw std::runtime_error("Socket is -1");
     if (socket > _maxFd)
         throw std::runtime_error("Index out of range");
+
     return FD_ISSET(socket, &_fds);
 }
 
@@ -90,13 +90,11 @@ bool PSELECT::skipableError() { return errno == EINTR || errno == EAGAIN || errn
 PPOLL::PPOLL(FileDescriptor fileDescriptor, event_t events, long int seconds, long int microseconds)
 {
     if (fileDescriptor == -1)
-    {
-        FLAKKARI_LOG_ERROR("Socket is -1");
         throw std::runtime_error("Socket is -1");
-    }
 
     if (_pollfds.size() < std::size_t(fileDescriptor))
         _pollfds.resize(fileDescriptor + 1);
+
     _pollfds[fileDescriptor] = pollfd{fileDescriptor, events, 0};
 
     _timeout.tv_sec = seconds;
@@ -115,6 +113,7 @@ void PPOLL::addSocket(FileDescriptor socket)
         throw std::runtime_error("Socket is -1");
     if (_pollfds.size() < std::size_t(socket))
         _pollfds.resize(socket + 1);
+
     _pollfds[socket] = pollfd{socket, POLLIN | POLLPRI, 0};
 }
 
@@ -124,6 +123,7 @@ void PPOLL::addSocket(FileDescriptor socket, event_t events)
         throw std::runtime_error("Socket is -1");
     if (_pollfds.size() < std::size_t(socket))
         _pollfds.resize(socket + 1);
+
     _pollfds[socket] = pollfd{socket, events, 0};
 }
 
@@ -152,6 +152,7 @@ pollfd &PPOLL::operator[](std::size_t index)
 {
     if (_pollfds.size() < index)
         throw std::runtime_error("Index out of range");
+
     return _pollfds[index];
 }
 
@@ -161,13 +162,15 @@ pollfd &PPOLL::operator[](FileDescriptor socket)
         throw std::runtime_error("Socket is -1");
     if (_pollfds.size() < std::size_t(socket))
         throw std::runtime_error("Index out of range");
+
     return _pollfds[socket];
 }
 
 bool PPOLL::isReady(pollfd fd)
 {
     if (fd.fd == -1)
         throw std::runtime_error("Socket is -1");
+
     return fd.revents & (POLLIN | POLLPRI);
 }
 
@@ -179,6 +182,7 @@ bool PPOLL::isReady(FileDescriptor socket)
         throw std::runtime_error("Index out of range");
     if (_pollfds[socket].revents & (POLLIN | POLLPRI))
         return true;
+
     return false;
 }
 
@@ -191,7 +195,7 @@ bool PPOLL::skipableError() { return errno == EINTR || errno == EAGAIN || errno
 WSA::WSA(FileDescriptor socket, int seconds, int microseconds)
 {
     if (socket == -1)
-        FLAKKARI_LOG_FATAL("Socket is -1");
+        throw std::runtime_error("Socket is -1");
 
     _sockets.reserve(MAX_POLLFD);
     _fdArray.reserve(MAX_POLLFD);

diff --git a/Flakkari/Network/Network.cpp b/Flakkari/Network/Network.cpp
@@ -17,7 +17,7 @@ void init()
     WSADATA WSAData;
 
     if (::WSAStartup(MAKEWORD(2, 2), &WSAData) != NO_ERROR)
-        FLAKKARI_LOG_FATAL("WSAStartup failed");
+        throw std::runtime_error("WSAStartup failed");
 #endif
 }
 

diff --git a/Flakkari/Network/Socket.cpp b/Flakkari/Network/Socket.cpp
@@ -26,20 +26,14 @@ void Socket::create(const std::shared_ptr<Address> &address)
 
     _socket = ::socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
     if (_socket == INVALID_SOCKET)
-    {
-        FLAKKARI_LOG_FATAL("Failed to create socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to create socket, error: " + STD_ERROR);
 
 #if __APPLE__
     int optval = 1;
     ::setsockopt(_socket, SOL_SOCKET, SO_NOSIGPIPE, &optval, sizeof(optval));
 #elif __linux__
     if (::setsockopt(_socket, SOL_SOCKET, SO_REUSEADDR | SO_REUSEPORT, "\001", 4))
-    {
-        FLAKKARI_LOG_FATAL("Failed to set socket to reuse address and port, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to set socket to reuse address and port, error: " + STD_ERROR);
 #endif
 }
 
@@ -51,19 +45,13 @@ void Socket::create(socket_t socket, const std::shared_ptr<Address> &address)
 #if _WIN32
     u_long mode = 1;
     if (::ioctlsocket(_socket, FIONBIO, &mode) != NO_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to set socket to non-blocking, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to set socket to non-blocking, error: " + STD_ERROR);
 #elif __APPLE__
     int optval = 1;
     ::setsockopt(_socket, SOL_SOCKET, SO_NOSIGPIPE, &optval, sizeof(optval));
 #elif __linux__
     if (::setsockopt(_socket, SOL_SOCKET, SO_REUSEADDR | SO_REUSEPORT, "\001", 4))
-    {
-        FLAKKARI_LOG_FATAL("Failed to set socket to reuse address and port, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to set socket to reuse address and port, error: " + STD_ERROR);
 #endif
 }
 
@@ -82,20 +70,14 @@ void Socket::create(const Address &address)
 
     _socket = ::socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
     if (_socket == INVALID_SOCKET)
-    {
-        FLAKKARI_LOG_FATAL("Failed to create socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to create socket, error: " + STD_ERROR);
 
 #if __APPLE__
     int optval = 1;
     ::setsockopt(_socket, SOL_SOCKET, SO_NOSIGPIPE, &optval, sizeof(optval));
 #elif __linux__
     if (::setsockopt(_socket, SOL_SOCKET, SO_REUSEADDR | SO_REUSEPORT, "\001", 4))
-    {
-        FLAKKARI_LOG_FATAL("Failed to set socket to reuse address and port, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to set socket to reuse address and port, error: " + STD_ERROR);
 #endif
 }
 
@@ -114,20 +96,14 @@ void Socket::create(ip_t address, port_t port, Address::IpType ip_type, Address:
 
     _socket = ::socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
     if (_socket == INVALID_SOCKET)
-    {
-        FLAKKARI_LOG_FATAL("Failed to create socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to create socket, error: " + STD_ERROR);
 
 #if __APPLE__
     int optval = 1;
     ::setsockopt(_socket, SOL_SOCKET, SO_NOSIGPIPE, &optval, sizeof(optval));
 #elif __linux__
     if (::setsockopt(_socket, SOL_SOCKET, SO_REUSEADDR | SO_REUSEPORT, "\001", 4))
-    {
-        FLAKKARI_LOG_FATAL("Failed to set socket to reuse address and port, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to set socket to reuse address and port, error: " + STD_ERROR);
 #endif
 }
 
@@ -141,19 +117,13 @@ void Socket::bind()
         return FLAKKARI_LOG_ERROR("Address is nullptr"), void();
 
     if (::bind(_socket, addr->ai_addr, (int) addr->ai_addrlen) == SOCKET_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to bind socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to bind socket, error: " + STD_ERROR);
 }
 
 void Socket::listen(int backlog)
 {
     if (::listen(_socket, backlog) == SOCKET_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to listen on socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to listen on socket, error: " + STD_ERROR);
 }
 
 void Socket::connect()
@@ -164,40 +134,25 @@ void Socket::connect()
         return FLAKKARI_LOG_ERROR("Address is nullptr"), void();
 
     if (::connect(_socket, addr->ai_addr, (int) addr->ai_addrlen) == SOCKET_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to connect socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to connect socket, error: " + STD_ERROR);
 }
 
 void Socket::disconnect()
 {
 #if _WIN32
     if (::shutdown(_socket, SD_BOTH) == SOCKET_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to disconnect socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to disconnect socket, error: " + STD_ERROR);
 #else
     if (::shutdown(_socket, SHUT_RDWR) == SOCKET_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to disconnect socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to disconnect socket, error: " + STD_ERROR);
 #endif
 
 #ifdef _WIN32
     if (::closesocket(_socket) == SOCKET_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to close socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to close socket, error: " + STD_ERROR);
 #else
     if (::close(_socket) == SOCKET_ERROR)
-    {
-        FLAKKARI_LOG_FATAL("Failed to close socket, error: " + STD_ERROR);
-        return;
-    }
+        throw std::runtime_error("Failed to close socket, error: " + STD_ERROR);
 #endif
 
 #ifdef _WIN32
@@ -214,10 +169,7 @@ std::shared_ptr<Socket> Socket::accept()
     SOCKET clientSocket = ::accept(_socket, (struct sockaddr *) &clientAddr, &clientAddrLen);
 
     if (clientSocket == INVALID_SOCKET)
-    {
-        FLAKKARI_LOG_FATAL("Failed to accept socket, error: " + STD_ERROR);
-        return nullptr;
-    }
+        throw std::runtime_error("Failed to accept socket, error: " + STD_ERROR);
 
     auto _ip_type = (clientAddr.ss_family == AF_INET)  ? Address::IpType::IPv4 :
                     (clientAddr.ss_family == AF_INET6) ? Address::IpType::IPv6 :

diff --git a/Flakkari/Protocol/Packet.hpp b/Flakkari/Protocol/Packet.hpp
@@ -162,12 +162,14 @@ template <typename Id> struct Packet {
     [[nodiscard]] bool deserialize(const Network::Buffer &buffer)
     {
         if (buffer.size() < sizeof(header))
-            return false;
+            return FLAKKARI_LOG_WARNING("Buffer is too small to deserialize a packet."), false;
         std::memcpy(&header, buffer.data(), sizeof(header));
         if (header._priority >= Priority::MAX_PRIORITY)
-            return false;
+            return FLAKKARI_LOG_WARNING("Priority is too big ("+ std::to_string((int)header._priority) +")"), false;
         if (header._apiVersion >= ApiVersion::MAX_VERSION)
-            return false;
+            return FLAKKARI_LOG_WARNING("ApiVersion is too big ("+ std::to_string((int)header._apiVersion) +")"), false;
+        if (header._commandId >= CommandId::MAX_COMMAND_ID)
+            return FLAKKARI_LOG_WARNING("CommandId is too big ("+ std::to_string((int)header._commandId) +")"), false;
         if (header._contentLength > buffer.size() - sizeof(header))
             return false;
         payload = buffer.extractData(sizeof(header), header._contentLength);

diff --git a/Flakkari/Server/Client/Client.cpp b/Flakkari/Server/Client/Client.cpp
@@ -17,11 +17,7 @@ Client::Client(const std::shared_ptr<Network::Address> &address, const std::stri
     _lastActivity = std::chrono::steady_clock::now();
 }
 
-Client::~Client()
-{
-    _isConnected = false;
-    _address->setId(-1);
-}
+Client::~Client() { _isConnected = false; }
 
 bool Client::isConnected(float timeout)
 {

diff --git a/Flakkari/Server/Client/Client.hpp b/Flakkari/Server/Client/Client.hpp
@@ -95,8 +95,6 @@ class Client {
     [[nodiscard]] Engine::ECS::Entity getEntity() const { return _entity; }
     void setEntity(Engine::ECS::Entity entity) { _entity = entity; }
 
-    [[nodiscard]] short getId() const { return _address->getId(); }
-
     [[nodiscard]] std::string getSceneName() const { return _sceneName; }
     void setSceneName(std::string sceneName) { _sceneName = sceneName; }
 

diff --git a/Flakkari/Server/Game/Game.cpp b/Flakkari/Server/Game/Game.cpp
@@ -46,7 +46,10 @@ Game::Game(const std::string &name, std::shared_ptr<nlohmann::json> config)
     _time = std::chrono::steady_clock::now();
 
     if ((*_config)["scenes"].empty())
-        throw std::runtime_error("Game: no scenes found");
+    {
+        FLAKKARI_LOG_ERROR("Game: no scenes found");
+        return;
+    }
 
     loadScene((*_config)["startGame"]);
     ResourceManager::GetInstance().addScene(config, (*_config)["startGame"]);

diff --git a/Flakkari/Server/Game/GameManager.cpp b/Flakkari/Server/Game/GameManager.cpp
@@ -19,7 +19,7 @@ namespace Flakkari {
 GameManager::GameManager(const std::string &gameDir) : _game_dir(gameDir)
 {
     if (_game_dir.empty())
-        FLAKKARI_LOG_FATAL("No game directory set: please set \"FLAKKARI_GAME_DIR\" environment variable");
+        throw std::runtime_error("No game directory set: please download games in the Games folder");
 
     for (const auto &entry : std::filesystem::directory_iterator(_game_dir))
     {