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

@@ -8,6 +8,7 @@
 #include <boost/property_tree/ini_parser.hpp>
 #include <boost/property_tree/ptree_fwd.hpp>
 #include <boost/filesystem/operations.hpp>
+#include <boost/log/trivial.hpp>
 
 #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 <wx/richmsgdlg.h>
+
 #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.
+    try {
-    for (const char *subdir : snapshot_subdirs)
+	    boost::filesystem::create_directory(snapshot_dir);
-    	copy_config_dir_single_level(data_dir / subdir, snapshot_dir / subdir);
+
-    snapshot.save_ini((snapshot_dir / "snapshot.ini").string());
+        // Backup the presets.
-    assert(m_snapshots.empty() || m_snapshots.back().time_captured <= snapshot.time_captured);
+        for (const char *subdir : snapshot_subdirs)
-    m_snapshots.emplace_back(std::move(snapshot));
+    	    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<wxWindow*>(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<wxWindow*>(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

src/slic3r/Config/Snapshot.hpp

@@ -127,6 +127,13 @@ private:
 	std::vector<Snapshot>			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

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) {
+    if (snapshot && ! take_config_snapshot_cancel_on_error(*app_config, snapshot_reason, "", _u8L("Continue with applying configuration changes?")))
-        SnapshotDB::singleton().take_snapshot(*app_config, snapshot_reason);
+        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();

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__

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",
+                    if (const Config::Snapshot *snapshot = Config::take_config_snapshot_report_error(
-                    Slic3r::GUI::Config::SnapshotDB::singleton().take_snapshot(
+                            *app_config, Config::Snapshot::SNAPSHOT_USER, dlg.GetValue().ToUTF8().data());
-                    *app_config, Slic3r::GUI::Config::Snapshot::SNAPSHOT_USER, dlg.GetValue().ToUTF8().data()).id);
+                        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))
+                    if (! Config::SnapshotDB::singleton().is_on_snapshot(*app_config) && 
-                        Config::SnapshotDB::singleton().take_snapshot(*app_config, Config::Snapshot::SNAPSHOT_BEFORE_ROLLBACK);
+                        ! 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

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 (! p->perform_updates(std::move(updates)) ||
-
+				! GUI::wxGetApp().run_wizard(GUI::ConfigWizard::RR_DATA_INCOMPAT))
-			if (!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<std::string> bundles, bool snapshot) const
+bool PresetUpdater::install_bundles_rsrc(std::vector<std::string> bundles, bool snapshot) const
 {
 	Updates updates;
 
@@ -860,7 +866,7 @@ void PresetUpdater::install_bundles_rsrc(std::vector<std::string> 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));
+		if (p->perform_updates(std::move(p->waiting_updates))) {
-		reload_configs_update_gui();
+			reload_configs_update_gui();
-		p->has_waiting_updates = false;
+			p->has_waiting_updates = false;
-		//return R_UPDATE_INSTALLED;
+		}
 	}
 	else {
 		BOOST_LOG_TRIVIAL(info) << "User refused the update";
-		//return R_UPDATE_REJECT;
 	}	
-	
 }
 
 }

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<std::string> bundles, bool snapshot = true) const;
+	bool install_bundles_rsrc(std::vector<std::string> bundles, bool snapshot = true) const;
 
 	void on_update_notification_confirm();
 private: