fixed issue arangodb#12304 (arangodb#12447)

dcosta72 · Aug 20, 2020 · 6ba0a6c · 6ba0a6c
1 parent d66b611
commit 6ba0a6c
Show file tree

Hide file tree

Showing 25 changed files with 459 additions and 557 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,14 @@
 devel
 -----
 
+* Fixed issue #12304: insert in transaction causing com.arangodb.ArangoDBException: 
+  Response: 500, Error: 4 - Builder value not yet sealed. 
+
+  This happened when too deeply-nested documents (more than 63 levels of nesting)
+  were inserted. While indefinite nesting is still not supported, the error message
+  has been corrected from the internal HTTP 500 error "Builder value not yet sealed"
+  to the correct HTTP 400 "Bad parameter".
+
 * Show optimizer rules with highest execution times in explain output.
 
 * Fixed that dropping a vanished follower works again. An exception response

diff --git a/arangod/Agency/v8-agency.cpp b/arangod/Agency/v8-agency.cpp
@@ -92,11 +92,7 @@ static void JS_ReadAgent(v8::FunctionCallbackInfo<v8::Value> const& args) {
   }
 
   query_t query = std::make_shared<Builder>();
-  int res = TRI_V8ToVPack(isolate, *query, args[0], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION(res);
-  }
+  TRI_V8ToVPack(isolate, *query, args[0], false);
 
   read_ret_t ret = agent->read(query);
 
@@ -125,11 +121,7 @@ static void JS_WriteAgent(v8::FunctionCallbackInfo<v8::Value> const& args) {
   }
 
   query_t query = std::make_shared<Builder>();
-  int res = TRI_V8ToVPack(isolate, *query, args[0], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION(res);
-  }
+  TRI_V8ToVPack(isolate, *query, args[0], false);
 
   write_ret_t ret = agent->write(query);
 

