From a88af1ac119f307b81786692e52bcda9cf0aad9f Mon Sep 17 00:00:00 2001
From: eelke <eelke@eelkeklein.nl>
Date: Sun, 23 Dec 2018 08:43:43 +0100
Subject: [PATCH] Improved connection error handling.

Just returning a boolean is too limiting. Using expection instead to easily pass on error message.
---
 pglab/CrudModel.cpp        | 36 +++++++++++++++---------------------
 pglab/OpenDatabase.cpp     | 28 ++++++++++++++--------------
 pglab/OpenDatabase.h       |  2 +-
 pgsql/Pgsql_Connection.cpp | 14 ++++++++++----
 pgsql/Pgsql_Connection.h   |  2 +-
 5 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/pglab/CrudModel.cpp b/pglab/CrudModel.cpp
index 5871591..7fb3343 100644
--- a/pglab/CrudModel.cpp
+++ b/pglab/CrudModel.cpp
@@ -435,26 +435,23 @@ std::tuple<bool, CrudModel::ModifiedRow> CrudModel::updateRow(const PendingRow &
 		int row_number = pending_row.row();
 		Pgsql::Connection db_update_conn;
 		auto dbconfig = m_database->config();
-		bool res = db_update_conn.connect(dbconfig.getKeywords(), dbconfig.getValues(), false);
-		if (res) {
-			auto result = db_update_conn.queryParam(buffer, params);
-			if (result && result.rows() == 1) {
+		db_update_conn.connect(dbconfig.getKeywords(), dbconfig.getValues(), false);
+		auto result = db_update_conn.queryParam(buffer, params);
+		if (result && result.rows() == 1) {
 
-				std::vector<Value> values;
-				auto row = *result.begin();
-				for (auto v : row) {
-					if (v.null())
-						values.push_back(Value());
-					else
-						values.push_back(std::string(v.c_str()));
-				}
-
-				ModifiedRow modified_row(row_number, values);
-
-				return { true, modified_row };
+			std::vector<Value> values;
+			auto row = *result.begin();
+			for (auto v : row) {
+				if (v.null())
+					values.push_back(Value());
+				else
+					values.push_back(std::string(v.c_str()));
 			}
-		}
 
+			ModifiedRow modified_row(row_number, values);
+
+			return { true, modified_row };
+		}
 	}
 	return { false, {} };
 }
@@ -527,10 +524,7 @@ std::tuple<bool, QString> CrudModel::removeRows(const std::set<IntegerRange<int>
 	try {
 		Pgsql::Connection db_update_conn;
 		auto dbconfig = m_database->config();
-		bool res = db_update_conn.connect(dbconfig.getKeywords(), dbconfig.getValues(), false);
-		if (!res) {
-			return { false, "Cannot connect to the database" };
-		}
+		db_update_conn.connect(dbconfig.getKeywords(), dbconfig.getValues(), false);
 
 		// First delete rows in table
 		QString delete_statement = createDeleteStatement();
diff --git a/pglab/OpenDatabase.cpp b/pglab/OpenDatabase.cpp
index 445224a..fa349f9 100644
--- a/pglab/OpenDatabase.cpp
+++ b/pglab/OpenDatabase.cpp
@@ -5,15 +5,18 @@
 
 Expected<OpenDatabase::OpenDatabaseSPtr> OpenDatabase::createOpenDatabase(const ConnectionConfig &cfg)
 {
-	OpenDatabaseSPtr odb(new OpenDatabase(cfg));
-	if (odb->Init()) {
+	return Expected<OpenDatabase::OpenDatabaseSPtr>::fromCode(
+		[&cfg] () -> auto {
+			OpenDatabaseSPtr odb(new OpenDatabase(cfg));
+			odb->Init();
+			return odb;
+		});
+//	catch (const PgException &ex) {
+//		return
+//	}
 
-        return odb;
-
-	}
-	//return Expected<ConnectionConfig>::fromException(std::out_of_range("Invalid row"));
-	return Expected<OpenDatabaseSPtr>::fromException(
-        std::runtime_error("Failed to get database information"));
+//	return Expected<OpenDatabaseSPtr>::fromException(
+//        std::runtime_error("Failed to get database information"));
 }
 
 OpenDatabase::OpenDatabase(const ConnectionConfig& cfg)
@@ -24,16 +27,13 @@ OpenDatabase::OpenDatabase(const ConnectionConfig& cfg)
 
 OpenDatabase::~OpenDatabase() = default;
 
-bool OpenDatabase::Init()
+void OpenDatabase::Init()
 {
 	Pgsql::Connection conn;
 	auto kw = m_config.getKeywords();
 	auto vals = m_config.getValues();
-	if (conn.connect(kw, vals, 0)) {
-		m_catalog->loadAll(conn, nullptr);
-        return true;
-	}
-	return false;
+	conn.connect(kw, vals, 0);
+	m_catalog->loadAll(conn, nullptr);
 }
 
 std::shared_ptr<PgDatabaseCatalog> OpenDatabase::catalog()
diff --git a/pglab/OpenDatabase.h b/pglab/OpenDatabase.h
index 1b4e5ea..93ac6a2 100644
--- a/pglab/OpenDatabase.h
+++ b/pglab/OpenDatabase.h
@@ -34,7 +34,7 @@ private:
 	TypeSelectionItemModel *m_typeSelectionModel = nullptr;
 
 	OpenDatabase(const ConnectionConfig& cfg);
-	bool Init();
+	void Init();
 };
 
 #endif // OPENDATABASE_H
diff --git a/pgsql/Pgsql_Connection.cpp b/pgsql/Pgsql_Connection.cpp
index 58563d8..65d9d48 100644
--- a/pgsql/Pgsql_Connection.cpp
+++ b/pgsql/Pgsql_Connection.cpp
@@ -54,15 +54,21 @@ bool Connection::connect(const char *params)
     return result;
 }
 
-bool Connection::connect(const char *const * keywords, const char* const * values, int expand_dbname)
+void Connection::connect(const char *const * keywords, const char* const * values, int expand_dbname)
 {
-	bool result = false;
 	conn = PQconnectdbParams(keywords, values, expand_dbname);
+	std::string error_msg;
 	if (conn) {
 		ConnStatusType status = PQstatus(conn);
-		result = (status == CONNECTION_OK);
+		if (status == CONNECTION_OK)
+			return;
+
+		error_msg = PQerrorMessage(conn);
 	}
-	return result;
+	else {
+		error_msg = "Unknown connection failure (maybe out of memory?)";
+	}
+	throw PgException(error_msg);
 }
 
 bool Connection::connectStart(const char* params)
diff --git a/pgsql/Pgsql_Connection.h b/pgsql/Pgsql_Connection.h
index a8dd048..6f5bcc1 100644
--- a/pgsql/Pgsql_Connection.h
+++ b/pgsql/Pgsql_Connection.h
@@ -50,7 +50,7 @@ namespace Pgsql {
 			return connect(params.toUtf8().data());
 		}
 
-		bool connect(const char *const * keywords, const char* const * values, int expand_dbname);
+		void connect(const char *const * keywords, const char* const * values, int expand_dbname);
 		bool connectStart(const char *params);
 		bool connectStart(const std::string &params)
 		{