Skip to content

Commit 3c6377d

Browse files
beekldbeekld
authored
fix: deflake hooks-ordering contract test (#563)
## Summary Fixes flaky contract tests `hooks/evaluation/executes beforeEvaluation hooks in registration order` and the corresponding `afterEvaluation` reverse-order test, which had been intermittently red on CI. Root cause: `ContractTestHook::PostCallback` was firing the harness notifications as fire-and-forget async chains (resolve -> connect -> write). The SDK invokes hooks sequentially, but the three async POSTs then race on the wire, so the harness can receive them in any order. Fix: send each callback synchronously via the boost::beast sync HTTP API. The sync path uses blocking syscalls rather than the reactor, so there is no deadlock on the single-thread io_context, and each POST completes before the next hook's call starts. Local repro on macOS over loopback: 2/50 fails before, 0/100 after. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Test-only contract hook infrastructure; no production SDK behavior change beyond blocking briefly during test callbacks. > > **Overview** > **`ContractTestHook::PostCallback`** no longer uses fire-and-forget async resolve/connect/write/read chains. Each harness notification is sent with **synchronous** Boost.Beast HTTP (resolve, connect, write, read, shutdown) so a POST finishes before the next hook stage runs. > > That preserves **callback arrival order** when the SDK invokes hooks sequentially, which stabilizes hooks-ordering contract tests that assert registration order for `beforeEvaluation` / `afterEvaluation`. > > The rest of the diff is import reordering and minor JSON serialization formatting; hook behavior is unchanged aside from callback delivery semantics. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fd2de8b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 5f8bf79 commit 3c6377d

1 file changed

Lines changed: 49 additions & 73 deletions

File tree

‎contract-tests/server-contract-tests/src/contract_test_hook.cpp‎

Lines changed: 49 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#include "contract_test_hook.hpp"
22

3+
#include <launchdarkly/detail/serialization/json_primitives.hpp>
4+
#include <launchdarkly/detail/serialization/json_value.hpp>
35
#include <launchdarkly/serialization/json_context.hpp>
46
#include <launchdarkly/serialization/json_evaluation_reason.hpp>
5-
#include <launchdarkly/detail/serialization/json_value.hpp>
6-
#include <launchdarkly/detail/serialization/json_primitives.hpp>
77

8-
#include <boost/asio/ip/tcp.hpp>
98
#include <boost/asio/connect.hpp>
9+
#include <boost/asio/ip/tcp.hpp>
1010
#include <boost/asio/post.hpp>
1111
#include <boost/beast/core.hpp>
1212
#include <boost/beast/http.hpp>
@@ -55,10 +55,8 @@ std::optional<std::string> ContractTestHook::GetErrorForStage(
5555

5656
void ContractTestHook::PostCallback(std::string const& stage,
5757
nlohmann::json const& payload) {
58-
// Parse the callback URI
5958
auto uri_result = boost::urls::parse_uri(config_.callbackUri);
6059
if (!uri_result) {
61-
// Invalid URI, skip callback
6260
return;
6361
}
6462

@@ -70,57 +68,44 @@ void ContractTestHook::PostCallback(std::string const& stage,
7068
target = "/";
7169
}
7270

73-
std::string body = payload.dump();
74-
75-
// Create a shared connection and post asynchronously without blocking
76-
// This uses fire-and-forget semantics - we don't wait for response
77-
auto conn = std::make_shared<beast::tcp_stream>(executor_);
78-
auto resolver = std::make_shared<tcp::resolver>(executor_);
79-
80-
// Capture everything needed by value in shared_ptrs
81-
auto req = std::make_shared<http::request<http::string_body>>();
82-
req->method(http::verb::post);
83-
req->target(target);
84-
req->version(11);
85-
req->set(http::field::host, host);
86-
req->set(http::field::user_agent, "cpp-server-sdk-contract-tests");
87-
req->set(http::field::content_type, "application/json");
88-
req->body() = body;
89-
req->prepare_payload();
90-
91-
// Start async resolution
92-
resolver->async_resolve(host, port,
93-
[conn, resolver, req](beast::error_code ec, tcp::resolver::results_type results) {
94-
if (ec) return; // Silently ignore resolution errors
95-
96-
// Connect asynchronously
97-
conn->async_connect(results,
98-
[conn, resolver, req](beast::error_code ec, tcp::resolver::results_type::endpoint_type) {
99-
if (ec) return; // Silently ignore connection errors
100-
101-
// Write request asynchronously
102-
http::async_write(*conn, *req,
103-
[conn, resolver, req](beast::error_code ec, std::size_t) {
104-
if (ec) return; // Silently ignore write errors
105-
106-
// Read response asynchronously (but don't wait for result)
107-
auto res = std::make_shared<http::response<http::string_body>>();
108-
auto buffer = std::make_shared<beast::flat_buffer>();
109-
http::async_read(*conn, *buffer, *res,
110-
[conn, resolver, req, res, buffer](beast::error_code ec, std::size_t) {
111-
// Close connection gracefully
112-
conn->socket().shutdown(tcp::socket::shutdown_both, ec);
113-
// Cleanup happens automatically via shared_ptr
114-
});
115-
});
116-
});
117-
});
71+
// Send synchronously so callbacks from sequential hook invocations arrive
72+
// at the harness in the order the SDK invoked them.
73+
beast::error_code ec;
74+
tcp::resolver resolver(executor_);
75+
auto results = resolver.resolve(host, port, ec);
76+
if (ec) {
77+
return;
78+
}
79+
80+
beast::tcp_stream stream(executor_);
81+
stream.connect(results, ec);
82+
if (ec) {
83+
return;
84+
}
85+
86+
http::request<http::string_body> req{http::verb::post, target, 11};
87+
req.set(http::field::host, host);
88+
req.set(http::field::user_agent, "cpp-server-sdk-contract-tests");
89+
req.set(http::field::content_type, "application/json");
90+
req.body() = payload.dump();
91+
req.prepare_payload();
92+
93+
http::write(stream, req, ec);
94+
if (ec) {
95+
return;
96+
}
97+
98+
beast::flat_buffer buffer;
99+
http::response<http::string_body> res;
100+
http::read(stream, buffer, res, ec);
101+
102+
beast::error_code ignore_ec;
103+
stream.socket().shutdown(tcp::socket::shutdown_both, ignore_ec);
118104
}
119105

120106
EvaluationSeriesData ContractTestHook::BeforeEvaluation(
121107
EvaluationSeriesContext const& series_context,
122108
EvaluationSeriesData data) {
123-
124109
// Check if we should throw an error for this stage
125110
auto error = GetErrorForStage("beforeEvaluation");
126111
if (error) {
@@ -149,12 +134,10 @@ EvaluationSeriesData ContractTestHook::BeforeEvaluation(
149134
// EvaluationSeriesContext
150135
nlohmann::json ctx;
151136
ctx["flagKey"] = std::string(series_context.FlagKey());
152-
ctx["context"] = nlohmann::json::parse(
153-
boost::json::serialize(
154-
boost::json::value_from(series_context.EvaluationContext())));
155-
ctx["defaultValue"] = nlohmann::json::parse(
156-
boost::json::serialize(
157-
boost::json::value_from(series_context.DefaultValue())));
137+
ctx["context"] = nlohmann::json::parse(boost::json::serialize(
138+
boost::json::value_from(series_context.EvaluationContext())));
139+
ctx["defaultValue"] = nlohmann::json::parse(boost::json::serialize(
140+
boost::json::value_from(series_context.DefaultValue())));
158141
ctx["method"] = std::string(series_context.Method());
159142
if (series_context.EnvironmentId()) {
160143
ctx["environmentId"] = std::string(*series_context.EnvironmentId());
@@ -181,7 +164,6 @@ EvaluationSeriesData ContractTestHook::AfterEvaluation(
181164
EvaluationSeriesContext const& series_context,
182165
EvaluationSeriesData data,
183166
launchdarkly::EvaluationDetail<launchdarkly::Value> const& detail) {
184-
185167
// Check if we should throw an error for this stage
186168
auto error = GetErrorForStage("afterEvaluation");
187169
if (error) {
@@ -195,12 +177,10 @@ EvaluationSeriesData ContractTestHook::AfterEvaluation(
195177
// EvaluationSeriesContext
196178
nlohmann::json ctx;
197179
ctx["flagKey"] = std::string(series_context.FlagKey());
198-
ctx["context"] = nlohmann::json::parse(
199-
boost::json::serialize(
200-
boost::json::value_from(series_context.EvaluationContext())));
201-
ctx["defaultValue"] = nlohmann::json::parse(
202-
boost::json::serialize(
203-
boost::json::value_from(series_context.DefaultValue())));
180+
ctx["context"] = nlohmann::json::parse(boost::json::serialize(
181+
boost::json::value_from(series_context.EvaluationContext())));
182+
ctx["defaultValue"] = nlohmann::json::parse(boost::json::serialize(
183+
boost::json::value_from(series_context.DefaultValue())));
204184
ctx["method"] = std::string(series_context.Method());
205185
if (series_context.EnvironmentId()) {
206186
ctx["environmentId"] = std::string(*series_context.EnvironmentId());
@@ -236,9 +216,7 @@ EvaluationSeriesData ContractTestHook::AfterEvaluation(
236216
return data;
237217
}
238218

239-
void ContractTestHook::AfterTrack(
240-
TrackSeriesContext const& series_context) {
241-
219+
void ContractTestHook::AfterTrack(TrackSeriesContext const& series_context) {
242220
// Check if we should throw an error for this stage
243221
auto error = GetErrorForStage("afterTrack");
244222
if (error) {
@@ -251,17 +229,15 @@ void ContractTestHook::AfterTrack(
251229

252230
// TrackSeriesContext
253231
nlohmann::json ctx;
254-
ctx["context"] = nlohmann::json::parse(
255-
boost::json::serialize(
256-
boost::json::value_from(series_context.TrackContext())));
232+
ctx["context"] = nlohmann::json::parse(boost::json::serialize(
233+
boost::json::value_from(series_context.TrackContext())));
257234
ctx["key"] = std::string(series_context.Key());
258235
if (series_context.MetricValue()) {
259236
ctx["metricValue"] = *series_context.MetricValue();
260237
}
261238
if (series_context.Data()) {
262-
ctx["data"] = nlohmann::json::parse(
263-
boost::json::serialize(
264-
boost::json::value_from(series_context.Data()->get())));
239+
ctx["data"] = nlohmann::json::parse(boost::json::serialize(
240+
boost::json::value_from(series_context.Data()->get())));
265241
}
266242
if (series_context.EnvironmentId()) {
267243
ctx["environmentId"] = std::string(*series_context.EnvironmentId());

0 commit comments

Comments
 (0)