diff --git a/arangod/Aql/Expression.cpp b/arangod/Aql/Expression.cpp
@@ -922,11 +922,8 @@ AqlValue Expression::invokeV8Function(ExpressionContext* expressionContext,
   auto& trx = expressionContext->trx();
   transaction::BuilderLeaser builder(&trx);
 
-  int res = TRI_V8ToVPack(isolate, *builder.get(), result, false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    THROW_ARANGO_EXCEPTION(res);
-  }
+  // can throw
+  TRI_V8ToVPack(isolate, *builder.get(), result, false);
 
   mustDestroy = true;  // builder = dynamic data
   return AqlValue(builder->slice(), builder->size());

diff --git a/arangod/Cluster/v8-cluster.cpp b/arangod/Cluster/v8-cluster.cpp
@@ -136,18 +136,10 @@ static void JS_CasAgency(v8::FunctionCallbackInfo<v8::Value> const& args) {
   std::string const key = TRI_ObjectToString(isolate, args[0]);
 
   VPackBuilder oldBuilder;
-  int res = TRI_V8ToVPack(isolate, oldBuilder, args[1], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION_PARAMETER("cannot convert <oldValue> to VPack");
-  }
+  TRI_V8ToVPack(isolate, oldBuilder, args[1], false);
 
   VPackBuilder newBuilder;
-  res = TRI_V8ToVPack(isolate, newBuilder, args[2], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION_PARAMETER("cannot convert <newValue> to VPack");
-  }
+  TRI_V8ToVPack(isolate, newBuilder, args[2], false);
 
   double ttl = 0.0;
   if (args.Length() > 3) {
@@ -315,11 +307,7 @@ static void JS_APIAgency(std::string const& envelope,
   }
 
   VPackBuilder builder;
-  int res = TRI_V8ToVPack(isolate, builder, args[0], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION_PARAMETER("cannot convert query to JSON");
-  }
+  TRI_V8ToVPack(isolate, builder, args[0], false);
 
   TRI_GET_GLOBALS();
   AgencyComm comm(v8g->_server);
@@ -404,11 +392,7 @@ static void JS_SetAgency(v8::FunctionCallbackInfo<v8::Value> const& args) {
   std::string const key = TRI_ObjectToString(isolate, args[0]);
 
   VPackBuilder builder;
-  int res = TRI_V8ToVPack(isolate, builder, args[1], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION_PARAMETER("cannot convert <value> to JSON");
-  }
+  TRI_V8ToVPack(isolate, builder, args[1], false);
 
   double ttl = 0.0;
   if (args.Length() > 2) {
@@ -903,11 +887,7 @@ static void JS_GetResponsibleShardClusterInfo(v8::FunctionCallbackInfo<v8::Value
   }
 
   VPackBuilder builder;
-  int res = TRI_V8ToVPack(isolate, builder, args[1], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION(res);
-  }
+  TRI_V8ToVPack(isolate, builder, args[1], false);
 
   ShardID shardId;
   CollectionID collectionId = TRI_ObjectToString(isolate, args[0]);
@@ -922,8 +902,8 @@ static void JS_GetResponsibleShardClusterInfo(v8::FunctionCallbackInfo<v8::Value
 
   bool usesDefaultShardingAttributes;
 
-  res = collInfo->getResponsibleShard(builder.slice(), documentIsComplete,
-                                      shardId, usesDefaultShardingAttributes);
+  int res = collInfo->getResponsibleShard(builder.slice(), documentIsComplete,
+                                          shardId, usesDefaultShardingAttributes);
 
   if (res != TRI_ERROR_NO_ERROR) {
     TRI_V8_THROW_EXCEPTION(res);

diff --git a/arangod/RestHandler/RestAdminExecuteHandler.cpp b/arangod/RestHandler/RestAdminExecuteHandler.cpp
@@ -166,29 +166,21 @@ RestStatus RestAdminExecuteHandler::execute() {
 
         VPackBuilder result;
         bool handled = false;
-        int res = TRI_ERROR_FAILED;
 
         if (returnAsJSON) {
           result.openObject(true);
           result.add(StaticStrings::Error, VPackValue(false));
           result.add(StaticStrings::Code, VPackValue(static_cast<int>(rest::ResponseCode::OK)));
           if (rv->IsObject()) {
-            res = TRI_V8ToVPack(isolate, result, rv, false);
+            TRI_V8ToVPack(isolate, result, rv, false);
             handled = true;
           }
           result.close();
         }
 
         if (!handled) {
           result.clear();
-
-          VPackBuilder temp;
-          res = TRI_V8ToVPack(isolate, temp, rv, false);
-          result.add(temp.slice());
-        }
-
-        if (res != TRI_ERROR_NO_ERROR) {
-          THROW_ARANGO_EXCEPTION(res);
+          TRI_V8ToVPack(isolate, result, rv, false);
         }
 
         generateResult(rest::ResponseCode::OK, result.slice());

diff --git a/arangod/V8Server/v8-analyzers.cpp b/arangod/V8Server/v8-analyzers.cpp
@@ -322,10 +322,7 @@ void JS_Create(v8::FunctionCallbackInfo<v8::Value> const& args) {
       propertiesSlice = propertiesBuilder.slice();
     } else if (args[2]->IsObject()) {
       auto value = args[2]->ToObject(TRI_IGETC).FromMaybe(v8::Local<v8::Object>());
-      auto res = TRI_V8ToVPack(isolate, propertiesBuilder, value, false);
-      if (TRI_ERROR_NO_ERROR != res) {
-        TRI_V8_THROW_EXCEPTION(res);
-      }
+      TRI_V8ToVPack(isolate, propertiesBuilder, value, false);
       propertiesSlice = propertiesBuilder.slice();
     } else if (!args[2]->IsNull()) {
       TRI_V8_THROW_TYPE_ERROR("<properties> must be an object");

diff --git a/arangod/V8Server/v8-collection.cpp b/arangod/V8Server/v8-collection.cpp
@@ -233,10 +233,7 @@ static int V8ToVPackNoKeyRevId(v8::Isolate* isolate, VPackBuilder& builder,
     if (strcmp(*str, "_key") != 0 && strcmp(*str, "_rev") != 0 &&
         strcmp(*str, "_id") != 0) {
       builder.add(VPackValue(*str));
-      int res = TRI_V8ToVPack(isolate, builder, o->Get(context, key).FromMaybe(v8::Local<v8::Value>()), false);
-      if (res != TRI_ERROR_NO_ERROR) {
-        return res;
-      }
+      TRI_V8ToVPack(isolate, builder, o->Get(context, key).FromMaybe(v8::Local<v8::Value>()), false);
     }
   }
   return TRI_ERROR_NO_ERROR;
@@ -1006,10 +1003,7 @@ static void JS_GetResponsibleShardVocbaseCol(v8::FunctionCallbackInfo<v8::Value>
     builder.add(StaticStrings::KeyString, VPackValue(TRI_ObjectToString(isolate, args[0])));
     builder.close();
   } else {
-    int res = TRI_V8ToVPack(isolate, builder, args[0], false);
-    if (res != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(res);
-    }
+    TRI_V8ToVPack(isolate, builder, args[0], false);
   }
   if (!builder.slice().isObject()) {
     TRI_V8_THROW_EXCEPTION_USAGE("getResponsibleShard(<object>)");
@@ -1150,22 +1144,13 @@ static void JS_PropertiesVocbaseCol(v8::FunctionCallbackInfo<v8::Value> const& a
 
     if (par->IsObject()) {
       VPackBuilder builder;
-      {
-        int res = TRI_V8ToVPack(isolate, builder, args[0], false);
-        if (res != TRI_ERROR_NO_ERROR) {
-          TRI_V8_THROW_EXCEPTION(res);
-        }
-      }
-
+      TRI_V8ToVPack(isolate, builder, args[0], false);
       TRI_ASSERT(builder.isClosed());
 
       auto res = methods::Collections::updateProperties(*consoleColl, builder.slice());
       if (res.fail() && ServerState::instance()->isCoordinator()) {
         TRI_V8_THROW_EXCEPTION(res);
       }
-
-      // TODO Review
-      // TODO API compatibility, for now we ignore if persisting fails...
     }
   }
 
@@ -1696,10 +1681,7 @@ static void JS_PregelStart(v8::FunctionCallbackInfo<v8::Value> const& args) {
   }
   VPackBuilder paramBuilder;
   if (argLength >= 4 && args[3]->IsObject()) {
-    int res = TRI_V8ToVPack(isolate, paramBuilder, args[3], false);
-    if (res != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(res);
-    }
+    TRI_V8ToVPack(isolate, paramBuilder, args[3], false);
   }
 
   auto& vocbase = GetContextVocBase(isolate);
@@ -1983,12 +1965,8 @@ static void InsertVocbaseCol(v8::Isolate* isolate,
   VPackBuilder builder(&vpackOptions);
 
   auto doOneDocument = [&](v8::Handle<v8::Value> obj) -> void {
-    int res = TRI_V8ToVPack(isolate, builder, obj, true);
-
-    if (res != TRI_ERROR_NO_ERROR) {
-      THROW_ARANGO_EXCEPTION(res);
-    }
-
+    TRI_V8ToVPack(isolate, builder, obj, true);
+
     if (isEdgeCollection && oldEdgeSignature) {
       // Just insert from and to. Check is done later.
       std::string tmpId(ExtractIdString(isolate, args[0]));

diff --git a/arangod/V8Server/v8-dispatcher.cpp b/arangod/V8Server/v8-dispatcher.cpp
@@ -211,14 +211,11 @@ static void JS_RegisterTask(v8::FunctionCallbackInfo<v8::Value> const& args) {
   auto parameters = std::make_shared<VPackBuilder>();
 
   if (TRI_HasProperty(context, isolate, obj, "params")) {
-    int res = TRI_V8ToVPack(isolate, *parameters,
-                            obj->Get(TRI_IGETC,
-                                     TRI_V8_ASCII_STRING(isolate, "params"))
-                                .FromMaybe(v8::Local<v8::Value>()),
-                            false);
-    if (res != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(res);
-    }
+    TRI_V8ToVPack(isolate, *parameters,
+                  obj->Get(TRI_IGETC,
+                           TRI_V8_ASCII_STRING(isolate, "params"))
+                      .FromMaybe(v8::Local<v8::Value>()),
+                  false);
   }
 
   command = "(function (params) { " + command + " } )(params);";

diff --git a/arangod/V8Server/v8-query.cpp b/arangod/V8Server/v8-query.cpp
@@ -409,11 +409,7 @@ static void JS_LookupByKeys(v8::FunctionCallbackInfo<v8::Value> const& args) {
   bindVars->add("@collection", VPackValue(collection->name()));
 
   VPackBuilder keys;
-  int res = TRI_V8ToVPack(isolate, keys, args[0], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION(res);
-  }
+  TRI_V8ToVPack(isolate, keys, args[0], false);
 
   bindVars->add(VPackValue("keys"));
   arangodb::aql::BindParameters::stripCollectionNames(keys.slice(), collection->name(),
@@ -458,10 +454,7 @@ static void JS_RemoveByKeys(v8::FunctionCallbackInfo<v8::Value> const& args) {
   bindVars->add("@collection", VPackValue(collection->name()));
   bindVars->add(VPackValue("keys"));
 
-  int res = TRI_V8ToVPack(isolate, *(bindVars.get()), args[0], false);
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION(res);
-  }
+  TRI_V8ToVPack(isolate, *(bindVars.get()), args[0], false);
   bindVars->close();
 
   std::string const queryString(

diff --git a/arangod/V8Server/v8-replication.cpp b/arangod/V8Server/v8-replication.cpp
@@ -185,11 +185,7 @@ static void SynchronizeReplication(v8::FunctionCallbackInfo<v8::Value> const& ar
   // treat the argument as an object from now on
   v8::Handle<v8::Object> object = v8::Handle<v8::Object>::Cast(args[0]);
   VPackBuilder builder;
-  int res = TRI_V8ToVPack(isolate, builder, args[0], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION(res);
-  }
+  TRI_V8ToVPack(isolate, builder, args[0], false);
 
   auto& vocbase = GetContextVocBase(isolate);
   std::string databaseName;
@@ -320,11 +316,7 @@ static void JS_SynchronizeReplicationFinalize(v8::FunctionCallbackInfo<v8::Value
   v8::Handle<v8::Object> object = v8::Handle<v8::Object>::Cast(args[0]);
 
   VPackBuilder builder;
-  int res = TRI_V8ToVPack(isolate, builder, args[0], false);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION(res);
-  }
+  TRI_V8ToVPack(isolate, builder, args[0], false);
 
   std::string database;
   if (TRI_HasProperty(context, isolate, object, "database")) {
@@ -472,11 +464,7 @@ static void ConfigureApplierReplication(v8::FunctionCallbackInfo<v8::Value> cons
     }
 
     VPackBuilder builder;
-    int res = TRI_V8ToVPack(isolate, builder, args[0], false);
-
-    if (res != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(res);
-    }
+    TRI_V8ToVPack(isolate, builder, args[0], false);
 
     std::string databaseName;
     if (applierType == APPLIER_DATABASE) {

diff --git a/arangod/V8Server/v8-ttl.cpp b/arangod/V8Server/v8-ttl.cpp
@@ -54,11 +54,7 @@ static void JS_TtlProperties(v8::FunctionCallbackInfo<v8::Value> const& args) {
   } else {
     // set properties
     VPackBuilder properties;
-
-    int res = TRI_V8ToVPack(isolate, properties, args[0], false);
-    if (res != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(res);
-    }
+    TRI_V8ToVPack(isolate, properties, args[0], false);
 
     result = methods::Ttl::setProperties(v8g->_server.getFeature<TtlFeature>(),
                                          properties.slice(), builder);

diff --git a/arangod/V8Server/v8-users.cpp b/arangod/V8Server/v8-users.cpp
@@ -117,10 +117,7 @@ void StoreUser(v8::FunctionCallbackInfo<v8::Value> const& args, bool replace) {
 
   VPackBuilder extras;
   if (args.Length() >= 4) {
-    int r = TRI_V8ToVPackSimple(isolate, extras, args[3]);
-    if (r != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(r);
-    }
+    TRI_V8ToVPack(isolate, extras, args[3], false, false);
   }
 
   auth::UserManager* um = AuthenticationFeature::instance()->userManager();
@@ -165,10 +162,7 @@ static void JS_UpdateUser(v8::FunctionCallbackInfo<v8::Value> const& args) {
 
   VPackBuilder extras;
   if (args.Length() >= 4) {
-    int r = TRI_V8ToVPackSimple(isolate, extras, args[3]);
-    if (r != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(r);
-    }
+    TRI_V8ToVPack(isolate, extras, args[3], false, false);
   }
 
   auth::UserManager* um = AuthenticationFeature::instance()->userManager();
@@ -444,10 +438,7 @@ static void JS_UpdateConfigData(v8::FunctionCallbackInfo<v8::Value> const& args)
   VPackBuilder merge;
   if (args.Length() > 2) {
     VPackBuilder value;
-    int res = TRI_V8ToVPackSimple(isolate, value, args[2]);
-    if (res != TRI_ERROR_NO_ERROR) {
-      TRI_V8_THROW_EXCEPTION(res);
-    }
+    TRI_V8ToVPack(isolate, value, args[2], false, false);
     merge.add(key, value.slice());
   } else {
     merge.add(key, VPackSlice::nullSlice());