feat: Optional log rotation (#3016)

This PR adds an option to disable log rotation.

benchmarks/util/log/LoggerBenchmark.cpp

@@ -11,6 +11,7 @@
 #include <barrier>
 #include <chrono>
 #include <cstddef>
+#include <cstdint>
 #include <filesystem>
 #include <memory>
 #include <string>
@@ -24,9 +25,15 @@ static constexpr auto kLOG_FORMAT = "%Y-%m-%d %H:%M:%S.%f %^%3!l:%n%$ - %v";
 
 struct BenchmarkLoggingInitializer {
     [[nodiscard]] static std::shared_ptr<spdlog::sinks::sink>
-    createFileSink(LogService::FileLoggingParams const& params)
+    createFileSink(std::string const& logDir, uint32_t sizeMB, uint32_t maxFiles)
     {
-        return LogService::createFileSink(params, kLOG_FORMAT);
+        return LogService::createFileSink(
+            LogService::FileLoggingParams{
+                .logDir = logDir,
+                .rotation = LogService::RotationParams{.sizeMB = sizeMB, .maxFiles = maxFiles},
+            },
+            kLOG_FORMAT
+        );
     }
 
     static Logger
@@ -68,11 +75,7 @@ benchmarkConcurrentFileLogging(benchmark::State& state)
         static constexpr size_t kTHREAD_COUNT = 1;
         spdlog::init_thread_pool(kQUEUE_SIZE, kTHREAD_COUNT);
 
-        auto fileSink = BenchmarkLoggingInitializer::createFileSink({
-            .logDir = logDir,
-            .rotationSizeMB = 5,
-            .dirMaxFiles = 25,
-        });
+        auto fileSink = BenchmarkLoggingInitializer::createFileSink(logDir, 5, 25);
 
         std::vector<std::thread> threads;
         threads.reserve(numThreads);

docs/config-description.md

@@ -230,7 +230,7 @@ This document provides a list of all available Clio configuration properties in
 - **Required**: False
 - **Type**: string
 - **Default value**: None
-- **Constraints**: None
+- **Constraints**: The value must be a valid IP address.
 - **Description**: The list of IP addresses to whitelist for DOS protection.
 
 ### dos_guard.max_fetches
@@ -342,7 +342,7 @@ This document provides a list of all available Clio configuration properties in
 - **Required**: True
 - **Type**: string
 - **Default value**: None
-- **Constraints**: None
+- **Constraints**: The value must be a valid IP address.
 - **Description**: List of proxy ip addresses. When Clio receives a request from proxy it will use `Forwarded` value (if any) as client ip. When this option is used together with `server.proxy.tokens` Clio will identify proxy by ip or by token.
 
 ### server.proxy.tokens.[]
@@ -561,6 +561,14 @@ Documentation can be found at: <https://github.com/gabime/spdlog/wiki/Custom-for
 - **Constraints**: The minimum value is `1`. The maximum value is `4294967295`.
 - **Description**: The maximum number of log files in the directory.
 
+### log.rotate
+
+- **Required**: True
+- **Type**: boolean
+- **Default value**: `True`
+- **Constraints**: None
+- **Description**: Enables or disables log file rotation. When disabled, a single log file is used without size-based rotation. Useful when rotation is managed externally (e.g., via logrotate).
+
 ### log.tag_style
 
 - **Required**: True

src/app/ClioApplication.cpp

@@ -92,9 +92,14 @@ ClioApplication::run(bool const useNgWebServer)
     boost::asio::io_context ioc{threads};
 
     // Rate limiter, to prevent abuse
-    auto whitelistHandler = web::dosguard::WhitelistHandler{config_};
+    auto whitelistHandler = web::dosguard::WhitelistHandler::create(config_);
+    if (not whitelistHandler.has_value()) {
+        LOG(util::LogService::fatal()) << whitelistHandler.error();
+        return EXIT_FAILURE;
+    }
+
     auto const dosguardWeights = web::dosguard::Weights::make(config_);
-    auto dosGuard = web::dosguard::DOSGuard{config_, whitelistHandler, dosguardWeights};
+    auto dosGuard = web::dosguard::DOSGuard{config_, *whitelistHandler, dosguardWeights};
     auto sweepHandler = web::dosguard::IntervalSweepHandler{config_, ioc, dosGuard};
 
     auto cache = data::LedgerCache{};
@@ -222,10 +227,15 @@ ClioApplication::run(bool const useNgWebServer)
         config_, backend, rpcEngine, etl, dosGuard
     );
 
-    auto const httpServer = web::makeHttpServer(config_, ioc, dosGuard, handler, cache);
+    auto const expectedHttpServer = web::makeHttpServer(config_, ioc, dosGuard, handler, cache);
+    if (not expectedHttpServer.has_value()) {
+        LOG(util::LogService::fatal()) << expectedHttpServer.error();
+        return EXIT_FAILURE;
+    }
+
     appStopper_.setOnStop(
         Stopper::makeOnStopCallback(
-            *httpServer,
+            **expectedHttpServer,
             *balancer,
             *etl,
             *subscriptions,

src/util/config/ConfigDefinition.cpp

@@ -312,7 +312,8 @@ getClioConfig()
          {"num_markers",
           ConfigValue{ConfigType::Integer}.optional().withConstraint(gValidateNumMarkers)},
 
-         {"dos_guard.whitelist.[]", Array{ConfigValue{ConfigType::String}.optional()}},
+         {"dos_guard.whitelist.[]",
+          Array{ConfigValue{ConfigType::String}.optional().withConstraint(gValidateIp)}},
          {"dos_guard.max_fetches",
           ConfigValue{ConfigType::Integer}.defaultValue(1000'000u).withConstraint(gValidateUint32)},
          {"dos_guard.max_connections",
@@ -361,7 +362,8 @@ getClioConfig()
          {"server.ws_max_sending_queue_size",
           ConfigValue{ConfigType::Integer}.defaultValue(1500).withConstraint(gValidateUint32)},
          {"server.__ng_web_server", ConfigValue{ConfigType::Boolean}.defaultValue(false)},
-         {"server.proxy.ips.[]", Array{ConfigValue{ConfigType::String}}},
+         {"server.proxy.ips.[]",
+          Array{ConfigValue{ConfigType::String}.withConstraint(gValidateIp)}},
          {"server.proxy.tokens.[]", Array{ConfigValue{ConfigType::String}}},
 
          {"prometheus.enabled", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
@@ -420,6 +422,8 @@ getClioConfig()
          {"log.directory_max_files",
           ConfigValue{ConfigType::Integer}.defaultValue(25).withConstraint(gValidateUint32)},
 
+         {"log.rotate", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
+
          {"log.tag_style",
           ConfigValue{ConfigType::String}.defaultValue("none").withConstraint(gValidateLogTag)},
 

src/util/config/ConfigDescription.hpp

@@ -357,6 +357,10 @@ Documentation can be found at: <https://github.com/gabime/spdlog/wiki/Custom-for
                     "file starts."},
         KV{.key = "log.directory_max_files",
            .value = "The maximum number of log files in the directory."},
+        KV{.key = "log.rotate",
+           .value = "Enables or disables log file rotation. When disabled, a single log file is "
+                    "used without size-based rotation. Useful when rotation is managed externally "
+                    "(e.g., via logrotate)."},
         KV{.key = "log.tag_style",
            .value = "Log tags are unique identifiers for log messages. `uint`/`int` starts logging "
                     "from 0 and increments, "

src/util/log/Logger.cpp

@@ -17,6 +17,7 @@
 #include <spdlog/formatter.h>
 #include <spdlog/logger.h>
 #include <spdlog/pattern_formatter.h>
+#include <spdlog/sinks/basic_file_sink.h>
 #include <spdlog/sinks/rotating_file_sink.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
 #include <spdlog/spdlog.h>
@@ -184,12 +185,18 @@ spdlog::sink_ptr
 LogService::createFileSink(FileLoggingParams const& params, std::string const& format)
 {
     std::filesystem::path const dirPath(params.logDir);
-    // the below are taken from user in MB, but spdlog needs it to be in bytes
-    auto const rotationSize = mbToBytes(params.rotationSizeMB);
+    auto fileSink = [&]() -> std::shared_ptr<spdlog::sinks::sink> {
+        auto const logPath = (dirPath / "clio.log").string();
+        if (params.rotation.has_value()) {
+            // rotation sizes are taken from user in MB, but spdlog needs bytes
+            auto const rotationSize = mbToBytes(params.rotation->sizeMB);
+            return std::make_shared<spdlog::sinks::rotating_file_sink_mt>(
+                logPath, rotationSize, params.rotation->maxFiles
+            );
+        }
+        return std::make_shared<spdlog::sinks::basic_file_sink_mt>(logPath, /*truncate=*/false);
+    }();
 
-    auto fileSink = std::make_shared<spdlog::sinks::rotating_file_sink_mt>(
-        (dirPath / "clio.log").string(), rotationSize, params.dirMaxFiles
-    );
     fileSink->set_level(spdlog::level::trace);
     fileSink->set_formatter(std::make_unique<spdlog::pattern_formatter>(format));
 
@@ -336,10 +343,17 @@ LogService::getSinks(config::ClioConfigDefinition const& config)
             }
         }
 
+        std::optional<RotationParams> rotation = std::nullopt;
+        if (config.get<bool>("log.rotate")) {
+            rotation = RotationParams{
+                .sizeMB = config.get<uint32_t>("log.rotation_size"),
+                .maxFiles = config.get<uint32_t>("log.directory_max_files"),
+            };
+        }
+
         FileLoggingParams const params{
             .logDir = logDir.value(),
-            .rotationSizeMB = config.get<uint32_t>("log.rotation_size"),
-            .dirMaxFiles = config.get<uint32_t>("log.directory_max_files"),
+            .rotation = rotation,
         };
         allSinks.push_back(createFileSink(params, format));
     }

src/util/log/Logger.hpp

@@ -28,6 +28,7 @@ class sink;  // NOLINT(readability-identifier-naming)
 struct BenchmarkLoggingInitializer;
 class LoggerFixture;
 struct LogServiceInitTests;
+struct LogFileRotationTests;
 
 namespace util {
 
@@ -229,6 +230,7 @@ class LogServiceState {
 protected:
     friend struct ::LogServiceInitTests;
     friend class ::LoggerFixture;
+    friend struct ::LogFileRotationTests;
     friend class Logger;
     friend class ::util::impl::OnAssert;
 
@@ -388,11 +390,14 @@ private:
         expected<std::vector<std::shared_ptr<spdlog::sinks::sink>>, std::string>
         getSinks(config::ClioConfigDefinition const& config);
 
+    struct RotationParams {
+        uint32_t sizeMB;
+        uint32_t maxFiles;
+    };
+
     struct FileLoggingParams {
         std::string logDir;
-
-        uint32_t rotationSizeMB;
-        uint32_t dirMaxFiles;
+        std::optional<RotationParams> rotation;  ///< nullopt when rotation is disabled
     };
 
     friend struct ::BenchmarkLoggingInitializer;

src/web/Server.hpp

@@ -375,7 +375,7 @@ using HttpServer = Server<HttpSession, SslHttpSession, HandlerType>;
  * @return The server instance
  */
 template <typename HandlerType>
-static std::shared_ptr<HttpServer<HandlerType>>
+static std::expected<std::shared_ptr<HttpServer<HandlerType>>, std::string>
 makeHttpServer(
     util::config::ClioConfigDefinition const& config,
     boost::asio::io_context& ioc,
@@ -388,19 +388,24 @@ makeHttpServer(
 
     auto expectedSslContext = ng::impl::makeServerSslContext(config);
     if (not expectedSslContext) {
-        LOG(log.error()) << "Failed to create SSL context: " << expectedSslContext.error();
-        return nullptr;
+        return std::unexpected(
+            fmt::format("Failed to create SSL context: {}", expectedSslContext.error())
+        );
     }
 
     auto const serverConfig = config.getObject("server");
-    auto const address = boost::asio::ip::make_address(serverConfig.get<std::string>("ip"));
+
+    auto const ipFromConfig = serverConfig.get<std::string>("ip");
+    boost::system::error_code ec;
+    auto const address = boost::asio::ip::make_address(ipFromConfig, ec);
+    if (ec.failed())
+        return std::unexpected(fmt::format("Invalid 'server.ip' config value: {}", ipFromConfig));
+
     auto const port = serverConfig.get<unsigned short>("port");
 
     auto expectedAdminVerification = makeAdminVerificationStrategy(config);
-    if (not expectedAdminVerification.has_value()) {
-        LOG(log.error()) << expectedAdminVerification.error();
-        throw std::logic_error{expectedAdminVerification.error()};
-    }
+    if (not expectedAdminVerification.has_value())
+        return std::unexpected(expectedAdminVerification.error());
 
     // If the transactions number is 200 per ledger, A client which subscribes everything will send
     // 400+ feeds for each ledger. we allow user delay 3 ledgers by default

src/web/dosguard/WhitelistHandler.cpp

@@ -8,32 +8,48 @@
 #include <algorithm>
 #include <functional>
 #include <regex>
-#include <stdexcept>
 #include <string>
 #include <string_view>
-#include <unordered_map>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 namespace web::dosguard {
 
-void
+WhitelistHandler::WhitelistHandler(Whitelist whitelist) : whitelist_(std::move(whitelist))
+{
+}
+
+std::expected<void, std::string>
 Whitelist::add(std::string_view net)
 {
     using namespace boost::asio;
 
     if (not isMask(net)) {
-        ips_.push_back(ip::make_address(net));
-        return;
+        boost::system::error_code ec;
+        auto const ip = ip::make_address(net, ec);
+        if (ec.failed())
+            return std::unexpected{fmt::format("Malformed whitelist ip address: {}. ", net)};
+        ips_.push_back(ip);
+        return {};
     }
 
     if (isV4(net)) {
-        subnetsV4_.push_back(ip::make_network_v4(net));
+        boost::system::error_code ec;
+        auto const net4 = ip::make_network_v4(net, ec);
+        if (ec.failed())
+            return std::unexpected{fmt::format("Malformed network: {}. ", net)};
+        subnetsV4_.push_back(net4);
     } else if (isV6(net)) {
-        subnetsV6_.push_back(ip::make_network_v6(net));
+        boost::system::error_code ec;
+        auto const net6 = ip::make_network_v6(net, ec);
+        if (ec.failed())
+            return std::unexpected{fmt::format("Malformed network: {}. ", net)};
+        subnetsV6_.push_back(net6);
     } else {
-        throw std::runtime_error(fmt::format("malformed network: {}", net.data()));
+        return std::unexpected{fmt::format("Malformed network: {}. ", net)};
     }
+    return {};
 }
 
 bool
@@ -41,7 +57,11 @@ Whitelist::isWhiteListed(std::string_view ip) const
 {
     using namespace boost::asio;
 
-    auto const addr = ip::make_address(ip);
+    boost::system::error_code ec;
+    auto const addr = ip::make_address(ip, ec);
+    if (ec.failed())
+        return false;
+
     if (std::ranges::find(ips_, addr) != std::end(ips_))
         return true;
 
@@ -95,7 +115,7 @@ Whitelist::isV6(std::string_view net)
 bool
 Whitelist::isMask(std::string_view net)
 {
-    return net.find('/') != std::string_view::npos;
+    return net.contains('/');
 }
 
 }  // namespace web::dosguard

src/web/dosguard/WhitelistHandler.hpp

@@ -14,11 +14,11 @@
 #include <fmt/format.h>
 
 #include <algorithm>
-#include <regex>
+#include <optional>
 #include <string>
 #include <string_view>
-#include <unordered_map>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 namespace web::dosguard {
@@ -36,16 +36,15 @@ public:
      * @brief Add network address to whitelist.
      *
      * @param net Network part of the ip address
-     * @throws std::runtime::error when the network address is not valid
+     * @return void on success, or an error string if the address is not valid
      */
-    void
+    std::expected<void, std::string>
     add(std::string_view net);
 
     /**
      * @brief Checks to see if ip address is whitelisted.
      *
      * @param ip IP address
-     * @throws std::runtime::error when the network address is not valid
      * @return true if the given IP is whitelisted; false otherwise
      */
     bool
@@ -76,21 +75,38 @@ class WhitelistHandler : public WhitelistHandlerInterface {
 
 public:
     /**
-     * @brief Adds all whitelisted IPs and masks from the given config.
+     * @brief Constructs a WhitelistHandler from an already-built Whitelist.
+     *
+     * @param whitelist The whitelist to use
+     */
+    explicit WhitelistHandler(Whitelist whitelist);
+
+    /**
+     * @brief Creates a WhitelistHandler by loading all whitelisted IPs and masks from config.
      *
      * @param config The Clio config to use
      * @param resolver The resolver to use for hostname resolution
+     * @return The WhitelistHandler on success, or an error string if any whitelist entry is invalid
      */
     template <SomeResolver HostnameResolverType = Resolver>
-    WhitelistHandler(
-        util::config::ClioConfigDefinition const& config,
-        HostnameResolverType&& resolver = {}
-    )
+    static std::expected<WhitelistHandler, std::string>
+    create(util::config::ClioConfigDefinition const& config, HostnameResolverType&& resolver = {})
     {
         std::unordered_set<std::string> const arr =
             getWhitelist(config, std::forward<HostnameResolverType>(resolver));
-        for (auto const& net : arr)
-            whitelist_.add(net);
+        Whitelist whitelist;
+        std::optional<std::string> errors;
+        for (auto const& net : arr) {
+            if (auto result = whitelist.add(net); !result.has_value()) {
+                if (!errors.has_value())
+                    errors.emplace();
+                errors->append(std::move(result).error());
+            }
+        }
+        if (errors.has_value()) {
+            return std::unexpected{std::move(errors).value()};
+        }
+        return WhitelistHandler(std::move(whitelist));
     }
 
     /**

tests/unit/rpc/RPCEngineTests.cpp

@@ -81,7 +81,9 @@ struct RPCEngineTest : util::prometheus::WithPrometheus,
 
     util::TagDecoratorFactory tagFactory{cfg};
     WorkQueue queue = WorkQueue::makeWorkQueue(cfg);
-    web::dosguard::WhitelistHandler whitelistHandler{cfg};
+    web::dosguard::WhitelistHandler whitelistHandler{
+        web::dosguard::WhitelistHandler::create(cfg).value()
+    };
     web::dosguard::Weights weights{1, {}};
     web::dosguard::DOSGuard dosGuard{cfg, whitelistHandler, weights};
     std::shared_ptr<MockHandlerProvider> handlerProvider = std::make_shared<MockHandlerProvider>();

tests/unit/util/log/LogServiceInitTests.cpp

@@ -66,6 +66,8 @@ protected:
         {"log.directory_max_files",
          ConfigValue{ConfigType::Integer}.defaultValue(25).withConstraint(config::gValidateUint32)},
 
+        {"log.rotate", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
+
         {"log.tag_style", ConfigValue{ConfigType::String}.defaultValue("none")},
     };
 
@@ -219,3 +221,21 @@ TEST_F(LogServiceInitTests, LogSizeAndHourRotationCannotBeZero)
         );
     }
 }
+
+TEST_F(LogServiceInitTests, RotateDefaultsToTrue)
+{
+    auto const parsingErrors = config_.parse(ConfigFileJson{boost::json::object{}});
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    EXPECT_TRUE(config_.get<bool>("log.rotate"));
+}
+
+TEST_F(LogServiceInitTests, RotationDisabledConfigParsesSuccessfully)
+{
+    auto const parsingErrors = config_.parse(
+        ConfigFileJson{boost::json::object{{"log", boost::json::object{{"rotate", false}}}}}
+    );
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    EXPECT_FALSE(config_.get<bool>("log.rotate"));
+}

tests/unit/util/log/LoggerTests.cpp

@@ -1,14 +1,30 @@
 #include "util/LoggerFixtures.hpp"
+#include "util/config/Array.hpp"
+#include "util/config/ConfigConstraints.hpp"
+#include "util/config/ConfigDefinition.hpp"
+#include "util/config/ConfigFileJson.hpp"
+#include "util/config/ConfigValue.hpp"
+#include "util/config/Types.hpp"
 #include "util/log/Logger.hpp"
 
+#include <boost/json/object.hpp>
+#include <boost/uuid/random_generator.hpp>
+#include <boost/uuid/uuid_io.hpp>
+#include <fmt/core.h>
+#include <fmt/format.h>
 #include <gtest/gtest.h>
 #include <spdlog/logger.h>
 #include <spdlog/spdlog.h>
 
 #include <cstddef>
+#include <filesystem>
 #include <memory>
 #include <string>
 using namespace util;
+using util::config::Array;
+using util::config::ConfigFileJson;
+using util::config::ConfigType;
+using util::config::ConfigValue;
 
 namespace {
 size_t
@@ -91,3 +107,138 @@ TEST_F(LoggerTest, ManyDynamicLoggers)
 
     ASSERT_EQ(loggersNum(), initialLoggers);
 }
+
+/**
+ * @brief Fixture for testing real log-file rotation behaviour.
+ *
+ * Unlike LoggerTest (which uses LoggerFixture's in-memory buffer), this fixture
+ * initialises the LogService with a real file sink and redirects all spdlog
+ * loggers to that sink so that written messages actually land on disk.
+ */
+struct LogFileRotationTests : ::testing::Test {
+    std::filesystem::path const tmpDir = std::filesystem::temp_directory_path() /
+        fmt::format("clio_log_rotation_tests_{}",
+                    boost::uuids::to_string(boost::uuids::random_generator{}()));
+
+    util::config::ClioConfigDefinition config{
+        {"log.channels.[].channel", Array{ConfigValue{ConfigType::String}}},
+        {"log.channels.[].level", Array{ConfigValue{ConfigType::String}}},
+
+        {"log.level", ConfigValue{ConfigType::String}.defaultValue("info")},
+
+        {"log.format",
+         ConfigValue{ConfigType::String}.defaultValue(R"(%Y-%m-%d %H:%M:%S.%f %^%3!l:%n%$ - %v)")},
+        {"log.is_async", ConfigValue{ConfigType::Boolean}.defaultValue(false)},
+
+        {"log.enable_console", ConfigValue{ConfigType::Boolean}.defaultValue(false)},
+
+        {"log.directory", ConfigValue{ConfigType::String}.optional()},
+
+        {"log.rotation_size",
+         ConfigValue{ConfigType::Integer}.defaultValue(2048).withConstraint(
+             util::config::gValidateUint32
+         )},
+
+        {"log.directory_max_files",
+         ConfigValue{ConfigType::Integer}.defaultValue(25).withConstraint(
+             util::config::gValidateUint32
+         )},
+
+        {"log.rotate", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
+
+        {"log.tag_style", ConfigValue{ConfigType::String}.defaultValue("none")},
+    };
+
+    LogFileRotationTests()
+    {
+        std::filesystem::remove_all(tmpDir);
+        if (LogServiceState::initialized())
+            LogServiceState::reset();
+    }
+
+    ~LogFileRotationTests() override
+    {
+        if (LogService::initialized())
+            LogService::reset();
+        // Leave state initialised so that subsequent tests can call reset().
+        LogServiceState::init(false, Severity::FTL, {});
+        std::filesystem::remove_all(tmpDir);
+    }
+
+    /**
+     * @brief Initialises LogService with the current config and redirects all
+     * existing spdlog loggers to the newly created file sink.
+     *
+     * LogService::init() skips updating sinks on loggers that already exist in
+     * the spdlog registry. Calling replaceSinks() here ensures every logger
+     * writes to the file sink regardless of prior test state.
+     */
+    void
+    initFileLogging() const
+    {
+        ASSERT_TRUE(LogService::init(config));
+        LogServiceState::replaceSinks(LogServiceState::sinks_);
+    }
+
+    /** @brief Returns the number of regular files in tmpDir_. */
+    [[nodiscard]] std::size_t
+    countLogFiles() const
+    {
+        std::size_t count = 0;
+        for (auto const& entry : std::filesystem::directory_iterator(tmpDir)) {
+            if (entry.is_regular_file())
+                ++count;
+        }
+        return count;
+    }
+};
+
+TEST_F(LogFileRotationTests, RotationDisabledProducesSingleLogFile)
+{
+    auto const parsingErrors = config.parse(
+        ConfigFileJson{boost::json::object{
+            {"log",
+             boost::json::object{
+                 {"directory", tmpDir.string()},
+                 {"rotate", false},
+             }}
+        }}
+    );
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    initFileLogging();
+
+    // Write enough data to trigger rotation if it were enabled (> 1 MB).
+    // Writing at error level flushes immediately because flush_on(err) is set.
+    Logger const log{"General"};
+    std::string const bigMessage(1000, 'x');
+    for (int i = 0; i < 1100; ++i)
+        log.error() << bigMessage;
+
+    EXPECT_EQ(countLogFiles(), 1u);
+}
+
+TEST_F(LogFileRotationTests, RotationEnabledProducesMultipleLogFiles)
+{
+    auto const parsingErrors = config.parse(
+        ConfigFileJson{boost::json::object{
+            {"log",
+             boost::json::object{
+                 {"directory", tmpDir.string()},
+                 {"rotate", true},
+                 {"rotation_size", 1},
+                 {"directory_max_files", 2},
+             }}
+        }}
+    );
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    initFileLogging();
+
+    Logger const log{"General"};
+    std::string const bigMessage(1000, 'x');
+    for (int i = 0; i < 1100; ++i)
+        log.error() << bigMessage;
+
+    EXPECT_GT(countLogFiles(), 1u);
+}

tests/unit/web/ServerTests.cpp

@@ -150,13 +150,15 @@ struct WebServerTest : public virtual ::testing::Test {
     boost::asio::io_context ctxSync;
     std::string const port = std::to_string(tests::util::generateFreePort());
     ClioConfigDefinition cfg{getParseServerConfig(generateJSONWithDynamicPort(port))};
-    dosguard::WhitelistHandler whitelistHandler{cfg};
+    dosguard::WhitelistHandler whitelistHandler{dosguard::WhitelistHandler::create(cfg).value()};
     dosguard::Weights dosguardWeights{1, {}};
     dosguard::DOSGuard dosGuard{cfg, whitelistHandler, dosguardWeights};
     dosguard::IntervalSweepHandler sweepHandler{cfg, ctxSync, dosGuard};
 
     ClioConfigDefinition cfgOverload{getParseServerConfig(generateJSONDataOverload(port))};
-    dosguard::WhitelistHandler whitelistHandlerOverload{cfgOverload};
+    dosguard::WhitelistHandler whitelistHandlerOverload{
+        dosguard::WhitelistHandler::create(cfgOverload).value()
+    };
     dosguard::DOSGuard dosGuardOverload{cfgOverload, whitelistHandlerOverload, dosguardWeights};
     dosguard::IntervalSweepHandler sweepHandlerOverload{cfgOverload, ctxSync, dosGuardOverload};
     // this ctx is for http server
@@ -216,11 +218,26 @@ makeServerSync(
     std::reference_wrapper<data::LedgerCacheInterface const> cache
 )
 {
-    return web::makeHttpServer(config, ioc, dosGuard, handler, cache);
+    auto result = web::makeHttpServer(config, ioc, dosGuard, handler, cache);
+    [&]() { ASSERT_TRUE(result.has_value()); }();
+    return std::move(result).value();
 }
 
 }  // namespace
 
+TEST_F(WebServerTest, InvalidIpAddress)
+{
+    auto jsonConfig = generateJSONWithDynamicPort(port);
+    jsonConfig.as_object()["server"].as_object()["ip"] = "not-an-ip";
+
+    auto cache = MockLedgerCache();
+    auto const e = std::make_shared<EchoExecutor>();
+    auto const result =
+        web::makeHttpServer(getParseServerConfig(jsonConfig), ctx, dosGuard, e, cache);
+    ASSERT_FALSE(result.has_value());
+    EXPECT_THAT(result.error(), testing::HasSubstr("Invalid 'server.ip' config value"));
+}
+
 TEST_F(WebServerTest, Http)
 {
     auto cache = MockLedgerCache();
@@ -294,8 +311,9 @@ TEST_F(WebServerTest, IncompleteSslConfig)
     jsonConfig.as_object()["ssl_key_file"] = sslKeyFile.path;
 
     auto cache = MockLedgerCache();
-    auto const server = makeServerSync(getParseServerConfig(jsonConfig), ctx, dosGuard, e, cache);
-    EXPECT_EQ(server, nullptr);
+    auto const result =
+        web::makeHttpServer(getParseServerConfig(jsonConfig), ctx, dosGuard, e, cache);
+    EXPECT_FALSE(result.has_value());
 }
 
 TEST_F(WebServerTest, WrongSslConfig)
@@ -307,8 +325,9 @@ TEST_F(WebServerTest, WrongSslConfig)
     jsonConfig.as_object()["ssl_cert_file"] = "wrong_path";
 
     auto cache = MockLedgerCache();
-    auto const server = makeServerSync(getParseServerConfig(jsonConfig), ctx, dosGuard, e, cache);
-    EXPECT_EQ(server, nullptr);
+    auto const result =
+        web::makeHttpServer(getParseServerConfig(jsonConfig), ctx, dosGuard, e, cache);
+    EXPECT_FALSE(result.has_value());
 }
 
 TEST_F(WebServerTest, Https)
@@ -694,9 +713,8 @@ TEST_F(WebServerTest, AdminErrorCfgTestBothAdminPasswordAndLocalAdminSet)
     )};
 
     MockLedgerCache cache;
-    EXPECT_THROW(
-        web::makeHttpServer(serverConfig, ctx, dosGuardOverload, e, cache), std::logic_error
-    );
+    auto const result = web::makeHttpServer(serverConfig, ctx, dosGuardOverload, e, cache);
+    EXPECT_FALSE(result.has_value());
 }
 
 TEST_F(WebServerTest, AdminErrorCfgTestBothAdminPasswordAndLocalAdminFalse)
@@ -719,9 +737,8 @@ TEST_F(WebServerTest, AdminErrorCfgTestBothAdminPasswordAndLocalAdminFalse)
     )};
 
     MockLedgerCache cache;
-    EXPECT_THROW(
-        web::makeHttpServer(serverConfig, ctx, dosGuardOverload, e, cache), std::logic_error
-    );
+    auto const result = web::makeHttpServer(serverConfig, ctx, dosGuardOverload, e, cache);
+    EXPECT_FALSE(result.has_value());
 }
 
 struct WebServerPrometheusTest : util::prometheus::WithPrometheus, WebServerTest {};

tests/unit/web/dosguard/WhitelistHandlerTests.cpp

@@ -11,6 +11,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <expected>
 #include <string>
 #include <string_view>
 #include <vector>
@@ -62,7 +63,9 @@ TEST_F(WhitelistHandlerTest, TestWhiteListIPV4)
     ClioConfigDefinition const cfg{
         getParseWhitelistHandlerConfig(boost::json::parse(kJSON_DATA_IP_V4))
     };
-    WhitelistHandler const whitelistHandler{cfg, mockResolver};
+    auto const result = WhitelistHandler::create(cfg, mockResolver);
+    ASSERT_TRUE(result.has_value());
+    auto const& whitelistHandler = *result;
 
     EXPECT_TRUE(whitelistHandler.isWhiteListed("192.168.1.10"));
     EXPECT_FALSE(whitelistHandler.isWhiteListed("193.168.0.123"));
@@ -86,7 +89,9 @@ TEST_F(WhitelistHandlerTest, TestWhiteListResolvesHostname)
     ClioConfigDefinition const cfg{
         getParseWhitelistHandlerConfig(boost::json::parse(kJSON_DATA_IP_V4))
     };
-    WhitelistHandler const whitelistHandler{cfg};
+    auto const result = WhitelistHandler::create(cfg);
+    ASSERT_TRUE(result.has_value());
+    auto const& whitelistHandler = *result;
 
     EXPECT_TRUE(whitelistHandler.isWhiteListed("127.0.0.1"));
     EXPECT_FALSE(whitelistHandler.isWhiteListed("193.168.0.123"));
@@ -110,10 +115,69 @@ TEST_F(WhitelistHandlerTest, TestWhiteListIPV6)
     ClioConfigDefinition const cfg{
         getParseWhitelistHandlerConfig(boost::json::parse(kJSON_DATA_IP_V6))
     };
-    WhitelistHandler const whitelistHandler{cfg};
+    auto const result = WhitelistHandler::create(cfg);
+    ASSERT_TRUE(result.has_value());
+    auto const& whitelistHandler = *result;
 
     EXPECT_TRUE(whitelistHandler.isWhiteListed("2002:1dd8:85a7:0000:0000:8a6e:0000:1111"));
     EXPECT_FALSE(whitelistHandler.isWhiteListed("2002:1dd8:85a7:1101:0000:8a6e:0000:1111"));
     EXPECT_TRUE(whitelistHandler.isWhiteListed("2001:0db8:85a3:0000:0000:8a2e:0000:0000"));
     EXPECT_TRUE(whitelistHandler.isWhiteListed("2001:0db8:85a3:0000:1111:8a2e:0370:7334"));
 }
+
+struct WhitelistTest : public virtual ::testing::Test {};
+
+TEST_F(WhitelistTest, AddValidIPV4)
+{
+    Whitelist whitelist;
+    EXPECT_TRUE(whitelist.add("1.2.3.4").has_value());
+}
+
+TEST_F(WhitelistTest, AddInvalidIP)
+{
+    Whitelist whitelist;
+    auto const result = whitelist.add("not-an-ip");
+    ASSERT_FALSE(result.has_value());
+    EXPECT_THAT(result.error(), testing::HasSubstr("not-an-ip"));
+}
+
+TEST_F(WhitelistTest, AddInvalidNetwork)
+{
+    Whitelist whitelist;
+    auto const result = whitelist.add("not-a-net/24");
+    ASSERT_FALSE(result.has_value());
+    EXPECT_THAT(result.error(), testing::HasSubstr("not-a-net/24"));
+}
+
+TEST_F(WhitelistTest, IsWhiteListedWithInvalidIP)
+{
+    Whitelist const whitelist;
+    EXPECT_FALSE(whitelist.isWhiteListed("not-an-ip"));
+}
+
+TEST_F(WhitelistHandlerTest, CreateWithInvalidIPFails)
+{
+    struct MockResolver {
+        MOCK_METHOD(std::vector<std::string>, resolve, (std::string_view, std::string_view));
+        MOCK_METHOD(std::vector<std::string>, resolve, (std::string_view));
+    };
+
+    static constexpr auto kJSON = R"JSON(
+        {
+            "dos_guard": {
+                "whitelist": ["not-an-ip"]
+            }
+        }
+    )JSON";
+
+    testing::StrictMock<MockResolver> mockResolver;
+    EXPECT_CALL(mockResolver, resolve(testing::_))
+        .WillOnce([](auto hostname) -> std::vector<std::string> {
+            return {std::string{hostname}};
+        });
+
+    ClioConfigDefinition const cfg{getParseWhitelistHandlerConfig(boost::json::parse(kJSON))};
+    auto const result = WhitelistHandler::create(cfg, mockResolver);
+    ASSERT_FALSE(result.has_value());
+    EXPECT_THAT(result.error(), testing::HasSubstr("not-an-ip"));
+}