Skip to content

Commit 3488e09

Browse files
committed
chore: log error when FDv2 filter key is invalid
1 parent 61174d8 commit 3488e09

6 files changed

Lines changed: 31 additions & 12 deletions

File tree

libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ network::HttpRequest MakeFDv2PollRequest(
4444
config::built::ServiceEndpoints const& endpoints,
4545
config::built::HttpProperties const& http_properties,
4646
data_model::Selector const& selector,
47-
std::optional<std::string> const& filter_key) {
47+
std::optional<std::string> const& filter_key,
48+
Logger const& logger) {
4849
config::builders::HttpPropertiesBuilder const builder(http_properties);
4950

5051
auto parsed = boost::urls::parse_uri(endpoints.PollingBaseUrl());
@@ -65,8 +66,14 @@ network::HttpRequest MakeFDv2PollRequest(
6566
if (selector.value) {
6667
u.params().append({"basis", selector.value->state});
6768
}
68-
if (filter_key && detail::ValidateFilterKey(*filter_key)) {
69-
u.params().append({"filter", *filter_key});
69+
if (filter_key) {
70+
if (detail::ValidateFilterKey(*filter_key)) {
71+
u.params().append({"filter", *filter_key});
72+
} else {
73+
LD_LOG(logger, LogLevel::kError)
74+
<< "data source config: provided filter is invalid, will "
75+
"request full environment instead";
76+
}
7077
}
7178

7279
return {std::string(u.buffer()), network::HttpMethod::kGet, builder.Build(),

libs/server-sdk/src/data_systems/fdv2/fdv2_polling_impl.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ network::HttpRequest MakeFDv2PollRequest(
1919
config::built::ServiceEndpoints const& endpoints,
2020
config::built::HttpProperties const& http_properties,
2121
data_model::Selector const& selector,
22-
std::optional<std::string> const& filter_key);
22+
std::optional<std::string> const& filter_key,
23+
Logger const& logger);
2324

2425
// Parse an HTTP response from the FDv2 polling endpoint through the protocol
2526
// handler and return the appropriate result. identity is used in log messages

libs/server-sdk/src/data_systems/fdv2/polling_initializer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ FDv2PollingInitializer::FDv2PollingInitializer(
2020
: request_(MakeFDv2PollRequest(endpoints,
2121
http_properties,
2222
std::move(selector),
23-
std::move(filter_key))),
23+
std::move(filter_key),
24+
logger)),
2425
requester_(executor, http_properties.Tls()),
2526
state_(std::make_shared<State>(logger)) {}
2627

libs/server-sdk/src/data_systems/fdv2/polling_synchronizer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ FDv2PollingSynchronizer::State::State(
3434
async::Future<network::HttpResult> FDv2PollingSynchronizer::State::Request(
3535
data_model::Selector const& selector) const {
3636
auto request = MakeFDv2PollRequest(endpoints_, http_properties_, selector,
37-
filter_key_);
37+
filter_key_, logger_);
3838

3939
// Promise must be in a shared_ptr because Requester requires callbacks
4040
// to be copy-constructible (stored in std::function).

libs/server-sdk/src/data_systems/fdv2/streaming_synchronizer.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,14 @@ void FDv2StreamingSynchronizer::State::OnConnect(HttpRequest* req) {
172172
if (latest_selector_.value) {
173173
u.params().set("basis", latest_selector_.value->state);
174174
}
175-
if (filter_key_ && detail::ValidateFilterKey(*filter_key_)) {
176-
u.params().set("filter", *filter_key_);
175+
if (filter_key_) {
176+
if (detail::ValidateFilterKey(*filter_key_)) {
177+
u.params().set("filter", *filter_key_);
178+
} else {
179+
LD_LOG(logger_, LogLevel::kError)
180+
<< "data source config: provided filter is invalid, will "
181+
"request full environment instead";
182+
}
177183
}
178184
req->target(u.encoded_target());
179185
}

libs/server-sdk/tests/fdv2_polling_impl_test.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,41 +110,45 @@ TEST(HandleFDv2PollResponseTest, NetworkErrorDoesNotSetFlag) {
110110
}
111111

112112
TEST(MakeFDv2PollRequestTest, BaseWithTrailingSlashDoesNotProduceDoubleSlash) {
113+
auto logger = MakeNullLogger();
113114
config::shared::built::ServiceEndpoints endpoints{"http://example.com/", "",
114115
""};
115116
auto props =
116117
config::shared::Defaults<config::shared::ServerSDK>::HttpProperties();
117118
auto req = MakeFDv2PollRequest(endpoints, props, data_model::Selector{},
118-
std::nullopt);
119+
std::nullopt, logger);
119120
EXPECT_EQ(req.Url(), "http://example.com/sdk/poll");
120121
}
121122

122123
TEST(MakeFDv2PollRequestTest, BaseWithSubpathTrailingSlashJoinsCleanly) {
124+
auto logger = MakeNullLogger();
123125
config::shared::built::ServiceEndpoints endpoints{
124126
"http://example.com/relay/", "", ""};
125127
auto props =
126128
config::shared::Defaults<config::shared::ServerSDK>::HttpProperties();
127129
auto req = MakeFDv2PollRequest(endpoints, props, data_model::Selector{},
128-
std::nullopt);
130+
std::nullopt, logger);
129131
EXPECT_EQ(req.Url(), "http://example.com/relay/sdk/poll");
130132
}
131133

132134
TEST(MakeFDv2PollRequestTest, ValidFilterKeyIsIncluded) {
135+
auto logger = MakeNullLogger();
133136
config::shared::built::ServiceEndpoints endpoints{"http://example.com", "",
134137
""};
135138
auto props =
136139
config::shared::Defaults<config::shared::ServerSDK>::HttpProperties();
137140
auto req = MakeFDv2PollRequest(endpoints, props, data_model::Selector{},
138-
std::string{"my-filter_1.0"});
141+
std::string{"my-filter_1.0"}, logger);
139142
EXPECT_EQ(req.Url(), "http://example.com/sdk/poll?filter=my-filter_1.0");
140143
}
141144

142145
TEST(MakeFDv2PollRequestTest, InvalidFilterKeyIsDropped) {
146+
auto logger = MakeNullLogger();
143147
config::shared::built::ServiceEndpoints endpoints{"http://example.com", "",
144148
""};
145149
auto props =
146150
config::shared::Defaults<config::shared::ServerSDK>::HttpProperties();
147151
auto req = MakeFDv2PollRequest(endpoints, props, data_model::Selector{},
148-
std::string{"has spaces"});
152+
std::string{"has spaces"}, logger);
149153
EXPECT_EQ(req.Url(), "http://example.com/sdk/poll");
150154
}

0 commit comments

Comments
 (0)