Changed error handling in TransferStatus, then-callbacks are now always called and...

diff --git a/src/addon/addon_manager.cpp b/src/addon/addon_manager.cpp
index e6c1b0a..a624bc5 100644 (file)
--- a/src/addon/addon_manager.cpp
+++ b/src/addon/addon_manager.cpp
@@ -221,11 +221,16 @@ AddonManager::request_check_online()
   {
     m_transfer_status = m_downloader.request_download(m_repository_url, "/addons/repository.nfo");
 
-    m_transfer_status->then([this]{
-        m_repository_addons = parse_addon_infos("/addons/repository.nfo");
-        m_has_been_updated = true;
-
+    m_transfer_status->then(
+      [this](bool success)
+      {
         m_transfer_status = {};
+
+        if (success)
+        {
+          m_repository_addons = parse_addon_infos("/addons/repository.nfo");
+          m_has_been_updated = true;
+        }
       });
 
     return m_transfer_status;
@@ -277,35 +282,38 @@ AddonManager::request_install_addon(const AddonId& addon_id)
     m_transfer_status = m_downloader.request_download(addon.get_url(), install_filename);
 
     m_transfer_status->then(
-      [this, install_filename, addon_id]
+      [this, install_filename, addon_id](bool success)
       {
-        // complete the addon install
-        Addon& repository_addon = get_repository_addon(addon_id);
+        m_transfer_status = {};
 
-        MD5 md5 = md5_from_file(install_filename);
-        if (repository_addon.get_md5() != md5.hex_digest())
+        if (success)
         {
-          if (PHYSFS_delete(install_filename.c_str()) == 0)
-          {
-            log_warning << "PHYSFS_delete failed: " << PHYSFS_getLastError() << std::endl;
-          }
+          // complete the addon install
+          Addon& repository_addon = get_repository_addon(addon_id);
 
-          throw std::runtime_error("Downloading Add-on failed: MD5 checksums differ");
-        }
-        else
-        {
-          const char* realdir = PHYSFS_getRealDir(install_filename.c_str());
-          if (!realdir)
+          MD5 md5 = md5_from_file(install_filename);
+          if (repository_addon.get_md5() != md5.hex_digest())
           {
-            throw std::runtime_error("PHYSFS_getRealDir failed: " + install_filename);
+            if (PHYSFS_delete(install_filename.c_str()) == 0)
+            {
+              log_warning << "PHYSFS_delete failed: " << PHYSFS_getLastError() << std::endl;
+            }
+
+            throw std::runtime_error("Downloading Add-on failed: MD5 checksums differ");
           }
           else
           {
-            add_installed_archive(install_filename, md5.hex_digest());
+            const char* realdir = PHYSFS_getRealDir(install_filename.c_str());
+            if (!realdir)
+            {
+              throw std::runtime_error("PHYSFS_getRealDir failed: " + install_filename);
+            }
+            else
+            {
+              add_installed_archive(install_filename, md5.hex_digest());
+            }
           }
         }
-
-        m_transfer_status = {};
       });
 
     return m_transfer_status;

diff --git a/src/addon/downloader.cpp b/src/addon/downloader.cpp
index fd8320b..b923813 100644 (file)
--- a/src/addon/downloader.cpp
+++ b/src/addon/downloader.cpp
@@ -48,6 +48,12 @@ size_t my_curl_physfs_write(void* ptr, size_t size, size_t nmemb, void* userdata
 
 } // namespace
 
+void
+TransferStatus::update()
+{
+  m_downloader.update();
+}
+
 class Transfer
 {
 private:
@@ -69,8 +75,8 @@ public:
     m_id(id),
     m_url(url),
     m_handle(),
-    m_error_buffer(),
-    m_status(new TransferStatus(id)),
+    m_error_buffer({'\0'}),
+    m_status(new TransferStatus(downloader, id)),
     m_fout(PHYSFS_openWrite(outfile.c_str()), PHYSFS_close)
   {
     if (!m_fout)
@@ -114,6 +120,11 @@ public:
     return m_status;
   }
 
+  const char* get_error_buffer() const
+  {
+    return m_error_buffer.data();
+  }
+
   TransferId get_id() const
   {
     return m_id;
@@ -272,19 +283,49 @@ Downloader::update()
     {
       case CURLMSG_DONE:
         {
-          log_info << "Download completed" << std::endl;
+          log_info << "Download completed with " << msg->data.result << std::endl;
           curl_multi_remove_handle(m_multi_handle, msg->easy_handle);
+
           auto it = std::find_if(m_transfers.begin(), m_transfers.end(),
                                  [&msg](const std::unique_ptr<Transfer>& rhs) {
                                    return rhs->get_curl_handle() == msg->easy_handle;
                                  });
           assert(it != m_transfers.end());
           TransferStatusPtr status = (*it)->get_status();
+          status->error_msg = (*it)->get_error_buffer();
           m_transfers.erase(it);
 
-          for(auto& callback : status->callbacks)
+          if (msg->data.result == CURLE_OK)
+          {
+            bool success = true;
+            for(auto& callback : status->callbacks)
+            {
+              try
+              {
+                callback(success);
+              }
+              catch(const std::exception& err)
+              {
+                success = false;
+                log_warning << "Exception in Downloader: " << err.what() << std::endl;
+                status->error_msg = err.what();
+              }
+            }
+          }
+          else
           {
-            callback();
+            log_warning << "Error: " << curl_easy_strerror(msg->data.result) << std::endl;
+            for(auto& callback : status->callbacks)
+            {
+              try
+              {
+                callback(false);
+              }
+              catch(const std::exception& err)
+              {
+                log_warning << "Illegal exception in Downloader: " << err.what() << std::endl;
+              }
+            }
           }
         }
         break;

diff --git a/src/addon/downloader.hpp b/src/addon/downloader.hpp
index de11557..c75c7e1 100644 (file)
--- a/src/addon/downloader.hpp
+++ b/src/addon/downloader.hpp
@@ -26,31 +26,36 @@
 #include <vector>
 
 typedef int TransferId;
+class Downloader;
 
 class TransferStatus
 {
 public:
-  enum Status { RUNNING, COMPLETED, ABORT, ERROR };
-
-public:
+  Downloader& m_downloader;
   TransferId id;
-  std::vector<std::function<void ()> > callbacks;
+  std::vector<std::function<void (bool)> > callbacks;
 
   int dltotal;
   int dlnow;
   int ultotal;
   int ulnow;
 
-  TransferStatus(TransferId id_) :
+  std::string error_msg;
+
+  TransferStatus(Downloader& downloader, TransferId id_) :
+    m_downloader(downloader),
     id(id_),
     callbacks(),
     dltotal(0),
     dlnow(0),
     ultotal(0),
-    ulnow(0)
+    ulnow(0),
+    error_msg()
   {}
 
-  void then(const std::function<void ()>& callback)
+  void update();
+
+  void then(const std::function<void (bool)>& callback)
   {
     callbacks.push_back(callback);
   }

diff --git a/src/gui/menu_manager.cpp b/src/gui/menu_manager.cpp
index 009a10a..32b6178 100644 (file)
--- a/src/gui/menu_manager.cpp
+++ b/src/gui/menu_manager.cpp
@@ -208,17 +208,7 @@ MenuManager::draw(DrawingContext& context)
   {
     if (m_dialog)
     {
-      try
-      {
-        m_dialog->update();
-      }
-      catch(const std::exception& err)
-      {
-        m_dialog = std::unique_ptr<Dialog>(new Dialog);
-        m_dialog->set_text(_("Error:\n") + err.what());
-        m_dialog->add_button(_("Ok"));
-      }
-
+      m_dialog->update();
       m_dialog->draw(context);
     }
     else if (current_menu())

diff --git a/src/supertux/menu/addon_menu.cpp b/src/supertux/menu/addon_menu.cpp
index b9ee6c3..13f8e0a 100644 (file)
--- a/src/supertux/menu/addon_menu.cpp
+++ b/src/supertux/menu/addon_menu.cpp
@@ -185,8 +185,13 @@ AddonMenu::menu_action(MenuItem* item)
     try
     {
       TransferStatusPtr status = m_addon_manager.request_check_online();
-      status->then([this]{
-          refresh();
+      status->then(
+        [this](bool success)
+        {
+          if (success)
+          {
+            refresh();
+          }
         });
       std::unique_ptr<DownloadDialog> dialog(new DownloadDialog(status));
       dialog->set_title("Downloading Add-On Repository Index");
@@ -226,16 +231,21 @@ AddonMenu::menu_action(MenuItem* item)
         auto addon_id = addon.get_id();
         TransferStatusPtr status = m_addon_manager.request_install_addon(addon_id);
 
-        status->then([this, addon_id]{
-            try
+        status->then(
+          [this, addon_id](bool success)
+          {
+            if (success)
             {
-              m_addon_manager.enable_addon(addon_id);
+              try
+              {
+                m_addon_manager.enable_addon(addon_id);
+              }
+              catch(const std::exception& err)
+              {
+                log_warning << "Enabling addon failed: " << err.what() << std::endl;
+              }
+              refresh();
             }
-            catch(const std::exception& err)
-            {
-              log_warning << "Enabling addon failed: " << err.what() << std::endl;
-            }
-            refresh();
           });
 
         std::unique_ptr<DownloadDialog> dialog(new DownloadDialog(status));

diff --git a/src/supertux/menu/download_dialog.cpp b/src/supertux/menu/download_dialog.cpp
index c8d1cfd..910dad0 100644 (file)
--- a/src/supertux/menu/download_dialog.cpp
+++ b/src/supertux/menu/download_dialog.cpp
@@ -31,15 +31,28 @@ DownloadDialog::DownloadDialog(TransferStatusPtr status) :
 
   update_text();
 
-  status->then([this]{
-      on_download_complete();
+  status->then(
+    [this](bool success)
+    {
+      if (success)
+      {
+        on_download_complete();
+      }
+      else
+      {
+        std::unique_ptr<Dialog> dialog(new Dialog);
+        dialog->set_text(_("Error:\n") + m_status->error_msg);
+        dialog->add_button(_("Ok"));
+        MenuManager::instance().set_dialog(std::move(dialog));
+      }
     });
 }
 
 void
 DownloadDialog::update()
 {
-  AddonManager::current()->update();
+  m_status->update();
+
   update_text();
 }