SERVER-65946 Have DBClient::call always throw exceptions on failure

8 files changed, 42 insertions, 75 deletions

diff --git a/src/mongo/client/dbclient_base.cpp b/src/mongo/client/dbclient_base.cpp
index 2c2bcf36412..30179b3914b 100644
--- a/src/mongo/client/dbclient_base.cpp
+++ b/src/mongo/client/dbclient_base.cpp
@@ -223,14 +223,14 @@ std::pair<rpc::UniqueReply, DBClientBase*> DBClientBase::runCommandWithTarget(
 
     Message replyMsg;
 
-    // We always want to throw if there was a network error, we do it here
-    // instead of passing 'true' for the 'assertOk' parameter so we can construct a
-    // more helpful error message. Note that call() can itself throw a socket exception.
-    uassert(ErrorCodes::HostUnreachable,
-            str::stream() << "network error while attempting to run "
-                          << "command '" << request.getCommandName() << "' "
-                          << "on host '" << host << "' ",
-            call(requestMsg, replyMsg, false, &host));
+    try {
+        call(requestMsg, replyMsg, &host);
+    } catch (DBException& e) {
+        e.addContext(str::stream() << str::stream() << "network error while attempting to run "
+                                   << "command '" << request.getCommandName() << "' "
+                                   << "on host '" << host << "' ");
+        throw;
+    }
 
     auto commandReply = parseCommandReplyMessage(host, replyMsg);
 
diff --git a/src/mongo/client/dbclient_base.h b/src/mongo/client/dbclient_base.h
index 3c933f6d3d3..4e77674b1ba 100644
--- a/src/mongo/client/dbclient_base.h
+++ b/src/mongo/client/dbclient_base.h
@@ -494,10 +494,9 @@ public:
      * 'actualServer' is set to the actual server where they call went if there was a choice (for
      * example SecondaryOk).
      */
-    virtual bool call(Message& toSend,
-                      Message& response,
-                      bool assertOk = true,
-                      std::string* actualServer = nullptr) = 0;
+    void call(Message& toSend, Message& response, std::string* actualServer = nullptr) {
+        _call(toSend, response, actualServer);
+    };
 
     virtual void say(Message& toSend,
                      bool isRetry = false,
@@ -716,6 +715,8 @@ protected:
     std::vector<std::string> _saslMechsForAuth;
 
 private:
+    virtual void _call(Message& toSend, Message& response, std::string* actualServer) = 0;
+
     /**
      * Implementation for getIndexes() and getReadyIndexes().
      */
diff --git a/src/mongo/client/dbclient_connection.cpp b/src/mongo/client/dbclient_connection.cpp
index 1b87829c1cd..56eb91e484c 100644
--- a/src/mongo/client/dbclient_connection.cpp
+++ b/src/mongo/client/dbclient_connection.cpp
@@ -678,29 +678,9 @@ Status DBClientConnection::recv(Message& m, int lastRequestId) {
     return Status::OK();
 }
 
-bool DBClientConnection::call(Message& toSend,
-                              Message& response,
-                              bool assertOk,
-                              string* actualServer) {
+void DBClientConnection::_call(Message& toSend, Message& response, string* actualServer) {
     checkConnection();
     ScopeGuard killSessionOnError([this] { _markFailed(kEndSession); });
-    auto maybeThrow = [&](const auto& errStatus) {
-        // TODO SERVER-65946 Remove the following log statement.
-        if (TestingProctor::instance().isEnabled()) {
-            LOGV2(6599701,
-                  "DBClient failed to communicate with the server",
-                  "error"_attr = errStatus,
-                  "server"_attr = getServerAddress(),
-                  "local"_attr = _session->local(),
-                  "remote"_attr = _session->remote());
-        }
-
-        if (assertOk)
-            uassertStatusOKWithContext(errStatus,
-                                       str::stream() << "dbclient error communicating with server "
-                                                     << getServerAddress());
-        return false;
-    };
 
     toSend.header().setId(nextMessageId());
     toSend.header().setResponseToMsgId(0);
@@ -723,7 +703,9 @@ bool DBClientConnection::call(Message& toSend,
               "DBClientConnection failed to send message",
               "connString"_attr = getServerAddress(),
               "error"_attr = redact(sinkStatus));
-        return maybeThrow(sinkStatus);
+        uassertStatusOKWithContext(sinkStatus,
+                                   str::stream() << "dbclient error communicating with server "
+                                                 << getServerAddress());
     }
 
     swm = _session->sourceMessage();
@@ -735,7 +717,9 @@ bool DBClientConnection::call(Message& toSend,
               "DBClientConnection failed to receive message",
               "connString"_attr = getServerAddress(),
               "error"_attr = redact(swm.getStatus()));
-        return maybeThrow(swm.getStatus());
+        uassertStatusOKWithContext(swm.getStatus(),
+                                   str::stream() << "dbclient error communicating with server "
+                                                 << getServerAddress());
     }
 
     if (response.operation() == dbCompressed) {
@@ -743,7 +727,6 @@ bool DBClientConnection::call(Message& toSend,
     }
 
     killSessionOnError.dismiss();
-    return true;
 }
 
 void DBClientConnection::setParentReplSetName(const string& replSetName) {
diff --git a/src/mongo/client/dbclient_connection.h b/src/mongo/client/dbclient_connection.h
index 61096ba59b3..a0382ea167f 100644
--- a/src/mongo/client/dbclient_connection.h
+++ b/src/mongo/client/dbclient_connection.h
@@ -209,10 +209,6 @@ public:
     void say(Message& toSend, bool isRetry = false, std::string* actualServer = nullptr) override;
     Status recv(Message& m, int lastRequestId) override;
 
-    bool call(Message& toSend,
-              Message& response,
-              bool assertOk,
-              std::string* actualServer) override;
     ConnectionString::ConnectionType type() const override {
         return ConnectionString::ConnectionType::kStandalone;
     }
@@ -319,6 +315,7 @@ private:
     void handleNotPrimaryResponse(const BSONObj& replyBody, StringData errorMsgFieldName);
     enum FailAction { kSetFlag, kEndSession, kReleaseSession };
     void _markFailed(FailAction action);
+    void _call(Message& toSend, Message& response, std::string* actualServer) override;
 
     // Contains the string for the replica set name of the host this is connected to.
     // Should be empty if this connection is not pointing to a replica set member.
diff --git a/src/mongo/client/dbclient_cursor.cpp b/src/mongo/client/dbclient_cursor.cpp
index 6fdbbd24195..035bb5ece61 100644
--- a/src/mongo/client/dbclient_cursor.cpp
+++ b/src/mongo/client/dbclient_cursor.cpp
@@ -127,7 +127,7 @@ bool DBClientCursor::init() {
     verify(_client);
     Message reply;
     try {
-        _client->call(toSend, reply, true, &_originalHost);
+        _client->call(toSend, reply, &_originalHost);
     } catch (const DBException&) {
         // log msg temp?
         LOGV2(20127, "DBClientCursor::init call() failed");
diff --git a/src/mongo/client/dbclient_cursor_test.cpp b/src/mongo/client/dbclient_cursor_test.cpp
index 21e9cfcf082..fa0c699b2e6 100644
--- a/src/mongo/client/dbclient_cursor_test.cpp
+++ b/src/mongo/client/dbclient_cursor_test.cpp
@@ -54,27 +54,6 @@ public:
         _serverAddress = HostAndPort("localhost", 27017);  // dummy server address.
     }
 
-    bool call(Message& toSend,
-              Message& response,
-              bool assertOk,
-              std::string* actualServer) override {
-
-        // Intercept request.
-        const auto reqId = nextMessageId();
-        toSend.header().setId(reqId);
-        toSend.header().setResponseToMsgId(0);
-        OpMsg::appendChecksum(&toSend);
-        _lastSent = toSend;
-
-        // Mock response.
-        response = _mockCallResponse;
-        response.header().setId(nextMessageId());
-        response.header().setResponseToMsgId(reqId);
-        OpMsg::appendChecksum(&response);
-
-        return true;
-    }
-
     Status recv(Message& m, int lastRequestId) override {
         m = _mockRecvResponse;
         return Status::OK();
@@ -102,6 +81,22 @@ public:
     }
 
 private:
+    void _call(Message& toSend, Message& response, std::string* actualServer) override {
+
+        // Intercept request.
+        const auto reqId = nextMessageId();
+        toSend.header().setId(reqId);
+        toSend.header().setResponseToMsgId(0);
+        OpMsg::appendChecksum(&toSend);
+        _lastSent = toSend;
+
+        // Mock response.
+        response = _mockCallResponse;
+        response.header().setId(nextMessageId());
+        response.header().setResponseToMsgId(reqId);
+        OpMsg::appendChecksum(&response);
+    }
+
     Message _mockCallResponse;
     Message _mockRecvResponse;
     Message _lastSent;
diff --git a/src/mongo/client/dbclient_rs.cpp b/src/mongo/client/dbclient_rs.cpp
index a48fe50a2fa..6d86e41a936 100644
--- a/src/mongo/client/dbclient_rs.cpp
+++ b/src/mongo/client/dbclient_rs.cpp
@@ -795,10 +795,7 @@ std::pair<rpc::UniqueReply, std::shared_ptr<DBClientBase>> DBClientReplicaSet::r
     return {std::move(out.first), std::move(conn)};
 }
 
-bool DBClientReplicaSet::call(Message& toSend,
-                              Message& response,
-                              bool assertOk,
-                              string* actualServer) {
+void DBClientReplicaSet::_call(Message& toSend, Message& response, string* actualServer) {
     LOGV2_DEBUG(20146,
                 3,
                 "dbclient_rs call to primary node in {replicaSet}",
@@ -809,10 +806,7 @@ bool DBClientReplicaSet::call(Message& toSend,
     if (actualServer)
         *actualServer = m->getServerAddress();
 
-    if (!m->call(toSend, response, assertOk, nullptr))
-        return false;
-
-    return true;
+    m->call(toSend, response);
 }
 
 void DBClientReplicaSet::_invalidateLastSecondaryOkCache(const Status& status) {
diff --git a/src/mongo/client/dbclient_rs.h b/src/mongo/client/dbclient_rs.h
index fa796039f2c..28ff556541a 100644
--- a/src/mongo/client/dbclient_rs.h
+++ b/src/mongo/client/dbclient_rs.h
@@ -194,11 +194,6 @@ public:
     int getMaxWireVersion() final;
     // ---- low level ------
 
-    bool call(Message& toSend,
-              Message& response,
-              bool assertOk,
-              std::string* actualServer) override;
-
     /**
      * Performs a "soft reset" by clearing all states relating to secondary nodes and
      * returning secondary connections to the pool.
@@ -243,6 +238,8 @@ private:
 
     DBClientConnection* checkPrimary();
 
+    void _call(Message& toSend, Message& response, std::string* actualServer) override;
+
     template <typename Authenticate>
     Status _runAuthLoop(Authenticate authCb);