Fix query error handling

Queries are executed in a separate thread from the query handler, and
exception handling needs to be done in that thread, so refactor
exception handling into Query::execute() and move query-specific logic
to Query::executeLogic().

Also simplify Query::executeLogic() to return the success string, since
they all throw on failure.

src/gui/query/apply_sort_query.h

@@ -33,16 +33,17 @@ public:
   ApplySortQuery(LootState& state, const std::vector<std::string>& plugins) :
     state_(state), plugins_(plugins) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(trace) << "User has accepted sorted load order, applying it.";
     state_.decrementUnappliedChangeCounter();
     state_.getCurrentGame().SetLoadOrder(plugins_);
-    callback->Success("");
+
+    return "";
   }
 
 private:
   LootState& state_;
-  std::vector<std::string> plugins_;
+  const std::vector<std::string> plugins_;
 };
 }
 

src/gui/query/cancel_find_query.h

@@ -34,13 +34,13 @@ class CancelFindQuery : public Query {
 public:
   CancelFindQuery(CefRefPtr<CefBrowser> browser) : browser_(browser) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     browser_->GetHost()->StopFinding(true);
-    callback->Success("");
+    return "";
   }
 
 private:
-  CefRefPtr<CefBrowser> browser_;
+  const CefRefPtr<CefBrowser> browser_;
 };
 }
 

src/gui/query/cancel_sort_query.h

@@ -36,12 +36,11 @@ public:
     MetadataQuery(state.getCurrentGame(), state.getLanguage().GetCode()),
     state_(state) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     state_.decrementUnappliedChangeCounter();
     state_.getCurrentGame().DecrementLoadOrderSortCount();
 
-    YAML::Node node(getGeneralMessages());
-    callback->Success(JSON::stringify(node));
+    return JSON::stringify(YAML::Node(getGeneralMessages()));
   }
 
 private:

src/gui/query/change_game_query.h

@@ -38,10 +38,10 @@ public:
     state_(state),
     gameFolder_(gameFolder) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     state_.changeGame(gameFolder_);
 
-    GetGameDataQuery::execute(callback);
+    return GetGameDataQuery::executeLogic();
   }
 
 private:

src/gui/query/clear_all_metadata_query.h

@@ -36,7 +36,7 @@ public:
     MetadataQuery(state.getCurrentGame(), state.getLanguage().GetCode()),
     game_(state.getCurrentGame()) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(debug) << "Clearing all user metadata.";
 
     // Record which plugins have userlist entries.
@@ -47,7 +47,8 @@ public:
     game_.GetUserlist().Save(game_.UserlistPath());
 
     BOOST_LOG_TRIVIAL(trace) << "Rederiving display metadata for " << userlistPluginNames.size() << " plugins that had user metadata.";
-    callback->Success(getDerivedMetadataJson(userlistPluginNames));
+
+    return getDerivedMetadataJson(userlistPluginNames);
   }
 
 private:

src/gui/query/clear_plugin_metadata_query.h

@@ -37,7 +37,7 @@ public:
     game_(state.getCurrentGame()),
     pluginName_(pluginName) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(debug) << "Clearing user metadata for plugin " << pluginName_;
 
     game_.GetUserlist().ErasePlugin(PluginMetadata(pluginName_));
@@ -46,14 +46,14 @@ public:
     // Now rederive the displayed metadata from the masterlist.
     YAML::Node derivedMetadata = generateDerivedMetadata(pluginName_);
     if (derivedMetadata.size() > 0)
-      callback->Success(JSON::stringify(derivedMetadata));
-    else
-      callback->Success("null");
+      return JSON::stringify(derivedMetadata);
+
+    return "null";
   }
 
 private:
   Game& game_;
-  std::string pluginName_;
+  const std::string pluginName_;
 };
 }
 

src/gui/query/close_settings_query.h

@@ -34,11 +34,11 @@ public:
   CloseSettingsQuery(LootState& state, YAML::Node settings) :
     GetInstalledGamesQuery(state), state_(state), settings_(settings) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(trace) << "Settings dialog closed and changes accepted, updating settings object.";
     state_.load(settings_);
 
-    GetInstalledGamesQuery::execute(callback);
+    return GetInstalledGamesQuery::executeLogic();
   }
 
 private:

src/gui/query/copy_content_query.h

@@ -34,11 +34,11 @@ class CopyContentQuery : public ClipboardQuery {
 public:
   CopyContentQuery(const YAML::Node& content) : content_(content) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
-    std::string text = "[spoiler][code]" + getContentAsText() + "[/code][/spoiler]";
+  std::string executeLogic() {
+    const std::string text = "[spoiler][code]" + getContentAsText() + "[/code][/spoiler]";
 
     copyToClipboard(text);
-    callback->Success("");
+    return "";
   }
 
 private:
@@ -53,7 +53,7 @@ private:
     return text;
   }
 
-  YAML::Node content_;
+  const YAML::Node content_;
 };
 }
 

src/gui/query/copy_load_order_query.h

@@ -33,7 +33,7 @@ public:
   CopyLoadOrderQuery(LootState& state, const std::vector<std::string>& plugins) :
     state_(state), plugins_(plugins) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     int numberOfIndexDigits = getNumberOfIndexDigits();
 
     size_t activeIndex = 0;
@@ -43,7 +43,7 @@ public:
     }
 
     copyToClipboard(stream.str());
-    callback->Success("");
+    return "";
   }
 
 private:
@@ -67,7 +67,7 @@ private:
   }
 
   LootState& state_;
