Return a string_view in Result::errorMessage

arangodb · jsteemann · Feb 10, 2021 · Jan 21, 2021 · Jan 21, 2021 · Jan 21, 2021
commit 83bb4f8dd4c573c4cc68af4b16ce9c0f9493f6e1
diff --git a/arangod/Agency/AgencyComm.cpp b/arangod/Agency/AgencyComm.cpp
@@ -430,7 +430,7 @@ int AgencyCommResult::errorCode() const {
 }
 
 std::string AgencyCommResult::errorMessage() const {
-  return asResult().errorMessage();
+  return std::string{asResult().errorMessage()};
 }
 
 std::pair<std::optional<int>, std::optional<std::string_view>> AgencyCommResult::parseBodyError() const {

diff --git a/arangod/Aql/Functions.cpp b/arangod/Aql/Functions.cpp
@@ -118,6 +118,7 @@
 #endif
 
 using namespace arangodb;
+using namespace arangodb::basics;
 using namespace arangodb::aql;
 using namespace std::chrono;
 using namespace date;
@@ -928,10 +929,11 @@ void getDocumentByIdentifier(transaction::Methods* trx, std::string& collectionN
     }
     if (res.is(TRI_ERROR_TRANSACTION_UNREGISTERED_COLLECTION)) {
       // special error message to indicate which collection was undeclared
-      THROW_ARANGO_EXCEPTION_MESSAGE(res.errorNumber(),
-                                     res.errorMessage() + ": " + collectionName +
-                                         " [" + AccessMode::typeString(AccessMode::Type::READ) +
-                                         "]");
+      THROW_ARANGO_EXCEPTION_MESSAGE(
+          res.errorNumber(),
+          StringUtils::concatT(res.errorMessage(), ": ", collectionName, " [",
+                               AccessMode::typeString(AccessMode::Type::READ),
+                               "]"));
     }
     THROW_ARANGO_EXCEPTION(res);
   }

diff --git a/arangod/Aql/IResearchViewExecutor.cpp b/arangod/Aql/IResearchViewExecutor.cpp
@@ -24,11 +24,12 @@
 
 #include "IResearchViewExecutor.h"
 
+#include "Aql/AqlCall.h"
 #include "Aql/ExecutionStats.h"
 #include "Aql/OutputAqlItemRow.h"
 #include "Aql/Query.h"
 #include "Aql/SingleRowFetcher.h"
-#include "AqlCall.h"
+#include "Basics/StringUtils.h"
 #include "IResearch/IResearchCommon.h"
 #include "IResearch/IResearchDocument.h"
 #include "IResearch/IResearchFilterFactory.h"
@@ -58,6 +59,7 @@
 
 using namespace arangodb;
 using namespace arangodb::aql;
