From 1c078b1f474ff814b3416919197ba215b3a0e114 Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Wed, 7 Jul 2021 15:17:21 +0200 Subject: [PATCH] Improved error handling when installing configuration updates: In case the configuration snapshot cannot be taken before installing configuration updates (because the current configuration state is invalid), ask user whether to continue or abort. --- src/slic3r/Config/Snapshot.cpp | 61 ++++++++++++++++++++++--- src/slic3r/Config/Snapshot.hpp | 7 +++ src/slic3r/GUI/ConfigWizard.cpp | 15 +++--- src/slic3r/GUI/ConfigWizard_private.hpp | 2 +- src/slic3r/GUI/GUI_App.cpp | 13 ++++-- src/slic3r/Utils/PresetUpdater.cpp | 42 +++++++++-------- src/slic3r/Utils/PresetUpdater.hpp | 2 +- 7 files changed, 103 insertions(+), 39 deletions(-) diff --git a/src/slic3r/Config/Snapshot.cpp b/src/slic3r/Config/Snapshot.cpp index 40aade783d..23713dd119 100644 --- a/src/slic3r/Config/Snapshot.cpp +++ b/src/slic3r/Config/Snapshot.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include "libslic3r/PresetBundle.hpp" #include "libslic3r/format.hpp" @@ -17,6 +18,13 @@ #include "libslic3r/FileParserError.hpp" #include "libslic3r/Utils.hpp" +#include "../GUI/GUI.hpp" +#include "../GUI/GUI_App.hpp" +#include "../GUI/I18N.hpp" +#include "../GUI/MainFrame.hpp" + +#include + #define SLIC3R_SNAPSHOTS_DIR "snapshots" #define SLIC3R_SNAPSHOT_FILE "snapshot.ini" @@ -435,14 +443,27 @@ const Snapshot& SnapshotDB::take_snapshot(const AppConfig &app_config, Snapshot: } boost::filesystem::path snapshot_dir = snapshot_db_dir / snapshot.id; - boost::filesystem::create_directory(snapshot_dir); - // Backup the presets. - for (const char *subdir : snapshot_subdirs) - copy_config_dir_single_level(data_dir / subdir, snapshot_dir / subdir); - snapshot.save_ini((snapshot_dir / "snapshot.ini").string()); - assert(m_snapshots.empty() || m_snapshots.back().time_captured <= snapshot.time_captured); - m_snapshots.emplace_back(std::move(snapshot)); + try { + boost::filesystem::create_directory(snapshot_dir); + + // Backup the presets. + for (const char *subdir : snapshot_subdirs) + copy_config_dir_single_level(data_dir / subdir, snapshot_dir / subdir); + snapshot.save_ini((snapshot_dir / "snapshot.ini").string()); + assert(m_snapshots.empty() || m_snapshots.back().time_captured <= snapshot.time_captured); + m_snapshots.emplace_back(std::move(snapshot)); + } catch (...) { + if (boost::filesystem::is_directory(snapshot_dir)) { + try { + // Clean up partially copied snapshot. + boost::filesystem::remove_all(snapshot_dir); + } catch (...) { + BOOST_LOG_TRIVIAL(error) << "Failed taking snapshot and failed removing the snapshot directory " << snapshot_dir; + } + } + throw; + } return m_snapshots.back(); } @@ -553,6 +574,32 @@ SnapshotDB& SnapshotDB::singleton() return instance; } +const Snapshot* take_config_snapshot_report_error(const AppConfig &app_config, Snapshot::Reason reason, const std::string &comment) +{ + try { + return &SnapshotDB::singleton().take_snapshot(app_config, reason, comment); + } catch (std::exception &err) { + show_error(static_cast(wxGetApp().mainframe), + _L("Taking a configuration snapshot failed.") + "\n\n" + from_u8(err.what())); + return nullptr; + } +} + +bool take_config_snapshot_cancel_on_error(const AppConfig &app_config, Snapshot::Reason reason, const std::string &comment, const std::string &message) +{ + try { + SnapshotDB::singleton().take_snapshot(app_config, reason, comment); + return true; + } catch (std::exception &err) { + wxRichMessageDialog dlg(static_cast(wxGetApp().mainframe), + _L("PrusaSlicer has encountered an error while taking a configuration snapshot.") + "\n\n" + from_u8(err.what()) + "\n\n" + from_u8(message), + _L("PrusaSlicer error"), + wxYES_NO); + dlg.SetYesNoLabels(_L("Continue"), _L("Abort")); + return dlg.ShowModal() == wxID_YES; + } +} + } // namespace Config } // namespace GUI } // namespace Slic3r diff --git a/src/slic3r/Config/Snapshot.hpp b/src/slic3r/Config/Snapshot.hpp index 48add8a1ad..f453006338 100644 --- a/src/slic3r/Config/Snapshot.hpp +++ b/src/slic3r/Config/Snapshot.hpp @@ -127,6 +127,13 @@ private: std::vector m_snapshots; }; +// Take snapshot on SnapshotDB::singleton(). If taking snapshot fails, report an error and return nullptr. +const Snapshot* take_config_snapshot_report_error(const AppConfig &app_config, Snapshot::Reason reason, const std::string &comment); + +// Take snapshot on SnapshotDB::singleton(). If taking snapshot fails, report "message", and present a "Continue" or "Abort" buttons to respond. +// Return true on success and on "Continue" to continue with the process (for example installation of presets). +bool take_config_snapshot_cancel_on_error(const AppConfig &app_config, Snapshot::Reason reason, const std::string &comment, const std::string &message); + } // namespace Config } // namespace GUI } // namespace Slic3r diff --git a/src/slic3r/GUI/ConfigWizard.cpp b/src/slic3r/GUI/ConfigWizard.cpp index e873d9b07c..c31bc02873 100644 --- a/src/slic3r/GUI/ConfigWizard.cpp +++ b/src/slic3r/GUI/ConfigWizard.cpp @@ -2453,7 +2453,7 @@ bool ConfigWizard::priv::check_and_install_missing_materials(Technology technolo return true; } -void ConfigWizard::priv::apply_config(AppConfig *app_config, PresetBundle *preset_bundle, const PresetUpdater *updater) +bool ConfigWizard::priv::apply_config(AppConfig *app_config, PresetBundle *preset_bundle, const PresetUpdater *updater) { const auto enabled_vendors = appconfig_new.vendors(); @@ -2508,14 +2508,14 @@ void ConfigWizard::priv::apply_config(AppConfig *app_config, PresetBundle *prese break; } - if (snapshot) { - SnapshotDB::singleton().take_snapshot(*app_config, snapshot_reason); - } + if (snapshot && ! take_config_snapshot_cancel_on_error(*app_config, snapshot_reason, "", _u8L("Continue with applying configuration changes?"))) + return false; if (install_bundles.size() > 0) { // Install bundles from resources. // Don't create snapshot - we've already done that above if applicable. - updater->install_bundles_rsrc(std::move(install_bundles), false); + if (! updater->install_bundles_rsrc(std::move(install_bundles), false)) + return false; } else { BOOST_LOG_TRIVIAL(info) << "No bundles need to be installed from resources"; } @@ -2607,6 +2607,8 @@ void ConfigWizard::priv::apply_config(AppConfig *app_config, PresetBundle *prese // Update the selections from the compatibilty. preset_bundle->export_selections(*app_config); + + return true; } void ConfigWizard::priv::update_presets_in_config(const std::string& section, const std::string& alias_key, bool add) { @@ -2817,7 +2819,8 @@ bool ConfigWizard::run(RunReason reason, StartPage start_page) p->set_start_page(start_page); if (ShowModal() == wxID_OK) { - p->apply_config(app.app_config, app.preset_bundle, app.preset_updater); + if (! p->apply_config(app.app_config, app.preset_bundle, app.preset_updater)) + return false; app.app_config->set_legacy_datadir(false); app.update_mode(); app.obj_manipul()->update_ui_from_settings(); diff --git a/src/slic3r/GUI/ConfigWizard_private.hpp b/src/slic3r/GUI/ConfigWizard_private.hpp index 6ca0619410..84def42025 100644 --- a/src/slic3r/GUI/ConfigWizard_private.hpp +++ b/src/slic3r/GUI/ConfigWizard_private.hpp @@ -611,7 +611,7 @@ struct ConfigWizard::priv bool on_bnt_finish(); bool check_and_install_missing_materials(Technology technology, const std::string &only_for_model_id = std::string()); - void apply_config(AppConfig *app_config, PresetBundle *preset_bundle, const PresetUpdater *updater); + bool apply_config(AppConfig *app_config, PresetBundle *preset_bundle, const PresetUpdater *updater); // #ys_FIXME_alise void update_presets_in_config(const std::string& section, const std::string& alias_key, bool add); #ifdef __linux__ diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index 3fd823852b..3973434375 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -1862,9 +1862,10 @@ void GUI_App::add_config_menu(wxMenuBar *menu) child->SetFont(normal_font()); if (dlg.ShowModal() == wxID_OK) - app_config->set("on_snapshot", - Slic3r::GUI::Config::SnapshotDB::singleton().take_snapshot( - *app_config, Slic3r::GUI::Config::Snapshot::SNAPSHOT_USER, dlg.GetValue().ToUTF8().data()).id); + if (const Config::Snapshot *snapshot = Config::take_config_snapshot_report_error( + *app_config, Config::Snapshot::SNAPSHOT_USER, dlg.GetValue().ToUTF8().data()); + snapshot != nullptr) + app_config->set("on_snapshot", snapshot->id); } break; case ConfigMenuSnapshots: @@ -1875,8 +1876,10 @@ void GUI_App::add_config_menu(wxMenuBar *menu) ConfigSnapshotDialog dlg(Slic3r::GUI::Config::SnapshotDB::singleton(), on_snapshot); dlg.ShowModal(); if (!dlg.snapshot_to_activate().empty()) { - if (! Config::SnapshotDB::singleton().is_on_snapshot(*app_config)) - Config::SnapshotDB::singleton().take_snapshot(*app_config, Config::Snapshot::SNAPSHOT_BEFORE_ROLLBACK); + if (! Config::SnapshotDB::singleton().is_on_snapshot(*app_config) && + ! Config::take_config_snapshot_cancel_on_error(*app_config, Config::Snapshot::SNAPSHOT_BEFORE_ROLLBACK, "", + GUI::format(_L("Continue to activate a configuration snapshot %1%?", ex.what())))) + break; try { app_config->set("on_snapshot", Config::SnapshotDB::singleton().restore_snapshot(dlg.snapshot_to_activate(), *app_config).id); // Enable substitutions, log both user and system substitutions. There should not be any substitutions performed when loading system diff --git a/src/slic3r/Utils/PresetUpdater.cpp b/src/slic3r/Utils/PresetUpdater.cpp index 7b4321be99..29d474dcaf 100644 --- a/src/slic3r/Utils/PresetUpdater.cpp +++ b/src/slic3r/Utils/PresetUpdater.cpp @@ -171,7 +171,7 @@ struct PresetUpdater::priv void check_install_indices() const; Updates get_config_updates(const Semver& old_slic3r_version) const; - void perform_updates(Updates &&updates, bool snapshot = true) const; + bool perform_updates(Updates &&updates, bool snapshot = true) const; void set_waiting_updates(Updates u); }; @@ -584,12 +584,14 @@ Updates PresetUpdater::priv::get_config_updates(const Semver &old_slic3r_version return updates; } -void PresetUpdater::priv::perform_updates(Updates &&updates, bool snapshot) const +bool PresetUpdater::priv::perform_updates(Updates &&updates, bool snapshot) const { if (updates.incompats.size() > 0) { if (snapshot) { BOOST_LOG_TRIVIAL(info) << "Taking a snapshot..."; - SnapshotDB::singleton().take_snapshot(*GUI::wxGetApp().app_config, Snapshot::SNAPSHOT_DOWNGRADE); + if (! GUI::Config::take_config_snapshot_cancel_on_error(*GUI::wxGetApp().app_config, Snapshot::SNAPSHOT_DOWNGRADE, "", + _u8L("Continue and install configuration updates?"))) + return false; } BOOST_LOG_TRIVIAL(info) << format("Deleting %1% incompatible bundles", updates.incompats.size()); @@ -604,7 +606,9 @@ void PresetUpdater::priv::perform_updates(Updates &&updates, bool snapshot) cons if (snapshot) { BOOST_LOG_TRIVIAL(info) << "Taking a snapshot..."; - SnapshotDB::singleton().take_snapshot(*GUI::wxGetApp().app_config, Snapshot::SNAPSHOT_UPGRADE); + if (! GUI::Config::take_config_snapshot_cancel_on_error(*GUI::wxGetApp().app_config, Snapshot::SNAPSHOT_UPGRADE, "", + _u8L("Continue and install configuration updates?"))) + return false; } BOOST_LOG_TRIVIAL(info) << format("Performing %1% updates", updates.updates.size()); @@ -648,6 +652,8 @@ void PresetUpdater::priv::perform_updates(Updates &&updates, bool snapshot) cons for (const auto &name : bundle.obsolete_presets.printers) { obsolete_remover("printer", name); } } } + + return true; } void PresetUpdater::priv::set_waiting_updates(Updates u) @@ -761,11 +767,9 @@ PresetUpdater::UpdateResult PresetUpdater::config_update(const Semver& old_slic3 // This effectively removes the incompatible bundles: // (snapshot is taken beforehand) - p->perform_updates(std::move(updates)); - - if (!GUI::wxGetApp().run_wizard(GUI::ConfigWizard::RR_DATA_INCOMPAT)) { + if (! p->perform_updates(std::move(updates)) || + ! GUI::wxGetApp().run_wizard(GUI::ConfigWizard::RR_DATA_INCOMPAT)) return R_INCOMPAT_EXIT; - } return R_INCOMPAT_CONFIGURED; } @@ -799,7 +803,8 @@ PresetUpdater::UpdateResult PresetUpdater::config_update(const Semver& old_slic3 const auto res = dlg.ShowModal(); if (res == wxID_OK) { BOOST_LOG_TRIVIAL(info) << "User wants to update..."; - p->perform_updates(std::move(updates)); + if (! p->perform_updates(std::move(updates))) + return R_INCOMPAT_EXIT; reload_configs_update_gui(); return R_UPDATE_INSTALLED; } @@ -828,7 +833,8 @@ PresetUpdater::UpdateResult PresetUpdater::config_update(const Semver& old_slic3 const auto res = dlg.ShowModal(); if (res == wxID_OK) { BOOST_LOG_TRIVIAL(debug) << "User agreed to perform the update"; - p->perform_updates(std::move(updates)); + if (! p->perform_updates(std::move(updates))) + return R_ALL_CANCELED; reload_configs_update_gui(); return R_UPDATE_INSTALLED; } @@ -848,7 +854,7 @@ PresetUpdater::UpdateResult PresetUpdater::config_update(const Semver& old_slic3 return R_NOOP; } -void PresetUpdater::install_bundles_rsrc(std::vector bundles, bool snapshot) const +bool PresetUpdater::install_bundles_rsrc(std::vector bundles, bool snapshot) const { Updates updates; @@ -860,7 +866,7 @@ void PresetUpdater::install_bundles_rsrc(std::vector bundles, bool updates.updates.emplace_back(std::move(path_in_rsrc), std::move(path_in_vendors), Version(), "", ""); } - p->perform_updates(std::move(updates), snapshot); + return p->perform_updates(std::move(updates), snapshot); } void PresetUpdater::on_update_notification_confirm() @@ -880,16 +886,14 @@ void PresetUpdater::on_update_notification_confirm() const auto res = dlg.ShowModal(); if (res == wxID_OK) { BOOST_LOG_TRIVIAL(debug) << "User agreed to perform the update"; - p->perform_updates(std::move(p->waiting_updates)); - reload_configs_update_gui(); - p->has_waiting_updates = false; - //return R_UPDATE_INSTALLED; + if (p->perform_updates(std::move(p->waiting_updates))) { + reload_configs_update_gui(); + p->has_waiting_updates = false; + } } else { BOOST_LOG_TRIVIAL(info) << "User refused the update"; - //return R_UPDATE_REJECT; - } - + } } } diff --git a/src/slic3r/Utils/PresetUpdater.hpp b/src/slic3r/Utils/PresetUpdater.hpp index acec7baf7c..d7eeb5604b 100644 --- a/src/slic3r/Utils/PresetUpdater.hpp +++ b/src/slic3r/Utils/PresetUpdater.hpp @@ -52,7 +52,7 @@ public: UpdateResult config_update(const Semver &old_slic3r_version, UpdateParams params) const; // "Update" a list of bundles from resources (behaves like an online update). - void install_bundles_rsrc(std::vector bundles, bool snapshot = true) const; + bool install_bundles_rsrc(std::vector bundles, bool snapshot = true) const; void on_update_notification_confirm(); private: