diff --git a/UnitTests/HttpInterface/api-transactions-noncluster-spec.rb b/UnitTests/HttpInterface/api-transactions-noncluster-spec.rb index c19f531ee1..e30fe22bc0 100644 --- a/UnitTests/HttpInterface/api-transactions-noncluster-spec.rb +++ b/UnitTests/HttpInterface/api-transactions-noncluster-spec.rb @@ -362,7 +362,7 @@ describe ArangoDB do doc.headers['content-type'].should eq("application/json; charset=utf-8") doc.parsed_response['error'].should eq(true) doc.parsed_response['code'].should eq(500) - doc.parsed_response['errorNum'].should eq(1650) #internal transactin error + doc.parsed_response['errorNum'].should eq(1650) #internal transaction error end end diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp index 998c263eaa..f6a7734a57 100644 --- a/arangod/Transaction/Methods.cpp +++ b/arangod/Transaction/Methods.cpp @@ -724,7 +724,7 @@ Result transaction::Methods::commit() { if (_state == nullptr || _state->status() != transaction::Status::RUNNING) { // transaction not created or not running - return Result(TRI_ERROR_TRANSACTION_INTERNAL); + return Result(TRI_ERROR_TRANSACTION_INTERNAL, "transaction not running on commit"); } ExecContext const* exe = ExecContext::CURRENT; @@ -751,7 +751,7 @@ Result transaction::Methods::commit() { Result transaction::Methods::abort() { if (_state == nullptr || _state->status() != transaction::Status::RUNNING) { // transaction not created or not running - return TRI_ERROR_TRANSACTION_INTERNAL; + return Result(TRI_ERROR_TRANSACTION_INTERNAL, "transaction not running on abort"); } CallbackInvoker invoker(this); @@ -769,16 +769,7 @@ Result transaction::Methods::abort() { /// @brief finish a transaction (commit or abort), based on the previous state Result transaction::Methods::finish(int errorNum) { - if (errorNum == TRI_ERROR_NO_ERROR) { - // there was no previous error, so we'll commit - return this->commit(); - } - - // there was a previous error, so we'll abort - this->abort(); - - // return original error number - return errorNum; + return finish(Result(errorNum)); } /// @brief finish a transaction (commit or abort), based on the previous state @@ -2844,7 +2835,7 @@ bool transaction::Methods::isLocked(LogicalCollection* document, Result transaction::Methods::lock(TRI_voc_cid_t cid, AccessMode::Type type) { if (_state == nullptr || _state->status() != transaction::Status::RUNNING) { - return TRI_ERROR_TRANSACTION_INTERNAL; + return Result(TRI_ERROR_TRANSACTION_INTERNAL, "transaction not running on lock"); } TransactionCollection* trxColl = trxCollection(cid, type); TRI_ASSERT(trxColl != nullptr); @@ -2855,7 +2846,7 @@ Result transaction::Methods::lock(TRI_voc_cid_t cid, Result transaction::Methods::unlock(TRI_voc_cid_t cid, AccessMode::Type type) { if (_state == nullptr || _state->status() != transaction::Status::RUNNING) { - return TRI_ERROR_TRANSACTION_INTERNAL; + return Result(TRI_ERROR_TRANSACTION_INTERNAL, "transaction not running on unlock"); } TransactionCollection* trxColl = trxCollection(cid, type); TRI_ASSERT(trxColl != nullptr); diff --git a/arangod/VocBase/Methods/Transactions.cpp b/arangod/VocBase/Methods/Transactions.cpp index 06e67781b3..293a15d93e 100644 --- a/arangod/VocBase/Methods/Transactions.cpp +++ b/arangod/VocBase/Methods/Transactions.cpp @@ -21,7 +21,6 @@ namespace arangodb { - Result executeTransaction( v8::Isolate* isolate, basics::ReadWriteLock& lock, @@ -310,10 +309,11 @@ Result executeTransactionJS( std::make_shared(vocbase, embed); // start actual transaction std::unique_ptr trx(new transaction::UserTransaction(transactionContext, readCollections, - writeCollections, exclusiveCollections, - trxOptions)); + writeCollections, exclusiveCollections, + trxOptions)); rv = trx->begin(); + if (rv.fail()) { return rv; } @@ -323,9 +323,12 @@ Result executeTransactionJS( result = action->Call(current, 1, &arguments); if (tryCatch.HasCaught()) { trx->abort(); - std::tuple rvTuple = extractArangoError(isolate, tryCatch); - if (std::get<1>(rvTuple)){ + std::tuple rvTuple = extractArangoError(isolate, tryCatch, TRI_ERROR_TRANSACTION_INTERNAL); + if (std::get<1>(rvTuple)) { rv = std::get<2>(rvTuple); + } else { + // some general error we don't know about + rv = Result(TRI_ERROR_TRANSACTION_INTERNAL, "an unknown error occured while executing the transaction"); } } } catch (arangodb::basics::Exception const& ex) { @@ -342,8 +345,7 @@ Result executeTransactionJS( return rv; } - rv = trx->commit(); - return rv; + return trx->commit(); } } // arangodb diff --git a/arangosh/Benchmark/test-cases.h b/arangosh/Benchmark/test-cases.h index b0743288ad..5228d095dd 100644 --- a/arangosh/Benchmark/test-cases.h +++ b/arangosh/Benchmark/test-cases.h @@ -1172,7 +1172,7 @@ struct TransactionCountTest : public BenchmarkOperation { TRI_AppendStringStringBuffer(buffer, "\\\"]; var startcount = c.count(); for (var " "i = 0; i < 50; ++i) { if (startcount + i !== " - "c.count()) { throw \\\"error\\\"; } c.save({ " + "c.count()) { throw \\\"error, counters deviate!\\\"; } c.save({ " "}); } }\" }"); *length = TRI_LengthStringBuffer(buffer); @@ -1318,7 +1318,7 @@ struct TransactionMultiTest : public BenchmarkOperation { "var r1 = 0; c1.toArray().forEach(function " "(d) { r1 += d.count }); var r2 = " "c2.document(\\\"sum\\\").count; if (r1 !== " - "r2) { throw \\\"error\\\"; }"); + "r2) { throw \\\"error, counters deviate!\\\"; }"); } TRI_AppendStringStringBuffer(buffer, " }\" }"); diff --git a/js/server/tests/aql/aql-query-cache-noncluster.js b/js/server/tests/aql/aql-query-cache-noncluster.js index 5b087558c2..46ef20d4ae 100644 --- a/js/server/tests/aql/aql-query-cache-noncluster.js +++ b/js/server/tests/aql/aql-query-cache-noncluster.js @@ -1,5 +1,5 @@ /*jshint globalstrict:false, strict:false, maxlen: 500 */ -/*global fail, assertEqual, assertTrue, assertFalse, AQL_EXECUTE, +/*global fail, assertEqual, assertTrue, assertFalse, assertMatch, AQL_EXECUTE, AQL_QUERY_CACHE_PROPERTIES, AQL_QUERY_CACHE_INVALIDATE */ //////////////////////////////////////////////////////////////////////////////// @@ -951,7 +951,7 @@ function ahuacatlQueryCacheTestSuite () { }); fail(); } catch (err) { - assertEqual("peng!", String(err)); + assertMatch(/peng!/, String(err)); } var result = AQL_EXECUTE(query, { "@collection": c1.name() }); diff --git a/lib/V8/v8-helper.h b/lib/V8/v8-helper.h index 8c83d27f39..eb0c5a8e78 100644 --- a/lib/V8/v8-helper.h +++ b/lib/V8/v8-helper.h @@ -24,9 +24,12 @@ #ifndef ARANGODB_V8_V8__HELPER_H #define ARANGODB_V8_V8__HELPER_H 1 -#include #include "v8-globals.h" #include "V8/v8-conv.h" +#include "V8/v8-utils.h" + +#include + namespace arangodb { inline std::string stringify(v8::Isolate* isolate, v8::Handle value) { @@ -82,53 +85,71 @@ public: } }; -inline bool isContextCanceled(v8::Isolate* isolate){ +inline bool isContextCanceled(v8::Isolate* isolate) { TRI_GET_GLOBALS(); return v8g->_canceled; } -inline std::tuple extractArangoError(v8::Isolate* isolate, v8::TryCatch& tryCatch){ +inline std::tuple extractArangoError(v8::Isolate* isolate, v8::TryCatch& tryCatch, int errorCode) { // function tries to receive arango error form tryCatch Object // return tuple: - // bool - can continue + // bool - can continue // bool - could convert // result - extracted arango error - std::tuple rv = {}; + std::tuple rv = {}; std::get<0>(rv) = true; std::get<1>(rv) = false; - if(!tryCatch.CanContinue()){ + if (!tryCatch.CanContinue()) { std::get<0>(rv) = false; + std::get<1>(rv) = true; + std::get<2>(rv).reset(TRI_ERROR_REQUEST_CANCELED); TRI_GET_GLOBALS(); v8g->_canceled = true; + return rv; } v8::Handle exception = tryCatch.Exception(); - if(!exception->IsObject()){ + if (exception->IsString()) { + // the error is a plain string + std::string errorMessage = *v8::String::Utf8Value(exception->ToString()); + std::get<1>(rv) = true; + std::get<2>(rv).reset(errorCode, errorMessage); + tryCatch.Reset(); + return rv; + } + + if (!exception->IsObject()) { + // we have no idea what this error is about + std::get<1>(rv) = true; + TRI_Utf8ValueNFC exception(tryCatch.Exception()); + char const* exceptionString = *exception; + if (exceptionString == nullptr) { + std::get<2>(rv).reset(errorCode, "JavaScript exception"); + } else { + std::get<2>(rv).reset(errorCode, exceptionString); + } return rv; } v8::Handle object = v8::Handle::Cast(exception); try { - - if(object->Has(TRI_V8_ASCII_STRING(isolate, "errorNum")) && - object->Has(TRI_V8_ASCII_STRING(isolate, "errorMessage")) - ) - { + if (object->Has(TRI_V8_ASCII_STRING(isolate, "errorNum")) && + object->Has(TRI_V8_ASCII_STRING(isolate, "errorMessage")) + ) { int errorNum = static_cast(TRI_ObjectToInt64(object->Get(TRI_V8_ASCII_STRING(isolate, "errorNum")))); - std::string errorMessage = *v8::String::Utf8Value(object->Get(TRI_V8_ASCII_STRING(isolate, "errorMessage"))); + std::string errorMessage = *v8::String::Utf8Value(object->Get(TRI_V8_ASCII_STRING(isolate, "errorMessage"))); std::get<1>(rv) = true; - std::get<2>(rv).reset(errorNum,errorMessage); + std::get<2>(rv).reset(errorNum, errorMessage); tryCatch.Reset(); return rv; } - if(object->Has(TRI_V8_ASCII_STRING(isolate, "name")) && - object->Has(TRI_V8_ASCII_STRING(isolate, "message")) - ) - { + if (object->Has(TRI_V8_ASCII_STRING(isolate, "name")) && + object->Has(TRI_V8_ASCII_STRING(isolate, "message")) + ) { std::string name = *v8::String::Utf8Value(object->Get(TRI_V8_ASCII_STRING(isolate, "name"))); std::string message = *v8::String::Utf8Value(object->Get(TRI_V8_ASCII_STRING(isolate, "message"))); if(name == "TypeError"){ diff --git a/lib/V8/v8-utils.cpp b/lib/V8/v8-utils.cpp index 86efa039ea..abefa3271a 100644 --- a/lib/V8/v8-utils.cpp +++ b/lib/V8/v8-utils.cpp @@ -4220,7 +4220,7 @@ void TRI_LogV8Exception(v8::Isolate* isolate, v8::TryCatch* tryCatch) { << exceptionString; } } else { - TRI_Utf8ValueNFC filename( message->GetScriptResourceName()); + TRI_Utf8ValueNFC filename(message->GetScriptResourceName()); char const* filenameString = *filename; // if ifdef is not used, the compiler will complain about linenum being // unused