From 358b48e4070b42821ed074cb079119f85c74544c Mon Sep 17 00:00:00 2001 From: whaffman Date: Mon, 22 Sep 2025 12:00:53 +0200 Subject: [PATCH] Refactor logging: replace LOG macros with Log class methods for improved clarity and consistency --- webserv/client/Client.cpp | 10 ++--- webserv/config/ConfigManager.cpp | 8 ++-- webserv/config/LocationConfig.cpp | 4 +- webserv/config/ServerConfig.cpp | 24 +++++------ webserv/log/Log.cpp | 71 ++++++++++++------------------- webserv/log/Log.hpp | 46 ++++++++------------ webserv/main.cpp | 2 +- webserv/server/Server.cpp | 40 ++++++++--------- webserv/socket/Socket.cpp | 12 +++--- 9 files changed, 95 insertions(+), 122 deletions(-) diff --git a/webserv/client/Client.cpp b/webserv/client/Client.cpp index 5b7e9ca..1a6cc2d 100644 --- a/webserv/client/Client.cpp +++ b/webserv/client/Client.cpp @@ -13,7 +13,7 @@ Client::Client(std::unique_ptr socket, Server &server, const ServerConfi Client::~Client() { - LOG_DEBUG("Client destructor called for fd: " + std::to_string(client_socket_->getFd())); + Log::debug("Client destructor called for fd: " + std::to_string(client_socket_->getFd())); server_.removeFromEpoll(*client_socket_); }; @@ -21,7 +21,7 @@ int Client::parseHeaderforContentLength(const std::string &request) // NOLINT { std::string header = "Content-Length: "; size_t pos = request.find(header); - LOG_DEBUG("Parsing header for Content-Length...\n" + header); + Log::debug("Parsing header for Content-Length...\n" + header); if (pos != std::string::npos) { size_t start = pos + header.length(); @@ -62,7 +62,7 @@ void Client::request() contentLength_ = parseHeaderforContentLength(header_); if (contentLength_ == -1) { - LOG_INFO("Received complete request:\n" + requestBuffer_ + "\n=== HEADER FINISHED\n"); + Log::info("Received complete request:\n" + requestBuffer_ + "\n=== HEADER FINISHED\n"); server_.responseReady(client_socket_->getFd()); } requestBuffer_.erase(0, headerEnd + 4); @@ -74,7 +74,7 @@ void Client::request() content_ += requestBuffer_; if (content_.size() >= contentLength_) { - LOG_INFO("Received complete request:\n" + header_ + content_ + "\n=== FULL REQUEST FINISHED\n"); + Log::info("Received complete request:\n" + header_ + content_ + "\n=== FULL REQUEST FINISHED\n"); server_.responseReady(client_socket_->getFd()); requestBuffer_.clear(); contentLength_ = -1; @@ -86,6 +86,6 @@ std::string Client::getResponse() const { std::string response = "HTTP/1.1 200 OK\r\nContent-Length: 32\r\n\r\nHello, World!"; response += " Server port " + std::to_string(server_config_.getPort()) + "\r\n"; - LOG_DEBUG("Sending response:\n" + response); + Log::debug("Sending response:\n" + response); return response; } \ No newline at end of file diff --git a/webserv/config/ConfigManager.cpp b/webserv/config/ConfigManager.cpp index 5bf5a92..984a40d 100644 --- a/webserv/config/ConfigManager.cpp +++ b/webserv/config/ConfigManager.cpp @@ -22,10 +22,10 @@ void ConfigManager::init(const std::string &filePath) { if (initialized_) { - LOG_WARN("ConfigManager is already initialized."); + Log::warning("ConfigManager is already initialized."); throw std::runtime_error("ConfigManager is already initialized."); } - LOG_INFO("Initializing ConfigManager with file: " + filePath); + Log::info("Initializing ConfigManager with file: " + filePath); parseConfigFile(filePath); initialized_ = true; } @@ -67,7 +67,7 @@ void removeComments(std::string &str) void ConfigManager::parseConfigFile(const std::string &filePath) { // Placeholder for actual file parsing logic - LOG_INFO("Parsing configuration file: " + filePath); + Log::info("Parsing configuration file: " + filePath); // Implement the parsing logic here std::ifstream file(filePath); @@ -108,7 +108,7 @@ void ConfigManager::parseConfigFile(const std::string &filePath) } // parseGlobalDeclarations(globalDeclarations); // Implement this function to handle global config - LOG_INFO("Global Declarations..."); + Log::info("Global Declarations..."); file.close(); } diff --git a/webserv/config/LocationConfig.cpp b/webserv/config/LocationConfig.cpp index b6529b2..1914fe0 100644 --- a/webserv/config/LocationConfig.cpp +++ b/webserv/config/LocationConfig.cpp @@ -30,12 +30,12 @@ void LocationConfig::parseDirectives(const std::string &declarations) if (directive == "autoindex") { autoIndex_ = (value == "on"); - LOG_INFO("Set autoindex to " + std::string(autoIndex_ ? "on" : "off")); + Log::info("Set autoindex to " + std::string(autoIndex_ ? "on" : "off")); } else if (directive == "index") { indexFile_ = value; - LOG_INFO("Set index file to " + indexFile_); + Log::info("Set index file to " + indexFile_); } else { diff --git a/webserv/config/ServerConfig.cpp b/webserv/config/ServerConfig.cpp index 8fc3979..3d6ea6c 100644 --- a/webserv/config/ServerConfig.cpp +++ b/webserv/config/ServerConfig.cpp @@ -15,7 +15,7 @@ ServerConfig::ServerConfig(std::string const &serverBlock) : port_(80) void ServerConfig::parseServerBlock(const std::string &block) { // Placeholder for actual server block parsing logic - LOG_INFO("Parsing server block..."); + Log::info("Parsing server block..."); // Placeholder for actual file parsing logic @@ -43,7 +43,7 @@ void ServerConfig::parseServerBlock(const std::string &block) } // Optionally parse the server block here std::string locationBlock = block.substr(bracePos + 1, closeBrace - bracePos - 1); - LOG_TRACE("Added location: " + locationPath); + Log::trace("Added location: " + locationPath); locations_.emplace(locationPath, locationBlock); pos = closeBrace + 1; } @@ -54,7 +54,7 @@ void ServerConfig::parseServerBlock(const std::string &block) void ServerConfig::parseDirectives(const std::string &declarations) { - LOG_INFO("Parsing server directives"); + Log::info("Parsing server directives"); std::string line; std::istringstream stream(declarations); while (std::getline(stream, line)) @@ -74,27 +74,27 @@ void ServerConfig::parseDirectives(const std::string &declarations) { throw std::runtime_error("Invalid port number: " + std::to_string(port_)); } - LOG_TRACE("Set port to " + std::to_string(port_)); + Log::trace("Set port to " + std::to_string(port_)); } else if (directive == "root") { root_ = value; - LOG_TRACE("Set root to " + root_); + Log::trace("Set root to " + root_); } else if (directive == "host") { host_ = value; - LOG_TRACE("Set host to " + host_); + Log::trace("Set host to " + host_); } else if (directive == "cgi_pass") { cgi_pass_ = value; - LOG_TRACE("Set cgi_pass to " + cgi_pass_); + Log::trace("Set cgi_pass to " + cgi_pass_); } else if (directive == "cgi_ext") { cgi_ext_ = value; - LOG_TRACE("Set cgi_ext to " + cgi_ext_); + Log::trace("Set cgi_ext to " + cgi_ext_); } else if (directive == "index") { @@ -103,7 +103,7 @@ void ServerConfig::parseDirectives(const std::string &declarations) while (lineStream >> indexFile) { index_files_.push_back(indexFile); - LOG_TRACE("Added index file: " + indexFile); + Log::trace("Added index file: " + indexFile); } } else if (directive == "error_page") @@ -111,11 +111,11 @@ void ServerConfig::parseDirectives(const std::string &declarations) int statusCode = std::stoi(value); std::string errorPagePath; lineStream >> errorPagePath; - LOG_TRACE("Set error_page for status " + std::to_string(statusCode) + " to " + errorPagePath); + Log::trace("Set error_page for status " + std::to_string(statusCode) + " to " + errorPagePath); } else { - LOG_WARN("Unknown directive: " + directive); + Log::warning("Unknown directive: " + directive); } } } @@ -127,7 +127,7 @@ const LocationConfig &ServerConfig::getLocation(const std::string &path) const { return locations_.at(path); } - LOG_ERROR("Location not found: " + path); + Log::error("Location not found: " + path); throw std::runtime_error("Location not found: " + path); } diff --git a/webserv/log/Log.cpp b/webserv/log/Log.cpp index fa3c1c1..72b927e 100644 --- a/webserv/log/Log.cpp +++ b/webserv/log/Log.cpp @@ -5,12 +5,12 @@ #include #include #include +#include Log::Log() { // get start time start_time_ = std::chrono::steady_clock::now(); - } void Log::setStdoutChannel(LogLevel logLevel) @@ -53,34 +53,18 @@ Log &Log::getInstance() return instance; } -void Log::log(LogLevel level, const std::string &message, const std::string &channel, - const std::map &context) +void Log::log(LogLevel level, const std::string &message, const std::map &context, + const std::source_location &location) { for (auto &it : channels_) - { - it.second->log(level, message, context); - } -} -void Log::log(LogLevel level, const std::string &message, const std::string &file, int line, - const std::string &function, const std::string &channel, - const std::map &context) -{ - for (auto &it : channels_) { std::string extendedMessage; - extendedMessage += message + " | "; - if (!file.empty()) - { - extendedMessage += std::filesystem::path(file).filename().string(); - } - if (line != -1) - { - extendedMessage += ":" + std::to_string(line); - } - if (!function.empty()) - { - extendedMessage += " (" + function + ")"; - } + extendedMessage += message + "\n\t| "; + + extendedMessage += std::filesystem::path(location.file_name()).filename().string(); + extendedMessage += ":" + std::to_string(location.line()) + ":" + std::to_string(location.column()); + extendedMessage += " (" + std::string(location.function_name()) + ")"; + // extendedMessage += " | " + message; it.second->log(level, extendedMessage, context); } @@ -94,39 +78,38 @@ int Log::getElapsedTime() return static_cast(elapsed); } -void Log::static_log(LogLevel level, const std::string &message, const std::string &file, int line, - const std::string &function, const std::string &channel, - const std::map &context) +void Log::trace(const std::string &message, const std::map &context, + const std::source_location &location) { - getInstance().log(level, message, file, line, function, channel, context); + getInstance().log(LogLevel::LOGLVL_TRACE, message, context, location); } -void Log::trace(const std::string &message, const std::map &context) +void Log::debug(const std::string &message, const std::map &context, + const std::source_location &location) { - getInstance().log(LogLevel::LOGLVL_TRACE, message, "stdout", context); + getInstance().log(LogLevel::LOGLVL_DEBUG, message, context, location); } -void Log::debug(const std::string &message, const std::map &context) +void Log::info(const std::string &message, const std::map &context, + const std::source_location &location) { - getInstance().log(LogLevel::LOGLVL_DEBUG, message, "stdout", context); + getInstance().log(LogLevel::LOGLVL_INFO, message, context, location); } -void Log::info(const std::string &message, const std::map &context) +void Log::warning(const std::string &message, const std::map &context, + const std::source_location &location) { - getInstance().log(LogLevel::LOGLVL_INFO, message, "stdout", context); + getInstance().log(LogLevel::LOGLVL_WARN, message, context, location); } -void Log::warning(const std::string &message, const std::map &context) +void Log::error(const std::string &message, const std::map &context, + const std::source_location &location) { - getInstance().log(LogLevel::LOGLVL_WARN, message, "stdout", context); + getInstance().log(LogLevel::LOGLVL_ERROR, message, context, location); } -void Log::error(const std::string &message, const std::map &context) +void Log::fatal(const std::string &message, const std::map &context, + const std::source_location &location) { - getInstance().log(LogLevel::LOGLVL_ERROR, message, "stdout", context); -} - -void Log::fatal(const std::string &message, const std::map &context) -{ - getInstance().log(LogLevel::LOGLVL_FATAL, message, "stdout", context); + getInstance().log(LogLevel::LOGLVL_FATAL, message, context, location); } \ No newline at end of file diff --git a/webserv/log/Log.hpp b/webserv/log/Log.hpp index b77af90..361e5b5 100644 --- a/webserv/log/Log.hpp +++ b/webserv/log/Log.hpp @@ -6,10 +6,9 @@ #include #include #include +#include #include #include - - /* TODO ACTUALLY WE USE C++20 SO WE CAN USE std::source_location TO AUTOMATICALLY CAPTURE FILE, LINE, FUNCTION @@ -24,14 +23,6 @@ void log(LogLevel level, const std::string& message, No macros needed!!!! */ -#define LOG(level, message) Log::static_log((level), (message), __FILE__, __LINE__, __FUNCTION__, "file", {}) - -#define LOG_TRACE(message) LOG(LogLevel::LOGLVL_TRACE, message) -#define LOG_INFO(message) LOG(LogLevel::LOGLVL_INFO, message) -#define LOG_DEBUG(message) LOG(LogLevel::LOGLVL_DEBUG, message) -#define LOG_WARN(message) LOG(LogLevel::LOGLVL_WARN, message) -#define LOG_ERROR(message) LOG(LogLevel::LOGLVL_ERROR, message) -#define LOG_FATAL(message) LOG(LogLevel::LOGLVL_FATAL, message) class Channel; // Forward declaration @@ -43,28 +34,27 @@ class Log Log &operator=(const Log &other) = delete; Log &&operator=(const Log &&other) = delete; - static void setFileChannel(const std::string &filename, std::ios_base::openmode mode = std::ios_base::app, LogLevel logLevel = LogLevel::LOGLVL_TRACE); + void log(LogLevel level, const std::string &message, const std::map &context, + const std::source_location &location); + + static void setFileChannel(const std::string &filename, std::ios_base::openmode mode = std::ios_base::app, + LogLevel logLevel = LogLevel::LOGLVL_TRACE); static void setStdoutChannel(LogLevel logLevel = LogLevel::LOGLVL_TRACE); - void log(LogLevel level, const std::string &message, const std::string &channel = "stdout", - const std::map &context = {}); - - void log(LogLevel level, const std::string &message, const std::string &file = "", int line = -1, - const std::string &function = "", const std::string &channel = "stdout", - const std::map &context = {}); - static int getElapsedTime(); - static void static_log(LogLevel level, const std::string &message, const std::string &file = "", int line = -1, - const std::string &function = "", const std::string &channel = "stdout", - const std::map &context = {}); - - static void trace(const std::string &message, const std::map &context = {}); - static void debug(const std::string &message, const std::map &context = {}); - static void info(const std::string &message, const std::map &context = {}); - static void warning(const std::string &message, const std::map &context = {}); - static void error(const std::string &message, const std::map &context = {}); - static void fatal(const std::string &message, const std::map &context = {}); + static void trace(const std::string &message, const std::map &context = {}, + const std::source_location &location = std::source_location::current()); + static void debug(const std::string &message, const std::map &context = {}, + const std::source_location &location = std::source_location::current()); + static void info(const std::string &message, const std::map &context = {}, + const std::source_location &location = std::source_location::current()); + static void warning(const std::string &message, const std::map &context = {}, + const std::source_location &location = std::source_location::current()); + static void error(const std::string &message, const std::map &context = {}, + const std::source_location &location = std::source_location::current()); + static void fatal(const std::string &message, const std::map &context = {}, + const std::source_location &location = std::source_location::current()); private: Log(); diff --git a/webserv/main.cpp b/webserv/main.cpp index 5b3d4a6..1fcbae6 100644 --- a/webserv/main.cpp +++ b/webserv/main.cpp @@ -18,7 +18,7 @@ int main(int argc, char **argv) } Log::setFileChannel("webserv.log", std::ios_base::app, LogLevel::LOGLVL_TRACE); Log::setStdoutChannel(LogLevel::LOGLVL_INFO); - LOG_INFO("\n======================\nStarting webserv...\n======================\n"); + Log::info("\n======================\nStarting webserv...\n======================\n"); ConfigManager::getInstance().init(argv[1]); // NOLINT Server server(ConfigManager::getInstance()); server.start(); diff --git a/webserv/server/Server.cpp b/webserv/server/Server.cpp index e941e90..edf3414 100644 --- a/webserv/server/Server.cpp +++ b/webserv/server/Server.cpp @@ -19,12 +19,12 @@ Server::Server(const ConfigManager &configManager) : epoll_fd_(epoll_create1(0)) const auto &serverConfigs = configManager.getServerConfigs(); if (serverConfigs.empty()) { - LOG_FATAL("No server configurations available."); + Log::fatal("No server configurations available."); throw std::runtime_error("No server configurations available."); } if (epoll_fd_ == -1) { - LOG_FATAL("epoll_create1 failed"); + Log::fatal("epoll_create1 failed"); throw std::runtime_error("epoll_create1 failed"); } } @@ -39,7 +39,7 @@ Server::~Server() void Server::start() { - LOG_INFO("Starting servers..."); + Log::info("Starting servers..."); // 1. Load server configurations for (const auto &config : configManager_.getServerConfigs()) @@ -48,7 +48,7 @@ void Server::start() } if (fdToConfig_.empty()) { - LOG_FATAL("No server sockets created."); + Log::fatal("No server sockets created."); throw std::runtime_error("No server sockets created."); } @@ -63,7 +63,7 @@ void Server::addToEpoll(const Socket &socket, uint32_t events) const event.data.fd = fd; if (epoll_ctl(epoll_fd_, EPOLL_CTL_ADD, fd, &event) == -1) { - LOG_ERROR("epoll_ctl ADD failed for fd: " + std::to_string(fd)); + Log::error("epoll_ctl ADD failed for fd: " + std::to_string(fd)); throw std::runtime_error("epoll_ctl ADD failed"); } } @@ -73,7 +73,7 @@ void Server::removeFromEpoll(const Socket &socket) const int filedes = socket.getFd(); if (epoll_ctl(epoll_fd_, EPOLL_CTL_DEL, filedes, nullptr) == -1) { - LOG_ERROR("epoll_ctl DEL failed for fd: " + std::to_string(filedes)); + Log::error("epoll_ctl DEL failed for fd: " + std::to_string(filedes)); throw std::runtime_error("epoll_ctl DEL failed"); } } @@ -91,11 +91,11 @@ void Server::setupServerSocket(const ServerConfig &config) listeners_.push_back(std::move(serverSocket)); fdToConfig_.insert({server_fd, std::cref(config)}); - LOG_INFO("Server listening on " + config.getHost() + ":" + std::to_string(config.getPort()) + "..."); + Log::info("Server listening on " + config.getHost() + ":" + std::to_string(config.getPort()) + "..."); } catch (const std::exception &e) { - LOG_ERROR("Error setting up server socket: " + std::string(e.what())); + Log::error("Error setting up server socket: " + std::string(e.what())); } } @@ -117,7 +117,7 @@ Socket &Server::getListener(int fd) const return *listener; } } - LOG_ERROR("Listener not found for fd: " + std::to_string(fd)); + Log::error("Listener not found for fd: " + std::to_string(fd)); throw std::runtime_error("Listener not found for fd: " + std::to_string(fd)); } @@ -128,7 +128,7 @@ Client &Server::getClient(int fd) const { return *(it->second); } - LOG_ERROR("Client not found for fd: " + std::to_string(fd)); + Log::error("Client not found for fd: " + std::to_string(fd)); throw std::runtime_error("Client not found for fd: " + std::to_string(fd)); } @@ -144,7 +144,7 @@ const ServerConfig &Server::getConfig(int fd) const { return (it->second.get()); } - LOG_ERROR("Config not found for fd: " + std::to_string(fd)); + Log::error("Config not found for fd: " + std::to_string(fd)); throw std::runtime_error("Config not found for fd: " + std::to_string(fd)); } @@ -158,20 +158,20 @@ void Server::handleRequest(struct epoll_event *event) const void Server::responseReady(int client_fd) const { - LOG_INFO("Response ready for client fd: " + std::to_string(client_fd)); + Log::info("Response ready for client fd: " + std::to_string(client_fd)); struct epoll_event ev{}; ev.events = EPOLLOUT; ev.data.fd = client_fd; if (epoll_ctl(epoll_fd_, EPOLL_CTL_MOD, client_fd, &ev) == -1) { - LOG_ERROR("epoll_ctl MOD failed for fd: " + std::to_string(client_fd)); + Log::error("epoll_ctl MOD failed for fd: " + std::to_string(client_fd)); throw std::runtime_error("epoll_ctl MOD failed"); } } void Server::eventLoop() { - LOG_INFO("Entering event loop..."); + Log::info("Entering event loop..."); const int MAX_EVENTS = 10; struct epoll_event events[MAX_EVENTS]; // NOLINT while (true) @@ -179,7 +179,7 @@ void Server::eventLoop() int nfds = epoll_wait(epoll_fd_, events, MAX_EVENTS, -1); // NOLINT if (nfds == -1) { - LOG_ERROR("epoll_wait failed"); + Log::error("epoll_wait failed"); throw std::runtime_error("epoll_wait failed"); } for (int i = 0; i < nfds; ++i) @@ -187,7 +187,7 @@ void Server::eventLoop() epoll_event &event = events[i]; // NOLINT(cppcoreguidelines-pro-bounds-constant-array-index) if ((event.events & EPOLLERR) > 0 || (event.events & EPOLLHUP) > 0) { - LOG_ERROR("Epoll error on fd " + std::to_string(event.data.fd)); + Log::error("Epoll error on fd " + std::to_string(event.data.fd)); removeFromEpoll(getListener(event.data.fd)); close(event.data.fd); } @@ -201,19 +201,19 @@ void Server::eventLoop() } else if ((event.events & EPOLLOUT) > 0) { - LOG_INFO("Attempting to send data to fd: " + std::to_string(event.data.fd)); + Log::info("Attempting to send data to fd: " + std::to_string(event.data.fd)); Client &client = getClient(event.data.fd); std::string response = client.getResponse(); const char *httpResponse = response.c_str(); ssize_t bytesSent = send(event.data.fd, httpResponse, strlen(httpResponse), 0); if (bytesSent < 0) { - LOG_ERROR("Send failed for fd: " + std::to_string(event.data.fd) + - " with error: " + std::strerror(errno)); + Log::error("Send failed for fd: " + std::to_string(event.data.fd) + + " with error: " + std::strerror(errno)); } else { - LOG_INFO("Sent " + std::to_string(bytesSent) + " bytes to fd: " + std::to_string(event.data.fd)); + Log::info("Sent " + std::to_string(bytesSent) + " bytes to fd: " + std::to_string(event.data.fd)); } clients_.erase(event.data.fd); } diff --git a/webserv/socket/Socket.cpp b/webserv/socket/Socket.cpp index 53afa72..fbfec79 100644 --- a/webserv/socket/Socket.cpp +++ b/webserv/socket/Socket.cpp @@ -14,7 +14,7 @@ Socket::Socket() : fd_(socket(AF_INET, SOCK_STREAM, 0)) { if (fd_ == -1) { - LOG_ERROR("Socket creation failed"); + Log::error("Socket creation failed"); throw std::runtime_error("Socket creation failed"); } int opt = 1; @@ -22,7 +22,7 @@ Socket::Socket() : fd_(socket(AF_INET, SOCK_STREAM, 0)) { close(fd_); fd_ = -1; - LOG_ERROR("setsockopt failed"); + Log::error("setsockopt failed"); throw std::runtime_error("setsockopt failed"); } setNonBlocking(); @@ -32,7 +32,7 @@ Socket::Socket(int fd) : fd_(fd) // NOLINT(readability-identifier-naming) { if (fd_ == -1) { - LOG_ERROR("Invalid file descriptor"); + Log::error("Invalid file descriptor"); throw std::runtime_error("Invalid file descriptor"); } setNonBlocking(); @@ -50,7 +50,7 @@ void Socket::listen(int backlog) const { if (::listen(fd_, backlog) < 0) { - LOG_ERROR("Listen failed"); + Log::error("Listen failed"); throw std::runtime_error("Listen failed"); } } @@ -73,7 +73,7 @@ std::unique_ptr Socket::accept() const int client_fd = ::accept(fd_, nullptr, nullptr); if (client_fd < 0) { - LOG_ERROR("Accept failed"); + Log::error("Accept failed"); throw std::runtime_error("Accept failed"); } return std::make_unique(client_fd); @@ -93,7 +93,7 @@ void Socket::setNonBlocking() const { if (fcntl(fd_, F_SETFL, O_NONBLOCK) < 0) { - LOG_ERROR("Failed to set non-blocking mode"); + Log::error("Failed to set non-blocking mode"); throw std::runtime_error("Failed to set non-blocking mode"); } }