From 5033c64a9b71986cd64f0963e690ac985dd0fd1a Mon Sep 17 00:00:00 2001 From: Ingo Ruhnke Date: Wed, 27 Aug 2014 00:50:48 +0200 Subject: [PATCH 1/1] Changed error handling in TransferStatus, then-callbacks are now always called and have a 'success' flag --- src/addon/addon_manager.cpp | 54 ++++++++++++++++++++--------------- src/addon/downloader.cpp | 51 +++++++++++++++++++++++++++++---- src/addon/downloader.hpp | 19 +++++++----- src/gui/menu_manager.cpp | 12 +------- src/supertux/menu/addon_menu.cpp | 30 ++++++++++++------- src/supertux/menu/download_dialog.cpp | 19 ++++++++++-- 6 files changed, 126 insertions(+), 59 deletions(-) diff --git a/src/addon/addon_manager.cpp b/src/addon/addon_manager.cpp index e6c1b0a4f..a624bc564 100644 --- 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 fd8320b54..b923813f5 100644 --- 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& 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 de1155709..c75c7e1f4 100644 --- a/src/addon/downloader.hpp +++ b/src/addon/downloader.hpp @@ -26,31 +26,36 @@ #include typedef int TransferId; +class Downloader; class TransferStatus { public: - enum Status { RUNNING, COMPLETED, ABORT, ERROR }; - -public: + Downloader& m_downloader; TransferId id; - std::vector > callbacks; + std::vector > 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& callback) + void update(); + + void then(const std::function& callback) { callbacks.push_back(callback); } diff --git a/src/gui/menu_manager.cpp b/src/gui/menu_manager.cpp index 009a10a49..32b61785d 100644 --- 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(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 b9ee6c380..13f8e0abb 100644 --- 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 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 dialog(new DownloadDialog(status)); diff --git a/src/supertux/menu/download_dialog.cpp b/src/supertux/menu/download_dialog.cpp index c8d1cfdf5..910dad078 100644 --- 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(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(); } -- 2.11.0