[price-quotes] Switch error handling to exceptions.

Allows for cleaner code with less state, less coupling of the GncQuotes
class, and better transfer of error messages to client code.

Also translates some error messages for presentation to users.

gnucash/gnome-utils/dialog-transfer.cpp

@@ -1785,18 +1785,19 @@ gnc_xfer_dialog_fetch (GtkButton *button, XferDialog *xferData)
 
     ENTER(" ");
 
-    GncQuotes quotes;
-    if (quotes.cmd_result() != 0)
+    try
     {
-        if (!quotes.error_msg().empty())
-            PWARN ("%s", quotes.error_msg().c_str());
-        LEAVE("quote retrieval failed");
-        return;
+        GncQuotes quotes;
+        gnc_set_busy_cursor(nullptr, TRUE);
+        quotes.fetch(xferData->book);
+        gnc_unset_busy_cursor(nullptr);
+    }
+    catch (const GncQuoteException& err)
+    {
+        gnc_unset_busy_cursor(nullptr);
+        PERR("Price retrieval failed: %s", err.what());
+        gnc_error_dialog(GTK_WINDOW(xferData->dialog), _("Price retrieval failed: %s"), err.what());
     }
-
-    gnc_set_busy_cursor (nullptr, TRUE);
-    quotes.fetch (xferData->book);
-    gnc_unset_busy_cursor (nullptr);
 
     /*the results should be in the price db now, but don't crash if not. */
     PriceReq pr;

gnucash/gnome/dialog-price-edit-db.cpp

@@ -559,19 +559,19 @@ gnc_prices_dialog_get_quotes_clicked (GtkWidget *widget, gpointer data)
     auto pdb_dialog = static_cast<PricesDialog *> (data);
 
     ENTER(" ");
-    GncQuotes quotes;
-    if (quotes.cmd_result() != 0)
+    try {
+        GncQuotes quotes;
+        gnc_set_busy_cursor (NULL, TRUE);
+        quotes.fetch (pdb_dialog->book);
+        gnc_unset_busy_cursor (NULL);
+    }
+    catch (const GncQuoteException& err)
     {
-        if (!quotes.error_msg().empty())
-            PWARN ("%s", quotes.error_msg().c_str());
-        LEAVE("quote retrieval failed");
-        return;
+        gnc_unset_busy_cursor(nullptr);
+        PERR("Price retrieval failed: %s", err.what());
+        gnc_error_dialog(GTK_WINDOW(pdb_dialog), _("Price retrieval failed: %s"), err.what());
     }
 
-    gnc_set_busy_cursor (NULL, TRUE);
-    quotes.fetch (pdb_dialog->book);
-    gnc_unset_busy_cursor (NULL);
-
     /* Without this, the summary bar on the accounts tab
      * won't reflect the new prices (bug #522095). */
     gnc_gui_refresh_all ();

gnucash/gnucash-commands.cpp

@@ -303,9 +303,9 @@ int
 Gnucash::quotes_info (void)
 {
     gnc_prefs_init ();
-    GncQuotes quotes;
-    if (quotes.cmd_result() == 0)
+    try
     {
+        GncQuotes quotes;
         std::cout << bl::format (bl::translate ("Found Finance::Quote version {1}.")) % quotes.version() << "\n";
         std::cout << bl::translate ("Finance::Quote sources: ");
         for (auto source : quotes.sources())
@@ -313,12 +313,9 @@ Gnucash::quotes_info (void)
         std::cout << std::endl;
         return 0;
     }
-    else
+    catch (const GncQuoteException& err)
     {
-        std::cerr << bl::translate ("Finance::Quote isn't "
-                                    "installed properly.") << "\n";
-        std::cerr << bl::translate ("Error message:") << "\n";
-        std::cerr << quotes.error_msg() << std::endl;
+        std::cout << err.what() << std::endl;
         return 1;
     }
 }
@@ -341,32 +338,24 @@ Gnucash::add_quotes (const bo_str& uri)
     if (qof_session_get_error(session) != ERR_BACKEND_NO_ERR)
         cleanup_and_exit_with_failure (session);
 
-    GncQuotes quotes;
-    if (quotes.cmd_result() == 0)
+    try
     {
+        GncQuotes quotes;
         std::cout << bl::format (bl::translate ("Found Finance::Quote version {1}.")) % quotes.version() << std::endl;
         auto quote_sources = quotes.sources_as_glist();
         gnc_quote_source_set_fq_installed (quotes.version().c_str(), quote_sources);
         g_list_free_full (quote_sources, g_free);
+        quotes.fetch(qof_session_get_book(session));
     }