+using namespace arangodb::basics;
 using namespace arangodb::iresearch;
 
 namespace {
@@ -609,8 +611,9 @@ void IResearchViewExecutorBase<Impl, Traits>::reset() {
       infos().filterCondition().toVelocyPack(builder, true);
       THROW_ARANGO_EXCEPTION_MESSAGE(
           rv.errorNumber(),
-          "failed to build filter while querying arangosearch view, query '" +
-              builder.toJson() + "': " + rv.errorMessage());
+          StringUtils::concatT("failed to build filter while querying "
+                               "arangosearch view, query '",
+                               builder.toJson(), "': ", rv.errorMessage()));
     }
 
     if (infos().volatileSort() || !_isInitialized) {

diff --git a/arangod/Aql/IResearchViewOptimizerRules.cpp b/arangod/Aql/IResearchViewOptimizerRules.cpp
@@ -53,6 +53,7 @@
 
 using namespace arangodb::iresearch;
 using namespace arangodb::aql;
+using namespace arangodb::basics;
 
 namespace {
 
@@ -137,8 +138,10 @@ bool optimizeSearchCondition(IResearchViewNode& viewNode, arangodb::aql::QueryCo
       *searchCondition.root());
 
     if (filterCreated.fail()) {
-      THROW_ARANGO_EXCEPTION_MESSAGE(filterCreated.errorNumber(),
-                                     "unsupported SEARCH condition: " + filterCreated.errorMessage());
+      THROW_ARANGO_EXCEPTION_MESSAGE(
+          filterCreated.errorNumber(),
+          StringUtils::concatT("unsupported SEARCH condition: ",
+                               filterCreated.errorMessage()));
     }
   }
 

diff --git a/arangod/Aql/QueryResult.h b/arangod/Aql/QueryResult.h
@@ -85,7 +85,7 @@ struct QueryResult {
   int errorNumber() const { return result.errorNumber(); }
   bool is(int errorNumber) const { return result.errorNumber() == errorNumber; }
   bool isNot(int errorNumber) const { return !is(errorNumber); }
-  std::string errorMessage() const { return result.errorMessage(); }
+  std::string_view errorMessage() const { return result.errorMessage(); }
 
  public:
   Result result;

diff --git a/arangod/Auth/Handler.h b/arangod/Auth/Handler.h
@@ -55,7 +55,7 @@ class HandlerResult {
   bool ok() const { return _result.ok(); }
   bool fail() const { return _result.fail(); }
   int errorNumber() const { return _result.errorNumber(); }
-  std::string errorMessage() const { return _result.errorMessage(); }
+  std::string_view errorMessage() const { return _result.errorMessage(); }
 
  protected:
   Result _result;

diff --git a/arangod/Auth/UserManager.cpp b/arangod/Auth/UserManager.cpp
@@ -157,8 +157,10 @@ static std::shared_ptr<VPackBuilder> QueryAllUsers(application_features::Applica
         (queryResult.result.is(TRI_ERROR_QUERY_KILLED))) {
       THROW_ARANGO_EXCEPTION(TRI_ERROR_REQUEST_CANCELED);
     }
-    THROW_ARANGO_EXCEPTION_MESSAGE(queryResult.result.errorNumber(),
-                                   "Error executing user query: " + queryResult.result.errorMessage());
+    THROW_ARANGO_EXCEPTION_MESSAGE(
+        queryResult.result.errorNumber(),
+        StringUtils::concatT("Error executing user query: ",
+                             queryResult.result.errorMessage()));
   }
 
   VPackSlice usersSlice = queryResult.data->slice();

diff --git a/arangod/Cluster/AgencyCallbackRegistry.cpp b/arangod/Cluster/AgencyCallbackRegistry.cpp
@@ -30,6 +30,7 @@
 #include "Basics/Exceptions.h"
 #include "Basics/ReadLocker.h"
 #include "Basics/Result.h"
+#include "Basics/StringUtils.h"
 #include "Basics/WriteLocker.h"
 #include "Cluster/AgencyCache.h"
 #include "Cluster/AgencyCallback.h"
@@ -42,6 +43,7 @@
 #include "RestServer/MetricsFeature.h"
 
 using namespace arangodb;
+using namespace arangodb::basics;
 
 AgencyCallbackRegistry::AgencyCallbackRegistry(application_features::ApplicationServer& server,
                                                std::string const& callbackBasePath)
@@ -88,7 +90,9 @@ Result AgencyCallbackRegistry::registerCallback(std::shared_ptr<AgencyCallback>
   }
 
   TRI_ASSERT(res.fail());
-  res.reset(res.errorNumber(), std::string("registering ") + (local ? "local " : "") + "callback failed: " + res.errorMessage());
+  res.reset(res.errorNumber(),
+            StringUtils::concatT("registering ", (local ? "local " : ""),
+                                 "callback failed: ", res.errorMessage()));
   LOG_TOPIC("b88f4", WARN, Logger::CLUSTER) << res.errorMessage();
 
   {

diff --git a/arangod/Cluster/ClusterMethods.cpp b/arangod/Cluster/ClusterMethods.cpp
@@ -3403,7 +3403,7 @@ arangodb::Result lockDBServerTransactions(network::ConnectionPool* pool,
       if (finalRes.errorNumber() == TRI_ERROR_LOCAL_LOCK_FAILED) {
         c = TRI_ERROR_LOCAL_LOCK_FAILED;
       }
-      finalRes = arangodb::Result(c, finalRes.errorMessage() + ", " + m);
+      finalRes = arangodb::Result(c, StringUtils::concatT(finalRes.errorMessage(), ", ", m));
     }
   };
   for (Future<network::Response>& f : futures) {
@@ -3965,7 +3965,8 @@ arangodb::Result hotBackupCoordinator(ClusterFeature& feature, VPackSlice const
     if (!result.ok()) {
       // Failed to go to backup mode
       result.reset(TRI_ERROR_HOT_BACKUP_INTERNAL,
-                   std::string("agency lock operation resulted in ") + result.errorMessage());
+                   StringUtils::concatT("agency lock operation resulted in ",
+                                        result.errorMessage()));
       LOG_TOPIC("6c73d", ERR, Logger::BACKUP) << result.errorMessage();
       events::CreateHotbackup(timeStamp + "_" + backupId, TRI_ERROR_HOT_BACKUP_INTERNAL);
       return result;
@@ -3996,7 +3997,8 @@ arangodb::Result hotBackupCoordinator(ClusterFeature& feature, VPackSlice const
       // release the lock
       releaseAgencyLock.fire();
       result.reset(TRI_ERROR_HOT_BACKUP_INTERNAL,
-                   std::string("failed to acquire agency dump: ") + result.errorMessage());
+                   StringUtils::concatT("failed to acquire agency dump: ",
+                                        result.errorMessage()));
       LOG_TOPIC("c014d", ERR, Logger::BACKUP) << result.errorMessage();
       events::CreateHotbackup(timeStamp + "_" + backupId, TRI_ERROR_HOT_BACKUP_INTERNAL);
       return result;
@@ -4101,9 +4103,9 @@ arangodb::Result hotBackupCoordinator(ClusterFeature& feature, VPackSlice const
       releaseAgencyLock.fire();
       result.reset(
           TRI_ERROR_HOT_BACKUP_INTERNAL,
-          std::string(
-              "failed to acquire global transaction lock on all db servers: ") +
-              result.errorMessage());
+          StringUtils::concatT(
+              "failed to acquire global transaction lock on all db servers: ",
+              result.errorMessage()));
       LOG_TOPIC("b7d09", ERR, Logger::BACKUP) << result.errorMessage();
       events::CreateHotbackup(timeStamp + "_" + backupId, result.errorNumber());
       return result;
@@ -4119,8 +4121,8 @@ arangodb::Result hotBackupCoordinator(ClusterFeature& feature, VPackSlice const
       // release the lock
       releaseAgencyLock.fire();
       result.reset(TRI_ERROR_HOT_BACKUP_INTERNAL,
-                   std::string("failed to hot backup on all db servers: ") +
-                       result.errorMessage());
+                   StringUtils::concatT(
+                       "failed to hot backup on all db servers: ", result.errorMessage()));
       LOG_TOPIC("6b333", ERR, Logger::BACKUP) << result.errorMessage();
       removeLocalBackups(pool, backupId, dbServers, dummy);
       events::CreateHotbackup(timeStamp + "_" + backupId, result.errorNumber());
@@ -4138,8 +4140,9 @@ arangodb::Result hotBackupCoordinator(ClusterFeature& feature, VPackSlice const
         removeLocalBackups(pool, backupId, dbServers, dummy);
       }
       result.reset(TRI_ERROR_HOT_BACKUP_INTERNAL,
-                   std::string("failed to acquire agency dump post backup: ") +
-                       result.errorMessage() + " backup's integrity is not guaranteed");
+          StringUtils::concatT(
+                       "failed to acquire agency dump post backup: ", result.errorMessage(),
+                       " backup's integrity is not guaranteed"));
       LOG_TOPIC("d4229", ERR, Logger::BACKUP) << result.errorMessage();
       events::CreateHotbackup(timeStamp + "_" + backupId, result.errorNumber());
       return result;

diff --git a/arangod/Cluster/DBServerAgencySync.cpp b/arangod/Cluster/DBServerAgencySync.cpp
@@ -47,6 +47,7 @@
 
 using namespace arangodb;
 using namespace arangodb::application_features;
+using namespace arangodb::basics;
 using namespace arangodb::methods;
 using namespace arangodb::rest;
 
@@ -345,7 +346,7 @@ DBServerAgencySyncResult DBServerAgencySync::execute() {
       } else {
         // Report an error:
         result = DBServerAgencySyncResult(
-          false, "Error in phase 2: " + tmp.errorMessage(), 0, 0);
+            false, StringUtils::concatT("Error in phase 2: ", tmp.errorMessage()), 0, 0);
       }
     } else {
       // This code should never run, it is only there to debug problems if

diff --git a/arangod/Cluster/FollowerInfo.cpp b/arangod/Cluster/FollowerInfo.cpp
@@ -265,8 +265,7 @@ Result FollowerInfo::remove(ServerID const& sid) {
       _docColl->vocbase().name() + "/" + std::to_string(_docColl->planId().id()) +
       ": " + TRI_errno_string(agencyRes.errorNumber());
   LOG_TOPIC("a0dcc", ERR, Logger::CLUSTER) << errorMessage;
-  agencyRes.resetErrorMessage<std::string>(std::move(errorMessage));
-  return agencyRes;
+  return Result{agencyRes.errorNumber(), std::move(errorMessage)};
 }
 
 //////////////////////////////////////////////////////////////////////////////

diff --git a/arangod/Cluster/SynchronizeShard.cpp b/arangod/Cluster/SynchronizeShard.cpp
@@ -67,6 +67,7 @@ using namespace arangodb::maintenance;
 using namespace arangodb::methods;
 using namespace arangodb::transaction;
 using namespace arangodb;
+using namespace arangodb::basics;
 using namespace arangodb::consensus;
 
 std::string const ENDPOINT("endpoint");
@@ -316,7 +317,7 @@ static arangodb::Result addShardFollower(
         }
       }
       return arangodb::Result(result.errorNumber(),
-                              errorMessage + ", " + result.errorMessage());
+                              StringUtils::concatT(errorMessage, ", ", result.errorMessage()));
     }
 
     LOG_TOPIC("79935", DEBUG, Logger::MAINTENANCE) << "addShardFollower: success";
