diff --git a/src/communication/bolt/v1/session.hpp b/src/communication/bolt/v1/session.hpp index 414ed755a..971853a5a 100644 --- a/src/communication/bolt/v1/session.hpp +++ b/src/communication/bolt/v1/session.hpp @@ -10,8 +10,8 @@ #include "communication/bolt/v1/constants.hpp" #include "communication/bolt/v1/state.hpp" #include "communication/bolt/v1/states/error.hpp" +#include "communication/bolt/v1/states/executing.hpp" #include "communication/bolt/v1/states/handshake.hpp" -#include "communication/bolt/v1/states/idle_result.hpp" #include "communication/bolt/v1/states/init.hpp" #include "communication/bolt/v1/decoder/chunked_decoder_buffer.hpp" diff --git a/src/communication/bolt/v1/states/idle_result.hpp b/src/communication/bolt/v1/states/executing.hpp similarity index 75% rename from src/communication/bolt/v1/states/idle_result.hpp rename to src/communication/bolt/v1/states/executing.hpp index e3a045f97..3dbdbeb5d 100644 --- a/src/communication/bolt/v1/states/idle_result.hpp +++ b/src/communication/bolt/v1/states/executing.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -136,47 +137,62 @@ State HandleRun(Session &session, State state, Marker marker) { return State::Close; } return State::Result; - // TODO: Remove duplication in error handling. - } catch (const utils::BasicException &e) { - // clear header success message - session.encoder_buffer_.Clear(); - bool fail_sent = session.encoder_.MessageFailure( - {{"code", "Memgraph.Exception"}, {"message", e.what()}}); - DLOG(WARNING) << fmt::format("Error message: {}", e.what()); - if (!fail_sent) { - DLOG(WARNING) << "Couldn't send failure message!"; - return State::Close; - } - if (!in_explicit_transaction) { - session.Abort(); - return State::ErrorIdle; - } - return State::ErrorWaitForRollback; - } catch (const utils::StacktraceException &e) { - // clear header success message - session.encoder_buffer_.Clear(); - bool fail_sent = session.encoder_.MessageFailure( - {{"code", "Memgraph.Exception"}, {"message", e.what()}}); - DLOG(WARNING) << fmt::format("Error message: {}", e.what()); - DLOG(WARNING) << fmt::format("Error trace: {}", e.trace()); - if (!fail_sent) { - DLOG(WARNING) << "Couldn't send failure message!"; - return State::Close; - } - if (!in_explicit_transaction) { - session.Abort(); - return State::ErrorIdle; - } - return State::ErrorWaitForRollback; } catch (const std::exception &e) { // clear header success message session.encoder_buffer_.Clear(); + DLOG(WARNING) << fmt::format("Error message: {}", e.what()); + if (const auto *p = dynamic_cast(&e)) { + DLOG(WARNING) << fmt::format("Error trace: {}", p->trace()); + } + + auto code_message = [&e]() -> std::pair { + if (const auto *p = dynamic_cast(&e)) { + // Clients expect 4 strings separated by dots. First being database name + // (for example: Neo, Memgraph...), second being either ClientError, + // TransientError or DatabaseError (or ClientNotification for warnings). + // ClientError means wrong query, do not retry. DatabaseError means + // something wrong in database, do not retry. TransientError means query + // failed, but if retried it may succeed, retry it. + // + // Third and fourth strings being namespace and specific error name. + // It is not really important what we put there since we don't expect + // any special handling of specific exceptions on client side, but we + // need to make sure that we don't accidentally return some exception + // name which clients handle in a special way. For example, if client + // receives *.TransientError.Transaction.Terminate it will not rerun + // query even though TransientError was returned, because of Neo's + // semantics of that error. + // + // QueryException was thrown, only changing the query or existing + // database data could make this query successful. Return ClientError to + // discourage retry of this query. + return {"Memgraph.ClientError.MemgraphError.MemgraphError", e.what()}; + } + if (const auto *p = dynamic_cast(&e)) { + // Exception not derived from QueryException was thrown which means that + // database probably aborted transaction because of some timeout, + // deadlock, serialization error or something similar. We return + // TransientError since retry of same transaction could succeed. + return {"Memgraph.TransientError.MemgraphError.MemgraphError", + e.what()}; + } + if (const auto *p = dynamic_cast(&e)) { + // std::bad_alloc was thrown, God knows in which state is database -> + // terminate. + LOG(FATAL) << "Memgraph is out of memory"; + } + // All exceptions used in memgraph are derived from BasicException. Since + // we caught some other exception we don't know what is going on. Return + // DatabaseError, log real message and return generic string. + LOG(ERROR) << "Unknown exception occurred during query execution " + << e.what(); + return {"Memgraph.DatabaseError.MemgraphError.MemgraphError", + "An unknown exception occurred, this is unexpected. Real message " + "should be in database logs."}; + }(); + bool fail_sent = session.encoder_.MessageFailure( - {{"code", "Memgraph.Exception"}, - {"message", - "An unknown exception occured, please contact your database " - "administrator."}}); - DLOG(WARNING) << fmt::format("std::exception {}", e.what()); + {{"code", code_message.first}, {"message", code_message.second}}); if (!fail_sent) { DLOG(WARNING) << "Couldn't send failure message!"; return State::Close; diff --git a/src/query/exceptions.hpp b/src/query/exceptions.hpp index 05848a2b8..202bc204a 100644 --- a/src/query/exceptions.hpp +++ b/src/query/exceptions.hpp @@ -6,7 +6,11 @@ namespace query { -/** @brief Base class of all query language related exceptions. +/** + * @brief Base class of all query language related exceptions. All exceptions + * derived from this one will be interpreted as ClientError-s, i. e. if client + * executes same query again without making modifications to the database data, + * query will fail again. */ class QueryException : public utils::BasicException { using utils::BasicException::BasicException; @@ -60,16 +64,6 @@ class TypeMismatchError : public SemanticException { name, datum, expected)) {} }; -class HintedAbortError : public QueryException { - public: - using QueryException::QueryException; - HintedAbortError() - : QueryException( - "Transaction was asked to abort, most likely because it was " - "executing longer than time specified by " - "--query-execution-time-sec flag") {} -}; - class UnprovidedParameterError : public QueryException { public: using QueryException::QueryException; @@ -84,20 +78,17 @@ class QueryRuntimeException : public QueryException { using QueryException::QueryException; }; -// TODO: Move this elsewhere, it has no place in query. -class DecoderException : public utils::StacktraceException { +// This one is inherited from BasicException and will be treated as +// TransientError, i. e. client will be encouraged to retry execution because it +// could succeed if executed again. +class HintedAbortError : public utils::BasicException { public: - using utils::StacktraceException::StacktraceException; -}; - -class PlanCompilationException : public utils::StacktraceException { - public: - using utils::StacktraceException::StacktraceException; -}; - -class PlanExecutionException : public utils::StacktraceException { - public: - using utils::StacktraceException::StacktraceException; + using utils::BasicException::BasicException; + HintedAbortError() + : utils::BasicException( + "Transaction was asked to abort, most likely because it was " + "executing longer than time specified by " + "--query-execution-time-sec flag") {} }; } // namespace query diff --git a/tests/drivers/python/transactions.py b/tests/drivers/python/transactions.py index 94ff95e01..3137a5a6f 100644 --- a/tests/drivers/python/transactions.py +++ b/tests/drivers/python/transactions.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 # -*- coding: utf-8 -*- -from neo4j.v1 import GraphDatabase, basic_auth, CypherError +from neo4j.v1 import GraphDatabase, basic_auth, ClientError, TransientError driver = GraphDatabase.driver("bolt://localhost:7687", auth=basic_auth("", ""), @@ -20,17 +20,39 @@ def tx_good(tx, name, name2): a = tx.run("CREATE (a:Person {name: $name}) RETURN a", name=name2).data() print(a[0]['a']) +def tx_too_long(tx): + a = tx.run("MATCH (a), (b), (c), (d), (e), (f) RETURN COUNT(*) AS cnt") + print(a[0]['cnt']) + def add_person(f, name, name2): with driver.session() as session: session.write_transaction(f, name, name2) +# Wrong query. try: add_person(tx_error, "mirko", "slavko") -except CypherError: +except ClientError: pass +# Correct query. add_person(tx_good, "mirka", "slavka") +# Setup for next query. +with driver.session() as session: + session.write_transaction( + lambda tx: tx.run("UNWIND range(1, 100000) AS x CREATE ()")) + +# Query that will run for a very long time, transient error expected. +try: + session = driver.session() + session.read_transaction(tx_too_long) +except TransientError: + print("transient error") +except: + assert False, "TransientError should have been thrown" +finally: + session.close() + driver.close() print("All ok!")