-    else
+    catch (const GncQuoteException& err)
     {
-        std::cerr << bl::translate ("No quotes retrieved. Finance::Quote isn't "
-                                    "installed properly.") << "\n";
-        std::cerr << bl::translate ("Error message:") << std::endl;
-        std::cerr << quotes.error_msg() << std::endl;
+        std::cerr << err.what() << std::endl;
     }
-    quotes.fetch (qof_session_get_book(session));
-
     qof_session_save(session, NULL);
     if (qof_session_get_error(session) != ERR_BACKEND_NO_ERR)
         cleanup_and_exit_with_failure (session);
 
     qof_session_destroy(session);
-
-    if (quotes.cmd_result() != 0)
-        std::cerr << bl::format (bl::translate ("Failed to add quotes to {1}.")) % *uri << "\n";
-
     qof_event_resume();
     return 0;
 }

gnucash/gnucash.cpp

@@ -174,25 +174,25 @@ scm_run_gnucash (void *data, [[maybe_unused]] int argc, [[maybe_unused]] char **
     gnc_hook_add_dangler(HOOK_UI_SHUTDOWN, (GFunc)gnc_file_quit, NULL, NULL);
 
     /* Install Price Quote Sources */
-    auto msg = bl::translate ("Checking Finance::Quote...").str(gnc_get_boost_locale());
 
-    GncQuotes quotes;
-    if (quotes.cmd_result() == 0)
+    try
     {
-        msg = (bl::format (bl::translate("Found Finance::Quote version {1}.")) % quotes.version()).str(gnc_get_boost_locale());
+        auto msg = bl::translate ("Checking Finance::Quote...").str(gnc_get_boost_locale());
+        GncQuotes quotes;
+            msg = (bl::format (bl::translate("Found Finance::Quote version {1}.")) % quotes.version()).str(gnc_get_boost_locale());
         auto quote_sources = quotes.sources_as_glist();
         gnc_quote_source_set_fq_installed (quotes.version().c_str(), quote_sources);
         g_list_free (quote_sources);
+        gnc_update_splash_screen (msg.c_str(), GNC_SPLASH_PERCENTAGE_UNKNOWN);
     }
-    else
+    catch (const GncQuoteException& err)
     {
-        msg = bl::translate("Unable to load Finance::Quote.").str(gnc_get_boost_locale());
+        auto msg = bl::translate("Unable to load Finance::Quote.").str(gnc_get_boost_locale());
         PINFO ("Attempt to load Finance::Quote returned this error message:\n");
-        PINFO ("%s", quotes.error_msg().c_str());
+        PINFO ("%s", err.what());
+        gnc_update_splash_screen (msg.c_str(), GNC_SPLASH_PERCENTAGE_UNKNOWN);
     }
 
-    gnc_update_splash_screen (msg.c_str(), GNC_SPLASH_PERCENTAGE_UNKNOWN);
-
     gnc_hook_run(HOOK_STARTUP, NULL);
 
     if (!user_file_spec->nofile && (fn = get_file_to_load (user_file_spec->file_to_load)) && *fn )

libgnucash/app-utils/CMakeLists.txt

@@ -50,6 +50,7 @@ set(app_utils_ALL_LIBRARIES
     gnc-engine
     ${Boost_FILESYSTEM_LIBRARY}
     ${Boost_PROPERTY_TREE_LIBRARY}
+    ${Boost_LOCALE_LIBRARY}
     ${GIO_LDFLAGS}
     ${LIBXML2_LDFLAGS}
     ${LIBXSLT_LDFLAGS}

libgnucash/app-utils/gnc-quotes.cpp

@@ -23,6 +23,7 @@
 #include <config.h>
 
 #include <algorithm>
+#include <stdexcept>
 #include <vector>
 #include <string>
 #include <iostream>
@@ -34,6 +35,7 @@
 #include <boost/property_tree/json_parser.hpp>
 #include <boost/iostreams/device/array.hpp>
 #include <boost/iostreams/stream_buffer.hpp>
+#include <boost/locale.hpp>
 #include <boost/asio.hpp>
 #include <glib.h>
 #include "gnc-commodity.hpp"
@@ -53,6 +55,7 @@ extern "C" {
 
 static const QofLogModule log_module = "gnc.price-quotes";
 
+namespace bl = boost::locale;
 namespace bp = boost::process;
 namespace bfs = boost::filesystem;
 namespace bpt = boost::property_tree;
@@ -60,6 +63,11 @@ namespace bio = boost::iostreams;
 
 using QuoteResult = std::tuple<int, StrVec, StrVec>;
 
+struct GncQuoteSourceError : public std::runtime_error
+{
+    GncQuoteSourceError(const std::string& err) : std::runtime_error(err) {}
+};
+
 CommVec
 gnc_quotes_get_quotable_commodities(const gnc_commodity_table * table);
 
@@ -85,8 +93,6 @@ public:
     void fetch (CommVec& commodities);
     void fetch (gnc_commodity *comm);
 
-    int cmd_result() const noexcept { return m_cmd_result; }
-    const std::string& error_msg() noexcept { return m_error_msg; }
     const std::string& version() noexcept { return m_version.empty() ? not_found : m_version; }
     const QuoteSources& sources() noexcept { return m_sources; }
     GList* sources_as_glist ();
@@ -101,8 +107,6 @@ private:
     CommVec m_comm_vec;
     std::string m_version;
     QuoteSources m_sources;
-    int m_cmd_result;
-    std::string m_error_msg;
     std::string m_fq_answer;
     QofBook *m_book;
     gnc_commodity *m_dflt_curr;
@@ -138,21 +142,25 @@ m_version{}, m_sources{}
     auto [rv, sources, errors] = run_cmd(args, empty_string);
     if (rv)
     {
-        PERR("Failed to initialize Finance::Quote %s", errors.front().c_str());
-        return;
+        std::string err{bl::translate("Failed to initialize Finance::Quote: ")};
+        for (auto err_line : errors)
+            err += err_line.empty() ? "" : err_line + "\n";
+        throw(GncQuoteSourceError(err));
     }
     if (!errors.empty())
     {
-        for(const auto& err : errors)
-            PERR("Finance::Quote check returned error %s", err.empty() ? "" : err.c_str());
-        return;
+        std::string err{bl::translate("Finance::Quote check returned error ")};
+        for(const auto& err_line : errors)
+            err += err.empty() ? "" : err_line + "\n";
+        throw(GncQuoteSourceError(err));
     }
     static const boost::regex version_fmt{"[0-9]\\.[0-9][0-9]"};
     auto version{sources.front()};
     if (version.empty() || !boost::regex_match(version, version_fmt))
     {
-        PERR("Invalid Finance::Quote Version %s", version.empty() ? "" : version.c_str());
-        return;
+        std::string err{bl::translate("Invalid Finance::Quote Version ")};
+            err +=  version.empty() ? "" : version;
+        throw(GncQuoteSourceError(err));
     }
     m_ready = true;
     sources.erase(sources.begin());
@@ -221,7 +229,7 @@ GncFQQuoteSource::run_cmd (const StrVec& args, const std::string& json_string) c
 
 /* GncQuotes implementation */
 GncQuotesImpl::GncQuotesImpl() : m_quotesource{new GncFQQuoteSource},
-m_version{}, m_sources{}, m_cmd_result{}, m_error_msg{}, m_book{qof_session_get_book(gnc_get_current_session())},
+m_version{}, m_sources{}, m_book{qof_session_get_book(gnc_get_current_session())},
 m_dflt_curr{gnc_default_currency()}
 {
     if (!m_quotesource->usable())
@@ -230,7 +238,7 @@ m_dflt_curr{gnc_default_currency()}
 }
 
 GncQuotesImpl::GncQuotesImpl(QofBook* book) : m_quotesource{new GncFQQuoteSource},
-m_version{}, m_sources{}, m_cmd_result{}, m_error_msg{}, m_book{book},
+m_version{}, m_sources{}, m_book{book},
 m_dflt_curr{gnc_default_currency()}
 {
     if (!m_quotesource->usable())
@@ -240,8 +248,7 @@ m_dflt_curr{gnc_default_currency()}
 
 GncQuotesImpl::GncQuotesImpl(QofBook* book, std::unique_ptr<GncQuoteSource> quote_source) :
 m_quotesource{std::move(quote_source)},
-m_version{}, m_sources{}, m_cmd_result{}, m_error_msg{}, m_book{book},
-m_dflt_curr{gnc_default_currency()}
+m_version{}, m_sources{}, m_book{book}, m_dflt_curr{gnc_default_currency()}
 {
     if (!m_quotesource->usable())
         return;
@@ -262,12 +269,7 @@ void
 GncQuotesImpl::fetch (QofBook *book)
 {
     if (!book)
-    {
-        m_cmd_result = 1;
-        m_error_msg = _("No book set");
-        m_error_msg += "\n";
-        return;
-    }
+        throw (GncQuoteException(bl::translate("GncQuotes::Fetch called with no book.")));
     auto commodities = gnc_quotes_get_quotable_commodities (
         gnc_commodity_table_get_table (book));
     fetch (commodities);
@@ -290,8 +292,7 @@ GncQuotesImpl::fetch (CommVec& commodities)
     m_book = qof_instance_get_book (m_comm_vec[0]);
 
     query_fq ();
-    if (m_cmd_result == 0)
-        parse_quotes ();
+    parse_quotes ();
 }
 
 static const std::vector <std::string>
@@ -336,14 +337,19 @@ GncQuotesImpl::query_fq (void)
     auto json_str{comm_vec_to_json_string()};
     auto [rv, quotes, errors] = m_quotesource->get_quotes(json_str);
     m_fq_answer.clear();
-    m_cmd_result = rv;
+
     if (rv == 0)
+    {
         for (auto line : quotes)
             m_fq_answer.append(line + "\n");
+    }
     else
-        for (auto line : errors)
-            m_error_msg.append(line + "\n");
-
+    {
+        std::string err_str;
+        for (auto line: errors)
+            err_str.append(line + "\n");
+        throw(GncQuoteException(err_str));
+    }
 //        for (auto line : quotes)
 //            PINFO("Output line retrieved from wrapper:\n%s", line.c_str());
 //
@@ -546,24 +552,35 @@ GncQuotesImpl::parse_quotes (void)
 {
     bpt::ptree pt;
     std::istringstream ss {m_fq_answer};
+    const char* what = nullptr;
 
     try
     {
         bpt::read_json (ss, pt);
     }
     catch (bpt::json_parser_error &e) {
-        m_cmd_result = -1;
-        m_error_msg = m_error_msg +
-                      _("Failed to parse result returned by Finance::Quote.") + "\n" +
-                      _("Error message:") + "\n" +
-                       e.what() + "\n";
-        return;
+        what = e.what();
+    }
+    catch (const std::runtime_error& e)
+    {
+        what = e.what();
+    }
+    catch (const std::logic_error& e)
+    {
+        what = e.what();
     }
     catch (...) {
-        m_cmd_result = -1;
-        m_error_msg = m_error_msg +
-                      _("Failed to parse result returned by Finance::Quote.") + "\n";
-        return;
+        std::string error_msg{_("Failed to parse result returned by Finance::Quote.")};
+        throw(GncQuoteException(error_msg));
+    }
+    if (what)
+    {
+        std::string error_msg{_("Failed to parse result returned by Finance::Quote.")};
+        error_msg += "\n";
+        error_msg += _("Error message:");
+        error_msg += "\n";
+        error_msg += what;
+        throw(GncQuoteException(error_msg));
     }
 
     auto pricedb{gnc_pricedb_get_db(m_book)};
@@ -672,9 +689,17 @@ gnc_quotes_get_quotable_commodities (const gnc_commodity_table * table)
 // Constructor - checks for presence of Finance::Quote and import version and quote sources
 GncQuotes::GncQuotes ()
 {
-    m_impl = std::make_unique<GncQuotesImpl> ();
+    try
+    {
+        m_impl = std::make_unique<GncQuotesImpl>();
+    }
+    catch (const GncQuoteSourceError& err)
+    {
+        throw(GncQuoteException(err.what()));
+    }
 }
 
+
 void
 GncQuotes::fetch (QofBook *book)
 {
@@ -691,16 +716,6 @@ void GncQuotes::fetch (gnc_commodity *comm)
     m_impl->fetch (comm);
 }
 
-const int GncQuotes::cmd_result() noexcept
-{
-    return m_impl->cmd_result ();
-}
-
-const std::string& GncQuotes::error_msg() noexcept
-{
-    return m_impl->error_msg ();
-}
-
 const std::string& GncQuotes::version() noexcept
 {
     return m_impl->version ();

libgnucash/app-utils/gnc-quotes.hpp

@@ -27,6 +27,7 @@
 #include <vector>
 #include <gnc-commodity.hpp>  // For CommVec alias
 #include <glib.h>
+#include <stdexcept>
 
 extern  "C" {
 #include <qofbook.h>
@@ -36,6 +37,11 @@ using StrVec = std::vector  <std::string>;
 using QuoteSources = StrVec;
 using CmdOutput = std::pair <StrVec, StrVec>;
 
+struct GncQuoteException : public std::runtime_error
+{
+    GncQuoteException(const std::string& msg) : std::runtime_error(msg) {}
+};
+
 const std::string not_found = std::string ("Not Found");
 
 class GncQuotesImpl;
@@ -54,8 +60,6 @@ public:
     // Fetch quote for the commodity if it has a quote source  set
     void fetch (gnc_commodity *comm);
 
-    const int cmd_result() noexcept;
-    const std::string& error_msg() noexcept;
     const std::string& version() noexcept;
     const QuoteSources& sources() noexcept;
     GList* sources_as_glist ();

libgnucash/app-utils/test/CMakeLists.txt

@@ -44,6 +44,7 @@ set(test_gnc_quotes_LIBS
         gnc-engine
         gtest
         ${Boost_FILESYSTEM_LIBRARY}
+        ${Boost_LOCALE_LIBRARY}
         ${Boost_PROPERTY_TREE_LIBRARY}
         ${Boost_SYSTEM_LIBRARY}
         )