@@ -977,8 +978,8 @@ ResultT<TRI_voc_tick_t> SynchronizeShard::catchupWithReadLock(
     Result res = startReadLockOnLeader(ep, database, collection.name(),
                                        clientId, lockJobId, true, timeout);
     if (!res.ok()) {
-      std::string errorMessage =
-        "SynchronizeShard: error in startReadLockOnLeader (soft):" + res.errorMessage();
+      auto errorMessage = StringUtils::concatT(
+          "SynchronizeShard: error in startReadLockOnLeader (soft):", res.errorMessage());
       return ResultT<TRI_voc_tick_t>::error(TRI_ERROR_INTERNAL, errorMessage);
     }
 
@@ -1034,8 +1035,8 @@ ResultT<TRI_voc_tick_t> SynchronizeShard::catchupWithReadLock(
     // We removed the readlock
     readLockGuard.cancel();
     if (!res.ok()) {
-      std::string errorMessage =
-          "synchronizeOneShard: error when cancelling soft read lock: " + res.errorMessage();
+      auto errorMessage = StringUtils::concatT(
+          "synchronizeOneShard: error when cancelling soft read lock: ", res.errorMessage());
       LOG_TOPIC("c37d1", INFO, Logger::MAINTENANCE) << errorMessage;
       _result.reset(TRI_ERROR_INTERNAL, errorMessage);
       return ResultT<TRI_voc_tick_t>::error(TRI_ERROR_INTERNAL, errorMessage);
@@ -1065,9 +1066,9 @@ Result SynchronizeShard::catchupWithExclusiveLock(
   Result res = startReadLockOnLeader(ep, database, collection.name(), clientId,
                                      lockJobId, false);
   if (!res.ok()) {
-    std::string errorMessage =
-      "SynchronizeShard: error in startReadLockOnLeader (hard):" + res.errorMessage();
-    return {TRI_ERROR_INTERNAL, errorMessage};
+    auto errorMessage = StringUtils::concatT(
+        "SynchronizeShard: error in startReadLockOnLeader (hard):", res.errorMessage());
+    return {TRI_ERROR_INTERNAL, std::move(errorMessage)};
   }
   auto readLockGuard = arangodb::scopeGuard([&, this]() {
     // Always cancel the read lock.
@@ -1170,11 +1171,11 @@ Result SynchronizeShard::catchupWithExclusiveLock(
       auto result = response.combinedResult();
 
       if (result.fail()) {
-        std::string const errorMessage =
-            "addShardFollower: could not add us to the leader's follower "
-            "list for " + database + "/" + shard +
-            ", error while recalculating count on leader: " +
-            result.errorMessage();
+        auto const errorMessage = StringUtils::concatT(
+            "addShardFollower: could not add us to the leader's follower list "
+            "for ",
+            database, "/", shard,
+            ", error while recalculating count on leader: ", result.errorMessage());
         LOG_TOPIC("22e0b", WARN, Logger::MAINTENANCE) << errorMessage;
         return arangodb::Result(result.errorNumber(), errorMessage);
       }

diff --git a/arangod/IResearch/IResearchAnalyzerFeature.cpp b/arangod/IResearch/IResearchAnalyzerFeature.cpp
@@ -2218,10 +2218,9 @@ Result IResearchAnalyzerFeature::cleanupAnalyzersCollection(irs::string_ref cons
     if (deleteResult.fail()) {
       return {
         TRI_ERROR_INTERNAL,
-        "failure to remove dangling analyzers from '" + std::string(database) + "' Aql error: (" +
-        std::to_string(deleteResult.errorNumber()) + " ) " +
-        deleteResult.errorMessage()
-      };
+              arangodb::basics::StringUtils::concatT(
+                  "failure to remove dangling analyzers from '", database,
+                  "' Aql error: (", deleteResult.errorNumber(), " ) ", deleteResult.errorMessage())};
     }
 
     static const auto queryUpdateString = arangodb::aql::QueryString(
@@ -2236,9 +2235,11 @@ Result IResearchAnalyzerFeature::cleanupAnalyzersCollection(irs::string_ref cons
     if (updateResult.fail()) {
       return {
         TRI_ERROR_INTERNAL,
-        "failure to restore dangling analyzers from '" + std::string(database) + "' Aql error: (" +
-        std::to_string(updateResult.errorNumber()) + " ) " +
-        updateResult.errorMessage()
+              arangodb::basics::StringUtils::concatT(
+                  "failure to restore dangling analyzers from '", database,
+                  "' Aql error: (", updateResult.errorNumber(), " ) ",
+                  updateResult.errorMessage())
+
       };
     }
 

diff --git a/arangod/IResearch/IResearchFeature.cpp b/arangod/IResearch/IResearchFeature.cpp
@@ -544,9 +544,10 @@ void registerSingleFactory(
     if (!res.ok()) {
       THROW_ARANGO_EXCEPTION_MESSAGE(
           res.errorNumber(),
-          std::string("failure registering IResearch link factory with index "
-                      "factory from feature '") +
-              engine.name() + "': " + res.errorMessage());
+          arangodb::basics::StringUtils::concatT(
+              "failure registering IResearch link factory with index "
+              "factory from feature '",
+              engine.name(), "': ", res.errorMessage()));
     }
   }
 }
@@ -667,7 +668,8 @@ void registerViewFactory(arangodb::application_features::ApplicationServer& serv
   if (!res.ok()) {
     THROW_ARANGO_EXCEPTION_MESSAGE(
         res.errorNumber(),
-        std::string("failure registering arangosearch view factory: ") + res.errorMessage());
+        arangodb::basics::StringUtils::concatT(
+            "failure registering arangosearch view factory: ", res.errorMessage()));
   }
 }