From 457b09f15c68ee8f54f18e5af209871d9ae8a54a Mon Sep 17 00:00:00 2001 From: eelke Date: Sun, 14 Aug 2022 08:04:21 +0200 Subject: [PATCH] Improved error reporting --- pglab/ConnectionConfigurationWidget.cpp | 17 ++-- pglab/DatabaseWindow.cpp | 8 ++ pglab/crud/CrudModel.cpp | 5 +- pglab/crud/CrudModel.h | 2 +- pglab/querytool/QueryTool.cpp | 5 +- pglab/querytool/QueryTool.h | 2 +- pglablib/ASyncDBConnection.cpp | 105 +++++++++++------------- pglablib/ASyncDBConnection.h | 14 +++- pgsql/Pgsql_Connection.cpp | 10 ++- pgsql/Pgsql_Connection.h | 12 +-- pgsql/Pgsql_Transaction.cpp | 16 ++-- pgsql/Pgsql_Transaction.h | 8 +- 12 files changed, 110 insertions(+), 94 deletions(-) diff --git a/pglab/ConnectionConfigurationWidget.cpp b/pglab/ConnectionConfigurationWidget.cpp index 72cf418..34dc744 100644 --- a/pglab/ConnectionConfigurationWidget.cpp +++ b/pglab/ConnectionConfigurationWidget.cpp @@ -254,13 +254,18 @@ void ConnectionConfigurationWidget::testConnection() cc.setPassword(password); auto qthis = QPointer(this); - QFuture result = QtConcurrent::run([cc] { - return TestConnection(cc); - }).then(qApp, [qthis](TestConnectionResult result) { - if (qthis) { - qthis.data()->handleTestResult(result); + QFuture result = QtConcurrent::run( + [cc] + { + return TestConnection(cc); } - }); + ).then(qApp, + [qthis](TestConnectionResult result) + { + if (qthis) + qthis.data()->handleTestResult(result); + } + ); } void ConnectionConfigurationWidget::handleTestResult(TestConnectionResult result) diff --git a/pglab/DatabaseWindow.cpp b/pglab/DatabaseWindow.cpp index 849b47b..d00a925 100644 --- a/pglab/DatabaseWindow.cpp +++ b/pglab/DatabaseWindow.cpp @@ -118,6 +118,14 @@ void DatabaseWindow::setConfig(const ConnectionConfig &config) if (qthis) qthis.data()->catalogLoaded(db); } + ).onFailed(qApp, [qthis](OpenDatabaseException &ex) + { + if (qthis) { + QMessageBox::critical(qthis.data(), "Error reading database", + ex.text()); + qthis.data()->close(); + } + } ); } diff --git a/pglab/crud/CrudModel.cpp b/pglab/crud/CrudModel.cpp index 188711f..253b9ba 100644 --- a/pglab/crud/CrudModel.cpp +++ b/pglab/crud/CrudModel.cpp @@ -9,7 +9,6 @@ #include "CustomDataRole.h" #include #include -#include #include #include "Pgsql_oids.h" #include "Pgsql_PgException.h" @@ -247,9 +246,9 @@ void CrudModel::initRowMapping() m_rowMapping.emplace_back(i); } -void CrudModel::connectionStateChanged(ASyncDBConnection::State state) +void CrudModel::connectionStateChanged(ASyncDBConnection::StateData state) { - switch (state) { + switch (state.State) { case ASyncDBConnection::State::NotConnected: break; case ASyncDBConnection::State::Connecting: diff --git a/pglab/crud/CrudModel.h b/pglab/crud/CrudModel.h index cd6bafb..e852832 100644 --- a/pglab/crud/CrudModel.h +++ b/pglab/crud/CrudModel.h @@ -259,7 +259,7 @@ private: private slots: void loadIntoModel(std::shared_ptr data); - void connectionStateChanged(ASyncDBConnection::State state); + void connectionStateChanged(ASyncDBConnection::StateData state); }; #endif // CRUDMODEL_H diff --git a/pglab/querytool/QueryTool.cpp b/pglab/querytool/QueryTool.cpp index 5e1868c..1133eb1 100644 --- a/pglab/querytool/QueryTool.cpp +++ b/pglab/querytool/QueryTool.cpp @@ -314,11 +314,12 @@ void QueryTool::queryTextChanged() setQueryTextChanged(true); } -void QueryTool::connectionStateChanged(ASyncDBConnection::State state) +void QueryTool::connectionStateChanged(ASyncDBConnection::StateData state) { QString iconname; - switch (state) { + switch (state.State) { case ASyncDBConnection::State::NotConnected: + QMessageBox::warning(this, "pglab", tr("Warning connection and any of its session state has been lost")); startConnect(); iconname = "red.png"; break; diff --git a/pglab/querytool/QueryTool.h b/pglab/querytool/QueryTool.h index f254991..2c99cc4 100644 --- a/pglab/querytool/QueryTool.h +++ b/pglab/querytool/QueryTool.h @@ -113,7 +113,7 @@ private slots: void query_ready(std::shared_ptr, qint64 elapsedms); void queryTextChanged(); - void connectionStateChanged(ASyncDBConnection::State state); + void connectionStateChanged(ASyncDBConnection::StateData state); void receiveNotice(Pgsql::ErrorDetails notice); void startConnect(); diff --git a/pglablib/ASyncDBConnection.cpp b/pglablib/ASyncDBConnection.cpp index 8580428..da76ba0 100644 --- a/pglablib/ASyncDBConnection.cpp +++ b/pglablib/ASyncDBConnection.cpp @@ -79,10 +79,10 @@ private: bool makeConnection(); void communicate(); - void doStateCallback(ASyncDBConnection::State state); + void doStateCallback(ASyncDBConnection::StateData state); /// Wait's for a command to come in and send's it to the server void waitForAndSendCommand(); - void doNewCommand(); + void doNewCommand(); void waitForResult(); @@ -106,33 +106,46 @@ void ASyncDBConnectionThread::run() SCOPE_EXIT { m_state = ASyncDBConnection::State::NotConnected; m_terminated = true; + m_connection.close(); +// doStateCallback(ASyncDBConnection::State::NotConnected); }; - while (!terminateRequested) { - // make or recover connection - if (makeConnection()) { - m_connection.setNoticeReceiver( - [this](const PGresult *result) { processNotice(result); }); - m_canceller = m_connection.getCancel(); + try { + while (!terminateRequested) { + + // make or recover connection + if (makeConnection()) { + m_connection.setNoticeReceiver( + [this](const PGresult *result) { processNotice(result); }); + m_canceller = m_connection.getCancel(); - // send commands and receive results - communicate(); - } - else { - // It is not possible to determine the source of the problem. - // Accept for PQconnectionNeedsPassword + // send commands and receive results + communicate(); + } + else { + // It is not possible to determine the source of the problem. + // Accept for PQconnectionNeedsPassword - // Pass problem to main thread and stop this thread + // Pass problem to main thread and stop this thread - // Main thread needs to know it has to restart connecting if it want's to. - // TODO: add status functions to help main thread so it doesn't have to remember - // everything reported through callbacks. + // Main thread needs to know it has to restart connecting if it want's to. + // TODO: add status functions to help main thread so it doesn't have to remember + // everything reported through callbacks. - break; - } - } - // close connection + break; + } + } + doStateCallback({ ASyncDBConnection::State::NotConnected, QString("terminating") }); + } + catch (const std::exception &ex) + { + doStateCallback({ ASyncDBConnection::State::NotConnected, QString::fromUtf8(ex.what()) }); + } + catch (...) + { + doStateCallback({ ASyncDBConnection::State::NotConnected, QString::fromUtf8("Unknown error") }); + } } @@ -146,19 +159,7 @@ bool ASyncDBConnectionThread::makeConnection() using namespace std::literals::chrono_literals; // start connecting -// auto keywords = m_config.getKeywords(); -// auto values = m_config.getValues(); QString conn_string = m_config.connectionString(); -#if false - try { - m_connection.connect(conn_string); //keywords, values, 0); - doStateCallback(ASyncDBConnection::State::Connected); - return true; - } - catch (Pgsql::PgConnectionError &) { - } - return false; -#else while (!terminateRequested) { bool ok = m_connection.connectStart(conn_string); //keywords, values); @@ -190,20 +191,21 @@ bool ASyncDBConnectionThread::makeConnection() if (res == wait_result_socket_event) { auto poll_state = m_connection.connectPoll(); if (poll_state == PGRES_POLLING_OK) { + qDebug() << "ASyncDBConnection Connected"; // if connected return true - doStateCallback(ASyncDBConnection::State::Connected); + doStateCallback({ ASyncDBConnection::State::Connected, "Success" }); return true; } else if (poll_state == PGRES_POLLING_FAILED) { - doStateCallback(ASyncDBConnection::State::NotConnected); + doStateCallback({ ASyncDBConnection::State::NotConnected, "Failed to connect" }); return false; } else if (poll_state == PGRES_POLLING_READING) { - doStateCallback(ASyncDBConnection::State::Connecting); + doStateCallback({ ASyncDBConnection::State::Connecting, "Negotiating" }); fd = FD_READ; } else if (poll_state == PGRES_POLLING_WRITING) { - doStateCallback(ASyncDBConnection::State::Connecting); + doStateCallback({ ASyncDBConnection::State::Connecting, "Negotiating" }); fd = FD_WRITE; } } @@ -216,7 +218,6 @@ bool ASyncDBConnectionThread::makeConnection() } } return false; -#endif } void ASyncDBConnectionThread::communicate() @@ -252,13 +253,9 @@ void ASyncDBConnectionThread::stop() m_stopEvent.set(); } -void ASyncDBConnectionThread::doStateCallback(ASyncDBConnection::State state) +void ASyncDBConnectionThread::doStateCallback(ASyncDBConnection::StateData state) { - m_state = state; -// std::lock_guard lg(m_stateCallback.m_mutex); -// if (m_stateCallback.m_func) { -// m_stateCallback.m_func(state); -// } + m_state = state.State; Q_EMIT asyncConnObject->onStateChanged(state); } @@ -289,21 +286,13 @@ void ASyncDBConnectionThread::doNewCommand() const Command &command = m_commandQueue.m_queue.front(); bool query_send = false; if (command.params.empty()) - query_send = m_connection.sendQuery(command.command.c_str()); + m_connection.sendQuery(command.command.c_str()); else - query_send = m_connection.sendQueryParams(command.command.c_str(), command.params); + m_connection.sendQueryParams(command.command.c_str(), command.params); - if (query_send) { - m_timer.start(); - doStateCallback(ASyncDBConnection::State::QuerySend); - } - else { - std::string error = m_connection.getErrorMessage(); - qDebug() << "Error sending query " << QString::fromStdString(error); - // todo: need to report the error - } + m_timer.start(); + doStateCallback(ASyncDBConnection::State::QuerySend); } - } bool ASyncDBConnectionThread::consumeResultInput() @@ -332,7 +321,7 @@ bool ASyncDBConnectionThread::consumeResultInput() // error during consume auto error_msg = m_connection.getErrorMessage(); qDebug() << "error while communicating with server " << QString::fromStdString(error_msg); - doStateCallback(ASyncDBConnection::State::NotConnected); + doStateCallback({ASyncDBConnection::State::NotConnected, QString::fromStdString(error_msg)}); finished = true; stop(); } diff --git a/pglablib/ASyncDBConnection.h b/pglablib/ASyncDBConnection.h index edec34d..df5789d 100644 --- a/pglablib/ASyncDBConnection.h +++ b/pglablib/ASyncDBConnection.h @@ -21,6 +21,7 @@ class ASyncDBConnectionThread; class ASyncDBConnection: public QObject { Q_OBJECT public: + enum class State { NotConnected, Connecting, @@ -30,6 +31,17 @@ public: Terminating ///< shutting down }; + class StateData { + public: + State State; + QString Message; + + StateData(enum State state, QString message = QString()) + : State(state) + , Message(message) + {} + }; + using on_result_callback = std::function>, qint64)>; explicit ASyncDBConnection(); @@ -51,7 +63,7 @@ public: bool cancel(); Q_SIGNALS: - void onStateChanged(ASyncDBConnection::State state); + void onStateChanged(ASyncDBConnection::StateData state); void onNotice(Pgsql::ErrorDetails notice); private: diff --git a/pgsql/Pgsql_Connection.cpp b/pgsql/Pgsql_Connection.cpp index 1122fc8..952d127 100644 --- a/pgsql/Pgsql_Connection.cpp +++ b/pgsql/Pgsql_Connection.cpp @@ -115,19 +115,21 @@ Result Connection::queryParam(const QString &command, const Params ¶ms) } -bool Connection::sendQuery(const char *query) +void Connection::sendQuery(const char *query) { int res = PQsendQuery(conn, query); - return res == 1; + if (res == 0) + throw PgConnectionError(PQerrorMessage(conn)); } -bool Connection::sendQueryParams(const char * command, const Params ¶ms) +void Connection::sendQueryParams(const char * command, const Params ¶ms) { int res = PQsendQueryParams(conn, command, params.size(), params.types(), params.values(), params.lengths(), params.formats(), 0); // text format - return res == 1; + if (res == 0) + throw PgConnectionError(PQerrorMessage(conn)); } std::shared_ptr Connection::getResult() diff --git a/pgsql/Pgsql_Connection.h b/pgsql/Pgsql_Connection.h index eab2e7c..c417634 100644 --- a/pgsql/Pgsql_Connection.h +++ b/pgsql/Pgsql_Connection.h @@ -85,17 +85,17 @@ namespace Pgsql { Result queryParam(const char * command, const Params ¶ms); Result queryParam(const QString &command, const Params ¶ms); - bool sendQuery(const char * query); - bool sendQuery(const std::string &command) + void sendQuery(const char * query); + void sendQuery(const std::string &command) { - return sendQuery(command.c_str()); + sendQuery(command.c_str()); } - bool sendQuery(const QString &command) + void sendQuery(const QString &command) { - return sendQuery(command.toUtf8().data()); + sendQuery(command.toUtf8().data()); } - bool sendQueryParams(const char * command, const Params ¶ms); + void sendQueryParams(const char * command, const Params ¶ms); std::shared_ptr getResult(); std::shared_ptr getResultNoThrow(); diff --git a/pgsql/Pgsql_Transaction.cpp b/pgsql/Pgsql_Transaction.cpp index 89d8cde..2a5c9d9 100644 --- a/pgsql/Pgsql_Transaction.cpp +++ b/pgsql/Pgsql_Transaction.cpp @@ -84,32 +84,32 @@ namespace Pgsql { return m_conn->queryParam(command, params); } - bool Transaction::sendQuery(const char * query) + void Transaction::sendQuery(const char * query) { BOOST_ASSERT(m_conn != nullptr); BOOST_ASSERT(m_committed == false); BOOST_ASSERT(m_rolledback == false); - return m_conn->sendQuery(query); + m_conn->sendQuery(query); } - bool Transaction::sendQuery(const std::string &command) + void Transaction::sendQuery(const std::string &command) { - return sendQuery(command.c_str()); + sendQuery(command.c_str()); } - bool Transaction::sendQuery(const QString &command) + void Transaction::sendQuery(const QString &command) { - return sendQuery(command.toUtf8().data()); + sendQuery(command.toUtf8().data()); } - bool Transaction::sendQueryParams(const char * command, const Params ¶ms) + void Transaction::sendQueryParams(const char * command, const Params ¶ms) { BOOST_ASSERT(m_conn != nullptr); BOOST_ASSERT(m_committed == false); BOOST_ASSERT(m_rolledback == false); - return m_conn->sendQueryParams(command, params); + m_conn->sendQueryParams(command, params); } std::shared_ptr Transaction::getResult() diff --git a/pgsql/Pgsql_Transaction.h b/pgsql/Pgsql_Transaction.h index d6a740d..3390001 100644 --- a/pgsql/Pgsql_Transaction.h +++ b/pgsql/Pgsql_Transaction.h @@ -25,10 +25,10 @@ namespace Pgsql { Result query(const QString &command); Result queryParam(const char * command, const Params ¶ms); Result queryParam(const QString &command, const Params ¶ms); - bool sendQuery(const char * query); - bool sendQuery(const std::string &command); - bool sendQuery(const QString &command); - bool sendQueryParams(const char * command, const Params ¶ms); + void sendQuery(const char * query); + void sendQuery(const std::string &command); + void sendQuery(const QString &command); + void sendQueryParams(const char * command, const Params ¶ms); std::shared_ptr getResult(); bool consumeInput(); bool isBusy();