From 80272e81c356c3e30139adcba33f2a8c15cf0aa4 Mon Sep 17 00:00:00 2001 From: eelke Date: Wed, 17 Aug 2022 18:18:10 +0200 Subject: [PATCH] Improve error handling --- pglab/catalog/models/TablesTableModel.cpp | 5 +- pglab/querytool/QueryTool.cpp | 136 ++++++++++------------ pglab/querytool/QueryTool.h | 2 +- pglablib/ASyncDBConnection.cpp | 7 +- pgsql/Pgsql_Connection.cpp | 2 +- 5 files changed, 68 insertions(+), 84 deletions(-) diff --git a/pglab/catalog/models/TablesTableModel.cpp b/pglab/catalog/models/TablesTableModel.cpp index 0eb41a0..4a07b9f 100644 --- a/pglab/catalog/models/TablesTableModel.cpp +++ b/pglab/catalog/models/TablesTableModel.cpp @@ -166,7 +166,8 @@ QVariant TablesTableModel::data(const QModelIndex &index, int role) const return getData(index); else if (role == CustomDataTypeRole) return getType(index.column()); - else if (role == CustomDataMeaningRole) { + else if (role == CustomDataMeaningRole) + { switch (index.column()) { case TotalSizeCol: case TableSizeCol: @@ -191,9 +192,7 @@ void TablesTableModel::StartLoadTableSizes(std::map oidIndex) .then(qApp, [p, oidIndex] (TableSizes sizes) { if (p) - { p.data()->PopulateSizes(std::move(oidIndex), std::move(sizes)); - } }); } diff --git a/pglab/querytool/QueryTool.cpp b/pglab/querytool/QueryTool.cpp index 1133eb1..f9addcb 100644 --- a/pglab/querytool/QueryTool.cpp +++ b/pglab/querytool/QueryTool.cpp @@ -16,6 +16,7 @@ #include "json/json.h" #include "OpenDatabase.h" #include "catalog/PgDatabaseCatalog.h" +#include "Pgsql_PgException.h" #include "QueryParamListController.h" #include "util.h" #include "UserConfiguration.h" @@ -155,16 +156,23 @@ void QueryTool::execute() auto cb = [this](Expected> res, qint64 elapsedms) { - if (res.valid()) { - auto && dbresult = res.get(); - QMetaObject::invokeMethod(this, "query_ready", - Q_ARG(std::shared_ptr, dbresult), - Q_ARG(qint64, elapsedms)); - } - else { - /// \todo handle error + try + { + auto dbres = res.get(); + QMetaObject::invokeMethod(this, "query_ready", + Q_ARG(std::shared_ptr, dbres), + Q_ARG(qint64, elapsedms)); + } + catch (Pgsql::PgException &ex) + { + //QMessageBox::critical(this, "pglab", QString::fromStdString(ex.what())); + QMetaObject::invokeMethod(this, [this, ex] () + { + QMessageBox::critical(this, "pglab", QString::fromStdString(ex.what())); + }); + } + - } }; try { @@ -454,78 +462,52 @@ std::string QueryTool::getCommandUtf8() const void QueryTool::query_ready(std::shared_ptr dbres, qint64 elapsedms) { - if (dbres) { - addLog("query_ready with result"); - auto st = dbres->resultStatus(); - if (st == PGRES_TUPLES_OK) { - //int n_rows = dbres->getRows(); - //QString rowcount_str = QString("rows: %1").arg(dbres->getRows()); + if (!dbres) + { + m_stopwatch.stop(); + addLog("query_ready with NO result"); + return; + } - auto result_model = std::make_shared(nullptr , dbres, - m_catalog); - TuplesResultWidget *trw = new TuplesResultWidget; - trw->setResult(result_model, elapsedms); - resultList.push_back(trw); - ui->tabWidget->addTab(trw, "Data"); - if (resultList.size() == 1) - ui->tabWidget->setCurrentWidget(trw); + addLog("query_ready with result"); + auto st = dbres->resultStatus(); + if (st == PGRES_TUPLES_OK) { + //int n_rows = dbres->getRows(); + //QString rowcount_str = QString("rows: %1").arg(dbres->getRows()); - } - else { - if (st == PGRES_COMMAND_OK) { - int tuples_affected = dbres->tuplesAffected(); - QString msg; - if (tuples_affected >= 0) - msg = tr("Query returned succesfully: %1 rows affected, execution time %2") - .arg(QString::number(tuples_affected)) - .arg(msfloatToHumanReadableString(elapsedms)); - else - msg = tr("Query returned succesfully, execution time %1") - .arg(msfloatToHumanReadableString(elapsedms)); + auto result_model = std::make_shared(nullptr , dbres, + m_catalog); + TuplesResultWidget *trw = new TuplesResultWidget; + trw->setResult(result_model, elapsedms); + resultList.push_back(trw); + ui->tabWidget->addTab(trw, "Data"); + if (resultList.size() == 1) + ui->tabWidget->setCurrentWidget(trw); - ui->messagesEdit->append(msg); + } + else { + if (st == PGRES_COMMAND_OK) { + int tuples_affected = dbres->tuplesAffected(); + QString msg; + if (tuples_affected >= 0) + msg = tr("Query returned succesfully: %1 rows affected, execution time %2") + .arg(QString::number(tuples_affected)) + .arg(msfloatToHumanReadableString(elapsedms)); + else + msg = tr("Query returned succesfully, execution time %1") + .arg(msfloatToHumanReadableString(elapsedms)); - ui->tabWidget->setCurrentWidget(ui->messageTab); - } - else { -// if (st == PGRES_EMPTY_QUERY) { -// statusBar()->showMessage(tr("Empty query.")); -// } -// else if (st == PGRES_COPY_OUT) { -// statusBar()->showMessage(tr("COPY OUT.")); -// } -// else if (st == PGRES_COPY_IN) { -// statusBar()->showMessage(tr("COPY IN.")); -// } -// else if (st == PGRES_BAD_RESPONSE) { -// statusBar()->showMessage(tr("BAD RESPONSE.")); -// } -// else if (st == PGRES_NONFATAL_ERROR) { -// statusBar()->showMessage(tr("NON FATAL ERROR.")); -// } -// else if (st == PGRES_FATAL_ERROR) { -// statusBar()->showMessage(tr("FATAL ERROR.")); -// } -// else if (st == PGRES_COPY_BOTH) { -// statusBar()->showMessage(tr("COPY BOTH shouldn't happen is for replication.")); -// } -// else if (st == PGRES_SINGLE_TUPLE) { -// statusBar()->showMessage(tr("SINGLE TUPLE result.")); -// } -// else { -// statusBar()->showMessage(tr("No tuples returned, possibly an error...")); -// } - ui->tabWidget->setCurrentWidget(ui->messageTab); - auto details = dbres->diagDetails(); - markError(details); - receiveNotice(details); - } - } - } - else { - m_stopwatch.stop(); - addLog("query_ready with NO result"); - } + ui->messagesEdit->append(msg); + + ui->tabWidget->setCurrentWidget(ui->messageTab); + } + else { + ui->tabWidget->setCurrentWidget(ui->messageTab); + auto details = dbres->diagDetails(); + markError(details); + receiveNotice(details); + } + } } void QueryTool::markError(const Pgsql::ErrorDetails &details) diff --git a/pglab/querytool/QueryTool.h b/pglab/querytool/QueryTool.h index 2c99cc4..35b833b 100644 --- a/pglab/querytool/QueryTool.h +++ b/pglab/querytool/QueryTool.h @@ -110,7 +110,7 @@ private: private slots: void explain_ready(ExplainRoot::SPtr explain); - void query_ready(std::shared_ptr, qint64 elapsedms); + void query_ready(std::shared_ptr, qint64 elapsedms); void queryTextChanged(); void connectionStateChanged(ASyncDBConnection::StateData state); diff --git a/pglablib/ASyncDBConnection.cpp b/pglablib/ASyncDBConnection.cpp index da76ba0..396b345 100644 --- a/pglablib/ASyncDBConnection.cpp +++ b/pglablib/ASyncDBConnection.cpp @@ -427,7 +427,7 @@ void ASyncDBConnection::closeConnection() bool ASyncDBConnection::send(const std::string &command, on_result_callback on_result) { - if (command.empty()) + if (command.empty() || state() == State::NotConnected) return false; { @@ -440,7 +440,10 @@ bool ASyncDBConnection::send(const std::string &command, on_result_callback on_r bool ASyncDBConnection::send(const std::string &command, Pgsql::Params params, on_result_callback on_result) { - { + if (command.empty() || state() == State::NotConnected) + return false; + + { std::lock_guard lg(m_threadData->m_commandQueue.m_mutex); m_threadData->m_commandQueue.m_queue.emplace(command, std::move(params), on_result); m_threadData->m_commandQueue.m_newEvent.set(); diff --git a/pgsql/Pgsql_Connection.cpp b/pgsql/Pgsql_Connection.cpp index 952d127..6e82f41 100644 --- a/pgsql/Pgsql_Connection.cpp +++ b/pgsql/Pgsql_Connection.cpp @@ -246,7 +246,7 @@ void Connection::throwError(PGresult *result) const { auto state = PQresultStatus(result); if (state == PGRES_BAD_RESPONSE) - ; // communication problem + throw PgException("Communication issue"); else if (state == PGRES_FATAL_ERROR) { auto details = Pgsql::ErrorDetails::createErrorDetailsFromPGresult(result);