Merge pull request #11018 from Dentomologist/add_updater_error_messages

Add updater error messages

Source/Core/Common/FileUtil.cpp

@@ -635,15 +635,19 @@ bool DeleteDirRecursively(const std::string& directory)
   return success;
 }
 
-// Create directory and copy contents (does not overwrite existing files)
-void CopyDir(const std::string& source_path, const std::string& dest_path, bool destructive)
+// Create directory and copy contents (optionally overwrites existing files)
+bool CopyDir(const std::string& source_path, const std::string& dest_path, const bool destructive)
 {
   if (source_path == dest_path)
-    return;
+    return true;
   if (!Exists(source_path))
-    return;
+    return false;
+
+  // Shouldn't be used to short circuit operations after an earlier failure
+  bool everything_copied = true;
+
   if (!Exists(dest_path))
-    File::CreateFullPath(dest_path);
+    everything_copied = File::CreateFullPath(dest_path) && everything_copied;
 
 #ifdef _WIN32
   WIN32_FIND_DATA ffd;
@@ -652,7 +656,7 @@ void CopyDir(const std::string& source_path, const std::string& dest_path, bool
   if (hFind == INVALID_HANDLE_VALUE)
   {
     FindClose(hFind);
-    return;
+    return false;
   }
 
   do
@@ -661,7 +665,7 @@ void CopyDir(const std::string& source_path, const std::string& dest_path, bool
 #else
   DIR* dirp = opendir(source_path.c_str());
   if (!dirp)
-    return;
+    return false;
 
   while (dirent* result = readdir(dirp))
   {
@@ -671,21 +675,21 @@ void CopyDir(const std::string& source_path, const std::string& dest_path, bool
     if (virtualName == "." || virtualName == "..")
       continue;
 
-    std::string source = source_path + DIR_SEP + virtualName;
-    std::string dest = dest_path + DIR_SEP + virtualName;
+    const std::string source = source_path + DIR_SEP + virtualName;
+    const std::string dest = dest_path + DIR_SEP + virtualName;
     if (IsDirectory(source))
     {
       if (!Exists(dest))
         File::CreateFullPath(dest + DIR_SEP);
-      CopyDir(source, dest, destructive);
+      everything_copied = CopyDir(source, dest, destructive) && everything_copied;
     }
     else if (!destructive && !Exists(dest))
     {
-      Copy(source, dest);
+      everything_copied = Copy(source, dest) && everything_copied;
     }
     else if (destructive)
     {
-      Rename(source, dest);
+      everything_copied = Rename(source, dest) && everything_copied;
     }
 #ifdef _WIN32
   } while (FindNextFile(hFind, &ffd) != 0);
@@ -694,6 +698,7 @@ void CopyDir(const std::string& source_path, const std::string& dest_path, bool
   }
   closedir(dirp);
 #endif
+  return everything_copied;
 }
 
 // Returns the current directory

Source/Core/Common/FileUtil.h

@@ -193,7 +193,7 @@ bool DeleteDirRecursively(const std::string& directory);
 std::string GetCurrentDir();
 
 // Create directory and copy contents (optionally overwrites existing files)
-void CopyDir(const std::string& source_path, const std::string& dest_path,
+bool CopyDir(const std::string& source_path, const std::string& dest_path,
              bool destructive = false);
 
 // Set the current directory to given directory

Source/Core/DolphinQt/MenuBar.cpp

@@ -558,12 +558,7 @@ void MenuBar::InstallUpdateManually()
   auto* const updater = new Updater(this->parentWidget(), manual_track,
                                     Config::Get(Config::MAIN_AUTOUPDATE_HASH_OVERRIDE));
 
-  if (!updater->CheckForUpdate())
-  {
-    ModalMessageBox::information(
-        this, tr("Update"),
-        tr("You are running the latest version available on this update track."));
-  }
+  updater->CheckForUpdate();
 }
 
 void MenuBar::AddHelpMenu()

Source/Core/DolphinQt/Updater.cpp

@@ -29,21 +29,19 @@ Updater::Updater(QWidget* parent, std::string update_track, std::string hash_ove
 
 void Updater::run()
 {
-  AutoUpdateChecker::CheckForUpdate(m_update_track, m_hash_override);
+  AutoUpdateChecker::CheckForUpdate(m_update_track, m_hash_override,
+                                    AutoUpdateChecker::CheckType::Automatic);
 }
 
-bool Updater::CheckForUpdate()
+void Updater::CheckForUpdate()
 {
-  m_update_available = false;
-  AutoUpdateChecker::CheckForUpdate(m_update_track, m_hash_override);
-
-  return m_update_available;
+  AutoUpdateChecker::CheckForUpdate(m_update_track, m_hash_override,
+                                    AutoUpdateChecker::CheckType::Manual);
 }
 
 void Updater::OnUpdateAvailable(const NewVersionInformation& info)
 {
   bool later = false;
-  m_update_available = true;
 
   std::optional<int> choice = RunOnObject(m_parent, [&] {
     QDialog* dialog = new QDialog(m_parent);

Source/Core/DolphinQt/Updater.h

@@ -21,11 +21,10 @@ public:
 
   void run() override;
   void OnUpdateAvailable(const NewVersionInformation& info) override;
-  bool CheckForUpdate();
+  void CheckForUpdate();
 
 private:
   QWidget* m_parent;
   std::string m_update_track;
   std::string m_hash_override;
-  bool m_update_available = false;
 };

Source/Core/UICommon/AutoUpdate.cpp

@@ -8,10 +8,12 @@
 #include <fmt/format.h>
 #include <picojson.h>
 
+#include "Common/CommonFuncs.h"
 #include "Common/CommonPaths.h"
 #include "Common/FileUtil.h"
 #include "Common/HttpRequest.h"
 #include "Common/Logging/Log.h"
+#include "Common/MsgHandler.h"
 #include "Common/StringUtil.h"
 #include "Common/Version.h"
 
@@ -159,7 +161,7 @@ static std::string GetPlatformID()
 }
 
 void AutoUpdateChecker::CheckForUpdate(std::string_view update_track,
-                                       std::string_view hash_override)
+                                       std::string_view hash_override, const CheckType check_type)
 {
   // Don't bother checking if updates are not supported or not enabled.
   if (!SystemSupportsAutoUpdates() || update_track.empty())
@@ -173,11 +175,14 @@ void AutoUpdateChecker::CheckForUpdate(std::string_view update_track,
   std::string url = fmt::format("https://dolphin-emu.org/update/check/v1/{}/{}/{}", update_track,
                                 version_hash, GetPlatformID());
 
+  const bool is_manual_check = check_type == CheckType::Manual;
+
   Common::HttpRequest req{std::chrono::seconds{10}};
   auto resp = req.Get(url);
   if (!resp)
   {
-    ERROR_LOG_FMT(COMMON, "Auto-update request failed");
+    if (is_manual_check)
+      CriticalAlertFmtT("Unable to contact update server.");
     return;
   }
   const std::string contents(reinterpret_cast<char*>(resp->data()), resp->size());
@@ -187,13 +192,15 @@ void AutoUpdateChecker::CheckForUpdate(std::string_view update_track,
   const std::string err = picojson::parse(json, contents);
   if (!err.empty())
   {
-    ERROR_LOG_FMT(COMMON, "Invalid JSON received from auto-update service: {}", err);
+    CriticalAlertFmtT("Invalid JSON received from auto-update service : {0}", err);
     return;
   }
   picojson::object obj = json.get<picojson::object>();
 
   if (obj["status"].get<std::string>() != "outdated")
   {
+    if (is_manual_check)
+      SuccessAlertFmtT("You are running the latest version available on this update track.");
     INFO_LOG_FMT(COMMON, "Auto-update status: we are up to date.");
     return;
   }
@@ -212,7 +219,7 @@ void AutoUpdateChecker::CheckForUpdate(std::string_view update_track,
 }
 
 void AutoUpdateChecker::TriggerUpdate(const AutoUpdateChecker::NewVersionInformation& info,
-                                      AutoUpdateChecker::RestartMode restart_mode)
+                                      const AutoUpdateChecker::RestartMode restart_mode)
 {
   // Check to make sure we don't already have an update triggered
   if (s_update_triggered)
@@ -241,8 +248,16 @@ void AutoUpdateChecker::TriggerUpdate(const AutoUpdateChecker::NewVersionInforma
 #ifdef __APPLE__
   // Copy the updater so it can update itself if needed.
   const std::string reloc_updater_path = UpdaterPath(true);
-  File::CopyDir(UpdaterPath(), reloc_updater_path);
-  chmod((reloc_updater_path + UPDATER_CONTENT_PATH).c_str(), 0700);
+  if (!File::CopyDir(UpdaterPath(), reloc_updater_path))
+  {
+    CriticalAlertFmtT("Unable to create updater copy.");
+    return;
+  }
+  if (chmod((reloc_updater_path + UPDATER_CONTENT_PATH).c_str(), 0700) != 0)
+  {
+    CriticalAlertFmtT("Unable to set permissions on updater copy.");
+    return;
+  }
 #endif
 
   // Run the updater!
@@ -261,12 +276,14 @@ void AutoUpdateChecker::TriggerUpdate(const AutoUpdateChecker::NewVersionInforma
   }
   else
   {
-    ERROR_LOG_FMT(COMMON, "Could not start updater process: error={}", GetLastError());
+    const std::string error = GetLastErrorString();
+    CriticalAlertFmtT("Could not start updater process: {0}", error);
   }
 #else
   if (popen(command_line.c_str(), "r") == nullptr)
   {
-    ERROR_LOG_FMT(COMMON, "Could not start updater process: error={}", errno);
+    const std::string error = LastStrerrorString();
+    CriticalAlertFmtT("Could not start updater process: {0}", error);
   }
 #endif
 

Source/Core/UICommon/AutoUpdate.h

@@ -13,9 +13,15 @@
 class AutoUpdateChecker
 {
 public:
+  enum class CheckType
+  {
+    Automatic,
+    Manual,
+  };
   // Initiates a check for updates in the background. Calls the OnUpdateAvailable callback if an
   // update is available, does "nothing" otherwise.
-  void CheckForUpdate(std::string_view update_track, std::string_view hash_override);
+  void CheckForUpdate(std::string_view update_track, std::string_view hash_override,
+                      CheckType check_type);
 
   static bool SystemSupportsAutoUpdates();