From 59d51eb527e5db67421b785566504cf9e8472289 Mon Sep 17 00:00:00 2001 From: drslebedev Date: Thu, 13 Aug 2026 12:20:23 +0200 Subject: [PATCH] fix(client): handle REST session creation failures Guard GET and SET requests before accessing failed REST sessions so hostname-resolution errors are returned through the command callback without terminating the worker. Use gai_strerror for resolver errors, add conditional DnsClient failure logging, and verify that the same RestClient remains usable afterward. Signed-off-by: drslebedev --- src/client/include/RestClientNative.hpp | 6 +- src/client/test/nghttp2_tests.cpp | 69 +++++++++++++++++++- src/rest/include/rest/RestUtils.hpp | 2 +- src/services/include/services/dns_client.hpp | 11 +++- 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/client/include/RestClientNative.hpp b/src/client/include/RestClientNative.hpp index 46c5f799..0707e209 100644 --- a/src/client/include/RestClientNative.hpp +++ b/src/client/include/RestClientNative.hpp @@ -659,7 +659,11 @@ struct RestClient : public ClientBase { switch (cmd.command) { case mdp::Command::Get: case mdp::Command::Set: { - auto session = ensureSession(ssl_ctx, sessions, sslSettings, cmd.topic); + auto session = ensureSession(ssl_ctx, sessions, sslSettings, cmd.topic); + if (!session) { + reportError(cmd, std::format("Could not create REST session for endpoint '{}': {}", cmd.topic.str(), session.error())); + continue; + } auto preferred = preferredMimeType(cmd.topic); session.value()->submitRequest(std::move(cmd), mode, std::move(preferred), {}); } break; diff --git a/src/client/test/nghttp2_tests.cpp b/src/client/test/nghttp2_tests.cpp index 4073f78e..e40bb4b7 100644 --- a/src/client/test/nghttp2_tests.cpp +++ b/src/client/test/nghttp2_tests.cpp @@ -129,7 +129,7 @@ TEST_CASE("Basic Client Constructor and API Tests", "[http2]") { TEST_CASE("GET HTTP", "[http2]") { using namespace opencmw::client; - auto serverThread = std::jthread([](std::stop_token stopToken) { + auto serverThread = std::jthread([](std::stop_token stopToken) { RestServer server; majordomo::rest::Settings settings{ .port = kServerPort, .protocols = majordomo::rest::Http2 }; REQUIRE(server.bind(settings)); @@ -408,6 +408,73 @@ TEST_CASE("GET/SET", "[http2]") { waitFor(responseCount, 3); } +TEST_CASE("REST client survives hostname resolution failure", "[http2]") { + std::atomic responseCount = 0; + mdp::Message failedResponse; + mdp::Message successfulResponse; + client::RestClient client; + + client::Command invalidRequest; + invalidRequest.command = mdp::Command::Set; + invalidRequest.clientRequestID = opencmw::IoBuffer("unresolvable"); + invalidRequest.topic = URI<>("http://opencmw-rest-client-test.invalid:12345/dns"); + invalidRequest.callback = [&failedResponse, &responseCount](const mdp::Message &msg) { + failedResponse = msg; + responseCount++; + }; + client.request(std::move(invalidRequest)); + + REQUIRE(waitFor(responseCount, 1, std::chrono::seconds(30))); + REQUIRE(failedResponse.command == mdp::Command::Final); + REQUIRE(failedResponse.topic.str().contains("opencmw-rest-client-test.invalid")); + REQUIRE(failedResponse.error.contains("opencmw-rest-client-test.invalid")); + REQUIRE(failedResponse.error.contains("Could not resolve address")); + + auto serverThread = std::jthread([](std::stop_token stopToken) { + RestServer server; + majordomo::rest::Settings settings{ .port = kServerPort, .protocols = majordomo::rest::Http2 }; + REQUIRE(server.bind(settings)); + + std::deque messages; + ensureMessageReceived(server, stopToken, messages); + REQUIRE(messages.size() >= 1); + const auto request = std::move(messages.front()); + messages.pop_front(); + REQUIRE(request.command == mdp::Command::Get); + REQUIRE(request.topic.path() == "/sayhello"); + + Message reply; + reply.command = mdp::Command::Final; + reply.clientRequestID = request.clientRequestID; + reply.topic = URI<>("/sayhello"); + reply.data = opencmw::IoBuffer("worker survived"); + server.handleResponse(std::move(reply)); + + ensureMessageReceived(server, stopToken, messages); // makes sure the response is sent + }); + + Stopper stopper(serverThread.get_stop_source()); + std::this_thread::sleep_for(std::chrono::milliseconds(300)); // give the server some time to start listening + + client::Command validRequest; + validRequest.command = mdp::Command::Get; + validRequest.clientRequestID = opencmw::IoBuffer("valid"); + validRequest.topic = URI<>(std::format("http://localhost:{}/sayhello", kServerPort)); + validRequest.callback = [&successfulResponse, &responseCount](const mdp::Message &msg) { + successfulResponse = msg; + responseCount++; + }; + client.request(std::move(validRequest)); + + REQUIRE(waitFor(responseCount, 2)); + INFO(successfulResponse.error); + REQUIRE(successfulResponse.command == mdp::Command::Final); + REQUIRE(successfulResponse.error.empty()); + REQUIRE(successfulResponse.data.asString() == "worker survived"); + REQUIRE(successfulResponse.clientRequestID.asString() == "valid"); + REQUIRE(successfulResponse.topic.path() == "/sayhello"); +} + TEST_CASE("Long polling example", "[http2]") { constexpr int kFooMessages = 50; diff --git a/src/rest/include/rest/RestUtils.hpp b/src/rest/include/rest/RestUtils.hpp index 0cb8d7b6..8c745b9a 100644 --- a/src/rest/include/rest/RestUtils.hpp +++ b/src/rest/include/rest/RestUtils.hpp @@ -333,7 +333,7 @@ struct TcpSocket { struct addrinfo *res; int status = getaddrinfo(host.data(), nullptr, &hints, &res); if (status != 0) { - return std::unexpected(std::format("Could not resolve address: {}", strerror(status))); + return std::unexpected(std::format("Could not resolve address '{}': {}", host, gai_strerror(status))); } address = AddrinfoPtr(res, freeaddrinfo); reinterpret_cast(address->ai_addr)->sin_port = htons(port); diff --git a/src/services/include/services/dns_client.hpp b/src/services/include/services/dns_client.hpp index a9508d2f..a2b0a882 100644 --- a/src/services/include/services/dns_client.hpp +++ b/src/services/include/services/dns_client.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -33,7 +34,9 @@ struct DnsClient { uri = std::move(uri).setQuery(query::serialise(filter)); _clientContext.get(uri.build(), [callback = std::move(callback)](const mdp::Message &msg) { - std::cout << msg.error << std::endl; + if (!msg.error.empty()) { + std::cerr << "DNS signal query failed for '" << msg.topic.str() << "': " << msg.error << '\n'; + } IoBuffer buf{ msg.data }; FlatEntryList resp; @@ -54,6 +57,9 @@ struct DnsClient { _clientContext.set( _endpoint, [callback = std::move(callback)](auto &msg) { + if (!msg.error.empty()) { + std::cerr << "DNS signal registration failed for '" << msg.topic.str() << "': " << msg.error << '\n'; + } FlatEntryList resp; IoBuffer buf{ msg.data }; if (!buf.empty()) { @@ -76,6 +82,9 @@ struct DnsClient { _clientContext.set( uri.build(), [callback = std::move(callback)](auto &msg) { + if (!msg.error.empty()) { + std::cerr << "DNS signal unregistration failed for '" << msg.topic.str() << "': " << msg.error << '\n'; + } FlatEntryList resp; IoBuffer buf{ msg.data }; if (!buf.empty()) {