-  std::vector<std::string> plugins_;
+  const std::vector<std::string> plugins_;
 };
 }
 

src/gui/query/copy_metadata_query.h

@@ -35,7 +35,7 @@ public:
   CopyMetadataQuery(LootState& state, const std::string& pluginName) :
     state_(state), pluginName_(pluginName) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(debug) << "Copying metadata for plugin " << pluginName_;
 
     // Get metadata from masterlist and userlist.
@@ -48,6 +48,8 @@ public:
     copyToClipboard(text);
 
     BOOST_LOG_TRIVIAL(info) << "Exported userlist metadata text for \"" << pluginName_ << "\": " << text;
+
+    return "";
   }
 
 private:

src/gui/query/discard_unapplied_changes_query.h

@@ -33,10 +33,11 @@ class DiscardUnappliedChangesQuery : public Query {
 public:
   DiscardUnappliedChangesQuery(LootState& state) : state_(state) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     while (state_.hasUnappliedChanges())
       state_.decrementUnappliedChangeCounter();
-    callback->Success("");
+
+    return "";
   }
 
 private:

src/gui/query/editor_closed_query.h

@@ -35,9 +35,12 @@ public:
     MetadataQuery(state.getCurrentGame(), state.getLanguage().GetCode()),
     state_(state), metadata_(metadata) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     try {
-      callback->Success(applyUserEdits());
+      const std::string json = applyUserEdits();
+      state_.decrementUnappliedChangeCounter();
+
+      return json;
     } catch (Error&) {
       throw;
     } catch (std::exception& e) {
@@ -51,7 +54,6 @@ public:
 
       throw std::runtime_error(error);
     }
-    state_.decrementUnappliedChangeCounter();
   }
 
 private:

src/gui/query/editor_opened_query.h

@@ -33,9 +33,9 @@ class EditorOpenedQuery : public Query {
 public:
   EditorOpenedQuery(LootState& state) : state_(state) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     state_.incrementUnappliedChangeCounter();
-    callback->Success("");
+    return "";
   }
 
 private:

src/gui/query/get_conflicting_plugins_query.h

@@ -37,7 +37,7 @@ public:
     game_(state.getCurrentGame()),
     pluginName_(pluginName) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(debug) << "Searching for plugins that conflict with " << pluginName_;
 
     // Checking for FormID overlap will only work if the plugins have been loaded, so check if
@@ -52,9 +52,9 @@ public:
     }
 
     if (node.size() > 0)
-      callback->Success(JSON::stringify(node));
-    else
-      callback->Success("[]");
+      return JSON::stringify(node);
+
+    return "[]";
   }
 
 private:
@@ -76,7 +76,7 @@ private:
   }
 
   Game& game_;
-  std::string pluginName_;
+  const std::string pluginName_;
 };
 }
 

src/gui/query/get_game_data_query.h

@@ -41,7 +41,7 @@ public:
     state_(state),
     frame_(frame) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     sendProgressUpdate(frame_, boost::locale::translate("Parsing, merging and evaluating metadata..."));
 
     // First clear CRC and condition caches, otherwise they could lead to incorrect evaluations.
@@ -64,7 +64,7 @@ public:
       } catch (...) {}
     }
 
-    callback->Success(generateJsonResponse(installed));
+    return generateJsonResponse(installed);
   }
 
 private:

src/gui/query/get_game_types_query.h

@@ -32,9 +32,9 @@ along with LOOT.  If not, see
 namespace loot {
 class GetGameTypesQuery : public Query {
 public:
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(info) << "Getting LOOT's supported languages.";
-    callback->Success(getGameTypesAsJson());
+    return getGameTypesAsJson();
   }
 
 private:

src/gui/query/get_init_errors_query.h

@@ -34,12 +34,12 @@ class GetInitErrorsQuery : public Query {
 public:
   GetInitErrorsQuery(LootState& state) : state_(state) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     YAML::Node node(state_.getInitErrors());
     if (node.size() > 0)
-      callback->Success(JSON::stringify(node));
-    else
-      callback->Success("null");
+      return JSON::stringify(node);
+
+    return "null";
   }
 
 private:

src/gui/query/get_installed_games_query.h

@@ -34,9 +34,9 @@ class GetInstalledGamesQuery : public Query {
 public:
   GetInstalledGamesQuery(LootState& state) : state_(state) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(info) << "Getting LOOT's detected games.";
-    callback->Success(getInstalledGamesAsJson());
+    return getInstalledGamesAsJson();
   }
 
 private:

src/gui/query/get_languages_query.h

@@ -32,9 +32,9 @@ along with LOOT.  If not, see
 namespace loot {
 class GetLanguagesQuery : public Query {
 public:
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(info) << "Getting LOOT's supported languages.";
-    callback->Success(getLanguagesAsJson());
+    return getLanguagesAsJson();
   }
 
 private:

src/gui/query/get_settings_query.h

@@ -34,13 +34,13 @@ class GetSettingsQuery : public Query {
 public:
   GetSettingsQuery(LootSettings& settings) : settings_(settings) {}
 
-  void execute(CefRefPtr<CefMessageRouterBrowserSide::Callback> callback) {
+  std::string executeLogic() {
     BOOST_LOG_TRIVIAL(info) << "Getting LOOT settings.";
-    callback->Success(JSON::stringify(settings_.toYaml()));
+    return JSON::stringify(settings_.toYaml());
   }
 
 private:
-  LootSettings& settings_;
+  const LootSettings& settings_;
 };
 }