Skip to content

More graceful MSIX enumeration failure handling #5329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JohnMcPMS merged 2 commits into from
Mar 28, 2025
Merged

More graceful MSIX enumeration failure handling #5329

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
477faee
More graceful failure handling
JohnMcPMS Mar 27, 2025
5506afe
break
JohnMcPMS Mar 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,50 @@ namespace AppInstaller::Repository::Microsoft

IIterable<Package> packages;
PackageManager packageManager;

if (scope == Manifest::ScopeEnum::Machine)
{
packages = packageManager.FindProvisionedPackages();
// May not be present on our oldest supported systems; simply ignore for the time being.
IPackageManager9 packageManager9 = packageManager.try_as<IPackageManager9>();
if (packageManager9)
{
packages = packageManager.FindProvisionedPackages();
}
else
{
AICLI_LOG(Repo, Warning, << "FindProvisionedPackages is not available on this version of Windows");
}
}
else
{
// TODO: Consider if Optional packages should also be enumerated
packages = packageManager.FindPackagesForUserWithPackageTypes({}, PackageTypes::Main | PackageTypes::Framework);
for (PackageTypes types : { PackageTypes::Main | PackageTypes::Framework, PackageTypes::Main, PackageTypes::Framework })
{
try
{
packages = packageManager.FindPackagesForUserWithPackageTypes({}, types);

@yao-msft yao-msft Mar 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if Main | Framework failed, does enumerating Main and Framework individually and combining the results work?

@JohnMcPMS JohnMcPMS Mar 27, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption is that this issue is caused by some package's state being corrupted. Hopefully if it is just one package, then it must either be a Main OR a Framework. Attempting to get that one will also fail, and we can enumerate the other list. But this is all speculation on the actual issue.

A MUCH larger change would surface this inability to enumerate the packages and allow the code with context to decide how to proceed like we do with the errors from available catalogs. But on thinking through the implications here, I think it is better to allow reduced functionality rather than hard error. In the cases where it matters, it probably means doing extra work. In the cases where it doesn't matter, it means success instead of the current error.

break;
}
catch (const winrt::hresult_error& hre)
{
if (hre.code() == E_NOT_SET)
{
// This OS issue occurs frequently enough that we will attempt to work around it by enumerating progressively fewer packages
AICLI_LOG(Repo, Warning, << "FindPackagesForUserWithPackageTypes returned E_NOT_SET for types: " << ToIntegral(types));
}
else
{
throw;
}
}
}
}

// Failed to retrieve even an empty package list; make sure that these cases have a log to indicate why.
if (!packages)
{
AICLI_LOG(Repo, Warning, << "MSIX package list not populated");
return;
}

// Reuse the same manifest object, as we will be setting the same values every time.
Expand Down