Revert "SERVER-53423: Make ConnectString::connect return a status instead of setting error message"

This reverts commit ad1169d34cbd457a4d0637230e615bdf1d177531.

1 files changed, 98 insertions, 57 deletions

diff --git a/src/mongo/rpc/op_msg_integration_test.cpp b/src/mongo/rpc/op_msg_integration_test.cpp
index 6d3bb094720..c3f285e46c5 100644
--- a/src/mongo/rpc/op_msg_integration_test.cpp
+++ b/src/mongo/rpc/op_msg_integration_test.cpp
@@ -46,7 +46,6 @@
 #include "mongo/util/scopeguard.h"
 
 namespace mongo {
-namespace {
 
 template <typename F>
 bool waitForCondition(F&& f) {
@@ -64,13 +63,6 @@ bool waitForCondition(F&& f) {
     return false;
 }
 
-std::unique_ptr<DBClientBase> getIntegrationTestConnection() {
-    auto swConn = unittest::getFixtureConnectionString().connect("integration_test");
-    uassertStatusOK(swConn.getStatus());
-    return std::move(swConn.getValue());
-}
-
-
 // Returns the connection name by filtering on the appName of a $currentOp command. If no result is
 // found, return an empty string.
 std::string getThreadNameByAppName(DBClientBase* conn, StringData appName) {
@@ -86,7 +78,10 @@ std::string getThreadNameByAppName(DBClientBase* conn, StringData appName) {
 }
 
 TEST(OpMsg, UnknownRequiredFlagClosesConnection) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     auto request = OpMsgRequest::fromDBAndBody("admin", BSON("ping" << 1)).serialize();
     OpMsg::setFlag(&request, 1u << 15);  // This should be the last required flag to be assigned.
@@ -96,7 +91,10 @@ TEST(OpMsg, UnknownRequiredFlagClosesConnection) {
 }
 
 TEST(OpMsg, UnknownOptionalFlagIsIgnored) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     auto request = OpMsgRequest::fromDBAndBody("admin", BSON("ping" << 1)).serialize();
     OpMsg::setFlag(&request, 1u << 31);  // This should be the last optional flag to be assigned.
@@ -108,7 +106,10 @@ TEST(OpMsg, UnknownOptionalFlagIsIgnored) {
 }
 
 TEST(OpMsg, FireAndForgetInsertWorks) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     conn->dropCollection("test.collection");
 
@@ -124,7 +125,10 @@ TEST(OpMsg, FireAndForgetInsertWorks) {
 }
 
 TEST(OpMsg, DocumentSequenceLargeDocumentMultiInsertWorks) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     conn->dropCollection("test.collection");
 
@@ -155,7 +159,10 @@ TEST(OpMsg, DocumentSequenceLargeDocumentMultiInsertWorks) {
 }
 
 TEST(OpMsg, DocumentSequenceMaxWriteBatchWorks) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     conn->dropCollection("test.collection");
 
@@ -275,7 +282,10 @@ TEST(OpMsg, CloseConnectionOnFireAndForgetNotWritablePrimaryError) {
 }
 
 TEST(OpMsg, DocumentSequenceReturnsWork) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     auto opMsgRequest = OpMsgRequest::fromDBAndBody("admin", BSON("echo" << 1));
     opMsgRequest.sequences.push_back({"example", {BSON("a" << 1), BSON("b" << 2)}});
@@ -316,7 +326,10 @@ void enableClientChecksum() {
 }
 
 void exhaustGetMoreTest(bool enableChecksum) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     // Only test exhaust against a standalone.
     if (conn->isReplicaSetMember() || conn->isMongos()) {
@@ -408,7 +421,10 @@ TEST(OpMsg, ServerHandlesExhaustGetMoreCorrectlyWithChecksum) {
 }
 
 TEST(OpMsg, FindIgnoresExhaust) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     // Only test exhaust against a standalone.
     if (conn->isReplicaSetMember() || conn->isMongos()) {
@@ -440,7 +456,10 @@ TEST(OpMsg, FindIgnoresExhaust) {
 }
 
 TEST(OpMsg, ServerDoesNotSetMoreToComeOnErrorInGetMore) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     // Only test exhaust against a standalone.
     if (conn->isReplicaSetMember() || conn->isMongos()) {
@@ -487,7 +506,10 @@ TEST(OpMsg, ServerDoesNotSetMoreToComeOnErrorInGetMore) {
 }
 
 TEST(OpMsg, MongosIgnoresExhaustForGetMore) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     if (!conn->isMongos()) {
         return;
@@ -536,9 +558,10 @@ TEST(OpMsg, MongosIgnoresExhaustForGetMore) {
 }
 
 TEST(OpMsg, ServerHandlesExhaustIsMasterCorrectly) {
-    auto swConn = unittest::getFixtureConnectionString().connect("integration_test");
-    uassertStatusOK(swConn.getStatus());
-    auto fixtureConn = std::move(swConn.getValue());
+    std::string errMsg;
+    auto fixtureConn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, fixtureConn);
     DBClientBase* conn = fixtureConn.get();
 
     if (fixtureConn->isReplicaSetMember()) {
@@ -598,9 +621,10 @@ TEST(OpMsg, ServerHandlesExhaustIsMasterCorrectly) {
 }
 
 TEST(OpMsg, ServerHandlesExhaustIsMasterWithTopologyChange) {
-    auto swConn = unittest::getFixtureConnectionString().connect("integration_test");
-    uassertStatusOK(swConn.getStatus());
-    auto fixtureConn = std::move(swConn.getValue());
+    std::string errMsg;
+    auto fixtureConn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, fixtureConn);
     DBClientBase* conn = fixtureConn.get();
 
     if (fixtureConn->isReplicaSetMember()) {
@@ -663,9 +687,10 @@ TEST(OpMsg, ServerHandlesExhaustIsMasterWithTopologyChange) {
 }
 
 TEST(OpMsg, ServerRejectsExhaustIsMasterWithoutMaxAwaitTimeMS) {
-    auto swConn = unittest::getFixtureConnectionString().connect("integration_test");
-    uassertStatusOK(swConn.getStatus());
-    auto fixtureConn = std::move(swConn.getValue());
+    std::string errMsg;
+    auto fixtureConn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, fixtureConn);
     DBClientBase* conn = fixtureConn.get();
 
     if (fixtureConn->isReplicaSetMember()) {
@@ -687,7 +712,10 @@ TEST(OpMsg, ServerRejectsExhaustIsMasterWithoutMaxAwaitTimeMS) {
 }
 
 void serverStatusCorrectlyShowsExhaustMetrics(std::string commandName) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     if (conn->isReplicaSetMember()) {
         // Don't run on replica sets as the RSM will use the streamable hello or isMaster protocol
@@ -729,9 +757,10 @@ void serverStatusCorrectlyShowsExhaustMetrics(std::string commandName) {
     ASSERT_OK(getStatusFromCommandResult(res));
 
     // Start a new connection to the server to check the serverStatus metrics.
-    auto conn2 =
-        std::move(unittest::getFixtureConnectionString().connect("integration_test").getValue());
-    uassert(ErrorCodes::SocketException, "connection failed", conn2);
+    std::string newErrMsg;
+    auto conn2 = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", newErrMsg));
+    uassert(ErrorCodes::SocketException, newErrMsg, conn2);
 
     auto serverStatusCmd = BSON("serverStatus" << 1);
     BSONObj serverStatusReply;
@@ -759,10 +788,11 @@ TEST(OpMsg, ServerStatusCorrectlyShowsExhaustIsMasterMetricsWithIsMasterAlias) {
 }
 
 void exhaustMetricSwitchingCommandNames(bool useLegacyCommandNameAtStart) {
+    std::string errMsg;
     const auto conn1AppName = "integration_test";
-    auto swConn1 = unittest::getFixtureConnectionString().connect(conn1AppName);
-    uassertStatusOK(swConn1.getStatus());
-    auto conn1 = std::move(swConn1.getValue());
+    auto conn1 = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect(conn1AppName, errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn1);
 
     if (conn1->isReplicaSetMember()) {
         // Don't run on replica sets as the RSM will use the streamable hello or isMaster protocol
@@ -809,9 +839,10 @@ void exhaustMetricSwitchingCommandNames(bool useLegacyCommandNameAtStart) {
     ASSERT_OK(getStatusFromCommandResult(res));
 
     // Start a new connection to the server to check the serverStatus metrics.
-    auto conn2 =
-        std::move(unittest::getFixtureConnectionString().connect("integration_test2").getValue());
-    uassert(ErrorCodes::SocketException, "connection failed", conn2);
+    std::string newErrMsg;
+    auto conn2 = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test2", newErrMsg));
+    uassert(ErrorCodes::SocketException, newErrMsg, conn2);
 
     std::string threadName;
     ASSERT(waitForCondition([&] {
@@ -896,10 +927,11 @@ TEST(OpMsg, ExhaustHelloMetricSwitchingCommandNames) {
 
 
 void exhaustMetricDecrementsOnNewOpAfterTerminatingExhaustStream(bool useLegacyCommandName) {
+    std::string errMsg;
     const auto conn1AppName = "integration_test";
-    auto swConn1 = unittest::getFixtureConnectionString().connect(conn1AppName);
-    uassertStatusOK(swConn1.getStatus());
-    auto conn1 = std::move(swConn1.getValue());
+    auto conn1 = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect(conn1AppName, errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn1);
 
     if (conn1->isReplicaSetMember()) {
         // Don't run on replica sets as the RSM will use the streamable hello or isMaster protocol
@@ -945,9 +977,10 @@ void exhaustMetricDecrementsOnNewOpAfterTerminatingExhaustStream(bool useLegacyC
     ASSERT_OK(getStatusFromCommandResult(res));
 
     // Start a new connection to the server to check the serverStatus metrics.
-    auto conn2 =
-        std::move(unittest::getFixtureConnectionString().connect("integration_test2").getValue());
-    uassert(ErrorCodes::SocketException, "connection 2 failed", conn2);
+    std::string newErrMsg;
+    auto conn2 = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test2", newErrMsg));
+    uassert(ErrorCodes::SocketException, newErrMsg, conn2);
 
     std::string threadName;
     ASSERT(waitForCondition([&] {
@@ -1009,10 +1042,11 @@ TEST(OpMsg, ExhaustHelloMetricDecrementsOnNewOpAfterTerminatingExhaustStream) {
 }
 
 void exhaustMetricOnNewExhaustAfterTerminatingExhaustStream(bool useLegacyCommandName) {
+    std::string errMsg;
     const auto conn1AppName = "integration_test";
-    auto swConn1 = unittest::getFixtureConnectionString().connect(conn1AppName);
-    uassertStatusOK(swConn1.getStatus());
-    auto conn1 = std::move(swConn1.getValue());
+    auto conn1 = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect(conn1AppName, errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn1);
 
     if (conn1->isReplicaSetMember()) {
         // Don't run on replica sets as the RSM will use the streamable hello or isMaster protocol
@@ -1058,9 +1092,10 @@ void exhaustMetricOnNewExhaustAfterTerminatingExhaustStream(bool useLegacyComman
     ASSERT_OK(getStatusFromCommandResult(res));
 
     // Start a new connection to the server to check the serverStatus metrics.
-    auto conn2 =
-        std::move(unittest::getFixtureConnectionString().connect("integration_test2").getValue());
-    uassert(ErrorCodes::SocketException, "connection failed", conn2);
+    std::string newErrMsg;
+    auto conn2 = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test2", newErrMsg));
+    uassert(ErrorCodes::SocketException, newErrMsg, conn2);
 
     std::string threadName;
     ASSERT(waitForCondition([&] {
@@ -1140,7 +1175,10 @@ TEST(OpMsg, ExhaustWithDBClientCursorBehavesCorrectly) {
     // correctly. The externally visible behavior should technically be the same as a non-exhaust
     // cursor. The exhaust cursor should ideally provide a performance win over non-exhaust, but we
     // don't measure that here.
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     // Only test exhaust against a standalone.
     if (conn->isReplicaSetMember() || conn->isMongos()) {
@@ -1190,7 +1228,10 @@ TEST(OpMsg, ExhaustWithDBClientCursorBehavesCorrectly) {
 
 void checksumTest(bool enableChecksum) {
     // The server replies with a checksum if and only if the request has a checksum.
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = std::unique_ptr<DBClientBase>(
+        unittest::getFixtureConnectionString().connect("integration_test", errMsg));
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     if (!enableChecksum) {
         disableClientChecksum();
@@ -1217,7 +1258,9 @@ TEST(OpMsg, ServerRepliesWithChecksumToRequestWithChecksum) {
 }
 
 TEST(OpMsg, ServerHandlesReallyLargeMessagesGracefully) {
-    auto conn = getIntegrationTestConnection();
+    std::string errMsg;
+    auto conn = unittest::getFixtureConnectionString().connect("integration_test", errMsg);
+    uassert(ErrorCodes::SocketException, errMsg, conn);
 
     auto buildInfo = conn->runCommand(OpMsgRequest::fromDBAndBody("admin", BSON("buildInfo" << 1)))
                          ->getCommandReply();
@@ -1256,10 +1299,9 @@ public:
             uri.setHelloOk(helloOk.get());
         }
 
-        auto swConn = connStr.connect(_appName, 0, &uri);
-        uassertStatusOK(swConn.getStatus());
-        auto conn = std::move(swConn.getValue());
-        uassert(ErrorCodes::SocketException, "connection failed", conn);
+        std::string errMsg;
+        auto conn = connStr.connect(_appName, errMsg, 0, &uri);
+        uassert(ErrorCodes::SocketException, errMsg, conn);
 
         _configureFailPoint(conn.get());
         return conn;
@@ -1318,5 +1360,4 @@ TEST(OpMsg, HelloOkCanBeDisabled) {
     ASSERT(!isHelloOk);
 }
 
-}  // namespace
 }  // namespace mongo