From 088418f29c9da6044d556b48d13ae3e3a89a27d2 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Tue, 21 Apr 2026 23:15:10 -0700 Subject: [PATCH 01/13] refactor: split json parsing/translation for future-proofing client/server distinction --- .../launchdarkly/data_model/change_set.hpp | 20 ++ .../launchdarkly/data_model/fdv2_change.hpp | 23 +- libs/internal/src/fdv2_protocol_handler.cpp | 95 +------ .../tests/fdv2_protocol_handler_test.cpp | 42 +--- libs/server-sdk/src/CMakeLists.txt | 2 + .../memory_store/memory_store.cpp | 11 +- .../memory_store/memory_store.hpp | 5 +- .../src/data_interfaces/item_change.hpp | 22 ++ .../source/fdv2_source_result.hpp | 16 +- .../fdv2/fdv2_changeset_translator.cpp | 97 +++++++ .../fdv2/fdv2_changeset_translator.hpp | 26 ++ .../data_systems/fdv2/fdv2_polling_impl.cpp | 33 ++- .../tests/fdv2_changeset_translator_test.cpp | 238 ++++++++++++++++++ .../tests/memory_store_apply_test.cpp | 42 ++-- 14 files changed, 508 insertions(+), 164 deletions(-) create mode 100644 libs/internal/include/launchdarkly/data_model/change_set.hpp create mode 100644 libs/server-sdk/src/data_interfaces/item_change.hpp create mode 100644 libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.cpp create mode 100644 libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp create mode 100644 libs/server-sdk/tests/fdv2_changeset_translator_test.cpp diff --git a/libs/internal/include/launchdarkly/data_model/change_set.hpp b/libs/internal/include/launchdarkly/data_model/change_set.hpp new file mode 100644 index 000000000..d720e39bc --- /dev/null +++ b/libs/internal/include/launchdarkly/data_model/change_set.hpp @@ -0,0 +1,20 @@ +#pragma once + +#include + +namespace launchdarkly::data_model { + +enum class ChangeSetType { + kFull = 0, + kPartial = 1, + kNone = 2, +}; + +template +struct ChangeSet { + ChangeSetType type; + Selector selector; + T data; +}; + +} // namespace launchdarkly::data_model diff --git a/libs/internal/include/launchdarkly/data_model/fdv2_change.hpp b/libs/internal/include/launchdarkly/data_model/fdv2_change.hpp index 0d45470aa..4600eb3a1 100644 --- a/libs/internal/include/launchdarkly/data_model/fdv2_change.hpp +++ b/libs/internal/include/launchdarkly/data_model/fdv2_change.hpp @@ -1,29 +1,28 @@ #pragma once -#include -#include -#include +#include #include +#include + +#include #include -#include #include namespace launchdarkly::data_model { struct FDv2Change { + enum class ChangeType { kPut, kDelete }; + + ChangeType change_type; + std::string kind; std::string key; - std::variant, ItemDescriptor> object; + uint64_t version; + boost::json::value object; // set for kPut; unused for kDelete }; struct FDv2ChangeSet { - enum class Type { - kFull = 0, - kPartial = 1, - kNone = 2, - }; - - Type type; + ChangeSetType type; std::vector changes; Selector selector; }; diff --git a/libs/internal/src/fdv2_protocol_handler.cpp b/libs/internal/src/fdv2_protocol_handler.cpp index d0564e45f..13b8e6823 100644 --- a/libs/internal/src/fdv2_protocol_handler.cpp +++ b/libs/internal/src/fdv2_protocol_handler.cpp @@ -1,11 +1,5 @@ #include -#include -#include -#include -#include -#include - #include #include @@ -20,66 +14,6 @@ static char const* const kGoodbye = "goodbye"; using Error = FDv2ProtocolHandler::Error; -// Returns the parsed FDv2Change on success, nullopt for unknown kinds (which -// should be silently skipped for forward-compatibility), or an Error if -// a known kind fails to deserialize. -static tl::expected, Error> ParsePut( - PutObject const& put) { - if (put.kind == "flag") { - auto result = boost::json::value_to< - tl::expected, JsonError>>( - put.object); - // One bad flag aborts the entire transfer so the store is never - // left in a partially-updated state. - if (!result) { - return tl::make_unexpected(Error::JsonParseError( - result.error(), - "could not deserialize flag '" + put.key + "'")); - } - if (!result->has_value()) { - return tl::make_unexpected(Error::JsonParseError( - "flag '" + put.key + "' object was null")); - } - return data_model::FDv2Change{ - put.key, - data_model::ItemDescriptor{std::move(**result)}}; - } - if (put.kind == "segment") { - auto result = boost::json::value_to< - tl::expected, JsonError>>( - put.object); - // One bad segment aborts the entire transfer so the store is never - // left in a partially-updated state. - if (!result) { - return tl::make_unexpected(Error::JsonParseError( - result.error(), - "could not deserialize segment '" + put.key + "'")); - } - if (!result->has_value()) { - return tl::make_unexpected(Error::JsonParseError( - "segment '" + put.key + "' object was null")); - } - return data_model::FDv2Change{ - put.key, data_model::ItemDescriptor{ - std::move(**result)}}; - } - // Silently skip unknown kinds for forward-compatibility. - return std::nullopt; -} - -static data_model::FDv2Change MakeDeleteChange(DeleteObject const& del) { - if (del.kind == "flag") { - return data_model::FDv2Change{ - del.key, - data_model::ItemDescriptor{ - data_model::Tombstone{static_cast(del.version)}}}; - } - return data_model::FDv2Change{ - del.key, - data_model::ItemDescriptor{ - data_model::Tombstone{static_cast(del.version)}}}; -} - FDv2ProtocolHandler::Result FDv2ProtocolHandler::HandleEvent( std::string_view event_type, boost::json::value const& data) { @@ -137,7 +71,7 @@ FDv2ProtocolHandler::Result FDv2ProtocolHandler::HandleServerIntent( // kNone or kUnknown: emit an empty changeset immediately. state_ = State::kInactive; return data_model::FDv2ChangeSet{ - data_model::FDv2ChangeSet::Type::kNone, {}, data_model::Selector{}}; + data_model::ChangeSetType::kNone, {}, data_model::Selector{}}; } return std::monostate{}; } @@ -158,14 +92,10 @@ FDv2ProtocolHandler::Result FDv2ProtocolHandler::HandlePutObject( Reset(); return Error::JsonParseError("put-object data was null"); } - auto change = ParsePut(**result); - if (!change) { - Reset(); - return std::move(change.error()); - } - if (*change) { - changes_.push_back(std::move(**change)); - } + auto const& put = **result; + changes_.push_back(data_model::FDv2Change{ + data_model::FDv2Change::ChangeType::kPut, put.kind, put.key, + static_cast(put.version), put.object}); return std::monostate{}; } @@ -186,11 +116,12 @@ FDv2ProtocolHandler::Result FDv2ProtocolHandler::HandleDeleteObject( return Error::JsonParseError("delete-object data was null"); } auto const& del = **result; - // Silently skip unknown kinds for forward-compatibility. - if (del.kind != "flag" && del.kind != "segment") { - return std::monostate{}; - } - changes_.push_back(MakeDeleteChange(del)); + changes_.push_back( + data_model::FDv2Change{data_model::FDv2Change::ChangeType::kDelete, + del.kind, + del.key, + static_cast(del.version), + {}}); return std::monostate{}; } @@ -215,8 +146,8 @@ FDv2ProtocolHandler::Result FDv2ProtocolHandler::HandlePayloadTransferred( } auto const& transferred = **result; auto type = (state_ == State::kPartial) - ? data_model::FDv2ChangeSet::Type::kPartial - : data_model::FDv2ChangeSet::Type::kFull; + ? data_model::ChangeSetType::kPartial + : data_model::ChangeSetType::kFull; data_model::FDv2ChangeSet changeset{ type, std::move(changes_), data_model::Selector{data_model::Selector::State{transferred.version, diff --git a/libs/internal/tests/fdv2_protocol_handler_test.cpp b/libs/internal/tests/fdv2_protocol_handler_test.cpp index 4a7d9da0d..fb428c243 100644 --- a/libs/internal/tests/fdv2_protocol_handler_test.cpp +++ b/libs/internal/tests/fdv2_protocol_handler_test.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -56,7 +57,7 @@ TEST(FDv2ProtocolHandlerTest, NoneIntentEmitsEmptyChangeSetImmediately) { auto* cs = std::get_if(&result); ASSERT_NE(cs, nullptr); - EXPECT_EQ(cs->type, data_model::FDv2ChangeSet::Type::kNone); + EXPECT_EQ(cs->type, data_model::ChangeSetType::kNone); EXPECT_TRUE(cs->changes.empty()); EXPECT_FALSE(cs->selector.value.has_value()); } @@ -81,7 +82,7 @@ TEST(FDv2ProtocolHandlerTest, FullIntentEmitsChangeSetOnPayloadTransferred) { auto* cs = std::get_if(&r3); ASSERT_NE(cs, nullptr); - EXPECT_EQ(cs->type, data_model::FDv2ChangeSet::Type::kFull); + EXPECT_EQ(cs->type, data_model::ChangeSetType::kFull); EXPECT_EQ(cs->changes.size(), 1u); EXPECT_EQ(cs->changes[0].key, "my-flag"); ASSERT_TRUE(cs->selector.value.has_value()); @@ -105,7 +106,7 @@ TEST(FDv2ProtocolHandlerTest, FullIntentAccumulatesMultipleObjects) { auto* cs = std::get_if(&result); ASSERT_NE(cs, nullptr); - EXPECT_EQ(cs->type, data_model::FDv2ChangeSet::Type::kFull); + EXPECT_EQ(cs->type, data_model::ChangeSetType::kFull); EXPECT_EQ(cs->changes.size(), 3u); } @@ -132,7 +133,7 @@ TEST(FDv2ProtocolHandlerTest, auto* cs = std::get_if(&result); ASSERT_NE(cs, nullptr); - EXPECT_EQ(cs->type, data_model::FDv2ChangeSet::Type::kPartial); + EXPECT_EQ(cs->type, data_model::ChangeSetType::kPartial); ASSERT_EQ(cs->changes.size(), 1u); EXPECT_EQ(cs->changes[0].key, "flag-2"); } @@ -153,7 +154,7 @@ TEST(FDv2ProtocolHandlerTest, PartialIntentEmitsPartialChangeSet) { auto* cs = std::get_if(&result); ASSERT_NE(cs, nullptr); - EXPECT_EQ(cs->type, data_model::FDv2ChangeSet::Type::kPartial); + EXPECT_EQ(cs->type, data_model::ChangeSetType::kPartial); EXPECT_EQ(cs->changes.size(), 1u); EXPECT_EQ(cs->changes[0].key, "my-seg"); ASSERT_TRUE(cs->selector.value.has_value()); @@ -164,7 +165,7 @@ TEST(FDv2ProtocolHandlerTest, PartialIntentEmitsPartialChangeSet) { // Unknown kind in put-object → silently skipped // ============================================================================ -TEST(FDv2ProtocolHandlerTest, UnknownKindInPutObjectIsSilentlySkipped) { +TEST(FDv2ProtocolHandlerTest, UnknownKindInPutObjectIsPassedThrough) { FDv2ProtocolHandler handler; handler.HandleEvent("server-intent", MakeServerIntent("xfer-full")); @@ -179,12 +180,11 @@ TEST(FDv2ProtocolHandlerTest, UnknownKindInPutObjectIsSilentlySkipped) { auto* cs = std::get_if(&result); ASSERT_NE(cs, nullptr); - // Only the known kind (flag) should appear. - EXPECT_EQ(cs->changes.size(), 1u); - EXPECT_EQ(cs->changes[0].key, "my-flag"); + // All kinds pass through; filtering happens in the translator. + EXPECT_EQ(cs->changes.size(), 2u); } -TEST(FDv2ProtocolHandlerTest, UnknownKindInDeleteObjectIsSilentlySkipped) { +TEST(FDv2ProtocolHandlerTest, UnknownKindInDeleteObjectIsPassedThrough) { FDv2ProtocolHandler handler; handler.HandleEvent("server-intent", MakeServerIntent("xfer-full")); @@ -198,9 +198,8 @@ TEST(FDv2ProtocolHandlerTest, UnknownKindInDeleteObjectIsSilentlySkipped) { auto* cs = std::get_if(&result); ASSERT_NE(cs, nullptr); - // Only the known kind (flag) should appear. - EXPECT_EQ(cs->changes.size(), 1u); - EXPECT_EQ(cs->changes[0].key, "my-flag"); + // All kinds pass through; filtering happens in the translator. + EXPECT_EQ(cs->changes.size(), 2u); } // ============================================================================ @@ -252,23 +251,6 @@ TEST(FDv2ProtocolHandlerTest, ErrorEventWithIdSetsServerId) { EXPECT_EQ(err->server_error->reason, "overloaded"); } -TEST(FDv2ProtocolHandlerTest, MalformedPutObjectReturnsJsonError) { - FDv2ProtocolHandler handler; - - handler.HandleEvent("server-intent", MakeServerIntent("xfer-full")); - - // 'object' field is missing required flag fields — deserialisation fails. - auto result = handler.HandleEvent( - "put-object", - boost::json::parse( - R"({"version":1,"kind":"flag","key":"f","object":{}})")); - - auto* err = std::get_if(&result); - ASSERT_NE(err, nullptr); - EXPECT_EQ(err->kind, FDv2ProtocolHandler::Error::Kind::kJsonError); - EXPECT_TRUE(err->json_error.has_value()); -} - // ============================================================================ // goodbye event → return Goodbye // ============================================================================ diff --git a/libs/server-sdk/src/CMakeLists.txt b/libs/server-sdk/src/CMakeLists.txt index 09f98465b..eb3841179 100644 --- a/libs/server-sdk/src/CMakeLists.txt +++ b/libs/server-sdk/src/CMakeLists.txt @@ -49,6 +49,8 @@ target_sources(${LIBNAME} data_systems/background_sync/detail/payload_filter_validation/payload_filter_validation.cpp data_systems/background_sync/sources/polling/polling_data_source.hpp data_systems/background_sync/sources/polling/polling_data_source.cpp + data_systems/fdv2/fdv2_changeset_translator.hpp + data_systems/fdv2/fdv2_changeset_translator.cpp data_systems/fdv2/fdv2_polling_impl.hpp data_systems/fdv2/fdv2_polling_impl.cpp data_systems/fdv2/polling_initializer.hpp diff --git a/libs/server-sdk/src/data_components/memory_store/memory_store.cpp b/libs/server-sdk/src/data_components/memory_store/memory_store.cpp index c71acf08a..2eee8ff03 100644 --- a/libs/server-sdk/src/data_components/memory_store/memory_store.cpp +++ b/libs/server-sdk/src/data_components/memory_store/memory_store.cpp @@ -84,15 +84,16 @@ bool MemoryStore::RemoveSegment(std::string const& key) { return segments_.erase(key) == 1; } -void MemoryStore::Apply(data_model::FDv2ChangeSet changeSet) { +void MemoryStore::Apply( + data_model::ChangeSet changeSet) { std::lock_guard lock{data_mutex_}; switch (changeSet.type) { - case data_model::FDv2ChangeSet::Type::kNone: + case data_model::ChangeSetType::kNone: return; - case data_model::FDv2ChangeSet::Type::kPartial: + case data_model::ChangeSetType::kPartial: break; - case data_model::FDv2ChangeSet::Type::kFull: + case data_model::ChangeSetType::kFull: initialized_ = true; flags_.clear(); segments_.clear(); @@ -101,7 +102,7 @@ void MemoryStore::Apply(data_model::FDv2ChangeSet changeSet) { detail::unreachable(); } - for (auto& change : changeSet.changes) { + for (auto& change : changeSet.data) { if (std::holds_alternative(change.object)) { flags_[change.key] = std::make_shared( std::move(std::get(change.object))); diff --git a/libs/server-sdk/src/data_components/memory_store/memory_store.hpp b/libs/server-sdk/src/data_components/memory_store/memory_store.hpp index e9a067881..7bda17d3f 100644 --- a/libs/server-sdk/src/data_components/memory_store/memory_store.hpp +++ b/libs/server-sdk/src/data_components/memory_store/memory_store.hpp @@ -1,9 +1,10 @@ #pragma once #include "../../data_interfaces/destination/idestination.hpp" +#include "../../data_interfaces/item_change.hpp" #include "../../data_interfaces/store/istore.hpp" -#include +#include #include #include @@ -46,7 +47,7 @@ class MemoryStore final : public data_interfaces::IStore, bool RemoveSegment(std::string const& key); - void Apply(data_model::FDv2ChangeSet changeSet); + void Apply(data_model::ChangeSet changeSet); MemoryStore() = default; ~MemoryStore() override = default; diff --git a/libs/server-sdk/src/data_interfaces/item_change.hpp b/libs/server-sdk/src/data_interfaces/item_change.hpp new file mode 100644 index 000000000..1c2cc8292 --- /dev/null +++ b/libs/server-sdk/src/data_interfaces/item_change.hpp @@ -0,0 +1,22 @@ +#pragma once + +#include +#include +#include + +#include +#include +#include + +namespace launchdarkly::server_side::data_interfaces { + +struct ItemChange { + std::string key; + std::variant, + data_model::ItemDescriptor> + object; +}; + +using ChangeSetData = std::vector; + +} // namespace launchdarkly::server_side::data_interfaces diff --git a/libs/server-sdk/src/data_interfaces/source/fdv2_source_result.hpp b/libs/server-sdk/src/data_interfaces/source/fdv2_source_result.hpp index 069fe5a38..a2a7589e3 100644 --- a/libs/server-sdk/src/data_interfaces/source/fdv2_source_result.hpp +++ b/libs/server-sdk/src/data_interfaces/source/fdv2_source_result.hpp @@ -1,6 +1,8 @@ #pragma once -#include +#include "../item_change.hpp" + +#include #include #include @@ -11,8 +13,6 @@ namespace launchdarkly::server_side::data_interfaces { /** * Result returned by IFDv2Initializer::Run and IFDv2Synchronizer::Next. - * - * Mirrors Java's FDv2SourceResult. */ struct FDv2SourceResult { using ErrorInfo = common::data_sources::DataSourceStatusErrorInfo; @@ -21,7 +21,7 @@ struct FDv2SourceResult { * A changeset was successfully received and is ready to apply. */ struct ChangeSet { - data_model::FDv2ChangeSet change_set; + data_model::ChangeSet change_set; /** If true, the server signaled that the client should fall back to * FDv1. */ bool fdv1_fallback; @@ -61,8 +61,12 @@ struct FDv2SourceResult { */ struct Timeout {}; - using Value = std::variant; + using Value = std::variant; Value value; }; diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.cpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.cpp new file mode 100644 index 000000000..70a2bd2be --- /dev/null +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.cpp @@ -0,0 +1,97 @@ +#include "fdv2_changeset_translator.hpp" + +#include +#include +#include +#include +#include + +#include +#include + +namespace launchdarkly::server_side::data_systems { + +using data_interfaces::ChangeSetData; +using data_interfaces::ItemChange; +using data_model::ChangeSet; +using data_model::ChangeSetType; +using data_model::FDv2ChangeSet; + +std::optional> FDv2ChangeSetTranslator::Translate( + FDv2ChangeSet const& change_set, + Logger const& logger) const { + if (change_set.type == ChangeSetType::kNone) { + return ChangeSet{ + change_set.type, change_set.selector, {}}; + } + + ChangeSetData changes; + changes.reserve(change_set.changes.size()); + + for (auto const& change : change_set.changes) { + if (change.change_type == data_model::FDv2Change::ChangeType::kDelete) { + if (change.kind == "flag") { + changes.push_back(ItemChange{ + change.key, data_model::ItemDescriptor{ + data_model::Tombstone{change.version}}}); + } else if (change.kind == "segment") { + changes.push_back(ItemChange{ + change.key, data_model::ItemDescriptor{ + data_model::Tombstone{change.version}}}); + } else { + LD_LOG(logger, LogLevel::kWarn) + << "FDv2: unknown kind '" << change.kind + << "' in delete-object, skipping"; + } + } else { + if (change.kind == "flag") { + auto result = boost::json::value_to< + tl::expected, JsonError>>( + change.object); + if (!result) { + LD_LOG(logger, LogLevel::kError) + << "FDv2: could not deserialize flag '" << change.key + << "'"; + return std::nullopt; + } + if (!result->has_value()) { + LD_LOG(logger, LogLevel::kWarn) + << "FDv2: flag '" << change.key + << "' object was null, skipping"; + continue; + } + changes.push_back(ItemChange{ + change.key, data_model::ItemDescriptor{ + std::move(**result)}}); + } else if (change.kind == "segment") { + auto result = boost::json::value_to, JsonError>>( + change.object); + if (!result) { + LD_LOG(logger, LogLevel::kError) + << "FDv2: could not deserialize segment '" << change.key + << "'"; + return std::nullopt; + } + if (!result->has_value()) { + LD_LOG(logger, LogLevel::kWarn) + << "FDv2: segment '" << change.key + << "' object was null, skipping"; + continue; + } + changes.push_back(ItemChange{ + change.key, data_model::ItemDescriptor{ + std::move(**result)}}); + } else { + LD_LOG(logger, LogLevel::kWarn) + << "FDv2: unknown kind '" << change.kind + << "' in put-object, skipping"; + } + } + } + + return ChangeSet{change_set.type, change_set.selector, + std::move(changes)}; +} + +} // namespace launchdarkly::server_side::data_systems diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp new file mode 100644 index 000000000..5763a31eb --- /dev/null +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp @@ -0,0 +1,26 @@ +#pragma once + +#include "../../data_interfaces/item_change.hpp" + +#include +#include +#include + +#include + +namespace launchdarkly::server_side::data_systems { + +class FDv2ChangeSetTranslator { + public: + /** + * Translates a changeset into typed changes ready to apply to the store. + * + * Unknown kinds are warned and skipped. If any known kind fails to + * deserialize, the entire changeset is aborted and nullopt is returned. + */ + std::optional> + Translate(data_model::FDv2ChangeSet const& change_set, + Logger const& logger) const; +}; + +} // namespace launchdarkly::server_side::data_systems diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp index df6fda9af..b09a85162 100644 --- a/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp @@ -1,4 +1,5 @@ #include "fdv2_polling_impl.hpp" +#include "fdv2_changeset_translator.hpp" #include #include @@ -17,6 +18,8 @@ static char const* const kErrorMissingEvents = "FDv2 polling response missing 'events' array"; static char const* const kErrorIncompletePayload = "FDv2 polling response did not contain a complete payload"; +static char const* const kErrorTranslation = + "FDv2 polling response could not be translated"; using data_interfaces::FDv2SourceResult; using ErrorInfo = FDv2SourceResult::ErrorInfo; @@ -57,7 +60,9 @@ network::HttpRequest MakeFDv2PollRequest( static FDv2SourceResult ParseFDv2PollEvents( boost::json::array const& events, - FDv2ProtocolHandler* protocol_handler) { + FDv2ProtocolHandler* protocol_handler, + FDv2ChangeSetTranslator const& translator, + Logger const& logger) { for (auto const& event_val : events) { auto const* event_obj = event_val.if_object(); if (!event_obj) { @@ -79,9 +84,16 @@ static FDv2SourceResult ParseFDv2PollEvents( std::string_view{event_type_str->data(), event_type_str->size()}, *event_data_val); - if (auto* changeset = std::get_if(&result)) { + if (auto* change_set = + std::get_if(&result)) { + auto typed = translator.Translate(*change_set, logger); + if (!typed) { + return FDv2SourceResult{FDv2SourceResult::Interrupted{ + MakeError(ErrorKind::kInvalidData, 0, kErrorTranslation), + false}}; + } return FDv2SourceResult{ - FDv2SourceResult::ChangeSet{std::move(*changeset), false}}; + FDv2SourceResult::ChangeSet{std::move(*typed), false}}; } if (auto* goodbye = std::get_if(&result)) { return FDv2SourceResult{ @@ -110,7 +122,9 @@ static FDv2SourceResult ParseFDv2PollEvents( static FDv2SourceResult ParseFDv2PollResponse( std::string const& body, - FDv2ProtocolHandler* protocol_handler) { + FDv2ProtocolHandler* protocol_handler, + FDv2ChangeSetTranslator const& translator, + Logger const& logger) { boost::system::error_code ec; auto parsed = boost::json::parse(body, ec); if (ec) { @@ -136,7 +150,8 @@ static FDv2SourceResult ParseFDv2PollResponse( MakeError(ErrorKind::kInvalidData, 0, kErrorMissingEvents), false}}; } - return ParseFDv2PollEvents(*events_arr, protocol_handler); + return ParseFDv2PollEvents(*events_arr, protocol_handler, translator, + logger); } data_interfaces::FDv2SourceResult HandleFDv2PollResponse( @@ -155,8 +170,8 @@ data_interfaces::FDv2SourceResult HandleFDv2PollResponse( if (res.Status() == 304) { return FDv2SourceResult{FDv2SourceResult::ChangeSet{ - data_model::FDv2ChangeSet{ - data_model::FDv2ChangeSet::Type::kNone, {}, {}}, + data_model::ChangeSet{ + data_model::ChangeSetType::kNone, data_model::Selector{}, {}}, false}}; } @@ -169,7 +184,9 @@ data_interfaces::FDv2SourceResult HandleFDv2PollResponse( false}}; } - auto result = ParseFDv2PollResponse(*body, protocol_handler); + FDv2ChangeSetTranslator translator; + auto result = + ParseFDv2PollResponse(*body, protocol_handler, translator, logger); if (auto* interrupted = std::get_if(&result.value)) { if (interrupted->error.Kind() == ErrorKind::kErrorResponse) { diff --git a/libs/server-sdk/tests/fdv2_changeset_translator_test.cpp b/libs/server-sdk/tests/fdv2_changeset_translator_test.cpp new file mode 100644 index 000000000..a5111ce39 --- /dev/null +++ b/libs/server-sdk/tests/fdv2_changeset_translator_test.cpp @@ -0,0 +1,238 @@ +#include + +#include + +#include +#include +#include + +#include + +using namespace launchdarkly; +using namespace launchdarkly::data_model; +using namespace launchdarkly::server_side::data_interfaces; +using namespace launchdarkly::server_side::data_systems; + +// Minimal valid flag JSON accepted by the Flag deserializer. +static char const* const kFlagJson = + R"({"key":"my-flag","on":true,"fallthrough":{"variation":0},)" + R"("variations":[true,false],"version":1})"; + +// Minimal valid segment JSON accepted by the Segment deserializer. +static char const* const kSegmentJson = + R"({"key":"my-seg","version":2,"rules":[],"included":[],"excluded":[]})"; + +static Logger MakeNullLogger() { + struct NullBackend : ILogBackend { + bool Enabled(LogLevel) noexcept override { return false; } + void Write(LogLevel, std::string) noexcept override {} + }; + return Logger{std::make_shared()}; +} + +// ============================================================================ +// kNone changeset +// ============================================================================ + +TEST(FDv2ChangeSetTranslatorTest, NoneChangeSetProducesEmptyTypedChangeSet) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ChangeSetType::kNone, {}, Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->type, ChangeSetType::kNone); + EXPECT_TRUE(result->data.empty()); +} + +// ============================================================================ +// Known kinds — put +// ============================================================================ + +TEST(FDv2ChangeSetTranslatorTest, PutFlagProducesTypedFlag) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ChangeSetType::kFull, + {FDv2Change{FDv2Change::ChangeType::kPut, "flag", + "my-flag", 1, boost::json::parse(kFlagJson)}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result->data.size(), 1u); + EXPECT_EQ(result->data[0].key, "my-flag"); + EXPECT_TRUE( + std::holds_alternative>(result->data[0].object)); +} + +TEST(FDv2ChangeSetTranslatorTest, PutSegmentProducesTypedSegment) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ + ChangeSetType::kFull, + {FDv2Change{FDv2Change::ChangeType::kPut, "segment", "my-seg", 2, + boost::json::parse(kSegmentJson)}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result->data.size(), 1u); + EXPECT_EQ(result->data[0].key, "my-seg"); + EXPECT_TRUE(std::holds_alternative>( + result->data[0].object)); +} + +// ============================================================================ +// Known kinds — delete +// ============================================================================ + +TEST(FDv2ChangeSetTranslatorTest, DeleteFlagProducesFlagTombstone) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ + ChangeSetType::kPartial, + {FDv2Change{FDv2Change::ChangeType::kDelete, "flag", "my-flag", 5, {}}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result->data.size(), 1u); + EXPECT_EQ(result->data[0].key, "my-flag"); + auto const* desc = + std::get_if>(&result->data[0].object); + ASSERT_NE(desc, nullptr); + EXPECT_EQ(desc->version, 5u); + EXPECT_FALSE(desc->item.has_value()); +} + +TEST(FDv2ChangeSetTranslatorTest, DeleteSegmentProducesSegmentTombstone) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ + ChangeSetType::kPartial, + {FDv2Change{ + FDv2Change::ChangeType::kDelete, "segment", "my-seg", 3, {}}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result->data.size(), 1u); + auto const* desc = + std::get_if>(&result->data[0].object); + ASSERT_NE(desc, nullptr); + EXPECT_EQ(desc->version, 3u); + EXPECT_FALSE(desc->item.has_value()); +} + +// ============================================================================ +// Unknown kind — skipped +// ============================================================================ + +TEST(FDv2ChangeSetTranslatorTest, UnknownKindInPutIsSkipped) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ + ChangeSetType::kFull, + {FDv2Change{FDv2Change::ChangeType::kPut, "experiment", "exp-1", 1, + boost::json::parse(R"({"key":"exp-1","version":1})")}, + FDv2Change{FDv2Change::ChangeType::kPut, "flag", "my-flag", 1, + boost::json::parse(kFlagJson)}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result->data.size(), 1u); + EXPECT_EQ(result->data[0].key, "my-flag"); +} + +TEST(FDv2ChangeSetTranslatorTest, UnknownKindInDeleteIsSkipped) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ + ChangeSetType::kFull, + {FDv2Change{ + FDv2Change::ChangeType::kDelete, "experiment", "exp-1", 1, {}}, + FDv2Change{FDv2Change::ChangeType::kDelete, "flag", "my-flag", 2, {}}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result->data.size(), 1u); + EXPECT_EQ(result->data[0].key, "my-flag"); +} + +// ============================================================================ +// Null object on put — skipped +// ============================================================================ + +TEST(FDv2ChangeSetTranslatorTest, NullObjectInPutFlagIsSkipped) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ChangeSetType::kFull, + {FDv2Change{FDv2Change::ChangeType::kPut, "flag", + "my-flag", 1, boost::json::value{nullptr}}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + EXPECT_TRUE(result->data.empty()); +} + +// ============================================================================ +// Deserialization failure — abort +// ============================================================================ + +TEST(FDv2ChangeSetTranslatorTest, MalformedFlagAbortsTranslation) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ChangeSetType::kFull, + {FDv2Change{FDv2Change::ChangeType::kPut, "flag", + "bad-flag", 1, boost::json::parse(R"({})")}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + EXPECT_FALSE(result.has_value()); +} + +TEST(FDv2ChangeSetTranslatorTest, MalformedSegmentAbortsTranslation) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + // A non-empty object missing required fields triggers a schema failure + // (the deserializer treats an empty object as null/skip, not an error). + FDv2ChangeSet raw{ + ChangeSetType::kFull, + {FDv2Change{FDv2Change::ChangeType::kPut, "segment", "bad-seg", 1, + boost::json::parse(R"({"key":"bad-seg"})")}}, + Selector{}}; + auto result = translator.Translate(raw, logger); + + EXPECT_FALSE(result.has_value()); +} + +// ============================================================================ +// Selector is preserved +// ============================================================================ + +TEST(FDv2ChangeSetTranslatorTest, SelectorIsPreserved) { + FDv2ChangeSetTranslator translator; + auto logger = MakeNullLogger(); + + FDv2ChangeSet raw{ + ChangeSetType::kFull, {}, Selector{Selector::State{7, "state-abc"}}}; + auto result = translator.Translate(raw, logger); + + ASSERT_TRUE(result.has_value()); + ASSERT_TRUE(result->selector.value.has_value()); + EXPECT_EQ(result->selector.value->state, "state-abc"); + EXPECT_EQ(result->selector.value->version, 7); +} diff --git a/libs/server-sdk/tests/memory_store_apply_test.cpp b/libs/server-sdk/tests/memory_store_apply_test.cpp index 003285c53..00ecf62b4 100644 --- a/libs/server-sdk/tests/memory_store_apply_test.cpp +++ b/libs/server-sdk/tests/memory_store_apply_test.cpp @@ -1,10 +1,14 @@ #include #include +#include + +#include #include using namespace launchdarkly::data_model; using namespace launchdarkly::server_side::data_components; +using namespace launchdarkly::server_side::data_interfaces; // --------------------------------------------------------------------------- // kNone tests @@ -27,7 +31,7 @@ TEST(MemoryStoreApplyTest, ApplyNone_IsNoOp) { {"segA", SegmentDescriptor(seg_a)}}, }); - store.Apply(FDv2ChangeSet{FDv2ChangeSet::Type::kNone, {}, Selector{}}); + store.Apply(ChangeSet{ChangeSetType::kNone, Selector{}, {}}); auto fetched_flag = store.GetFlag("flagA"); ASSERT_TRUE(fetched_flag); @@ -39,7 +43,7 @@ TEST(MemoryStoreApplyTest, ApplyNone_IsNoOp) { TEST(MemoryStoreApplyTest, ApplyNone_DoesNotInitialize) { MemoryStore store; - store.Apply(FDv2ChangeSet{FDv2ChangeSet::Type::kNone, {}, Selector{}}); + store.Apply(ChangeSet{ChangeSetType::kNone, Selector{}, {}}); EXPECT_FALSE(store.Initialized()); } @@ -50,7 +54,7 @@ TEST(MemoryStoreApplyTest, ApplyNone_DoesNotInitialize) { TEST(MemoryStoreApplyTest, ApplyFull_SetsInitialized) { MemoryStore store; ASSERT_FALSE(store.Initialized()); - store.Apply(FDv2ChangeSet{FDv2ChangeSet::Type::kFull, {}, Selector{}}); + store.Apply(ChangeSet{ChangeSetType::kFull, Selector{}, {}}); EXPECT_TRUE(store.Initialized()); } @@ -64,11 +68,11 @@ TEST(MemoryStoreApplyTest, ApplyFull_StoresItems) { seg_a.version = 1; seg_a.key = "segA"; - store.Apply(FDv2ChangeSet{ - FDv2ChangeSet::Type::kFull, - std::vector{{"flagA", FlagDescriptor(flag_a)}, - {"segA", SegmentDescriptor(seg_a)}}, + store.Apply(ChangeSet{ + ChangeSetType::kFull, Selector{}, + ChangeSetData{ItemChange{"flagA", FlagDescriptor(flag_a)}, + ItemChange{"segA", SegmentDescriptor(seg_a)}}, }); auto fetched_flag = store.GetFlag("flagA"); @@ -114,11 +118,11 @@ TEST(MemoryStoreApplyTest, ApplyFull_ClearsExistingItems) { seg_b.version = 1; seg_b.key = "segB"; - store.Apply(FDv2ChangeSet{ - FDv2ChangeSet::Type::kFull, - std::vector{{"flagC", FlagDescriptor(flag_c)}, - {"segB", SegmentDescriptor(seg_b)}}, + store.Apply(ChangeSet{ + ChangeSetType::kFull, Selector{}, + ChangeSetData{ItemChange{"flagC", FlagDescriptor(flag_c)}, + ItemChange{"segB", SegmentDescriptor(seg_b)}}, }); EXPECT_FALSE(store.GetFlag("flagA")); @@ -157,11 +161,11 @@ TEST(MemoryStoreApplyTest, ApplyPartial_AppliesItems) { seg_a_new.version = 6; seg_a_new.key = "segA"; - store.Apply(FDv2ChangeSet{ - FDv2ChangeSet::Type::kPartial, - std::vector{{"flagA", FlagDescriptor(flag_a_new)}, - {"segA", SegmentDescriptor(seg_a_new)}}, + store.Apply(ChangeSet{ + ChangeSetType::kPartial, Selector{}, + ChangeSetData{ItemChange{"flagA", FlagDescriptor(flag_a_new)}, + ItemChange{"segA", SegmentDescriptor(seg_a_new)}}, }); ASSERT_TRUE(store.GetFlag("flagA")); @@ -205,11 +209,11 @@ TEST(MemoryStoreApplyTest, ApplyPartial_PreservesUnchangedItems) { seg_b_new.version = 2; seg_b_new.key = "segB"; - store.Apply(FDv2ChangeSet{ - FDv2ChangeSet::Type::kPartial, - std::vector{{"flagB", FlagDescriptor(flag_b_new)}, - {"segB", SegmentDescriptor(seg_b_new)}}, + store.Apply(ChangeSet{ + ChangeSetType::kPartial, Selector{}, + ChangeSetData{ItemChange{"flagB", FlagDescriptor(flag_b_new)}, + ItemChange{"segB", SegmentDescriptor(seg_b_new)}}, }); ASSERT_TRUE(store.GetFlag("flagA")); From c990e300709392a851c90ce4d3bf06f119ce0353 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Tue, 21 Apr 2026 23:34:15 -0700 Subject: [PATCH 02/13] refactor: make the changeset translator a function, not a class --- libs/server-sdk/src/CMakeLists.txt | 4 +- ...tor.cpp => fdv2_changeset_translation.cpp} | 6 +- .../fdv2/fdv2_changeset_translation.hpp | 23 ++++++++ .../fdv2/fdv2_changeset_translator.hpp | 26 --------- .../data_systems/fdv2/fdv2_polling_impl.cpp | 13 ++--- ...pp => fdv2_changeset_translation_test.cpp} | 57 ++++++++----------- 6 files changed, 55 insertions(+), 74 deletions(-) rename libs/server-sdk/src/data_systems/fdv2/{fdv2_changeset_translator.cpp => fdv2_changeset_translation.cpp} (96%) create mode 100644 libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.hpp delete mode 100644 libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp rename libs/server-sdk/tests/{fdv2_changeset_translator_test.cpp => fdv2_changeset_translation_test.cpp} (79%) diff --git a/libs/server-sdk/src/CMakeLists.txt b/libs/server-sdk/src/CMakeLists.txt index eb3841179..f8fac7fb3 100644 --- a/libs/server-sdk/src/CMakeLists.txt +++ b/libs/server-sdk/src/CMakeLists.txt @@ -49,8 +49,8 @@ target_sources(${LIBNAME} data_systems/background_sync/detail/payload_filter_validation/payload_filter_validation.cpp data_systems/background_sync/sources/polling/polling_data_source.hpp data_systems/background_sync/sources/polling/polling_data_source.cpp - data_systems/fdv2/fdv2_changeset_translator.hpp - data_systems/fdv2/fdv2_changeset_translator.cpp + data_systems/fdv2/fdv2_changeset_translation.hpp + data_systems/fdv2/fdv2_changeset_translation.cpp data_systems/fdv2/fdv2_polling_impl.hpp data_systems/fdv2/fdv2_polling_impl.cpp data_systems/fdv2/polling_initializer.hpp diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.cpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp similarity index 96% rename from libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.cpp rename to libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp index 70a2bd2be..8e138ced4 100644 --- a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.cpp +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp @@ -1,4 +1,4 @@ -#include "fdv2_changeset_translator.hpp" +#include "fdv2_changeset_translation.hpp" #include #include @@ -17,9 +17,9 @@ using data_model::ChangeSet; using data_model::ChangeSetType; using data_model::FDv2ChangeSet; -std::optional> FDv2ChangeSetTranslator::Translate( +std::optional> TranslateChangeSet( FDv2ChangeSet const& change_set, - Logger const& logger) const { + Logger const& logger) { if (change_set.type == ChangeSetType::kNone) { return ChangeSet{ change_set.type, change_set.selector, {}}; diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.hpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.hpp new file mode 100644 index 000000000..8e3aea9f1 --- /dev/null +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.hpp @@ -0,0 +1,23 @@ +#pragma once + +#include "../../data_interfaces/item_change.hpp" + +#include +#include +#include + +#include + +namespace launchdarkly::server_side::data_systems { + +/** + * Translates an FDv2ChangeSet into typed changes ready to apply to the store. + * + * Unknown kinds are warned and skipped. If any known kind fails to + * deserialize, the entire changeset is aborted and nullopt is returned. + */ +std::optional> +TranslateChangeSet(data_model::FDv2ChangeSet const& change_set, + Logger const& logger); + +} // namespace launchdarkly::server_side::data_systems diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp deleted file mode 100644 index 5763a31eb..000000000 --- a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translator.hpp +++ /dev/null @@ -1,26 +0,0 @@ -#pragma once - -#include "../../data_interfaces/item_change.hpp" - -#include -#include -#include - -#include - -namespace launchdarkly::server_side::data_systems { - -class FDv2ChangeSetTranslator { - public: - /** - * Translates a changeset into typed changes ready to apply to the store. - * - * Unknown kinds are warned and skipped. If any known kind fails to - * deserialize, the entire changeset is aborted and nullopt is returned. - */ - std::optional> - Translate(data_model::FDv2ChangeSet const& change_set, - Logger const& logger) const; -}; - -} // namespace launchdarkly::server_side::data_systems diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp index b09a85162..4deb66d70 100644 --- a/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp @@ -1,5 +1,5 @@ #include "fdv2_polling_impl.hpp" -#include "fdv2_changeset_translator.hpp" +#include "fdv2_changeset_translation.hpp" #include #include @@ -61,7 +61,6 @@ network::HttpRequest MakeFDv2PollRequest( static FDv2SourceResult ParseFDv2PollEvents( boost::json::array const& events, FDv2ProtocolHandler* protocol_handler, - FDv2ChangeSetTranslator const& translator, Logger const& logger) { for (auto const& event_val : events) { auto const* event_obj = event_val.if_object(); @@ -86,7 +85,7 @@ static FDv2SourceResult ParseFDv2PollEvents( if (auto* change_set = std::get_if(&result)) { - auto typed = translator.Translate(*change_set, logger); + auto typed = TranslateChangeSet(*change_set, logger); if (!typed) { return FDv2SourceResult{FDv2SourceResult::Interrupted{ MakeError(ErrorKind::kInvalidData, 0, kErrorTranslation), @@ -123,7 +122,6 @@ static FDv2SourceResult ParseFDv2PollEvents( static FDv2SourceResult ParseFDv2PollResponse( std::string const& body, FDv2ProtocolHandler* protocol_handler, - FDv2ChangeSetTranslator const& translator, Logger const& logger) { boost::system::error_code ec; auto parsed = boost::json::parse(body, ec); @@ -150,8 +148,7 @@ static FDv2SourceResult ParseFDv2PollResponse( MakeError(ErrorKind::kInvalidData, 0, kErrorMissingEvents), false}}; } - return ParseFDv2PollEvents(*events_arr, protocol_handler, translator, - logger); + return ParseFDv2PollEvents(*events_arr, protocol_handler, logger); } data_interfaces::FDv2SourceResult HandleFDv2PollResponse( @@ -184,9 +181,7 @@ data_interfaces::FDv2SourceResult HandleFDv2PollResponse( false}}; } - FDv2ChangeSetTranslator translator; - auto result = - ParseFDv2PollResponse(*body, protocol_handler, translator, logger); + auto result = ParseFDv2PollResponse(*body, protocol_handler, logger); if (auto* interrupted = std::get_if(&result.value)) { if (interrupted->error.Kind() == ErrorKind::kErrorResponse) { diff --git a/libs/server-sdk/tests/fdv2_changeset_translator_test.cpp b/libs/server-sdk/tests/fdv2_changeset_translation_test.cpp similarity index 79% rename from libs/server-sdk/tests/fdv2_changeset_translator_test.cpp rename to libs/server-sdk/tests/fdv2_changeset_translation_test.cpp index a5111ce39..f64bc359a 100644 --- a/libs/server-sdk/tests/fdv2_changeset_translator_test.cpp +++ b/libs/server-sdk/tests/fdv2_changeset_translation_test.cpp @@ -1,6 +1,6 @@ #include -#include +#include #include #include @@ -34,12 +34,11 @@ static Logger MakeNullLogger() { // kNone changeset // ============================================================================ -TEST(FDv2ChangeSetTranslatorTest, NoneChangeSetProducesEmptyTypedChangeSet) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, NoneChangeSetProducesEmptyTypedChangeSet) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ChangeSetType::kNone, {}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); EXPECT_EQ(result->type, ChangeSetType::kNone); @@ -50,15 +49,14 @@ TEST(FDv2ChangeSetTranslatorTest, NoneChangeSetProducesEmptyTypedChangeSet) { // Known kinds — put // ============================================================================ -TEST(FDv2ChangeSetTranslatorTest, PutFlagProducesTypedFlag) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, PutFlagProducesTypedFlag) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ChangeSetType::kFull, {FDv2Change{FDv2Change::ChangeType::kPut, "flag", "my-flag", 1, boost::json::parse(kFlagJson)}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); ASSERT_EQ(result->data.size(), 1u); @@ -67,8 +65,7 @@ TEST(FDv2ChangeSetTranslatorTest, PutFlagProducesTypedFlag) { std::holds_alternative>(result->data[0].object)); } -TEST(FDv2ChangeSetTranslatorTest, PutSegmentProducesTypedSegment) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, PutSegmentProducesTypedSegment) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ @@ -76,7 +73,7 @@ TEST(FDv2ChangeSetTranslatorTest, PutSegmentProducesTypedSegment) { {FDv2Change{FDv2Change::ChangeType::kPut, "segment", "my-seg", 2, boost::json::parse(kSegmentJson)}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); ASSERT_EQ(result->data.size(), 1u); @@ -89,15 +86,14 @@ TEST(FDv2ChangeSetTranslatorTest, PutSegmentProducesTypedSegment) { // Known kinds — delete // ============================================================================ -TEST(FDv2ChangeSetTranslatorTest, DeleteFlagProducesFlagTombstone) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, DeleteFlagProducesFlagTombstone) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ ChangeSetType::kPartial, {FDv2Change{FDv2Change::ChangeType::kDelete, "flag", "my-flag", 5, {}}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); ASSERT_EQ(result->data.size(), 1u); @@ -109,8 +105,7 @@ TEST(FDv2ChangeSetTranslatorTest, DeleteFlagProducesFlagTombstone) { EXPECT_FALSE(desc->item.has_value()); } -TEST(FDv2ChangeSetTranslatorTest, DeleteSegmentProducesSegmentTombstone) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, DeleteSegmentProducesSegmentTombstone) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ @@ -118,7 +113,7 @@ TEST(FDv2ChangeSetTranslatorTest, DeleteSegmentProducesSegmentTombstone) { {FDv2Change{ FDv2Change::ChangeType::kDelete, "segment", "my-seg", 3, {}}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); ASSERT_EQ(result->data.size(), 1u); @@ -133,8 +128,7 @@ TEST(FDv2ChangeSetTranslatorTest, DeleteSegmentProducesSegmentTombstone) { // Unknown kind — skipped // ============================================================================ -TEST(FDv2ChangeSetTranslatorTest, UnknownKindInPutIsSkipped) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, UnknownKindInPutIsSkipped) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ @@ -144,15 +138,14 @@ TEST(FDv2ChangeSetTranslatorTest, UnknownKindInPutIsSkipped) { FDv2Change{FDv2Change::ChangeType::kPut, "flag", "my-flag", 1, boost::json::parse(kFlagJson)}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); ASSERT_EQ(result->data.size(), 1u); EXPECT_EQ(result->data[0].key, "my-flag"); } -TEST(FDv2ChangeSetTranslatorTest, UnknownKindInDeleteIsSkipped) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, UnknownKindInDeleteIsSkipped) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ @@ -161,7 +154,7 @@ TEST(FDv2ChangeSetTranslatorTest, UnknownKindInDeleteIsSkipped) { FDv2Change::ChangeType::kDelete, "experiment", "exp-1", 1, {}}, FDv2Change{FDv2Change::ChangeType::kDelete, "flag", "my-flag", 2, {}}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); ASSERT_EQ(result->data.size(), 1u); @@ -172,15 +165,14 @@ TEST(FDv2ChangeSetTranslatorTest, UnknownKindInDeleteIsSkipped) { // Null object on put — skipped // ============================================================================ -TEST(FDv2ChangeSetTranslatorTest, NullObjectInPutFlagIsSkipped) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, NullObjectInPutFlagIsSkipped) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ChangeSetType::kFull, {FDv2Change{FDv2Change::ChangeType::kPut, "flag", "my-flag", 1, boost::json::value{nullptr}}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); EXPECT_TRUE(result->data.empty()); @@ -190,21 +182,19 @@ TEST(FDv2ChangeSetTranslatorTest, NullObjectInPutFlagIsSkipped) { // Deserialization failure — abort // ============================================================================ -TEST(FDv2ChangeSetTranslatorTest, MalformedFlagAbortsTranslation) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, MalformedFlagAbortsTranslation) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ChangeSetType::kFull, {FDv2Change{FDv2Change::ChangeType::kPut, "flag", "bad-flag", 1, boost::json::parse(R"({})")}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); EXPECT_FALSE(result.has_value()); } -TEST(FDv2ChangeSetTranslatorTest, MalformedSegmentAbortsTranslation) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, MalformedSegmentAbortsTranslation) { auto logger = MakeNullLogger(); // A non-empty object missing required fields triggers a schema failure @@ -214,7 +204,7 @@ TEST(FDv2ChangeSetTranslatorTest, MalformedSegmentAbortsTranslation) { {FDv2Change{FDv2Change::ChangeType::kPut, "segment", "bad-seg", 1, boost::json::parse(R"({"key":"bad-seg"})")}}, Selector{}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); EXPECT_FALSE(result.has_value()); } @@ -223,13 +213,12 @@ TEST(FDv2ChangeSetTranslatorTest, MalformedSegmentAbortsTranslation) { // Selector is preserved // ============================================================================ -TEST(FDv2ChangeSetTranslatorTest, SelectorIsPreserved) { - FDv2ChangeSetTranslator translator; +TEST(FDv2ChangeSetTranslationTest, SelectorIsPreserved) { auto logger = MakeNullLogger(); FDv2ChangeSet raw{ ChangeSetType::kFull, {}, Selector{Selector::State{7, "state-abc"}}}; - auto result = translator.Translate(raw, logger); + auto result = TranslateChangeSet(raw, logger); ASSERT_TRUE(result.has_value()); ASSERT_TRUE(result->selector.value.has_value()); From 09b2ef44982f9c6cef7e555bd61a28407bee7476 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 22 Apr 2026 10:24:08 -0700 Subject: [PATCH 03/13] refactor: split up a long function into a few smaller ones --- .../fdv2/fdv2_changeset_translation.cpp | 110 +++++++++++------- 1 file changed, 66 insertions(+), 44 deletions(-) diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp index 8e138ced4..639215a1b 100644 --- a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp @@ -17,6 +17,68 @@ using data_model::ChangeSet; using data_model::ChangeSetType; using data_model::FDv2ChangeSet; +static std::optional TranslateDelete( + data_model::FDv2Change const& change, + Logger const& logger) { + if (change.kind == "flag") { + return ItemChange{change.key, + data_model::ItemDescriptor{ + data_model::Tombstone{change.version}}}; + } + if (change.kind == "segment") { + return ItemChange{change.key, + data_model::ItemDescriptor{ + data_model::Tombstone{change.version}}}; + } + LD_LOG(logger, LogLevel::kWarn) << "FDv2: unknown kind '" << change.kind + << "' in delete-object, skipping"; + return std::nullopt; +} + +static bool TranslatePutFlag(data_model::FDv2Change const& change, + ChangeSetData* changes, + Logger const& logger) { + auto result = boost::json::value_to< + tl::expected, JsonError>>( + change.object); + if (!result) { + LD_LOG(logger, LogLevel::kError) + << "FDv2: could not deserialize flag '" << change.key << "'"; + return false; + } + if (!result->has_value()) { + LD_LOG(logger, LogLevel::kWarn) + << "FDv2: flag '" << change.key << "' object was null, skipping"; + return true; + } + changes->push_back(ItemChange{ + change.key, + data_model::ItemDescriptor{std::move(**result)}}); + return true; +} + +static bool TranslatePutSegment(data_model::FDv2Change const& change, + ChangeSetData* changes, + Logger const& logger) { + auto result = boost::json::value_to< + tl::expected, JsonError>>( + change.object); + if (!result) { + LD_LOG(logger, LogLevel::kError) + << "FDv2: could not deserialize segment '" << change.key << "'"; + return false; + } + if (!result->has_value()) { + LD_LOG(logger, LogLevel::kWarn) + << "FDv2: segment '" << change.key << "' object was null, skipping"; + return true; + } + changes->push_back(ItemChange{ + change.key, + data_model::ItemDescriptor{std::move(**result)}}); + return true; +} + std::optional> TranslateChangeSet( FDv2ChangeSet const& change_set, Logger const& logger) { @@ -30,58 +92,18 @@ std::optional> TranslateChangeSet( for (auto const& change : change_set.changes) { if (change.change_type == data_model::FDv2Change::ChangeType::kDelete) { - if (change.kind == "flag") { - changes.push_back(ItemChange{ - change.key, data_model::ItemDescriptor{ - data_model::Tombstone{change.version}}}); - } else if (change.kind == "segment") { - changes.push_back(ItemChange{ - change.key, data_model::ItemDescriptor{ - data_model::Tombstone{change.version}}}); - } else { - LD_LOG(logger, LogLevel::kWarn) - << "FDv2: unknown kind '" << change.kind - << "' in delete-object, skipping"; + if (auto item = TranslateDelete(change, logger)) { + changes.push_back(std::move(*item)); } } else { if (change.kind == "flag") { - auto result = boost::json::value_to< - tl::expected, JsonError>>( - change.object); - if (!result) { - LD_LOG(logger, LogLevel::kError) - << "FDv2: could not deserialize flag '" << change.key - << "'"; + if (!TranslatePutFlag(change, &changes, logger)) { return std::nullopt; } - if (!result->has_value()) { - LD_LOG(logger, LogLevel::kWarn) - << "FDv2: flag '" << change.key - << "' object was null, skipping"; - continue; - } - changes.push_back(ItemChange{ - change.key, data_model::ItemDescriptor{ - std::move(**result)}}); } else if (change.kind == "segment") { - auto result = boost::json::value_to, JsonError>>( - change.object); - if (!result) { - LD_LOG(logger, LogLevel::kError) - << "FDv2: could not deserialize segment '" << change.key - << "'"; + if (!TranslatePutSegment(change, &changes, logger)) { return std::nullopt; } - if (!result->has_value()) { - LD_LOG(logger, LogLevel::kWarn) - << "FDv2: segment '" << change.key - << "' object was null, skipping"; - continue; - } - changes.push_back(ItemChange{ - change.key, data_model::ItemDescriptor{ - std::move(**result)}}); } else { LD_LOG(logger, LogLevel::kWarn) << "FDv2: unknown kind '" << change.kind From 309eb037ead8c5ec523a533afd9f3d50ee1cc241 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 22 Apr 2026 14:18:12 -0700 Subject: [PATCH 04/13] refactor: restore original parameter order --- .../include/launchdarkly/data_model/change_set.hpp | 2 +- .../fdv2/fdv2_changeset_translation.cpp | 6 +++--- .../src/data_systems/fdv2/fdv2_polling_impl.cpp | 2 +- libs/server-sdk/tests/memory_store_apply_test.cpp | 14 +++++++------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/libs/internal/include/launchdarkly/data_model/change_set.hpp b/libs/internal/include/launchdarkly/data_model/change_set.hpp index d720e39bc..972c24f4e 100644 --- a/libs/internal/include/launchdarkly/data_model/change_set.hpp +++ b/libs/internal/include/launchdarkly/data_model/change_set.hpp @@ -13,8 +13,8 @@ enum class ChangeSetType { template struct ChangeSet { ChangeSetType type; - Selector selector; T data; + Selector selector; }; } // namespace launchdarkly::data_model diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp index 639215a1b..6e7b700b2 100644 --- a/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp @@ -84,7 +84,7 @@ std::optional> TranslateChangeSet( Logger const& logger) { if (change_set.type == ChangeSetType::kNone) { return ChangeSet{ - change_set.type, change_set.selector, {}}; + change_set.type, {}, change_set.selector}; } ChangeSetData changes; @@ -112,8 +112,8 @@ std::optional> TranslateChangeSet( } } - return ChangeSet{change_set.type, change_set.selector, - std::move(changes)}; + return ChangeSet{change_set.type, std::move(changes), + change_set.selector}; } } // namespace launchdarkly::server_side::data_systems diff --git a/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp b/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp index 4deb66d70..54b9dc867 100644 --- a/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp +++ b/libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp @@ -168,7 +168,7 @@ data_interfaces::FDv2SourceResult HandleFDv2PollResponse( if (res.Status() == 304) { return FDv2SourceResult{FDv2SourceResult::ChangeSet{ data_model::ChangeSet{ - data_model::ChangeSetType::kNone, data_model::Selector{}, {}}, + data_model::ChangeSetType::kNone, {}, data_model::Selector{}}, false}}; } diff --git a/libs/server-sdk/tests/memory_store_apply_test.cpp b/libs/server-sdk/tests/memory_store_apply_test.cpp index 00ecf62b4..e853f9808 100644 --- a/libs/server-sdk/tests/memory_store_apply_test.cpp +++ b/libs/server-sdk/tests/memory_store_apply_test.cpp @@ -31,7 +31,7 @@ TEST(MemoryStoreApplyTest, ApplyNone_IsNoOp) { {"segA", SegmentDescriptor(seg_a)}}, }); - store.Apply(ChangeSet{ChangeSetType::kNone, Selector{}, {}}); + store.Apply(ChangeSet{ChangeSetType::kNone, {}, Selector{}}); auto fetched_flag = store.GetFlag("flagA"); ASSERT_TRUE(fetched_flag); @@ -43,7 +43,7 @@ TEST(MemoryStoreApplyTest, ApplyNone_IsNoOp) { TEST(MemoryStoreApplyTest, ApplyNone_DoesNotInitialize) { MemoryStore store; - store.Apply(ChangeSet{ChangeSetType::kNone, Selector{}, {}}); + store.Apply(ChangeSet{ChangeSetType::kNone, {}, Selector{}}); EXPECT_FALSE(store.Initialized()); } @@ -54,7 +54,7 @@ TEST(MemoryStoreApplyTest, ApplyNone_DoesNotInitialize) { TEST(MemoryStoreApplyTest, ApplyFull_SetsInitialized) { MemoryStore store; ASSERT_FALSE(store.Initialized()); - store.Apply(ChangeSet{ChangeSetType::kFull, Selector{}, {}}); + store.Apply(ChangeSet{ChangeSetType::kFull, {}, Selector{}}); EXPECT_TRUE(store.Initialized()); } @@ -70,9 +70,9 @@ TEST(MemoryStoreApplyTest, ApplyFull_StoresItems) { store.Apply(ChangeSet{ ChangeSetType::kFull, - Selector{}, ChangeSetData{ItemChange{"flagA", FlagDescriptor(flag_a)}, ItemChange{"segA", SegmentDescriptor(seg_a)}}, + Selector{}, }); auto fetched_flag = store.GetFlag("flagA"); @@ -120,9 +120,9 @@ TEST(MemoryStoreApplyTest, ApplyFull_ClearsExistingItems) { store.Apply(ChangeSet{ ChangeSetType::kFull, - Selector{}, ChangeSetData{ItemChange{"flagC", FlagDescriptor(flag_c)}, ItemChange{"segB", SegmentDescriptor(seg_b)}}, + Selector{}, }); EXPECT_FALSE(store.GetFlag("flagA")); @@ -163,9 +163,9 @@ TEST(MemoryStoreApplyTest, ApplyPartial_AppliesItems) { store.Apply(ChangeSet{ ChangeSetType::kPartial, - Selector{}, ChangeSetData{ItemChange{"flagA", FlagDescriptor(flag_a_new)}, ItemChange{"segA", SegmentDescriptor(seg_a_new)}}, + Selector{}, }); ASSERT_TRUE(store.GetFlag("flagA")); @@ -211,9 +211,9 @@ TEST(MemoryStoreApplyTest, ApplyPartial_PreservesUnchangedItems) { store.Apply(ChangeSet{ ChangeSetType::kPartial, - Selector{}, ChangeSetData{ItemChange{"flagB", FlagDescriptor(flag_b_new)}, ItemChange{"segB", SegmentDescriptor(seg_b_new)}}, + Selector{}, }); ASSERT_TRUE(store.GetFlag("flagA")); From 5b93349c52c1546d6599cc06c6435209c8bbd176 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 22 Apr 2026 16:47:56 -0700 Subject: [PATCH 05/13] fix: fix the build on linux --- libs/server-sdk/tests/fdv2_changeset_translation_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/server-sdk/tests/fdv2_changeset_translation_test.cpp b/libs/server-sdk/tests/fdv2_changeset_translation_test.cpp index f64bc359a..bc8a086f2 100644 --- a/libs/server-sdk/tests/fdv2_changeset_translation_test.cpp +++ b/libs/server-sdk/tests/fdv2_changeset_translation_test.cpp @@ -170,7 +170,7 @@ TEST(FDv2ChangeSetTranslationTest, NullObjectInPutFlagIsSkipped) { FDv2ChangeSet raw{ChangeSetType::kFull, {FDv2Change{FDv2Change::ChangeType::kPut, "flag", - "my-flag", 1, boost::json::value{nullptr}}}, + "my-flag", 1, boost::json::value(nullptr)}}, Selector{}}; auto result = TranslateChangeSet(raw, logger); From ea05ff406c9d523c8beb9bb96cc21f5c93d8b1f2 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 12:25:26 -0700 Subject: [PATCH 06/13] feat(sse): add Builder::on_connect hook for per-connect request mutation --- .../include/launchdarkly/sse/client.hpp | 35 +- libs/server-sent-events/src/client.cpp | 29 +- libs/server-sent-events/src/curl_client.cpp | 430 +++++----- libs/server-sent-events/src/curl_client.hpp | 140 ++-- .../tests/curl_client_test.cpp | 791 ++++++++++++------ 5 files changed, 895 insertions(+), 530 deletions(-) diff --git a/libs/server-sent-events/include/launchdarkly/sse/client.hpp b/libs/server-sent-events/include/launchdarkly/sse/client.hpp index 500467fd0..709b2b40e 100644 --- a/libs/server-sent-events/include/launchdarkly/sse/client.hpp +++ b/libs/server-sent-events/include/launchdarkly/sse/client.hpp @@ -34,6 +34,8 @@ class Builder { using EventReceiver = std::function; using LogCallback = std::function; using ErrorCallback = std::function; + using ConnectionHook = + std::function*)>; /** * Create a builder for the given URL. If the port is omitted, 443 is @@ -161,18 +163,37 @@ class Builder { * - HTTP proxies: "http://proxy:port" * - HTTPS proxies: "https://proxy:port" * - SOCKS4 proxies: "socks4://proxy:port" - * - SOCKS5 proxies: "socks5://proxy:port" or "socks5://user:pass@proxy:port" + * - SOCKS5 proxies: "socks5://proxy:port" or + * "socks5://user:pass@proxy:port" * - SOCKS5 with DNS through proxy: "socks5h://proxy:port" * - * Passing an empty string explicitly disables proxy (overrides environment variables). - * Passing std::nullopt (or not calling this method) uses environment variables. + * Passing an empty string explicitly disables proxy (overrides environment + * variables). Passing std::nullopt (or not calling this method) uses + * environment variables. * - * @param url Proxy URL, empty string to disable, or std::nullopt for environment variables + * @param url Proxy URL, empty string to disable, or std::nullopt for + * environment variables * @return Reference to this builder. - * @throws std::runtime_error if proxy is configured without CURL networking support + * @throws std::runtime_error if proxy is configured without CURL networking + * support */ Builder& proxy(std::optional url); + /** + * Register a hook invoked immediately before each connection attempt, + * including the initial connect and every reconnect. The hook receives + * a mutable request seeded with the Builder's defaults; it may modify + * the request's target, headers, method, and body. + * + * Host, port, and scheme are fixed at build time and cannot be changed + * by the hook. + * + * @param hook Callback invoked with a mutable request before each + * connection attempt. + * @return Reference to this builder. + */ + Builder& on_connect(ConnectionHook hook); + /** * Builds a Client. The shared pointer is necessary to extend the lifetime * of the Client to encompass each asynchronous operation that it performs. @@ -195,6 +216,7 @@ class Builder { bool skip_verify_peer_; std::optional custom_ca_file_; std::optional proxy_url_; + ConnectionHook connection_hook_; }; /** @@ -213,7 +235,8 @@ class Client { * when the SDK detects invalid data from the stream and needs to * reconnect. The backoff mechanism prevents rapid reconnection attempts * that could overload the service. - * @param reason A description of why the restart was triggered (for logging) + * @param reason A description of why the restart was triggered (for + * logging) */ virtual void async_restart(std::string const& reason) = 0; }; diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index 99e8fdb10..caf96d410 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -74,6 +74,7 @@ class FoxyClient : public Client, Builder::EventReceiver receiver, Builder::LogCallback logger, Builder::ErrorCallback errors, + Builder::ConnectionHook connection_hook, std::optional maybe_ssl) : ssl_context_(std::move(maybe_ssl)), host_(std::move(host)), @@ -85,6 +86,7 @@ class FoxyClient : public Client, event_receiver_(std::move(receiver)), logger_(std::move(logger)), errors_(std::move(errors)), + connection_hook_(std::move(connection_hook)), body_parser_(std::nullopt), session_(std::nullopt), last_event_id_(std::nullopt), @@ -185,6 +187,9 @@ class FoxyClient : public Client, } void do_run() { + if (connection_hook_) { + connection_hook_(&req_); + } session_->async_connect( host_, port_, beast::bind_front_handler(&FoxyClient::on_connect, @@ -496,6 +501,11 @@ class FoxyClient : public Client, // which the client communicates error conditions to the user. Builder::ErrorCallback errors_; + // Optional hook invoked before each connection attempt with a mutable + // request, allowing the caller to vary the request per connect (e.g. + // updating query parameters from external state). + Builder::ConnectionHook connection_hook_; + // Customized parser (see parser.hpp) which repeatedly receives chunks of // data and parses them into SSE events. It cannot be reused across // connections, hence the optional so it can be destroyed easily. @@ -624,6 +634,11 @@ Builder& Builder::proxy(std::optional url) { return *this; } +Builder& Builder::on_connect(ConnectionHook hook) { + connection_hook_ = std::move(hook); + return *this; +} + std::shared_ptr Builder::build() { auto uri_components = boost::urls::parse_uri(url_); if (!uri_components) { @@ -667,12 +682,12 @@ std::shared_ptr Builder::build() { : uri_components->scheme(); #ifdef LD_CURL_NETWORKING - bool use_https = uri_components->scheme_id() == boost::urls::scheme::https; - return std::make_shared( - net::make_strand(executor_), request, host, service, - connect_timeout_, read_timeout_, write_timeout_, - initial_reconnect_delay_, receiver_, logging_cb_, error_cb_, - skip_verify_peer_, custom_ca_file_, use_https, proxy_url_); + bool use_https = uri_components->scheme_id() == boost::urls::scheme::https; + return std::make_shared( + net::make_strand(executor_), request, host, service, connect_timeout_, + read_timeout_, write_timeout_, initial_reconnect_delay_, receiver_, + logging_cb_, error_cb_, connection_hook_, skip_verify_peer_, + custom_ca_file_, use_https, proxy_url_); #else std::optional ssl; if (uri_components->scheme_id() == boost::urls::scheme::https) { @@ -694,7 +709,7 @@ std::shared_ptr Builder::build() { return std::make_shared( net::make_strand(executor_), request, host, service, connect_timeout_, read_timeout_, write_timeout_, initial_reconnect_delay_, receiver_, - logging_cb_, error_cb_, std::move(ssl)); + logging_cb_, error_cb_, connection_hook_, std::move(ssl)); #endif } diff --git a/libs/server-sent-events/src/curl_client.cpp b/libs/server-sent-events/src/curl_client.cpp index c99b6830a..1e08f9495 100644 --- a/libs/server-sent-events/src/curl_client.cpp +++ b/libs/server-sent-events/src/curl_client.cpp @@ -28,41 +28,37 @@ constexpr auto kCurlTransferAbort = 1; constexpr auto kHttpStatusCodeMovedPermanently = 301; constexpr auto kHttpStatusCodeTemporaryRedirect = 307; -CurlClient::CurlClient(boost::asio::any_io_executor executor, - http::request req, - std::string host, - std::string port, - std::optional connect_timeout, - std::optional read_timeout, - std::optional write_timeout, - std::optional - initial_reconnect_delay, - Builder::EventReceiver receiver, - Builder::LogCallback logger, - Builder::ErrorCallback errors, - bool skip_verify_peer, - std::optional custom_ca_file, - bool use_https, - std::optional proxy_url) - : - host_(std::move(host)), - port_(std::move(port)), - event_receiver_(std::move(receiver)), - logger_(std::move(logger)), - errors_(std::move(errors)), - use_https_(use_https), - backoff_timer_(executor), - multi_manager_(CurlMultiManager::create(executor)), - backoff_(initial_reconnect_delay.value_or(kDefaultInitialReconnectDelay), - kDefaultMaxBackoffDelay) { +CurlClient::CurlClient( + boost::asio::any_io_executor executor, + http::request req, + std::string host, + std::string port, + std::optional connect_timeout, + std::optional read_timeout, + std::optional write_timeout, + std::optional initial_reconnect_delay, + Builder::EventReceiver receiver, + Builder::LogCallback logger, + Builder::ErrorCallback errors, + Builder::ConnectionHook connection_hook, + bool skip_verify_peer, + std::optional custom_ca_file, + bool use_https, + std::optional proxy_url) + : host_(std::move(host)), + port_(std::move(port)), + event_receiver_(std::move(receiver)), + logger_(std::move(logger)), + errors_(std::move(errors)), + connection_hook_(std::move(connection_hook)), + use_https_(use_https), + backoff_timer_(executor), + multi_manager_(CurlMultiManager::create(executor)), + backoff_(initial_reconnect_delay.value_or(kDefaultInitialReconnectDelay), + kDefaultMaxBackoffDelay) { request_context_ = std::make_shared( - build_url(req), - std::move(req), - connect_timeout, - read_timeout, - write_timeout, - std::move(custom_ca_file), - std::move(proxy_url), + build_url(req), std::move(req), connect_timeout, read_timeout, + write_timeout, std::move(custom_ca_file), std::move(proxy_url), skip_verify_peer); } @@ -77,14 +73,14 @@ void CurlClient::async_connect() { } void CurlClient::async_restart(std::string const& reason) { - boost::asio::post(backoff_timer_.get_executor(), - [self = shared_from_this(), reason]() { - // Close the socket to abort the current transfer. - // CURL will detect the error and call the completion - // handler, which will trigger backoff and reconnection. - self->log_message("async_restart: aborting transfer due to " + reason); - self->request_context_->abort_transfer(); - }); + boost::asio::post(backoff_timer_.get_executor(), [self = shared_from_this(), + reason]() { + // Close the socket to abort the current transfer. + // CURL will detect the error and call the completion + // handler, which will trigger backoff and reconnection. + self->log_message("async_restart: aborting transfer due to " + reason); + self->request_context_->abort_transfer(); + }); } void CurlClient::do_run() { @@ -92,59 +88,56 @@ void CurlClient::do_run() { return; } + if (connection_hook_) { + connection_hook_(&request_context_->req); + request_context_->url = build_url(request_context_->req); + } + auto ctx = request_context_; auto weak_self = weak_from_this(); std::weak_ptr weak_ctx = ctx; - ctx->set_callbacks(Callbacks([weak_self, weak_ctx](const std::string& message) { - if (auto ctx = weak_ctx.lock()) { - if (auto self = weak_self.lock()) { - boost::asio::post( - self->backoff_timer_. - get_executor(), - [self, message]() { - self->async_backoff(message); - }); - } - } - }, - [weak_self, weak_ctx](const Event& event) { - if (auto ctx = weak_ctx.lock()) { - if (auto self = weak_self.lock()) { - boost::asio::post( - self->backoff_timer_. - get_executor(), - [self, event]() { - self->event_receiver_(event); - }); - } - } - }, - [weak_self, weak_ctx](const Error& error) { - if (auto ctx = weak_ctx.lock()) { - if (const auto self = weak_self.lock()) { - // report_error does an asio post. - self->report_error(error); - } - } - }, - [weak_self, weak_ctx]() { - if (auto ctx = weak_ctx.lock()) { - if (const auto self = weak_self.lock()) { - boost::asio::post( - self->backoff_timer_. - get_executor(), - [self]() { - self->backoff_.succeed(); - }); - } - } - }, [weak_self, weak_ctx](const std::string& message) { - if (auto ctx = weak_ctx.lock()) { - if (const auto self = weak_self.lock()) { - self->log_message(message); - } - } - })); + ctx->set_callbacks(Callbacks( + [weak_self, weak_ctx](std::string const& message) { + if (auto ctx = weak_ctx.lock()) { + if (auto self = weak_self.lock()) { + boost::asio::post( + self->backoff_timer_.get_executor(), + [self, message]() { self->async_backoff(message); }); + } + } + }, + [weak_self, weak_ctx](Event const& event) { + if (auto ctx = weak_ctx.lock()) { + if (auto self = weak_self.lock()) { + boost::asio::post( + self->backoff_timer_.get_executor(), + [self, event]() { self->event_receiver_(event); }); + } + } + }, + [weak_self, weak_ctx](Error const& error) { + if (auto ctx = weak_ctx.lock()) { + if (auto const self = weak_self.lock()) { + // report_error does an asio post. + self->report_error(error); + } + } + }, + [weak_self, weak_ctx]() { + if (auto ctx = weak_ctx.lock()) { + if (auto const self = weak_self.lock()) { + boost::asio::post(self->backoff_timer_.get_executor(), + [self]() { self->backoff_.succeed(); }); + } + } + }, + [weak_self, weak_ctx](std::string const& message) { + if (auto ctx = weak_ctx.lock()) { + if (auto const self = weak_self.lock()) { + self->log_message(message); + } + } + })); // Start request using CURL multi (non-blocking) PerformRequestWithMulti(multi_manager_, ctx); } @@ -155,14 +148,14 @@ void CurlClient::async_backoff(std::string const& reason) { std::stringstream msg; msg << "backing off in (" << std::chrono::duration_cast(backoff_.delay()) - .count() + .count() << ") seconds due to " << reason; log_message(msg.str()); auto weak_self = weak_from_this(); backoff_timer_.expires_after(backoff_.delay()); - backoff_timer_.async_wait([weak_self](const boost::system::error_code& ec) { + backoff_timer_.async_wait([weak_self](boost::system::error_code const& ec) { if (auto self = weak_self.lock()) { self->on_backoff(ec); } @@ -177,13 +170,14 @@ void CurlClient::on_backoff(boost::system::error_code const& ec) { } std::string CurlClient::build_url( - const http::request& req) const { - const std::string scheme = use_https_ ? "https" : "http"; + http::request const& req) const { + std::string const scheme = use_https_ ? "https" : "http"; std::string url = scheme + "://" + host_; // Add port if it's not the default service name - // port_ can be either a port number (like "8123") or service name (like "https"/"http") + // port_ can be either a port number (like "8123") or service name (like + // "https"/"http") if (port_ != "https" && port_ != "http") { url += ":" + port_; } @@ -198,15 +192,15 @@ bool CurlClient::SetupCurlOptions(CURL* curl, RequestContext& context) { // Helper macro to check curl_easy_setopt return values // Returns false on error to signal setup failure -#define CURL_SETOPT_CHECK(handle, option, parameter) \ - do { \ - CURLcode code = curl_easy_setopt(handle, option, parameter); \ - if (code != CURLE_OK) { \ - context.log_message("curl_easy_setopt failed for " #option ": " + \ - std::string(curl_easy_strerror(code))); \ - return false; \ - } \ - } while(0) +#define CURL_SETOPT_CHECK(handle, option, parameter) \ + do { \ + CURLcode code = curl_easy_setopt(handle, option, parameter); \ + if (code != CURLE_OK) { \ + context.log_message("curl_easy_setopt failed for " #option ": " + \ + std::string(curl_easy_strerror(code))); \ + return false; \ + } \ + } while (0) // Set URL CURL_SETOPT_CHECK(curl, CURLOPT_URL, context.url.c_str()); @@ -229,8 +223,7 @@ bool CurlClient::SetupCurlOptions(CURL* curl, // Set request body if present if (!context.req.body().empty()) { - CURL_SETOPT_CHECK(curl, CURLOPT_POSTFIELDS, - context.req.body().c_str()); + CURL_SETOPT_CHECK(curl, CURLOPT_POSTFIELDS, context.req.body().c_str()); CURL_SETOPT_CHECK(curl, CURLOPT_POSTFIELDSIZE, context.req.body().size()); } @@ -245,7 +238,8 @@ bool CurlClient::SetupCurlOptions(CURL* curl, // Add Last-Event-ID if we have one from previous connection if (context.last_event_id && !context.last_event_id->empty()) { - std::string last_event_header = "Last-Event-ID: " + *context.last_event_id; + std::string last_event_header = + "Last-Event-ID: " + *context.last_event_id; headers = curl_slist_append(headers, last_event_header.c_str()); } @@ -288,7 +282,8 @@ bool CurlClient::SetupCurlOptions(CURL* curl, if (context.proxy_url) { CURL_SETOPT_CHECK(curl, CURLOPT_PROXY, context.proxy_url->c_str()); } - // If proxy_url_ is std::nullopt, CURL will use environment variables (default behavior) + // If proxy_url_ is std::nullopt, CURL will use environment variables + // (default behavior) // Set callbacks CURL_SETOPT_CHECK(curl, CURLOPT_WRITEFUNCTION, WriteCallback); @@ -324,7 +319,7 @@ int CurlClient::ProgressCallback(void* clientp, // Check if we've exceeded the read timeout if (context->read_timeout) { - const auto now = std::chrono::steady_clock::now(); + auto const now = std::chrono::steady_clock::now(); // If download amount has changed, update the last progress time if (dlnow != context->last_download_amount) { @@ -332,9 +327,9 @@ int CurlClient::ProgressCallback(void* clientp, context->last_progress_time = now; } else { // No new data - check if we've exceeded the timeout - const auto elapsed = std::chrono::duration_cast< - std::chrono::milliseconds>( - now - context->last_progress_time); + auto const elapsed = + std::chrono::duration_cast( + now - context->last_progress_time); if (elapsed > *context->read_timeout) { return kCurlTransferAbort; @@ -350,12 +345,12 @@ int CurlClient::ProgressCallback(void* clientp, // https://curl.se/libcurl/c/CURLOPT_OPENSOCKETFUNCTION.html curl_socket_t CurlClient::OpenSocketCallback(void* clientp, curlsocktype purpose, - const curl_sockaddr* address) { + curl_sockaddr const* address) { auto* context = static_cast(clientp); // Create the socket - curl_socket_t sockfd = socket(address->family, address->socktype, - address->protocol); + curl_socket_t sockfd = + socket(address->family, address->socktype, address->protocol); // Store it so we can close it during shutdown if (sockfd != CURL_SOCKET_BAD) { @@ -368,7 +363,7 @@ curl_socket_t CurlClient::OpenSocketCallback(void* clientp, // Callback for writing response data // // https://curl.se/libcurl/c/CURLOPT_WRITEFUNCTION.html -size_t CurlClient::WriteCallback(const char* data, +size_t CurlClient::WriteCallback(char const* data, size_t size, size_t nmemb, void* userp) { @@ -376,7 +371,7 @@ size_t CurlClient::WriteCallback(const char* data, auto* context = static_cast(userp); if (context->is_shutting_down()) { - return 0; // Abort the transfer + return 0; // Abort the transfer } // Set up the event receiver callback for the parser @@ -388,7 +383,7 @@ size_t CurlClient::WriteCallback(const char* data, context->receive(std::move(event)); }); - const std::string_view data_view(data, total_size); + std::string_view const data_view(data, total_size); context->parser_reader->put(data_view); return total_size; @@ -397,15 +392,15 @@ size_t CurlClient::WriteCallback(const char* data, // Callback for reading request headers // // https://curl.se/libcurl/c/CURLOPT_HEADERFUNCTION.html -size_t CurlClient::HeaderCallback(const char* buffer, +size_t CurlClient::HeaderCallback(char const* buffer, size_t size, size_t nitems, void* userdata) { - const size_t total_size = size * nitems; + size_t const total_size = size * nitems; auto* context = static_cast(userdata); // Check for Content-Type header - if (const std::string header(buffer, total_size); + if (std::string const header(buffer, total_size); header.find("Content-Type:") == 0 || header.find("content-type:") == 0) { if (header.find("text/event-stream") == std::string::npos) { @@ -419,15 +414,15 @@ size_t CurlClient::HeaderCallback(const char* buffer, void CurlClient::PerformRequestWithMulti( std::shared_ptr multi_manager, std::shared_ptr context) { - if (context->is_shutting_down()) { return; } - // Initialize parser for new connection (last_event_id is tracked separately) + // Initialize parser for new connection (last_event_id is tracked + // separately) context->init_parser(); - std::shared_ptrcurl (curl_easy_init(), curl_easy_cleanup); + std::shared_ptr curl(curl_easy_init(), curl_easy_cleanup); if (!curl) { if (context->is_shutting_down()) { return; @@ -439,7 +434,8 @@ void CurlClient::PerformRequestWithMulti( curl_slist* headers = nullptr; if (!SetupCurlOptions(curl.get(), &headers, *context)) { - // setup_curl_options returned false, indicating an error (it already logged the error) + // setup_curl_options returned false, indicating an error (it already + // logged the error) if (context->is_shutting_down()) { return; @@ -452,112 +448,123 @@ void CurlClient::PerformRequestWithMulti( // Add handle to multi manager for async processing // Headers will be freed automatically by CurlMultiManager std::weak_ptr weak_context = context; - multi_manager->add_handle(curl, headers, [weak_context](std::shared_ptr easy, CurlMultiManager::Result result) { - auto context = weak_context.lock(); - if (!context) { - return; - } - - // Check if this was a read timeout from the multi manager - if (result.type == CurlMultiManager::Result::Type::ReadTimeout) { - if (!context->is_shutting_down()) { - context->error(errors::ReadTimeout{context->read_timeout}); - context->backoff("read timeout - no data received"); + multi_manager->add_handle( + curl, headers, + [weak_context](std::shared_ptr easy, + CurlMultiManager::Result result) { + auto context = weak_context.lock(); + if (!context) { + return; } - return; - } - // Handle CURLcode result - CURLcode res = result.curl_code; - - // Get response code - long response_code = 0; - curl_easy_getinfo(easy.get(), CURLINFO_RESPONSE_CODE, &response_code); - - // Handle HTTP status codes - auto status = static_cast(response_code); - auto status_class = http::to_status_class(status); + // Check if this was a read timeout from the multi manager + if (result.type == CurlMultiManager::Result::Type::ReadTimeout) { + if (!context->is_shutting_down()) { + context->error(errors::ReadTimeout{context->read_timeout}); + context->backoff("read timeout - no data received"); + } + return; + } + // Handle CURLcode result + CURLcode res = result.curl_code; - if (context->is_shutting_down()) { - return; - } + // Get response code + long response_code = 0; + curl_easy_getinfo(easy.get(), CURLINFO_RESPONSE_CODE, + &response_code); - if (status_class == http::status_class::redirection) { - // The internal CURL handling of redirects failed. - // This situation is likely the result of a missing redirect header - // or empty header. - context->error(errors::NotRedirectable{}); - return; - } + // Handle HTTP status codes + auto status = static_cast(response_code); + auto status_class = http::to_status_class(status); - // Handle result - if (res != CURLE_OK) { if (context->is_shutting_down()) { return; } - // Check if the error was due to progress callback aborting (read timeout) - if (res == CURLE_ABORTED_BY_CALLBACK && context->read_timeout) { - context->error(errors::ReadTimeout{context->read_timeout}); - context->backoff("aborting read of response body (timeout)"); - } else { - std::string error_msg = "CURL error: " + std::string(curl_easy_strerror(res)); - context->backoff(error_msg); + if (status_class == http::status_class::redirection) { + // The internal CURL handling of redirects failed. + // This situation is likely the result of a missing redirect + // header or empty header. + context->error(errors::NotRedirectable{}); + return; } - return; - } + // Handle result + if (res != CURLE_OK) { + if (context->is_shutting_down()) { + return; + } - if (status_class == http::status_class::successful) { - if (status == http::status::no_content) { - if (!context->is_shutting_down()) { - context->error(errors::UnrecoverableClientError{http::status::no_content}); + // Check if the error was due to progress callback aborting + // (read timeout) + if (res == CURLE_ABORTED_BY_CALLBACK && context->read_timeout) { + context->error(errors::ReadTimeout{context->read_timeout}); + context->backoff( + "aborting read of response body (timeout)"); + } else { + std::string error_msg = + "CURL error: " + std::string(curl_easy_strerror(res)); + context->backoff(error_msg); } + return; } - context->reset_backoff(); - // Connection ended normally, reconnect - if (!context->is_shutting_down()) { - context->backoff("connection closed normally"); + + if (status_class == http::status_class::successful) { + if (status == http::status::no_content) { + if (!context->is_shutting_down()) { + context->error(errors::UnrecoverableClientError{ + http::status::no_content}); + } + return; + } + context->reset_backoff(); + // Connection ended normally, reconnect + if (!context->is_shutting_down()) { + context->backoff("connection closed normally"); + } + return; } - return; - } - if (status_class == http::status_class::client_error) { - if (!context->is_shutting_down()) { - bool recoverable = (status == http::status::bad_request || - status == http::status::request_timeout || - status == http::status::too_many_requests); - - if (recoverable) { - std::stringstream ss; - ss << "HTTP status " << static_cast(status); - context->backoff(ss.str()); - } else { - context->error(errors::UnrecoverableClientError{status}); + if (status_class == http::status_class::client_error) { + if (!context->is_shutting_down()) { + bool recoverable = + (status == http::status::bad_request || + status == http::status::request_timeout || + status == http::status::too_many_requests); + + if (recoverable) { + std::stringstream ss; + ss << "HTTP status " << static_cast(status); + context->backoff(ss.str()); + } else { + context->error( + errors::UnrecoverableClientError{status}); + } } + return; } - return; - } - // Server error or other - backoff and retry - if (!context->is_shutting_down()) { - std::stringstream ss; - ss << "HTTP status " << static_cast(status); - context->backoff(ss.str()); - } - }, context->read_timeout); + // Server error or other - backoff and retry + if (!context->is_shutting_down()) { + std::stringstream ss; + ss << "HTTP status " << static_cast(status); + context->backoff(ss.str()); + } + }, + context->read_timeout); } void CurlClient::async_shutdown(std::function completion) { - boost::asio::post(backoff_timer_.get_executor(), [self = shared_from_this(), - completion = std::move(completion)]() { - self->do_shutdown(completion); - }); + boost::asio::post( + backoff_timer_.get_executor(), + [self = shared_from_this(), completion = std::move(completion)]() { + self->do_shutdown(completion); + }); } -void CurlClient::do_shutdown(const std::function& completion) { +void CurlClient::do_shutdown(std::function const& completion) { request_context_->shutdown(); backoff_timer_.cancel(); @@ -572,11 +579,10 @@ void CurlClient::log_message(std::string const& message) { } void CurlClient::report_error(Error error) { - boost::asio::post(backoff_timer_.get_executor(), - [errors = errors_, error = std::move(error)]() { - errors(error); - }); + boost::asio::post( + backoff_timer_.get_executor(), + [errors = errors_, error = std::move(error)]() { errors(error); }); } -} // namespace launchdarkly::sse +} // namespace launchdarkly::sse #endif // LD_CURL_NETWORKING \ No newline at end of file diff --git a/libs/server-sent-events/src/curl_client.hpp b/libs/server-sent-events/src/curl_client.hpp index e210791d1..330681c7f 100644 --- a/libs/server-sent-events/src/curl_client.hpp +++ b/libs/server-sent-events/src/curl_client.hpp @@ -2,8 +2,8 @@ #ifdef LD_CURL_NETWORKING -#include #include +#include #include "backoff.hpp" #include "parser.hpp" @@ -38,11 +38,11 @@ using launchdarkly::network::CurlMultiManager; class CurlClient final : public Client, public std::enable_shared_from_this { /** - * Structure containing callbacks between the CURL interactions and the - * IO executor. Callbacks are set while a connection is being established, - * instead of at construction time, to allow the use of weak_from_self. - * The weak_from_self method cannot be used during the constructor. - */ + * Structure containing callbacks between the CURL interactions and the + * IO executor. Callbacks are set while a connection is being established, + * instead of at construction time, to allow the use of weak_from_self. + * The weak_from_self method cannot be used during the constructor. + */ struct Callbacks { std::function do_backoff; std::function on_receive; @@ -50,31 +50,28 @@ class CurlClient final : public Client, std::function reset_backoff; std::function log_message; - Callbacks( - std::function do_backoff, - std::function on_receive, - std::function on_error, - std::function reset_backoff, - std::function log_message - ) : - do_backoff(std::move(do_backoff)), - on_receive(std::move(on_receive)), - on_error(std::move(on_error)), - reset_backoff(std::move(reset_backoff)), - log_message(std::move(log_message)) { - } + Callbacks(std::function do_backoff, + std::function on_receive, + std::function on_error, + std::function reset_backoff, + std::function log_message) + : do_backoff(std::move(do_backoff)), + on_receive(std::move(on_receive)), + on_error(std::move(on_error)), + reset_backoff(std::move(reset_backoff)), + log_message(std::move(log_message)) {} }; /** - * The request context represents the state required by the executing CURL - * request. Not directly including the shared data in the CurlClient allows - * for easy separation of its lifetime from that of the CURL client. This - * facilitates destruction of the CurlClient being used to stop in-progress - * requests. - * - * The CURL client can be destructed and pending tasks will still - * have a valid RequestContext and will detect the shutdown. - */ + * The request context represents the state required by the executing CURL + * request. Not directly including the shared data in the CurlClient allows + * for easy separation of its lifetime from that of the CURL client. This + * facilitates destruction of the CurlClient being used to stop in-progress + * requests. + * + * The CURL client can be destructed and pending tasks will still + * have a valid RequestContext and will detect the shutdown. + */ class RequestContext { // Only items used by both the curl thread and the executor/main // thread need to be mutex protected. @@ -84,7 +81,7 @@ class CurlClient final : public Client, // End mutex protected items. std::optional callbacks_; - public: + public: // SSE parser using common parser from parser.hpp using ParserBody = detail::EventBody>; std::unique_ptr parser_body; @@ -97,16 +94,18 @@ class CurlClient final : public Client, std::chrono::steady_clock::time_point last_progress_time; curl_off_t last_download_amount; - const http::request req; - const std::string url; - const std::optional connect_timeout; - const std::optional read_timeout; - const std::optional write_timeout; - const std::optional custom_ca_file; - const std::optional proxy_url; - const bool skip_verify_peer; - - void backoff(const std::string& message) { + // These are thread-safe, because they are only accessed from + // CurlClient's strand after construction. + http::request req; + std::string url; + std::optional const connect_timeout; + std::optional const read_timeout; + std::optional const write_timeout; + std::optional const custom_ca_file; + std::optional const proxy_url; + bool const skip_verify_peer; + + void backoff(std::string const& message) { std::lock_guard lock(mutex_); if (shutting_down_) { return; @@ -116,7 +115,7 @@ class CurlClient final : public Client, } } - void error(const Error& error) { + void error(Error const& error) { std::lock_guard lock(mutex_); if (shutting_down_) { return; @@ -126,7 +125,7 @@ class CurlClient final : public Client, } } - void receive(const Event& event) { + void receive(Event const& event) { std::lock_guard lock(mutex_); if (shutting_down_) { return; @@ -146,7 +145,7 @@ class CurlClient final : public Client, } } - void log_message(const std::string& message) { + void log_message(std::string const& message) { std::lock_guard lock(mutex_); if (shutting_down_) { return; @@ -161,9 +160,7 @@ class CurlClient final : public Client, callbacks_ = std::move(callbacks); } - bool is_shutting_down() { - return shutting_down_; - } + bool is_shutting_down() { return shutting_down_; } void set_curl_socket(curl_socket_t curl_socket) { std::lock_guard lock(mutex_); @@ -197,7 +194,6 @@ class CurlClient final : public Client, } } - RequestContext(std::string url, http::request req, std::optional connect_timeout, @@ -205,28 +201,28 @@ class CurlClient final : public Client, std::optional write_timeout, std::optional custom_ca_file, std::optional proxy_url, - bool skip_verify_peer - ) : shutting_down_(false), - curl_socket_(CURL_SOCKET_BAD), - last_download_amount(0), - req(std::move(req)), - url(std::move(url)), - connect_timeout(connect_timeout), - read_timeout(read_timeout), - write_timeout(write_timeout), - custom_ca_file(std::move(custom_ca_file)), - proxy_url(std::move(proxy_url)), - skip_verify_peer(skip_verify_peer) { - } + bool skip_verify_peer) + : shutting_down_(false), + curl_socket_(CURL_SOCKET_BAD), + last_download_amount(0), + req(std::move(req)), + url(std::move(url)), + connect_timeout(connect_timeout), + read_timeout(read_timeout), + write_timeout(write_timeout), + custom_ca_file(std::move(custom_ca_file)), + proxy_url(std::move(proxy_url)), + skip_verify_peer(skip_verify_peer) {} void init_parser() { parser_body = std::make_unique(); - parser_reader = std::make_unique(*parser_body); + parser_reader = + std::make_unique(*parser_body); parser_reader->init(); } }; -public: + public: CurlClient(boost::asio::any_io_executor executor, http::request req, std::string host, @@ -238,6 +234,7 @@ class CurlClient final : public Client, Builder::EventReceiver receiver, Builder::LogCallback logger, Builder::ErrorCallback errors, + Builder::ConnectionHook connection_hook, bool skip_verify_peer, std::optional custom_ca_file, bool use_https, @@ -249,32 +246,32 @@ class CurlClient final : public Client, void async_shutdown(std::function completion) override; void async_restart(std::string const& reason) override; -private: + private: void do_run(); - void do_shutdown(const std::function& completion); + void do_shutdown(std::function const& completion); void async_backoff(std::string const& reason); void on_backoff(boost::system::error_code const& ec); static void PerformRequestWithMulti( std::shared_ptr multi_manager, std::shared_ptr context); - static size_t WriteCallback(const char* data, + static size_t WriteCallback(char const* data, size_t size, size_t nmemb, void* userp); - static size_t HeaderCallback(const char* buffer, + static size_t HeaderCallback(char const* buffer, size_t size, size_t nitems, void* userdata); - static curl_socket_t OpenSocketCallback(void* clientp, - curlsocktype purpose, - const struct curl_sockaddr* - address); + static curl_socket_t OpenSocketCallback( + void* clientp, + curlsocktype purpose, + const struct curl_sockaddr* address); void log_message(std::string const& message); void report_error(Error error); - std::string build_url(const http::request& req) const; + std::string build_url(http::request const& req) const; static bool SetupCurlOptions(CURL* curl, curl_slist** headers, RequestContext& context); @@ -293,6 +290,7 @@ class CurlClient final : public Client, Builder::EventReceiver event_receiver_; Builder::LogCallback logger_; Builder::ErrorCallback errors_; + Builder::ConnectionHook connection_hook_; bool use_https_; boost::asio::steady_timer backoff_timer_; @@ -300,6 +298,6 @@ class CurlClient final : public Client, Backoff backoff_; }; -} // namespace launchdarkly::sse +} // namespace launchdarkly::sse #endif // LD_CURL_NETWORKING \ No newline at end of file diff --git a/libs/server-sent-events/tests/curl_client_test.cpp b/libs/server-sent-events/tests/curl_client_test.cpp index 5ac9476c7..bfabe7bd1 100644 --- a/libs/server-sent-events/tests/curl_client_test.cpp +++ b/libs/server-sent-events/tests/curl_client_test.cpp @@ -1,7 +1,7 @@ #ifdef LD_CURL_NETWORKING -#include #include +#include #include @@ -24,8 +24,8 @@ namespace { // C++17-compatible latch replacement // https://en.cppreference.com/w/cpp/thread/latch.html class SimpleLatch { -public: - explicit SimpleLatch(const std::size_t count) : count_(count) {} + public: + explicit SimpleLatch(std::size_t const count) : count_(count) {} void count_down() { std::lock_guard lock(mutex_); @@ -35,13 +35,13 @@ class SimpleLatch { cv_.notify_all(); } - template + template bool wait_for(std::chrono::duration timeout) { std::unique_lock lock(mutex_); return cv_.wait_for(lock, timeout, [this] { return count_ == 0; }); } -private: + private: std::mutex mutex_; std::condition_variable cv_; std::size_t count_; @@ -49,7 +49,7 @@ class SimpleLatch { // Helper to synchronize event reception in tests class EventCollector { -public: + public: void add_event(Event event) { std::lock_guard lock(mutex_); events_.push_back(std::move(event)); @@ -62,14 +62,18 @@ class EventCollector { cv_.notify_all(); } - bool wait_for_events(size_t count, std::chrono::milliseconds timeout = 5000ms) { + bool wait_for_events(size_t count, + std::chrono::milliseconds timeout = 5000ms) { std::unique_lock lock(mutex_); - return cv_.wait_for(lock, timeout, [&] { return events_.size() >= count; }); + return cv_.wait_for(lock, timeout, + [&] { return events_.size() >= count; }); } - bool wait_for_errors(size_t count, std::chrono::milliseconds timeout = 5000ms) { + bool wait_for_errors(size_t count, + std::chrono::milliseconds timeout = 5000ms) { std::unique_lock lock(mutex_); - return cv_.wait_for(lock, timeout, [&] { return errors_.size() >= count; }); + return cv_.wait_for(lock, timeout, + [&] { return errors_.size() >= count; }); } std::vector events() const { @@ -82,7 +86,7 @@ class EventCollector { return errors_; } -private: + private: mutable std::mutex mutex_; std::condition_variable cv_; std::vector events_; @@ -91,7 +95,7 @@ class EventCollector { // Helper to run io_context in background thread class IoContextRunner { -public: + public: IoContextRunner() : work_guard_(boost::asio::make_work_guard(ioc_)) { thread_ = std::thread([this] { ioc_.run(); }); } @@ -106,9 +110,10 @@ class IoContextRunner { boost::asio::io_context& context() { return ioc_; } -private: + private: boost::asio::io_context ioc_; - boost::asio::executor_work_guard work_guard_; + boost::asio::executor_work_guard + work_guard_; std::thread thread_; }; @@ -126,9 +131,11 @@ TEST(CurlClientTest, ConnectsToHttpServer) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -144,14 +151,17 @@ TEST(CurlClientTest, ConnectsToHttpServer) { TEST(CurlClientTest, HandlesMultipleEvents) { MockSSEServer server; - auto port = server.start(TestHandlers::multiple_events({"event1", "event2", "event3"})); + auto port = server.start( + TestHandlers::multiple_events({"event1", "event2", "event3"})); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -171,23 +181,26 @@ TEST(CurlClientTest, HandlesMultipleEvents) { TEST(CurlClientTest, ParsesEventWithType) { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto close) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - send_sse_event(SSEFormatter::event("test data", "custom-type")); - std::this_thread::sleep_for(10ms); - close(); - }); + send_sse_event(SSEFormatter::event("test data", "custom-type")); + std::this_thread::sleep_for(10ms); + close(); + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -204,23 +217,26 @@ TEST(CurlClientTest, ParsesEventWithType) { TEST(CurlClientTest, ParsesEventWithId) { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto close) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - send_sse_event(SSEFormatter::event("test data", "", "event-123")); - std::this_thread::sleep_for(10ms); - close(); - }); + send_sse_event(SSEFormatter::event("test data", "", "event-123")); + std::this_thread::sleep_for(10ms); + close(); + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -237,23 +253,26 @@ TEST(CurlClientTest, ParsesEventWithId) { TEST(CurlClientTest, ParsesMultiLineData) { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto close) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - send_sse_event(SSEFormatter::event("line1\nline2\nline3")); - std::this_thread::sleep_for(10ms); - close(); - }); + send_sse_event(SSEFormatter::event("line1\nline2\nline3")); + std::this_thread::sleep_for(10ms); + close(); + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -268,29 +287,33 @@ TEST(CurlClientTest, ParsesMultiLineData) { } TEST(CurlClientTest, HandlesComments) { - GTEST_SKIP() << "Comment filtering is not yet implemented in the SSE parser"; + GTEST_SKIP() + << "Comment filtering is not yet implemented in the SSE parser"; MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto close) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - // Send a comment (should be ignored) - send_sse_event(SSEFormatter::comment("this is a comment")); - // Send an actual event - send_sse_event(SSEFormatter::event("real data")); - std::this_thread::sleep_for(10ms); - close(); - }); + // Send a comment (should be ignored) + send_sse_event(SSEFormatter::comment("this is a comment")); + // Send an actual event + send_sse_event(SSEFormatter::event("real data")); + std::this_thread::sleep_for(10ms); + close(); + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -311,7 +334,8 @@ TEST(CurlClientTest, SupportsPostMethod) { MockSSEServer server; std::string received_method; - auto port = server.start([&](auto const& req, auto send_response, auto send_sse_event, auto close) { + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { received_method = std::string(req.method_string()); http::response res{http::status::ok, 11}; @@ -327,11 +351,13 @@ TEST(CurlClientTest, SupportsPostMethod) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .method(http::verb::post) - .body("test body") - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .method(http::verb::post) + .body("test body") + .build(); client->async_connect(); @@ -347,7 +373,8 @@ TEST(CurlClientTest, SupportsReportMethod) { MockSSEServer server; std::string received_method; - auto port = server.start([&](auto const& req, auto send_response, auto send_sse_event, auto close) { + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { received_method = std::string(req.method_string()); http::response res{http::status::ok, 11}; @@ -363,11 +390,13 @@ TEST(CurlClientTest, SupportsReportMethod) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .method(http::verb::report) - .body("test body") - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .method(http::verb::report) + .body("test body") + .build(); client->async_connect(); @@ -385,7 +414,8 @@ TEST(CurlClientTest, SendsCustomHeaders) { MockSSEServer server; std::string custom_header_value; - auto port = server.start([&](auto const& req, auto send_response, auto send_sse_event, auto close) { + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { auto it = req.find("X-Custom-Header"); if (it != req.end()) { custom_header_value = std::string(it->value()); @@ -404,10 +434,12 @@ TEST(CurlClientTest, SendsCustomHeaders) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .header("X-Custom-Header", "custom-value") - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .header("X-Custom-Header", "custom-value") + .build(); client->async_connect(); @@ -428,10 +460,12 @@ TEST(CurlClientTest, Handles404Error) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .errors([&](Error e) { collector.add_error(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .errors([&](Error e) { collector.add_error(std::move(e)); }) + .build(); client->async_connect(); @@ -446,12 +480,14 @@ TEST(CurlClientTest, Handles404Error) { TEST(CurlClientTest, Handles500Error) { // 500 errors are treated as transient server errors and should trigger - // backoff/retry behavior, not error callbacks. This is correct SSE client behavior. + // backoff/retry behavior, not error callbacks. This is correct SSE client + // behavior. std::atomic connection_attempts{0}; auto handler = [&](auto const&, auto send_response, auto, auto) { ++connection_attempts; - http::response res{http::status::internal_server_error, 11}; + http::response res{ + http::status::internal_server_error, 11}; res.body() = "Error"; res.prepare_payload(); send_response(res); @@ -463,11 +499,13 @@ TEST(CurlClientTest, Handles500Error) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .errors([&](Error e) { collector.add_error(std::move(e)); }) - .initial_reconnect_delay(50ms) // Short delay for test - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .errors([&](Error e) { collector.add_error(std::move(e)); }) + .initial_reconnect_delay(50ms) // Short delay for test + .build(); client->async_connect(); @@ -475,7 +513,8 @@ TEST(CurlClientTest, Handles500Error) { // Wait a bit to let multiple reconnection attempts happen std::this_thread::sleep_for(300ms); - // Verify that multiple reconnection attempts occurred (backoff/retry behavior) + // Verify that multiple reconnection attempts occurred (backoff/retry + // behavior) EXPECT_GE(connection_attempts.load(), 2); // Verify no error callbacks were invoked (5xx are not reported as errors) @@ -492,17 +531,19 @@ TEST(CurlClientTest, FollowsRedirects) { MockSSEServer redirect_server; MockSSEServer target_server; - auto target_port = target_server.start(TestHandlers::simple_event("redirected")); - auto redirect_port = redirect_server.start( - TestHandlers::redirect("http://localhost:" + std::to_string(target_port) + "/") - ); + auto target_port = + target_server.start(TestHandlers::simple_event("redirected")); + auto redirect_port = redirect_server.start(TestHandlers::redirect( + "http://localhost:" + std::to_string(target_port) + "/")); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(redirect_port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(redirect_port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -520,7 +561,8 @@ TEST(CurlClientTest, FollowsRedirects) { TEST(CurlClientTest, ShutdownStopsClient) { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto) { + auto port = server.start([](auto const&, auto send_response, + auto send_sse_event, auto) { http::response res{http::status::ok, 11}; res.set(http::field::content_type, "text/event-stream"); res.chunked(true); @@ -536,9 +578,11 @@ TEST(CurlClientTest, ShutdownStopsClient) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -563,9 +607,11 @@ TEST(CurlClientTest, CanShutdownBeforeConnection) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); // Shutdown immediately without connecting SimpleLatch shutdown_latch(1); @@ -574,8 +620,9 @@ TEST(CurlClientTest, CanShutdownBeforeConnection) { } TEST(CurlClientTest, HandlesImmediateClose) { - // Immediate connection close is treated as a transient network error and should trigger - // backoff/retry behavior, not error callbacks. This is correct SSE client behavior. + // Immediate connection close is treated as a transient network error and + // should trigger backoff/retry behavior, not error callbacks. This is + // correct SSE client behavior. std::atomic connection_attempts{0}; auto handler = [&](auto const&, auto, auto, auto close) { @@ -589,11 +636,13 @@ TEST(CurlClientTest, HandlesImmediateClose) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .errors([&](Error e) { collector.add_error(std::move(e)); }) - .initial_reconnect_delay(50ms) // Short delay for test - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .errors([&](Error e) { collector.add_error(std::move(e)); }) + .initial_reconnect_delay(50ms) // Short delay for test + .build(); client->async_connect(); @@ -601,7 +650,8 @@ TEST(CurlClientTest, HandlesImmediateClose) { // Wait a bit to let multiple reconnection attempts happen std::this_thread::sleep_for(300ms); - // Verify that multiple reconnection attempts occurred (backoff/retry behavior) + // Verify that multiple reconnection attempts occurred (backoff/retry + // behavior) EXPECT_GE(connection_attempts.load(), 2); // Verify no error callbacks were invoked (connection errors trigger retry) @@ -616,34 +666,37 @@ TEST(CurlClientTest, HandlesImmediateClose) { TEST(CurlClientTest, RespectsReadTimeout) { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - // Send one event - send_sse_event(SSEFormatter::event("first")); + // Send one event + send_sse_event(SSEFormatter::event("first")); - // Then wait longer than read timeout without sending anything - std::this_thread::sleep_for(5000ms); - }); + // Then wait longer than read timeout without sending anything + std::this_thread::sleep_for(5000ms); + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .errors([&](Error e) { - std::cerr << "Error" << e.index() << std::endl; - collector.add_error(std::move(e)); - }) - .logger([&](const std::string& message) { - std::cerr << "log_message" << message << std::endl; - }) - .read_timeout(500ms) // Short timeout for test - .initial_reconnect_delay(50ms) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .errors([&](Error e) { + std::cerr << "Error" << e.index() << std::endl; + collector.add_error(std::move(e)); + }) + .logger([&](std::string const& message) { + std::cerr << "log_message" << message << std::endl; + }) + .read_timeout(500ms) // Short timeout for test + .initial_reconnect_delay(50ms) + .build(); client->async_connect(); @@ -661,23 +714,27 @@ TEST(CurlClientTest, RespectsReadTimeout) { TEST(CurlClientTest, DestructorCleansUpProperly) { { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); - - // Keep sending events - for (int i = 0; i < 100; i++) { - send_sse_event(SSEFormatter::event("event " + std::to_string(i))); - std::this_thread::sleep_for(10ms); - } - }); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); + + // Keep sending events + for (int i = 0; i < 100; i++) { + send_sse_event( + SSEFormatter::event("event " + std::to_string(i))); + std::this_thread::sleep_for(10ms); + } + }); EventCollector collector; IoContextRunner runner; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); ASSERT_TRUE(collector.wait_for_events(1)); @@ -691,23 +748,26 @@ TEST(CurlClientTest, DestructorCleansUpProperly) { TEST(CurlClientTest, HandlesEmptyEventData) { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto close) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - send_sse_event(SSEFormatter::event("")); - std::this_thread::sleep_for(10ms); - close(); - }); + send_sse_event(SSEFormatter::event("")); + std::this_thread::sleep_for(10ms); + close(); + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -723,24 +783,27 @@ TEST(CurlClientTest, HandlesEmptyEventData) { TEST(CurlClientTest, HandlesEventWithOnlyType) { MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto close) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - // Send event with type but empty data - send_sse_event("event: heartbeat\ndata: \n\n"); - std::this_thread::sleep_for(10ms); - close(); - }); + // Send event with type but empty data + send_sse_event("event: heartbeat\ndata: \n\n"); + std::this_thread::sleep_for(10ms); + close(); + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -760,7 +823,8 @@ TEST(CurlClientTest, HandlesRapidEvents) { constexpr int num_events = 100; // num_events needs to be captured for MSVC. - auto port = server.start([num_events](auto const&, auto send_response, auto send_sse_event, auto close) { + auto port = server.start([num_events](auto const&, auto send_response, + auto send_sse_event, auto close) { http::response res{http::status::ok, 11}; res.set(http::field::content_type, "text/event-stream"); res.chunked(true); @@ -777,9 +841,11 @@ TEST(CurlClientTest, HandlesRapidEvents) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); @@ -799,7 +865,8 @@ TEST(CurlClientTest, ShutdownDuringBackoffDelay) { auto handler = [&](auto const&, auto send_response, auto, auto) { ++connection_attempts; // Return 500 to trigger backoff - http::response res{http::status::internal_server_error, 11}; + http::response res{ + http::status::internal_server_error, 11}; res.body() = "Error"; res.prepare_payload(); send_response(res); @@ -811,10 +878,13 @@ TEST(CurlClientTest, ShutdownDuringBackoffDelay) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .initial_reconnect_delay(2000ms) // Long delay to ensure we shutdown during wait - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .initial_reconnect_delay( + 2000ms) // Long delay to ensure we shutdown during wait + .build(); client->async_connect(); @@ -841,7 +911,8 @@ TEST(CurlClientTest, ShutdownDuringDataReception) { SimpleLatch server_sending(1); SimpleLatch client_received_some(1); - auto handler = [&](auto const&, auto send_response, auto send_sse_event, auto) { + auto handler = [&](auto const&, auto send_response, auto send_sse_event, + auto) { http::response res{http::status::ok, 11}; res.set(http::field::content_type, "text/event-stream"); res.chunked(true); @@ -849,13 +920,15 @@ TEST(CurlClientTest, ShutdownDuringDataReception) { // Send events continuously for (int i = 0; i < 100; i++) { - if (!send_sse_event(SSEFormatter::event("event " + std::to_string(i)))) { + if (!send_sse_event( + SSEFormatter::event("event " + std::to_string(i)))) { return; // Connection closed or error - stop sending } if (i == 2) { server_sending.count_down(); } - std::this_thread::sleep_for(10ms); // Slow enough to allow shutdown mid-stream + std::this_thread::sleep_for( + 10ms); // Slow enough to allow shutdown mid-stream } }; @@ -866,14 +939,15 @@ TEST(CurlClientTest, ShutdownDuringDataReception) { // Shared ptr to prevent handling events during destruction. auto collector = std::make_shared(); - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([collector, &client_received_some](Event e) { - collector->add_event(std::move(e)); - if (collector->events().size() >= 2) { - client_received_some.count_down(); - } - }) - .build(); + auto client = Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([collector, &client_received_some](Event e) { + collector->add_event(std::move(e)); + if (collector->events().size() >= 2) { + client_received_some.count_down(); + } + }) + .build(); client->async_connect(); @@ -896,7 +970,8 @@ TEST(CurlClientTest, ShutdownDuringProgressCallback) { // This ensures we can abort during slow data transfer SimpleLatch server_started(1); - auto handler = [&](auto const&, auto send_response, auto send_sse_event, auto) { + auto handler = [&](auto const&, auto send_response, auto send_sse_event, + auto) { http::response res{http::status::ok, 11}; res.set(http::field::content_type, "text/event-stream"); res.chunked(true); @@ -906,7 +981,8 @@ TEST(CurlClientTest, ShutdownDuringProgressCallback) { // Send one event then pause (simulating slow connection) send_sse_event(SSEFormatter::event("first")); - std::this_thread::sleep_for(5000ms); // Pause to simulate slow connection + std::this_thread::sleep_for( + 5000ms); // Pause to simulate slow connection }; MockSSEServer server; @@ -915,10 +991,13 @@ TEST(CurlClientTest, ShutdownDuringProgressCallback) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .read_timeout(10000ms) // Long timeout so ProgressCallback is called but doesn't abort - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .read_timeout(10000ms) // Long timeout so ProgressCallback is + // called but doesn't abort + .build(); client->async_connect(); @@ -945,9 +1024,11 @@ TEST(CurlClientTest, MultipleShutdownCalls) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); ASSERT_TRUE(collector.wait_for_events(1)); @@ -970,24 +1051,28 @@ TEST(CurlClientTest, MultipleShutdownCalls) { TEST(CurlClientTest, ShutdownAfterConnectionClosed) { // Tests shutdown when connection has already ended naturally MockSSEServer server; - auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto close) { - http::response res{http::status::ok, 11}; - res.set(http::field::content_type, "text/event-stream"); - res.chunked(true); - send_response(res); + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); - send_sse_event(SSEFormatter::event("only event")); - std::this_thread::sleep_for(10ms); - close(); // Server closes connection - }); + send_sse_event(SSEFormatter::event("only event")); + std::this_thread::sleep_for(10ms); + close(); // Server closes connection + }); IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .initial_reconnect_delay(500ms) // Will try to reconnect after close - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .initial_reconnect_delay( + 500ms) // Will try to reconnect after close + .build(); client->async_connect(); ASSERT_TRUE(collector.wait_for_events(1)); @@ -1002,10 +1087,12 @@ TEST(CurlClientTest, ShutdownAfterConnectionClosed) { } TEST(CurlClientTest, ShutdownDuringConnectionAttempt) { - // Server that delays before responding to test shutdown during connection phase + // Server that delays before responding to test shutdown during connection + // phase SimpleLatch connection_started(1); - auto handler = [&](auto const&, auto send_response, auto send_sse_event, auto close) { + auto handler = [&](auto const&, auto send_response, auto send_sse_event, + auto close) { connection_started.count_down(); // Delay before responding std::this_thread::sleep_for(500ms); @@ -1026,15 +1113,18 @@ TEST(CurlClientTest, ShutdownDuringConnectionAttempt) { IoContextRunner runner; EventCollector collector; - auto client = Builder(runner.context().get_executor(), "http://localhost:" + std::to_string(port)) - .receiver([&](Event e) { collector.add_event(std::move(e)); }) - .build(); + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .build(); client->async_connect(); // Wait for connection to start but shutdown before it completes ASSERT_TRUE(connection_started.wait_for(5000ms)); - std::this_thread::sleep_for(50ms); // Give CURL time to start but not finish + std::this_thread::sleep_for( + 50ms); // Give CURL time to start but not finish auto shutdown_start = std::chrono::steady_clock::now(); SimpleLatch shutdown_latch(1); @@ -1048,4 +1138,237 @@ TEST(CurlClientTest, ShutdownDuringConnectionAttempt) { // Should not have received any events since we shutdown during connection EXPECT_EQ(0, collector.events().size()); } + +// on_connect hook tests + +TEST(CurlClientTest, OnConnectHookInvokedBeforeRequest) { + MockSSEServer server; + auto port = server.start(TestHandlers::simple_event("hello")); + + IoContextRunner runner; + EventCollector collector; + + std::atomic hook_calls{0}; + std::string seen_target; + std::mutex target_mutex; + + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port) + "/initial-path") + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .on_connect([&](http::request* req) { + ++hook_calls; + std::lock_guard lock(target_mutex); + seen_target = std::string(req->target()); + }) + .build(); + + // Connect and let the hook fire. + client->async_connect(); + + // Verify the hook ran and saw the construction-time target. + ASSERT_TRUE(collector.wait_for_events(1)); + EXPECT_GE(hook_calls.load(), 1); + { + std::lock_guard lock(target_mutex); + EXPECT_EQ("/initial-path", seen_target); + } + + SimpleLatch shutdown_latch(1); + client->async_shutdown([&] { shutdown_latch.count_down(); }); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); +} + +TEST(CurlClientTest, OnConnectHookCanMutateTarget) { + MockSSEServer server; + std::string seen_target; + std::mutex target_mutex; + + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { + { + std::lock_guard lock(target_mutex); + seen_target = std::string(req.target()); + } + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); + send_sse_event(SSEFormatter::event("ok")); + std::this_thread::sleep_for(10ms); + close(); + }); + + IoContextRunner runner; + EventCollector collector; + + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port) + "/original") + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .on_connect([](http::request* req) { + req->target("/mutated?param=value"); + }) + .build(); + + // Connect; hook overrides the target before the request is sent. + client->async_connect(); + + // Verify the server received the mutated target, not the original URL's + // path. + ASSERT_TRUE(collector.wait_for_events(1)); + { + std::lock_guard lock(target_mutex); + EXPECT_EQ("/mutated?param=value", seen_target); + } + + SimpleLatch shutdown_latch(1); + client->async_shutdown([&] { shutdown_latch.count_down(); }); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); +} + +TEST(CurlClientTest, OnConnectHookCanMutateHeaders) { + MockSSEServer server; + std::string seen_header; + std::mutex header_mutex; + + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { + if (auto it = req.find("X-Hook-Header"); it != req.end()) { + std::lock_guard lock(header_mutex); + seen_header = std::string(it->value()); + } + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); + send_sse_event(SSEFormatter::event("ok")); + std::this_thread::sleep_for(10ms); + close(); + }); + + IoContextRunner runner; + EventCollector collector; + + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .on_connect([](http::request* req) { + req->set("X-Hook-Header", "from-hook"); + }) + .build(); + + // Connect; hook adds a custom header before the request is sent. + client->async_connect(); + + // Verify the server received the hook-injected header. + ASSERT_TRUE(collector.wait_for_events(1)); + { + std::lock_guard lock(header_mutex); + EXPECT_EQ("from-hook", seen_header); + } + + SimpleLatch shutdown_latch(1); + client->async_shutdown([&] { shutdown_latch.count_down(); }); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); +} + +TEST(CurlClientTest, OnConnectHookInvokedOnEachReconnect) { + MockSSEServer server; + auto port = server.start( + [](auto const&, auto send_response, auto send_sse_event, auto close) { + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); + send_sse_event(SSEFormatter::event("event")); + std::this_thread::sleep_for(10ms); + close(); + }); + + IoContextRunner runner; + EventCollector collector; + + std::atomic hook_calls{0}; + + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .initial_reconnect_delay(50ms) + .on_connect( + [&](http::request*) { ++hook_calls; }) + .build(); + + // Connect; the server closes after each event so the client reconnects. + client->async_connect(); + + // Verify the hook fires on every connection attempt, not just the first. + ASSERT_TRUE(collector.wait_for_events(3)); + EXPECT_GE(hook_calls.load(), 3); + + SimpleLatch shutdown_latch(1); + client->async_shutdown([&] { shutdown_latch.count_down(); }); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); +} + +TEST(CurlClientTest, OnConnectHookSeesPreviousMutations) { + MockSSEServer server; + std::vector seen_targets; + std::mutex targets_mutex; + std::condition_variable targets_cv; + + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { + { + std::lock_guard lock(targets_mutex); + seen_targets.push_back(std::string(req.target())); + targets_cv.notify_all(); + } + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); + send_sse_event(SSEFormatter::event("event")); + std::this_thread::sleep_for(10ms); + close(); + }); + + IoContextRunner runner; + EventCollector collector; + + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port) + "/start") + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .initial_reconnect_delay(50ms) + .on_connect([](http::request* req) { + req->target(std::string(req->target()) + "/x"); + }) + .build(); + + // Connect; on each reconnect the hook appends "/x" to whatever target it + // sees. + client->async_connect(); + + // Verify successive hook invocations see their own previous mutations + // rather than a reset to the construction-time target. + { + std::unique_lock lock(targets_mutex); + ASSERT_TRUE(targets_cv.wait_for( + lock, 5000ms, [&] { return seen_targets.size() >= 3; })); + } + + { + std::lock_guard lock(targets_mutex); + EXPECT_EQ("/start/x", seen_targets[0]); + EXPECT_EQ("/start/x/x", seen_targets[1]); + EXPECT_EQ("/start/x/x/x", seen_targets[2]); + } + + SimpleLatch shutdown_latch(1); + client->async_shutdown([&] { shutdown_latch.count_down(); }); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); +} #endif // LD_CURL_NETWORKING From 577746a26a228f48fe8c46d50ef280e291670609 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 14:02:11 -0700 Subject: [PATCH 07/13] fix: make the tests work for both FoxyClient and Curl --- libs/server-sent-events/src/client.cpp | 7 +- .../{curl_client_test.cpp => client_test.cpp} | 64 +++++++++---------- 2 files changed, 37 insertions(+), 34 deletions(-) rename libs/server-sent-events/tests/{curl_client_test.cpp => client_test.cpp} (96%) diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index caf96d410..74d556abe 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -283,6 +283,9 @@ class FoxyClient : public Client, return; } + host_ = new_url->host(); + port_ = + new_url->has_port() ? new_url->port() : new_url->scheme(); req_.set(http::field::host, new_url->host()); req_.target(new_url->encoded_target()); } else { @@ -665,7 +668,9 @@ std::shared_ptr Builder::build() { std::string host = uri_components->host(); request.set(http::field::host, host); - request.target(uri_components->encoded_target()); + // RFC 7230: an empty path in origin-form must be sent as "/". + auto target = uri_components->encoded_target(); + request.target(target.empty() ? "/" : target); if (uri_components->has_scheme()) { if (!(uri_components->scheme_id() == boost::urls::scheme::http || diff --git a/libs/server-sent-events/tests/curl_client_test.cpp b/libs/server-sent-events/tests/client_test.cpp similarity index 96% rename from libs/server-sent-events/tests/curl_client_test.cpp rename to libs/server-sent-events/tests/client_test.cpp index bfabe7bd1..0298576bb 100644 --- a/libs/server-sent-events/tests/curl_client_test.cpp +++ b/libs/server-sent-events/tests/client_test.cpp @@ -1,4 +1,3 @@ -#ifdef LD_CURL_NETWORKING #include #include @@ -121,7 +120,7 @@ class IoContextRunner { // Basic connectivity tests -TEST(CurlClientTest, ConnectsToHttpServer) { +TEST(ClientTest, ConnectsToHttpServer) { MockSSEServer server; auto port = server.start(TestHandlers::simple_event("hello world")); @@ -149,7 +148,7 @@ TEST(CurlClientTest, ConnectsToHttpServer) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, HandlesMultipleEvents) { +TEST(ClientTest, HandlesMultipleEvents) { MockSSEServer server; auto port = server.start( TestHandlers::multiple_events({"event1", "event2", "event3"})); @@ -179,7 +178,7 @@ TEST(CurlClientTest, HandlesMultipleEvents) { // SSE parsing tests -TEST(CurlClientTest, ParsesEventWithType) { +TEST(ClientTest, ParsesEventWithType) { MockSSEServer server; auto port = server.start( [](auto const&, auto send_response, auto send_sse_event, auto close) { @@ -215,7 +214,7 @@ TEST(CurlClientTest, ParsesEventWithType) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, ParsesEventWithId) { +TEST(ClientTest, ParsesEventWithId) { MockSSEServer server; auto port = server.start( [](auto const&, auto send_response, auto send_sse_event, auto close) { @@ -251,7 +250,7 @@ TEST(CurlClientTest, ParsesEventWithId) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, ParsesMultiLineData) { +TEST(ClientTest, ParsesMultiLineData) { MockSSEServer server; auto port = server.start( [](auto const&, auto send_response, auto send_sse_event, auto close) { @@ -286,7 +285,7 @@ TEST(CurlClientTest, ParsesMultiLineData) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, HandlesComments) { +TEST(ClientTest, HandlesComments) { GTEST_SKIP() << "Comment filtering is not yet implemented in the SSE parser"; @@ -330,7 +329,7 @@ TEST(CurlClientTest, HandlesComments) { // HTTP method tests -TEST(CurlClientTest, SupportsPostMethod) { +TEST(ClientTest, SupportsPostMethod) { MockSSEServer server; std::string received_method; @@ -369,7 +368,7 @@ TEST(CurlClientTest, SupportsPostMethod) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, SupportsReportMethod) { +TEST(ClientTest, SupportsReportMethod) { MockSSEServer server; std::string received_method; @@ -410,7 +409,7 @@ TEST(CurlClientTest, SupportsReportMethod) { // HTTP header tests -TEST(CurlClientTest, SendsCustomHeaders) { +TEST(ClientTest, SendsCustomHeaders) { MockSSEServer server; std::string custom_header_value; @@ -453,7 +452,7 @@ TEST(CurlClientTest, SendsCustomHeaders) { // HTTP status code tests -TEST(CurlClientTest, Handles404Error) { +TEST(ClientTest, Handles404Error) { MockSSEServer server; auto port = server.start(TestHandlers::http_error(http::status::not_found)); @@ -478,7 +477,7 @@ TEST(CurlClientTest, Handles404Error) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, Handles500Error) { +TEST(ClientTest, Handles500Error) { // 500 errors are treated as transient server errors and should trigger // backoff/retry behavior, not error callbacks. This is correct SSE client // behavior. @@ -527,7 +526,7 @@ TEST(CurlClientTest, Handles500Error) { // Redirect tests -TEST(CurlClientTest, FollowsRedirects) { +TEST(ClientTest, FollowsRedirects) { MockSSEServer redirect_server; MockSSEServer target_server; @@ -559,7 +558,7 @@ TEST(CurlClientTest, FollowsRedirects) { // Connection lifecycle tests -TEST(CurlClientTest, ShutdownStopsClient) { +TEST(ClientTest, ShutdownStopsClient) { MockSSEServer server; auto port = server.start([](auto const&, auto send_response, auto send_sse_event, auto) { @@ -600,7 +599,7 @@ TEST(CurlClientTest, ShutdownStopsClient) { EXPECT_LT(shutdown_duration, 2000ms); } -TEST(CurlClientTest, CanShutdownBeforeConnection) { +TEST(ClientTest, CanShutdownBeforeConnection) { MockSSEServer server; auto port = server.start(TestHandlers::simple_event("test")); @@ -619,7 +618,7 @@ TEST(CurlClientTest, CanShutdownBeforeConnection) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, HandlesImmediateClose) { +TEST(ClientTest, HandlesImmediateClose) { // Immediate connection close is treated as a transient network error and // should trigger backoff/retry behavior, not error callbacks. This is // correct SSE client behavior. @@ -664,7 +663,7 @@ TEST(CurlClientTest, HandlesImmediateClose) { // Timeout tests -TEST(CurlClientTest, RespectsReadTimeout) { +TEST(ClientTest, RespectsReadTimeout) { MockSSEServer server; auto port = server.start( [](auto const&, auto send_response, auto send_sse_event, auto) { @@ -711,7 +710,7 @@ TEST(CurlClientTest, RespectsReadTimeout) { EXPECT_TRUE(shutdown_latch.wait_for(100ms)); } -TEST(CurlClientTest, DestructorCleansUpProperly) { +TEST(ClientTest, DestructorCleansUpProperly) { { MockSSEServer server; auto port = server.start( @@ -746,7 +745,7 @@ TEST(CurlClientTest, DestructorCleansUpProperly) { // Test passing indicates proper cleanup in destructor } -TEST(CurlClientTest, HandlesEmptyEventData) { +TEST(ClientTest, HandlesEmptyEventData) { MockSSEServer server; auto port = server.start( [](auto const&, auto send_response, auto send_sse_event, auto close) { @@ -781,7 +780,7 @@ TEST(CurlClientTest, HandlesEmptyEventData) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, HandlesEventWithOnlyType) { +TEST(ClientTest, HandlesEventWithOnlyType) { MockSSEServer server; auto port = server.start( [](auto const&, auto send_response, auto send_sse_event, auto close) { @@ -818,7 +817,7 @@ TEST(CurlClientTest, HandlesEventWithOnlyType) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, HandlesRapidEvents) { +TEST(ClientTest, HandlesRapidEvents) { MockSSEServer server; constexpr int num_events = 100; @@ -858,7 +857,7 @@ TEST(CurlClientTest, HandlesRapidEvents) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, ShutdownDuringBackoffDelay) { +TEST(ClientTest, ShutdownDuringBackoffDelay) { // This ensures clean shutdown during backoff/retry wait period std::atomic connection_attempts{0}; @@ -906,7 +905,7 @@ TEST(CurlClientTest, ShutdownDuringBackoffDelay) { EXPECT_EQ(1, connection_attempts.load()); } -TEST(CurlClientTest, ShutdownDuringDataReception) { +TEST(ClientTest, ShutdownDuringDataReception) { // This covers the branch where we abort during SSE data parsing SimpleLatch server_sending(1); SimpleLatch client_received_some(1); @@ -966,7 +965,7 @@ TEST(CurlClientTest, ShutdownDuringDataReception) { EXPECT_LT(shutdown_duration, 2000ms); } -TEST(CurlClientTest, ShutdownDuringProgressCallback) { +TEST(ClientTest, ShutdownDuringProgressCallback) { // This ensures we can abort during slow data transfer SimpleLatch server_started(1); @@ -1016,7 +1015,7 @@ TEST(CurlClientTest, ShutdownDuringProgressCallback) { EXPECT_LT(shutdown_duration, 2000ms); } -TEST(CurlClientTest, MultipleShutdownCalls) { +TEST(ClientTest, MultipleShutdownCalls) { // Ensures multiple shutdown calls don't cause issues (idempotency test) MockSSEServer server; auto port = server.start(TestHandlers::simple_event("test")); @@ -1048,7 +1047,7 @@ TEST(CurlClientTest, MultipleShutdownCalls) { EXPECT_TRUE(shutdown_latch3.wait_for(5000ms)); } -TEST(CurlClientTest, ShutdownAfterConnectionClosed) { +TEST(ClientTest, ShutdownAfterConnectionClosed) { // Tests shutdown when connection has already ended naturally MockSSEServer server; auto port = server.start( @@ -1086,7 +1085,7 @@ TEST(CurlClientTest, ShutdownAfterConnectionClosed) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, ShutdownDuringConnectionAttempt) { +TEST(ClientTest, ShutdownDuringConnectionAttempt) { // Server that delays before responding to test shutdown during connection // phase SimpleLatch connection_started(1); @@ -1141,7 +1140,7 @@ TEST(CurlClientTest, ShutdownDuringConnectionAttempt) { // on_connect hook tests -TEST(CurlClientTest, OnConnectHookInvokedBeforeRequest) { +TEST(ClientTest, OnConnectHookInvokedBeforeRequest) { MockSSEServer server; auto port = server.start(TestHandlers::simple_event("hello")); @@ -1179,7 +1178,7 @@ TEST(CurlClientTest, OnConnectHookInvokedBeforeRequest) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, OnConnectHookCanMutateTarget) { +TEST(ClientTest, OnConnectHookCanMutateTarget) { MockSSEServer server; std::string seen_target; std::mutex target_mutex; @@ -1227,7 +1226,7 @@ TEST(CurlClientTest, OnConnectHookCanMutateTarget) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, OnConnectHookCanMutateHeaders) { +TEST(ClientTest, OnConnectHookCanMutateHeaders) { MockSSEServer server; std::string seen_header; std::mutex header_mutex; @@ -1274,7 +1273,7 @@ TEST(CurlClientTest, OnConnectHookCanMutateHeaders) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, OnConnectHookInvokedOnEachReconnect) { +TEST(ClientTest, OnConnectHookInvokedOnEachReconnect) { MockSSEServer server; auto port = server.start( [](auto const&, auto send_response, auto send_sse_event, auto close) { @@ -1313,7 +1312,7 @@ TEST(CurlClientTest, OnConnectHookInvokedOnEachReconnect) { EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -TEST(CurlClientTest, OnConnectHookSeesPreviousMutations) { +TEST(ClientTest, OnConnectHookSeesPreviousMutations) { MockSSEServer server; std::vector seen_targets; std::mutex targets_mutex; @@ -1371,4 +1370,3 @@ TEST(CurlClientTest, OnConnectHookSeesPreviousMutations) { client->async_shutdown([&] { shutdown_latch.count_down(); }); EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } -#endif // LD_CURL_NETWORKING From 4f540b18a75534c35824a4037be8f40adda299ab Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 14:20:39 -0700 Subject: [PATCH 08/13] docs: update a comment to be clearer --- libs/server-sent-events/src/curl_client.hpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/server-sent-events/src/curl_client.hpp b/libs/server-sent-events/src/curl_client.hpp index 330681c7f..f0df1bb02 100644 --- a/libs/server-sent-events/src/curl_client.hpp +++ b/libs/server-sent-events/src/curl_client.hpp @@ -94,8 +94,11 @@ class CurlClient final : public Client, std::chrono::steady_clock::time_point last_progress_time; curl_off_t last_download_amount; - // These are thread-safe, because they are only accessed from - // CurlClient's strand after construction. + // Mutated on the strand in do_run() before each transfer, and read by + // libcurl via raw pointers (CURLOPT_URL, CURLOPT_POSTFIELDS) for the + // duration of the transfer. Safe because the next do_run() only fires + // after the previous transfer's completion callback, so reads and + // writes never overlap. http::request req; std::string url; std::optional const connect_timeout; From 653653bca60d6d67b0ea2ef94ca845611358f433 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 14:54:20 -0700 Subject: [PATCH 09/13] fix: change a shutdown timeout to match the other tests --- libs/server-sent-events/tests/client_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/server-sent-events/tests/client_test.cpp b/libs/server-sent-events/tests/client_test.cpp index 0298576bb..2f705dafc 100644 --- a/libs/server-sent-events/tests/client_test.cpp +++ b/libs/server-sent-events/tests/client_test.cpp @@ -707,7 +707,7 @@ TEST(ClientTest, RespectsReadTimeout) { SimpleLatch shutdown_latch(1); client->async_shutdown([&] { shutdown_latch.count_down(); }); - EXPECT_TRUE(shutdown_latch.wait_for(100ms)); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } TEST(ClientTest, DestructorCleansUpProperly) { From ab97f30f205abc729142ce45e4dcd93bafb711ba Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 16:38:39 -0700 Subject: [PATCH 10/13] docs: clarify a comment --- libs/server-sent-events/include/launchdarkly/sse/client.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/server-sent-events/include/launchdarkly/sse/client.hpp b/libs/server-sent-events/include/launchdarkly/sse/client.hpp index 709b2b40e..7e21596b6 100644 --- a/libs/server-sent-events/include/launchdarkly/sse/client.hpp +++ b/libs/server-sent-events/include/launchdarkly/sse/client.hpp @@ -186,7 +186,9 @@ class Builder { * the request's target, headers, method, and body. * * Host, port, and scheme are fixed at build time and cannot be changed - * by the hook. + * by the hook. The Last-Event-ID header is managed by the client and + * any value the hook sets for it will be replaced before the request + * is sent. * * @param hook Callback invoked with a mutable request before each * connection attempt. From 17710f102a532197ab29d5ff227e5ced99755357 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 16:50:38 -0700 Subject: [PATCH 11/13] fix: handle changing the body in the callback --- libs/server-sent-events/src/client.cpp | 5 +- libs/server-sent-events/src/curl_client.cpp | 2 + libs/server-sent-events/tests/client_test.cpp | 64 +++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index 74d556abe..fb3eb1a05 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -190,6 +190,9 @@ class FoxyClient : public Client, if (connection_hook_) { connection_hook_(&req_); } + + req_.prepare_payload(); + session_->async_connect( host_, port_, beast::bind_front_handler(&FoxyClient::on_connect, @@ -663,8 +666,6 @@ std::shared_ptr Builder::build() { } } - request.prepare_payload(); - std::string host = uri_components->host(); request.set(http::field::host, host); diff --git a/libs/server-sent-events/src/curl_client.cpp b/libs/server-sent-events/src/curl_client.cpp index 1e08f9495..0e50723e7 100644 --- a/libs/server-sent-events/src/curl_client.cpp +++ b/libs/server-sent-events/src/curl_client.cpp @@ -93,6 +93,8 @@ void CurlClient::do_run() { request_context_->url = build_url(request_context_->req); } + request_context_->req.prepare_payload(); + auto ctx = request_context_; auto weak_self = weak_from_this(); std::weak_ptr weak_ctx = ctx; diff --git a/libs/server-sent-events/tests/client_test.cpp b/libs/server-sent-events/tests/client_test.cpp index 2f705dafc..38f6f5cbe 100644 --- a/libs/server-sent-events/tests/client_test.cpp +++ b/libs/server-sent-events/tests/client_test.cpp @@ -1370,3 +1370,67 @@ TEST(ClientTest, OnConnectHookSeesPreviousMutations) { client->async_shutdown([&] { shutdown_latch.count_down(); }); EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } + +TEST(ClientTest, OnConnectHookCanMutateBody) { + MockSSEServer server; + std::string received_body; + std::string received_content_length; + std::mutex received_mutex; + + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { + { + std::lock_guard lock(received_mutex); + received_body = req.body(); + if (auto it = req.find(http::field::content_length); + it != req.end()) { + received_content_length = std::string(it->value()); + } + } + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); + send_sse_event(SSEFormatter::event("ok")); + std::this_thread::sleep_for(10ms); + close(); + }); + + IoContextRunner runner; + EventCollector collector; + + std::string const build_time_body = "short"; + std::string const hook_body = + "this-body-is-much-longer-than-the-build-time-body"; + ASSERT_GT(hook_body.size(), build_time_body.size()); + + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .method(http::verb::post) + .body(build_time_body) + .on_connect([&](http::request* req) { + req->body() = hook_body; + }) + .build(); + + // Connect; hook replaces the body with one larger than the build-time body. + client->async_connect(); + + // Verify the server received the full hook body and a matching + // Content-Length, not the stale build-time value. + ASSERT_TRUE(collector.wait_for_events(1)); + { + std::lock_guard lock(received_mutex); + EXPECT_EQ(hook_body, received_body) + << "Server received a truncated body. Content-Length header was '" + << received_content_length << "' (hook set body of size " + << hook_body.size() << ")."; + EXPECT_EQ(std::to_string(hook_body.size()), received_content_length); + } + + SimpleLatch shutdown_latch(1); + client->async_shutdown([&] { shutdown_latch.count_down(); }); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); +} From 4ec32df416a14af7a1412c536c263cd02e822a36 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 17:26:19 -0700 Subject: [PATCH 12/13] fix: override last-event-id in the curl branch --- libs/server-sent-events/src/curl_client.cpp | 17 +++-- libs/server-sent-events/tests/client_test.cpp | 70 +++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/libs/server-sent-events/src/curl_client.cpp b/libs/server-sent-events/src/curl_client.cpp index 0e50723e7..58f18cbbf 100644 --- a/libs/server-sent-events/src/curl_client.cpp +++ b/libs/server-sent-events/src/curl_client.cpp @@ -93,6 +93,16 @@ void CurlClient::do_run() { request_context_->url = build_url(request_context_->req); } + // Set Last-Event-ID if we have one from a previous connection, otherwise + // erase it to override any value set by the hook. + if (request_context_->last_event_id && + !request_context_->last_event_id->empty()) { + request_context_->req.set("last-event-id", + *request_context_->last_event_id); + } else { + request_context_->req.erase("last-event-id"); + } + request_context_->req.prepare_payload(); auto ctx = request_context_; @@ -238,13 +248,6 @@ bool CurlClient::SetupCurlOptions(CURL* curl, headers = curl_slist_append(headers, header.c_str()); } - // Add Last-Event-ID if we have one from previous connection - if (context.last_event_id && !context.last_event_id->empty()) { - std::string last_event_header = - "Last-Event-ID: " + *context.last_event_id; - headers = curl_slist_append(headers, last_event_header.c_str()); - } - if (headers) { CURL_SETOPT_CHECK(curl, CURLOPT_HTTPHEADER, headers); } diff --git a/libs/server-sent-events/tests/client_test.cpp b/libs/server-sent-events/tests/client_test.cpp index 38f6f5cbe..9fa1c692e 100644 --- a/libs/server-sent-events/tests/client_test.cpp +++ b/libs/server-sent-events/tests/client_test.cpp @@ -1434,3 +1434,73 @@ TEST(ClientTest, OnConnectHookCanMutateBody) { client->async_shutdown([&] { shutdown_latch.count_down(); }); EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); } + +TEST(ClientTest, OnConnectHookLastEventIdIsManagedByClient) { + MockSSEServer server; + std::vector> last_event_id_observations; + std::mutex received_mutex; + std::condition_variable received_cv; + + auto port = server.start([&](auto const& req, auto send_response, + auto send_sse_event, auto close) { + { + std::lock_guard lock(received_mutex); + std::size_t count = req.count("Last-Event-ID"); + auto it = req.find("Last-Event-ID"); + std::string value = + (it != req.end()) ? std::string(it->value()) : ""; + last_event_id_observations.emplace_back(count, value); + received_cv.notify_all(); + } + + http::response res{http::status::ok, 11}; + res.set(http::field::content_type, "text/event-stream"); + res.chunked(true); + send_response(res); + send_sse_event(SSEFormatter::event("data", "", "evt-42")); + std::this_thread::sleep_for(10ms); + close(); + }); + + IoContextRunner runner; + EventCollector collector; + + auto client = + Builder(runner.context().get_executor(), + "http://localhost:" + std::to_string(port)) + .receiver([&](Event e) { collector.add_event(std::move(e)); }) + .initial_reconnect_delay(50ms) + .on_connect([](http::request* req) { + req->set("last-event-id", "from-hook"); + }) + .build(); + + // Connect; the server closes after each event so the client reconnects. + client->async_connect(); + + // Verify the documented Last-Event-ID contract: the client manages this + // header and overrides any value set by the hook. On first connect no + // event ID has been seen, so the header should be absent. On reconnect + // the client should send exactly one Last-Event-ID with the most recent + // event's ID, not the hook's value. + { + std::unique_lock lock(received_mutex); + ASSERT_TRUE(received_cv.wait_for(lock, 5000ms, [&] { + return last_event_id_observations.size() >= 2; + })); + } + + { + std::lock_guard lock(received_mutex); + EXPECT_EQ(0u, last_event_id_observations[0].first) + << "first connect should send no Last-Event-ID; got '" + << last_event_id_observations[0].second << "'"; + EXPECT_EQ(1u, last_event_id_observations[1].first) + << "reconnect should send exactly one Last-Event-ID header"; + EXPECT_EQ("evt-42", last_event_id_observations[1].second); + } + + SimpleLatch shutdown_latch(1); + client->async_shutdown([&] { shutdown_latch.count_down(); }); + EXPECT_TRUE(shutdown_latch.wait_for(5000ms)); +} From c7d13d8ab6c00c5180b9b755f2de15fc0ddfcc59 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 27 Apr 2026 23:27:12 -0700 Subject: [PATCH 13/13] fix: fix a minor edge case --- libs/server-sent-events/src/client.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index fb3eb1a05..e5ae7a8b6 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -187,6 +187,10 @@ class FoxyClient : public Client, } void do_run() { + if (shutting_down_) { + return; + } + if (connection_hook_) { connection_hook_(&req_); }