Properly catch and handle database errors

Do not use a singleton for the database.

fix #3203

louloulibs/utils/reload.cpp

@@ -1,11 +1,28 @@
+#include <utils/reload.hpp>
+#include <database/database.hpp>
 #include <config/config.hpp>
+#include <utils/xdg.hpp>
 #include <logger/logger.hpp>
 
+void open_database()
+{
+  const auto db_filename = Config::get("db_name", xdg_data_path("biboumi.sqlite"));
+  log_info("Opening database: ", db_filename);
+  Database::open(db_filename);
+  log_info("database successfully opened.");
+}
+
 void reload_process()
 {
   Config::read_conf();
   // Destroy the logger instance, to be recreated the next time a log
   // line needs to be written
   Logger::instance().reset();
-  log_debug("Configuration and logger reloaded.");
+  log_info("Configuration and logger reloaded.");
+  try {
+      open_database();
+    } catch (const litesql::DatabaseError&) {
+      log_warning("Re-using the previous database.");
     }
+}
+

louloulibs/utils/reload.hpp

@@ -1,10 +1,4 @@
 #pragma once
 
-
+void open_database();
-/**
- * Reload the server's configuration, and close the logger (so that it
- * closes its files etc, to take into account the new configuration)
- */
 void reload_process();
-
-

src/database/database.cpp

@@ -2,8 +2,6 @@
 #ifdef USE_DATABASE
 
 #include <database/database.hpp>
-#include <config/config.hpp>
-#include <utils/xdg.hpp>
 #include <logger/logger.hpp>
 #include <string>
 
@@ -11,37 +9,36 @@ using namespace std::string_literals;
 
 std::unique_ptr<db::BibouDB> Database::db;
 
-db::BibouDB& Database::get_db()
+void Database::open(const std::string& filename, const std::string& db_type)
 {
-  if (!Database::db)
+  try
     {
-      const std::string db_filename = Config::get("db_name",
+      auto new_db = std::make_unique<db::BibouDB>(db_type,
-                                                  xdg_data_path("biboumi.sqlite"));
+                                             "database="s + filename);
-      Database::db = std::make_unique<db::BibouDB>("sqlite3",
+      if (new_db->needsUpgrade())
-                                                   "database="s + db_filename);
+        new_db->upgrade();
+      Database::db.reset(new_db.release());
+    } catch (const litesql::DatabaseError& e) {
+      log_error("Failed to open database ", filename, ". ", e.what());
+      throw;
     }
-
-  if (Database::db->needsUpgrade())
-    Database::db->upgrade();
-
-  return *Database::db.get();
 }
 
 void Database::set_verbose(const bool val)
 {
-  Database::get_db().verbose = val;
+  Database::db->verbose = val;
 }
 
 db::IrcServerOptions Database::get_irc_server_options(const std::string& owner,
                                                       const std::string& server)
 {
   try {
-    auto options = litesql::select<db::IrcServerOptions>(Database::get_db(),
+    auto options = litesql::select<db::IrcServerOptions>(*Database::db,
                              db::IrcServerOptions::Owner == owner &&
                              db::IrcServerOptions::Server == server).one();
     return options;
   } catch (const litesql::NotFound& e) {
-    db::IrcServerOptions options(Database::get_db());
+    db::IrcServerOptions options(*Database::db);
     options.owner = owner;
     options.server = server;
     // options.update();
@@ -54,13 +51,13 @@ db::IrcChannelOptions Database::get_irc_channel_options(const std::string& owner
                                                         const std::string& channel)
 {
   try {
-    auto options = litesql::select<db::IrcChannelOptions>(Database::get_db(),
+    auto options = litesql::select<db::IrcChannelOptions>(*Database::db,
                                                          db::IrcChannelOptions::Owner == owner &&
                                                          db::IrcChannelOptions::Server == server &&
                                                          db::IrcChannelOptions::Channel == channel).one();
     return options;
   } catch (const litesql::NotFound& e) {
-    db::IrcChannelOptions options(Database::get_db());
+    db::IrcChannelOptions options(*Database::db);
     options.owner = owner;
     options.server = server;
     options.channel = channel;

src/database/database.hpp

@@ -26,7 +26,7 @@ public:
   template<typename PersistentType>
   static size_t count()
   {
-    return litesql::select<PersistentType>(Database::get_db()).count();
+    return litesql::select<PersistentType>(*Database::db).count();
   }
   /**
    * Return the object from the db. Create it beforehand (with all default
@@ -42,11 +42,11 @@ public:
                                                                            const std::string& channel);
 
   static void close();
+  static void open(const std::string& filename, const std::string& db_type="sqlite3");
+
 
 private:
   static std::unique_ptr<db::BibouDB> db;
-
-  static db::BibouDB& get_db();
 };
 #endif /* USE_DATABASE */
 

src/main.cpp

@@ -3,22 +3,15 @@
 #include <network/poller.hpp>
 #include <config/config.hpp>
 #include <logger/logger.hpp>
-#include <utils/reload.hpp>
 #include <utils/xdg.hpp>
-
+#include <utils/reload.hpp>
-#include <iostream>
-#include <memory>
-#include <atomic>
-
-#include <signal.h>
 
 #ifdef CARES_FOUND
 # include <network/dns_handler.hpp>
 #endif
 
-#ifdef SYSTEMD_FOUND
+#include <atomic>
-# include <systemd/sd-daemon.h>
+#include <signal.h>
-#endif
 
 // A flag set by the SIGINT signal handler.
 static volatile std::atomic<bool> stop(false);
@@ -71,6 +64,12 @@ int main(int ac, char** av)
   if (hostname.empty())
     return config_help("hostname");
 
+  try {
+      open_database();
+    } catch (...) {
+      return 1;
+    }
+
   // Block the signals we want to manage. They will be unblocked only during
   // the epoll_pwait or ppoll calls. This avoids some race conditions,
   // explained in man 2 pselect on linux

tests/database.cpp

@@ -7,7 +7,7 @@
 TEST_CASE("Database")
 {
 #ifdef USE_DATABASE
-  Config::set("db_name", ":memory:");
+  Database::open(":memory:");
   Database::set_verbose(false);
 
   SECTION("Basic retrieve and update")