SERVER-31734 Add Status::withContext() and addContext()

These prepend the message with a context string, leaving everything else alone.

9 files changed, 65 insertions, 42 deletions

diff --git a/src/mongo/base/status.cpp b/src/mongo/base/status.cpp
index a5bd0a59066..8b615c5b98b 100644
--- a/src/mongo/base/status.cpp
+++ b/src/mongo/base/status.cpp
@@ -53,6 +53,10 @@ Status::Status(ErrorCodes::Error code, StringData reason) : Status(code, reason.
 Status::Status(ErrorCodes::Error code, const mongoutils::str::stream& reason)
     : Status(code, std::string(reason)) {}
 
+Status Status::withContext(StringData reasonPrefix) const {
+    return isOK() ? Status::OK() : Status(code(), reasonPrefix + causedBy(reason()));
+}
+
 std::ostream& operator<<(std::ostream& os, const Status& status) {
     return os << status.codeString() << " " << status.reason();
 }
diff --git a/src/mongo/base/status.h b/src/mongo/base/status.h
index 7c9a3c0ed3c..f01c07219a1 100644
--- a/src/mongo/base/status.h
+++ b/src/mongo/base/status.h
@@ -71,6 +71,9 @@ public:
      *
      * For OK Statuses prefer using Status::OK(). If code is OK, the remaining arguments are
      * ignored.
+     *
+     * For adding context to the reason string, use withContext/addContext rather than making a new
+     * Status manually.
      */
     MONGO_COMPILER_COLD_FUNCTION Status(ErrorCodes::Error code, std::string reason);
     MONGO_COMPILER_COLD_FUNCTION Status(ErrorCodes::Error code, const char* reason);
@@ -87,6 +90,18 @@ public:
     inline ~Status();
 
     /**
+     * Returns a new Status with the same data as this, but with the reason string prefixed with
+     * reasonPrefix and our standard " :: caused by :: " separator. The new reason is not visible to
+     * any other Statuses that share the same ErrorInfo object.
+     *
+     * No-op when called on an OK status.
+     */
+    Status withContext(StringData reasonPrefix) const;
+    void addContext(StringData reasonPrefix) {
+        *this = this->withContext(reasonPrefix);
+    }
+
+    /**
      * Only compares codes. Ignores reason strings.
      */
     bool operator==(const Status& other) const {
diff --git a/src/mongo/base/status_test.cpp b/src/mongo/base/status_test.cpp
index 121c30f7ff5..149f712cd15 100644
--- a/src/mongo/base/status_test.cpp
+++ b/src/mongo/base/status_test.cpp
@@ -62,6 +62,18 @@ TEST(Basic, Compare) {
     ASSERT_NE(errMax, Status::OK());
 }
 
+TEST(Basic, WithContext) {
+    const Status orig(ErrorCodes::MaxError, "error");
+
+    const auto copy = orig.withContext("context");
+    ASSERT_EQ(copy.code(), ErrorCodes::MaxError);
+    ASSERT(str::startsWith(copy.reason(), "context ")) << copy.reason();
+    ASSERT(str::endsWith(copy.reason(), " error")) << copy.reason();
+
+    ASSERT_EQ(orig.code(), ErrorCodes::MaxError);
+    ASSERT_EQ(orig.reason(), "error");
+}
+
 TEST(Cloning, Copy) {
     Status orig(ErrorCodes::MaxError, "error");
     ASSERT_EQUALS(orig.refCount(), 1U);
diff --git a/src/mongo/client/dbclient_rs.cpp b/src/mongo/client/dbclient_rs.cpp
index a4e7a5df146..10547fc66c1 100644
--- a/src/mongo/client/dbclient_rs.cpp
+++ b/src/mongo/client/dbclient_rs.cpp
@@ -540,10 +540,10 @@ unique_ptr<DBClientCursor> DBClientReplicaSet::query(const string& ns,
 
                 return checkSlaveQueryResult(std::move(cursor));
             } catch (const DBException& ex) {
-                const Status status = ex.toStatus();
-                lastNodeErrMsg = str::stream() << "can't query replica set node "
-                                               << _lastSlaveOkHost << ": " << status.reason();
-                _invalidateLastSlaveOkCache({status.code(), lastNodeErrMsg});
+                const Status status = ex.toStatus(str::stream() << "can't query replica set node "
+                                                                << _lastSlaveOkHost);
+                lastNodeErrMsg = status.reason();
+                _invalidateLastSlaveOkCache(status);
             }
         }
 
@@ -589,11 +589,10 @@ BSONObj DBClientReplicaSet::findOne(const string& ns,
 
                 return conn->findOne(ns, query, fieldsToReturn, queryOptions);
             } catch (const DBException& ex) {
-                const Status status = ex.toStatus();
-                lastNodeErrMsg = str::stream() << "can't findone replica set node "
-                                               << _lastSlaveOkHost.toString() << ": "
-                                               << status.reason();
-                _invalidateLastSlaveOkCache({status.code(), lastNodeErrMsg});
+                const Status status = ex.toStatus(str::stream() << "can't findone replica set node "
+                                                                << _lastSlaveOkHost.toString());
+                lastNodeErrMsg = status.reason();
+                _invalidateLastSlaveOkCache(status);
             }
         }
 
@@ -779,11 +778,11 @@ void DBClientReplicaSet::say(Message& toSend, bool isRetry, string* actualServer
                     _lazyState._secondaryQueryOk = true;
                     _lazyState._lastClient = conn;
                 } catch (const DBException& ex) {
-                    const Status status = ex.toStatus();
-                    lastNodeErrMsg = str::stream() << "can't callLazy replica set node "
-                                                   << _lastSlaveOkHost.toString() << ": "
-                                                   << status.reason();
-                    _invalidateLastSlaveOkCache({status.code(), lastNodeErrMsg});
+                    const Status status =
+                        ex.toStatus(str::stream() << "can't callLazy replica set node "
+                                                  << _lastSlaveOkHost.toString());
+                    lastNodeErrMsg = status.reason();
+                    _invalidateLastSlaveOkCache(status);
 
                     continue;
                 }
diff --git a/src/mongo/db/repl/initial_syncer.cpp b/src/mongo/db/repl/initial_syncer.cpp
index afb7eba5bbd..f3a84eb2ad3 100644
--- a/src/mongo/db/repl/initial_syncer.cpp
+++ b/src/mongo/db/repl/initial_syncer.cpp
@@ -1379,11 +1379,7 @@ Status InitialSyncer::_checkForShutdownAndConvertStatus_inlock(const Status& sta
         return Status(ErrorCodes::CallbackCanceled, message + ": initial syncer is shutting down");
     }
 
-    if (!status.isOK()) {
-        return Status(status.code(), message + ": " + status.reason());
-    }
-
-    return Status::OK();
+    return status.withContext(message);
 }
 
 Status InitialSyncer::_scheduleWorkAndSaveHandle_inlock(
diff --git a/src/mongo/db/repl/initial_syncer_test.cpp b/src/mongo/db/repl/initial_syncer_test.cpp
index e5272881b2d..28a7954019c 100644
--- a/src/mongo/db/repl/initial_syncer_test.cpp
+++ b/src/mongo/db/repl/initial_syncer_test.cpp
@@ -3850,10 +3850,9 @@ TEST_F(InitialSyncerTest, GetInitialSyncProgressReturnsCorrectProgress) {
     ASSERT_EQUALS(attempts.nFields(), 1) << attempts;
     BSONObj attempt0 = attempts["0"].Obj();
     ASSERT_EQUALS(attempt0.nFields(), 3) << attempt0;
-    ASSERT_EQUALS(
-        attempt0.getStringField("status"),
-        std::string(
-            "FailedToParse: error cloning databases: fail on clone -- listDBs injected failure"))
+    ASSERT_EQUALS(attempt0.getStringField("status"),
+                  std::string("FailedToParse: error cloning databases :: caused by :: fail on "
+                              "clone -- listDBs injected failure"))
         << attempt0;
     ASSERT_EQUALS(attempt0["durationMillis"].type(), NumberInt) << attempt0;
     ASSERT_EQUALS(attempt0.getStringField("syncSource"), std::string("localhost:27017"))
@@ -3958,10 +3957,9 @@ TEST_F(InitialSyncerTest, GetInitialSyncProgressReturnsCorrectProgress) {
     ASSERT_EQUALS(attempts.nFields(), 1) << progress;
     attempt0 = attempts["0"].Obj();
     ASSERT_EQUALS(attempt0.nFields(), 3) << attempt0;
-    ASSERT_EQUALS(
-        attempt0.getStringField("status"),
-        std::string(
-            "FailedToParse: error cloning databases: fail on clone -- listDBs injected failure"))
+    ASSERT_EQUALS(attempt0.getStringField("status"),
+                  std::string("FailedToParse: error cloning databases :: caused by :: fail on "
+                              "clone -- listDBs injected failure"))
         << attempt0;
     ASSERT_EQUALS(attempt0["durationMillis"].type(), NumberInt) << attempt0;
     ASSERT_EQUALS(attempt0.getStringField("syncSource"), std::string("localhost:27017"))
@@ -4007,10 +4005,9 @@ TEST_F(InitialSyncerTest, GetInitialSyncProgressReturnsCorrectProgress) {
 
     attempt0 = attempts["0"].Obj();
     ASSERT_EQUALS(attempt0.nFields(), 3) << attempt0;
-    ASSERT_EQUALS(
-        attempt0.getStringField("status"),
-        std::string(
-            "FailedToParse: error cloning databases: fail on clone -- listDBs injected failure"))
+    ASSERT_EQUALS(attempt0.getStringField("status"),
+                  std::string("FailedToParse: error cloning databases :: caused by :: fail on "
+                              "clone -- listDBs injected failure"))
         << attempt0;
     ASSERT_EQUALS(attempt0["durationMillis"].type(), NumberInt) << attempt0;
     ASSERT_EQUALS(attempt0.getStringField("syncSource"), std::string("localhost:27017"))
diff --git a/src/mongo/db/repl/rollback_impl.cpp b/src/mongo/db/repl/rollback_impl.cpp
index c08fd9e6ced..31c34c016ba 100644
--- a/src/mongo/db/repl/rollback_impl.cpp
+++ b/src/mongo/db/repl/rollback_impl.cpp
@@ -169,12 +169,12 @@ Status RollbackImpl::_transitionToRollback(OperationContext* opCtx) {
 
         auto status = _replicationCoordinator->setFollowerMode(MemberState::RS_ROLLBACK);
         if (!status.isOK()) {
-            std::string msg = str::stream()
-                << "Cannot transition from " << _replicationCoordinator->getMemberState().toString()
-                << " to " << MemberState(MemberState::RS_ROLLBACK).toString()
-                << causedBy(status.reason());
-            log() << msg;
-            return Status(status.code(), msg);
+            status.addContext(str::stream() << "Cannot transition from "
+                                            << _replicationCoordinator->getMemberState().toString()
+                                            << " to "
+                                            << MemberState(MemberState::RS_ROLLBACK).toString());
+            log() << status;
+            return status;
         }
     }
     return Status::OK();
diff --git a/src/mongo/s/catalog/sharding_catalog_client_impl.cpp b/src/mongo/s/catalog/sharding_catalog_client_impl.cpp
index 54346ec4e59..10ef8131969 100644
--- a/src/mongo/s/catalog/sharding_catalog_client_impl.cpp
+++ b/src/mongo/s/catalog/sharding_catalog_client_impl.cpp
@@ -1004,7 +1004,7 @@ Status ShardingCatalogClientImpl::applyChunkOpsDeprecated(OperationContext* opCt
                                    << ". Result: " << response.getValue().response;
         }
 
-        return Status(status.code(), errMsg);
+        return status.withContext(errMsg);
     }
 
     return Status::OK();
diff --git a/src/mongo/util/assert_util.h b/src/mongo/util/assert_util.h
index ba6cfb94d33..b5c0665fd53 100644
--- a/src/mongo/util/assert_util.h
+++ b/src/mongo/util/assert_util.h
@@ -69,12 +69,12 @@ public:
         return reason().c_str();
     }
 
-    virtual void addContext(const std::string& str) {
-        _status = Status(code(), str + causedBy(reason()));
+    virtual void addContext(StringData context) {
+        _status.addContext(context);
     }
 
-    Status toStatus(const std::string& context) const {
-        return Status(code(), context + causedBy(*this));
+    Status toStatus(StringData context) const {
+        return _status.withContext(context);
     }
     const Status& toStatus() const {
         return _status;