commit 91ee267ccd1bb6d9f976153e195572a5ee9746d8
parent 596b1b0497c74fac3d743b7600113e3f73170b6a
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date: Tue, 9 Apr 2024 10:31:47 -0700
Improve MSStore installation success rate by trying Restart or Cancel when applicable (#4356)
After calling to InstallProductAsync, try to check status of existing
install items and call Restart or Cancel if applicable.
After our operation failed, clean up the installation queue by calling
cancel.
I manually validated by opening Store app, install a test app and pause
the installation. Then use winget to install the same app. App
installation failed before the change and succeeded after the change.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/winget-cli/pull/4356)
Diffstat:
3 files changed, 154 insertions(+), 3 deletions(-)
diff --git a/src/AppInstallerCommonCore/MSStore.cpp b/src/AppInstallerCommonCore/MSStore.cpp
@@ -95,6 +95,87 @@ namespace AppInstaller::MSStore
return result;
}
+
+ enum class CheckExistingItemResult
+ {
+ None,
+ Restart,
+ Cancel,
+ };
+
+ CheckExistingItemResult CheckRestartOrCancelForPossibleExistingOperation(const IVectorView<AppInstallItem>& installItems)
+ {
+ CheckExistingItemResult result = CheckExistingItemResult::None;
+
+ for (auto const& installItem : installItems)
+ {
+ const auto& status = installItem.GetCurrentStatus();
+ switch (status.InstallState())
+ {
+ case AppInstallState::Canceled:
+ case AppInstallState::Error:
+ // For these states, always do a cancel;
+ result = CheckExistingItemResult::Cancel;
+ return result;
+ case AppInstallState::Paused:
+ case AppInstallState::PausedLowBattery:
+ case AppInstallState::PausedWiFiRecommended:
+ case AppInstallState::PausedWiFiRequired:
+ case AppInstallState::ReadyToDownload:
+ // For these states, set result to restart and continue the loop to see if future items need cancel.
+ result = CheckExistingItemResult::Restart;
+ break;
+ }
+ }
+
+ return result;
+ }
+
+ bool DoesInstallItemsContainProduct(const IVectorView<AppInstallItem>& installItems, std::wstring_view productId)
+ {
+ for (auto const& installItem : installItems)
+ {
+ if (Utility::CaseInsensitiveEquals(installItem.ProductId(), productId))
+ {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ // Returns true if Restart or Cancel happened. False otherwise.
+ HRESULT RestartOrCancelExistingOperationIfNecessary(const IVectorView<AppInstallItem>& installItems, AppInstallManager& installManager, std::wstring_view productId)
+ {
+ auto existingItemResult = CheckRestartOrCancelForPossibleExistingOperation(installItems);
+
+ if (existingItemResult == CheckExistingItemResult::Cancel || existingItemResult == CheckExistingItemResult::Restart)
+ {
+ if (existingItemResult == CheckExistingItemResult::Cancel)
+ {
+ installManager.Cancel(productId);
+
+ // Wait for at most 10 seconds for install item to be removed from queue.
+ for (int i = 0; i < 50; ++i)
+ {
+ Sleep(200);
+ if (!DoesInstallItemsContainProduct(installManager.AppInstallItems(), productId))
+ {
+ return S_OK;
+ }
+ }
+
+ RETURN_HR(HRESULT_FROM_WIN32(ERROR_TIMEOUT));
+ }
+ else
+ {
+ installManager.Restart(productId);
+ return S_OK;
+ }
+ }
+
+ return S_FALSE;
+ }
}
HRESULT MSStoreOperation::StartAndWaitForOperation(IProgressCallback& progress)
@@ -102,12 +183,14 @@ namespace AppInstaller::MSStore
// Best effort verifying/acquiring product ownership.
std::ignore = EnsureFreeEntitlement(m_productId, m_scope);
- if (m_type == MSStoreOperationType::Install || m_type == MSStoreOperationType::Repair)
+ if (m_type == MSStoreOperationType::Update)
+ {
+ return UpdatePackage(progress);
+ }
+ else
{
return InstallPackage(progress);
}
-
- return UpdatePackage(progress);
}
HRESULT MSStoreOperation::InstallPackage(IProgressCallback& progress)
@@ -149,6 +232,22 @@ namespace AppInstaller::MSStore
winrt::hstring(),
installOptions).get();
+ // Check if we need to restart or cancel existing items.
+ auto restartOrCancelResult = RestartOrCancelExistingOperationIfNecessary(installItems, installManager, m_productId);
+ RETURN_IF_FAILED(restartOrCancelResult);
+
+ // If restart or cancel happened, try again.
+ if (restartOrCancelResult == S_OK)
+ {
+ // Try again
+ installItems = installManager.StartProductInstallAsync(
+ m_productId, // ProductId
+ winrt::hstring(), // FlightId
+ L"WinGetCli", // ClientId
+ winrt::hstring(),
+ installOptions).get();
+ }
+
return WaitForOperation(installItems, progress);
}
@@ -175,11 +274,48 @@ namespace AppInstaller::MSStore
std::vector<AppInstallItem> installItemVector{ installItem };
IVectorView<AppInstallItem> installItems = winrt::single_threaded_vector(std::move(installItemVector)).GetView();
+ // Check if we need to restart or cancel existing items.
+ auto restartOrCancelResult = RestartOrCancelExistingOperationIfNecessary(installItems, installManager, m_productId);
+ RETURN_IF_FAILED(restartOrCancelResult);
+
+ // If restart or cancel happened, try again.
+ if (restartOrCancelResult == S_OK)
+ {
+ // Try again
+ installItem = installManager.SearchForUpdatesAsync(
+ m_productId, // ProductId
+ winrt::hstring(), // SkuId
+ winrt::hstring(),
+ winrt::hstring(), // ClientId
+ updateOptions
+ ).get();
+
+ if (!installItem)
+ {
+ return APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE;
+ }
+
+ installItemVector.clear();
+ installItemVector.emplace_back(installItem);
+ installItems = winrt::single_threaded_vector(std::move(installItemVector)).GetView();
+ }
+
return WaitForOperation(installItems, progress);
}
HRESULT MSStoreOperation::WaitForOperation(IVectorView<AppInstallItem>& installItems, IProgressCallback& progress)
{
+ auto cancelIfOperationFailed = wil::scope_exit(
+ [&]()
+ {
+ try
+ {
+ AppInstallManager installManager;
+ installManager.Cancel(m_productId);
+ }
+ CATCH_LOG();
+ });
+
for (auto const& installItem : installItems)
{
AICLI_LOG(Core, Info, <<
@@ -242,6 +378,7 @@ namespace AppInstaller::MSStore
{
AICLI_LOG(Core, Info, << "Asked to shutdown while installing AppInstaller.");
progress.OnProgress(overallProgressMax, overallProgressMax, ProgressType::Percent);
+ cancelIfOperationFailed.release();
return S_OK;
}
}
@@ -250,6 +387,11 @@ namespace AppInstaller::MSStore
Sleep(100);
}
+ if (SUCCEEDED(errorCode))
+ {
+ cancelIfOperationFailed.release();
+ }
+
return errorCode;
}
}
diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp
@@ -104,6 +104,11 @@ namespace AppInstaller::Utility
return ToLower(a) == ToLower(b);
}
+ bool CaseInsensitiveEquals(std::wstring_view a, std::wstring_view b)
+ {
+ return ToLower(a) == ToLower(b);
+ }
+
bool CaseInsensitiveContains(const std::vector<std::string_view>& a, std::string_view b)
{
auto B = ToLower(b);
diff --git a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h
@@ -108,6 +108,10 @@ namespace AppInstaller::Utility
// Use this if one of the values is a known value, and thus ToLower is sufficient.
bool CaseInsensitiveEquals(std::string_view a, std::string_view b);
+ // Compares the two UTF16 strings in a case insensitive manner.
+ // Use this if one of the values is a known value, and thus ToLower is sufficient.
+ bool CaseInsensitiveEquals(std::wstring_view a, std::wstring_view b);
+
// Returns if a UTF8 string is contained within a vector in a case insensitive manner.
bool CaseInsensitiveContains(const std::vector<std::string_view>& a, std::string_view b);
