Skip to content

Commit 85e9a36

Browse files
committed
[TEST] Cover the synchronous refusal and the instrumented constructor
Codecov had three patch lines nothing reached. Both are reachable. Send() carries the same construction_result_ guard as SendAsync, and nothing called it after a failed construction. The case builds an operation whose header list cannot be allocated and calls Send(): without the guard the request goes out carrying none of its headers and the server sees it, five runs out of five, so the case fails rather than merely covering the line. The other line is the multi handle in the constructor that takes thread instrumentation. Both constructors reach it through the same helper, so the case uses that overload with a null instrumentation and holds that it reports a handle it could not create, the same way the plain one does. Patch coverage for this branch measured with the abiv2-preview configuration the Codecov job uses: three missing lines before, none after. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 01cc6da commit 85e9a36

1 file changed

Lines changed: 77 additions & 0 deletions

File tree

‎ext/test/http/curl_http_test.cc‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@
1616
#include <atomic>
1717
#include <chrono>
1818
#include <condition_variable>
19+
#include <cstdio>
1920
#include <cstdlib>
2021
#include <cstring>
2122
#include <map>
23+
#include <memory>
2224
#include <mutex>
2325
#include <sstream>
2426
#include <string>
@@ -35,6 +37,7 @@
3537
#include "opentelemetry/nostd/shared_ptr.h"
3638
#include "opentelemetry/nostd/string_view.h"
3739
#include "opentelemetry/sdk/common/global_log_handler.h"
40+
#include "opentelemetry/sdk/common/thread_instrumentation.h"
3841
#include "opentelemetry/version.h"
3942

4043
constexpr int HTTP_PORT{19000};
@@ -994,6 +997,80 @@ TEST_F(BasicCurlHttpTests, AFailedMultiHandleAllocationIsReported)
994997
<< "a multi handle that could not be created was not reported, captured: " << text;
995998
}
996999

1000+
// SendAsync refuses a request whose header list could not be built, and Send has to refuse it
1001+
// the same way, or a synchronous caller puts one on the wire carrying none of its headers.
1002+
TEST_F(BasicCurlHttpTests, ASynchronousSendRefusesAFailedHeaderAllocation)
1003+
{
1004+
ASSERT_TRUE(g_curl_hooks_installed);
1005+
received_requests_.clear();
1006+
1007+
// The operation keeps references to these, so they outlive it.
1008+
const http_client::HttpSslOptions no_ssl;
1009+
const http_client::Body body;
1010+
const http_client::Headers headers = {{"X-Test", "1"}};
1011+
const http_client::Compression compression = http_client::Compression::kNone;
1012+
1013+
std::unique_ptr<curl::HttpOperation> operation;
1014+
{
1015+
// Only the constructor allocates under the switch. Send needs its own allocations to work.
1016+
FailCurlMalloc fail;
1017+
operation.reset(new curl::HttpOperation(http_client::Method::Get, "http://127.0.0.1:19000/get/",
1018+
no_ssl, nullptr, headers, body, compression));
1019+
}
1020+
1021+
const CURLcode result = operation->Send();
1022+
1023+
size_t requests_seen = 0;
1024+
{
1025+
std::unique_lock<std::mutex> lock_requests(mtx_requests);
1026+
requests_seen = received_requests_.size();
1027+
}
1028+
1029+
EXPECT_EQ(static_cast<size_t>(0), requests_seen)
1030+
<< "a synchronous request went out after its setup had failed";
1031+
1032+
if (CURLE_OUT_OF_MEMORY != result)
1033+
{
1034+
GTEST_SKIP() << "this libcurl consumed the failing allocation before the header list, "
1035+
<< "reported as: " << curl_easy_strerror(result);
1036+
}
1037+
EXPECT_EQ(CURLE_OUT_OF_MEMORY, operation->GetLastResultCode())
1038+
<< "the refusal was not recorded as the last result";
1039+
}
1040+
1041+
// Both constructors reach the multi handle through the same helper, so the overload that takes
1042+
// thread instrumentation reports a failed one too. A null instrumentation is enough to pick it.
1043+
TEST_F(BasicCurlHttpTests, AFailedMultiHandleIsReportedForAnInstrumentedClient)
1044+
{
1045+
ASSERT_TRUE(g_curl_hooks_installed);
1046+
1047+
{
1048+
auto warmup = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create();
1049+
ASSERT_TRUE(warmup != nullptr);
1050+
warmup->FinishAllSessions();
1051+
}
1052+
1053+
auto *capture = new CapturingLogHandler();
1054+
auto previous = opentelemetry::sdk::common::internal_log::GlobalLogHandler::GetLogHandler();
1055+
opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(
1056+
nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(capture));
1057+
1058+
{
1059+
FailCurlCalloc fail;
1060+
auto client = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create(
1061+
std::shared_ptr<opentelemetry::sdk::common::ThreadInstrumentation>{});
1062+
ASSERT_TRUE(client != nullptr);
1063+
client->FinishAllSessions();
1064+
}
1065+
1066+
const std::string text = capture->Text();
1067+
opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(previous);
1068+
1069+
EXPECT_NE(std::string::npos, text.find("curl_multi_init failed"))
1070+
<< "the instrumented constructor did not report a multi handle it could not create, "
1071+
<< "captured: " << text;
1072+
}
1073+
9971074
// Reporting the failure does not by itself stop the client taking requests, so this holds what
9981075
// taking one leads to. The IO loop resets the multi handle whenever curl_multi_perform rejects
9991076
// it, so a client built on a null handle repairs itself and answers; if the allocation is still

0 commit comments

Comments
 (0)