don't mask errors with fake OOM messages (#2872)

arangod/RocksDBEngine/RocksDBCollection.cpp

@@ -381,7 +381,7 @@ static std::shared_ptr<Index> findIndex(
 
   if (!value.isString()) {
     // Compatibility with old v8-vocindex.
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_OUT_OF_MEMORY);
+    THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "invalid index type definition");
   }
 
   std::string tmp = value.copyString();
@@ -1369,7 +1369,7 @@ RocksDBOperationResult RocksDBCollection::insertDocument(
   for (std::shared_ptr<Index> const& idx : _indexes) {
     innerRes.reset(idx->insert(trx, revisionId, doc, false));
 
-    // in case of no-memory, return immediately
+    // in case of OOM return immediately
     if (innerRes.is(TRI_ERROR_OUT_OF_MEMORY)) {
       return innerRes;
     }
@@ -1433,7 +1433,7 @@ RocksDBOperationResult RocksDBCollection::removeDocument(
     Result tmpres = idx->remove(trx, revisionId, doc, false);
     resInner.reset(tmpres);
 
-    // in case of no-memory, return immediately
+    // in case of OOM return immediately
     if (resInner.is(TRI_ERROR_OUT_OF_MEMORY)) {
       return resInner;
     }

arangod/RocksDBEngine/RocksDBIndexFactory.cpp

@@ -57,42 +57,38 @@ static int ProcessIndexFields(VPackSlice const definition,
   TRI_ASSERT(builder.isOpenObject());
   std::unordered_set<StringRef> fields;
 
-  try {
+  VPackSlice fieldsSlice = definition.get("fields");
-    VPackSlice fieldsSlice = definition.get("fields");
+  builder.add(VPackValue("fields"));
-    builder.add(VPackValue("fields"));
+  builder.openArray();
-    builder.openArray();
+  if (fieldsSlice.isArray()) {
-    if (fieldsSlice.isArray()) {
+    // "fields" is a list of fields
-      // "fields" is a list of fields
+    for (auto const& it : VPackArrayIterator(fieldsSlice)) {
-      for (auto const& it : VPackArrayIterator(fieldsSlice)) {
+      if (!it.isString()) {
-        if (!it.isString()) {
+        return TRI_ERROR_BAD_PARAMETER;
-          return TRI_ERROR_BAD_PARAMETER;
-        }
-
-        StringRef f(it);
-
-        if (f.empty() || (create && f == StaticStrings::IdString)) {
-          // accessing internal attributes is disallowed
-          return TRI_ERROR_BAD_PARAMETER;
-        }
-
-        if (fields.find(f) != fields.end()) {
-          // duplicate attribute name
-          return TRI_ERROR_BAD_PARAMETER;
-        }
-
-        fields.insert(f);
-        builder.add(it);
       }
-    }
 
-    if (fields.empty() || (numFields > 0 && (int)fields.size() != numFields)) {
+      StringRef f(it);
-      return TRI_ERROR_BAD_PARAMETER;
-    }
 
-    builder.close();
+      if (f.empty() || (create && f == StaticStrings::IdString)) {
-  } catch (...) {
+        // accessing internal attributes is disallowed
-    return TRI_ERROR_OUT_OF_MEMORY;
+        return TRI_ERROR_BAD_PARAMETER;
+      }
+
+      if (fields.find(f) != fields.end()) {
+        // duplicate attribute name
+        return TRI_ERROR_BAD_PARAMETER;
+      }
+
+      fields.insert(f);
+      builder.add(it);
+    }
   }
+
+  if (fields.empty() || (numFields > 0 && (int)fields.size() != numFields)) {
+    return TRI_ERROR_BAD_PARAMETER;
+  }
+
+  builder.close();
   return TRI_ERROR_NO_ERROR;
 }
 
@@ -293,77 +289,65 @@ int RocksDBIndexFactory::enhanceIndexDefinition(VPackSlice const definition,
   }
 
   TRI_ASSERT(enhanced.isEmpty());
+
+  VPackObjectBuilder b(&enhanced);
+  current = definition.get("id");
+  uint64_t id = 0;
+  if (current.isNumber()) {
+    id = current.getNumericValue<uint64_t>();
+  } else if (current.isString()) {
+    id = basics::StringUtils::uint64(current.copyString());
+  }
+  if (id > 0) {
+    enhanced.add("id", VPackValue(std::to_string(id)));
+  }
+
+  if (create && !isCoordinator) {
+    if (!definition.hasKey("objectId")) {
+      enhanced.add("objectId",
+                    VPackValue(std::to_string(TRI_NewTickServer())));
+    }
+  }
+
+  enhanced.add("type", VPackValue(Index::oldtypeName(type)));
+
   int res = TRI_ERROR_INTERNAL;
 
-  try {
+  switch (type) {
-    VPackObjectBuilder b(&enhanced);
+    case Index::TRI_IDX_TYPE_PRIMARY_INDEX:
-    current = definition.get("id");
+    case Index::TRI_IDX_TYPE_EDGE_INDEX: {
-    uint64_t id = 0;
+      break;
-    if (current.isNumber()) {
-      id = current.getNumericValue<uint64_t>();
-    } else if (current.isString()) {
-      id = basics::StringUtils::uint64(current.copyString());
-    }
-    if (id > 0) {
-      enhanced.add("id", VPackValue(std::to_string(id)));
     }
 
-    if (create && !isCoordinator) {
+    case Index::TRI_IDX_TYPE_GEO1_INDEX:
-      if (!definition.hasKey("objectId")) {
+      res = EnhanceJsonIndexGeo1(definition, enhanced, create);
-        enhanced.add("objectId",
+      break;
-                     VPackValue(std::to_string(TRI_NewTickServer())));
+
-      }
+    case Index::TRI_IDX_TYPE_GEO2_INDEX:
+      res = EnhanceJsonIndexGeo2(definition, enhanced, create);
+      break;
+
+    case Index::TRI_IDX_TYPE_HASH_INDEX:
+      res = EnhanceJsonIndexHash(definition, enhanced, create);
+      break;
+
+    case Index::TRI_IDX_TYPE_SKIPLIST_INDEX:
+      res = EnhanceJsonIndexSkiplist(definition, enhanced, create);
+      break;
+
+    case Index::TRI_IDX_TYPE_PERSISTENT_INDEX:
+      res = EnhanceJsonIndexPersistent(definition, enhanced, create);
+      break;
+
+    case Index::TRI_IDX_TYPE_FULLTEXT_INDEX:
+      res = EnhanceJsonIndexFulltext(definition, enhanced, create);
+      break;
+
+    case Index::TRI_IDX_TYPE_UNKNOWN:
+    default: {
+      res = TRI_ERROR_BAD_PARAMETER;
+      break;
     }
-    // breaks lookupIndex()
-    /*else {
-      if (!definition.hasKey("objectId")) {
-        // objectId missing, but must be present
-        return TRI_ERROR_INTERNAL;
-      }
-    }*/
-
-    enhanced.add("type", VPackValue(Index::oldtypeName(type)));
-
-    switch (type) {
-      case Index::TRI_IDX_TYPE_PRIMARY_INDEX:
-      case Index::TRI_IDX_TYPE_EDGE_INDEX: {
-        break;
-      }
-
-      case Index::TRI_IDX_TYPE_GEO1_INDEX:
-        res = EnhanceJsonIndexGeo1(definition, enhanced, create);
-        break;
-
-      case Index::TRI_IDX_TYPE_GEO2_INDEX:
-        res = EnhanceJsonIndexGeo2(definition, enhanced, create);
-        break;
-
-      case Index::TRI_IDX_TYPE_HASH_INDEX:
-        res = EnhanceJsonIndexHash(definition, enhanced, create);
-        break;
-
-      case Index::TRI_IDX_TYPE_SKIPLIST_INDEX:
-        res = EnhanceJsonIndexSkiplist(definition, enhanced, create);
-        break;
-
-      case Index::TRI_IDX_TYPE_PERSISTENT_INDEX:
-        res = EnhanceJsonIndexPersistent(definition, enhanced, create);
-        break;
-
-      case Index::TRI_IDX_TYPE_FULLTEXT_INDEX:
-        res = EnhanceJsonIndexFulltext(definition, enhanced, create);
-        break;
-
-      case Index::TRI_IDX_TYPE_UNKNOWN:
-      default: {
-        res = TRI_ERROR_BAD_PARAMETER;
-        break;
-      }
-    }
-
-  } catch (...) {
-    // TODO Check for different type of Errors
-    return TRI_ERROR_OUT_OF_MEMORY;
   }
 
   return res;

arangod/RocksDBEngine/RocksDBVPackIndex.cpp

@@ -532,13 +532,10 @@ Result RocksDBVPackIndex::insertInternal(transaction::Methods* trx,
   std::vector<RocksDBKey> elements;
   std::vector<uint64_t> hashes;
   int res;
-  try {
+  {
+    // rethrow all types of exceptions from here...
     transaction::BuilderLeaser leased(trx);
     res = fillElement(*(leased.get()), revisionId, doc, elements, hashes);
-  } catch (basics::Exception const& ex) {
-    res = ex.code();
-  } catch (...) {
-    res = TRI_ERROR_OUT_OF_MEMORY;
   }
 
   if (res != TRI_ERROR_NO_ERROR) {
@@ -604,14 +601,11 @@ Result RocksDBVPackIndex::removeInternal(transaction::Methods* trx,
   std::vector<uint64_t> hashes;
 
   int res;
-  try {
+  {
+    // rethrow all types of exceptions from here...
     transaction::BuilderLeaser leased(trx);
     res = fillElement(*(leased.get()), revisionId, doc, elements, hashes);
-  } catch (basics::Exception const& ex) {
+  } 
-    res = ex.code();
-  } catch (...) {
-    res = TRI_ERROR_OUT_OF_MEMORY;
-  }
 
   if (res != TRI_ERROR_NO_ERROR) {
     return IndexResult(res, this);