commit 2e285f1bda556b783098c5d7588fcddbdc082d7c parent 53ac5d42b65c96317245c6473044607920bef8d5 Author: JohnMcPMS <johnmcp@microsoft.com> Date: Fri, 17 Sep 2021 23:38:36 -0700 Source resilience (#1470) Improves handling of errors from one source when multiple are configured by capturing the errors and including them as part of the source open and search results. Some commands will continue to fail if one of the sources has an error to prevent said error from affecting a change to the system. The commands that still fail are `install`, `import`, and `upgrade --all|<specific>` and they now indicate which source failed. This change also replaces the experimental `msstore` source with the more official but still green `msstore` source (previously called `storepreview`). This source is now capable of providing basic search functionality against the entire Store catalog. It does not provide any correlation for installed packages however. In addition, the store source is being moved out of experimental. Diffstat:
43 files changed, 865 insertions(+), 214 deletions(-)
diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt @@ -370,7 +370,8 @@ realloc REALTIME REBOOTPROMPT Redistributable -Refactor +refactor +refactored refactoring REFCLSID regex @@ -384,6 +385,7 @@ RESOLVESOURCE RESTSOURCE resw resx +rethrowing roadmap robuffer rowcount @@ -464,6 +466,7 @@ STRINGIFY STRINGIZE stringstream strstr +strtoll subcontext SUBLANG subresource diff --git a/doc/Settings.md b/doc/Settings.md @@ -127,16 +127,6 @@ The `experimentalFeatures` settings involve the configuration of these "experime }, ``` -### experimentalMSStore - -Microsoft Store App support in WinGet is currently implemented as an experimental feature. It supports a curated list of utility apps from Microsoft Store. You can enable the feature as shown below. - -```json - "experimentalFeatures": { - "experimentalMSStore": true - }, -``` - ### directMSI This feature enables the Windows Package Manager to directly install MSI packages with the MSI APIs rather than through msiexec. diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json @@ -114,8 +114,8 @@ "type": "boolean", "default": false }, - "experimentalMSStore": { - "description": "Experimental support for Microsoft Store source", + "dependencies": { + "description": "Support for package dependencies", "type": "boolean", "default": false }, diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj @@ -226,6 +226,11 @@ <SubSystem Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Windows</SubSystem> </Link> </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(WingetDisableTestHooks)'=='true'"> + <ClCompile> + <PreprocessorDefinitions>AICLI_DISABLE_TEST_HOOKS;%(PreprocessorDefinitions)</PreprocessorDefinitions> + </ClCompile> + </ItemDefinitionGroup> <ItemGroup> <ClInclude Include="Argument.h" /> <ClInclude Include="ChannelStreams.h" /> diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp @@ -861,52 +861,9 @@ namespace AppInstaller::CLI command->Execute(context); } - // Exceptions that may occur in the process of executing an arbitrary command - catch (const wil::ResultException& re) - { - // Even though they are logged at their source, log again here for completeness. - Logging::Telemetry().LogException(command->FullName(), "wil::ResultException", re.what()); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << - GetUserPresentableMessage(re) << std::endl; - return re.GetErrorCode(); - } - catch (const winrt::hresult_error& hre) - { - std::string message = GetUserPresentableMessage(hre); - Logging::Telemetry().LogException(command->FullName(), "winrt::hresult_error", message); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << - message << std::endl; - return hre.code(); - } - catch (const Settings::GroupPolicyException& e) - { - auto policy = Settings::TogglePolicy::GetPolicy(e.Policy()); - context.Reporter.Error() << Resource::String::DisabledByGroupPolicy << ": "_liv << policy.PolicyName() << std::endl; - return APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY; - } - catch (const Resource::ResourceOpenException& e) - { - Logging::Telemetry().LogException(command->FullName(), "ResourceOpenException", e.what()); - context.Reporter.Error() << GetUserPresentableMessage(e) << std::endl; - return APPINSTALLER_CLI_ERROR_MISSING_RESOURCE_FILE; - } - catch (const std::exception& e) - { - Logging::Telemetry().LogException(command->FullName(), "std::exception", e.what()); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << - GetUserPresentableMessage(e) << std::endl; - return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; - } catch (...) { - LOG_CAUGHT_EXCEPTION(); - Logging::Telemetry().LogException(command->FullName(), "unknown", {}); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl; - return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; + context.SetTerminationHR(Workflow::HandleException(context, std::current_exception())); } if (SUCCEEDED(context.GetTerminationHR())) diff --git a/src/AppInstallerCLICore/Commands/ExportCommand.cpp b/src/AppInstallerCLICore/Commands/ExportCommand.cpp @@ -53,11 +53,14 @@ namespace AppInstaller::CLI void ExportCommand::ExecuteInternal(Execution::Context& context) const { + context.SetFlags(Execution::ContextFlag::TreatSourceFailuresAsWarning); + context << Workflow::ReportExecutionStage(Workflow::ExecutionStage::Discovery) << Workflow::OpenSource << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << Workflow::SearchSourceForMany << + Workflow::HandleSearchResultFailures << Workflow::EnsureMatchesFromSearchResult(true) << Workflow::SelectVersionsToExport << Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) << diff --git a/src/AppInstallerCLICore/Commands/InstallCommand.cpp b/src/AppInstallerCLICore/Commands/InstallCommand.cpp @@ -122,6 +122,8 @@ namespace AppInstaller::CLI void InstallCommand::ExecuteInternal(Context& context) const { + context.SetFlags(ContextFlag::ShowSearchResultsOnPartialFailure); + context << Workflow::ReportExecutionStage(ExecutionStage::Discovery) << Workflow::GetManifest << diff --git a/src/AppInstallerCLICore/Commands/ListCommand.cpp b/src/AppInstallerCLICore/Commands/ListCommand.cpp @@ -70,10 +70,13 @@ namespace AppInstaller::CLI void ListCommand::ExecuteInternal(Execution::Context& context) const { + context.SetFlags(Execution::ContextFlag::TreatSourceFailuresAsWarning); + context << Workflow::OpenSource << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << Workflow::SearchSourceForMany << + Workflow::HandleSearchResultFailures << Workflow::EnsureMatchesFromSearchResult(true) << Workflow::ReportListResult(); } diff --git a/src/AppInstallerCLICore/Commands/SearchCommand.cpp b/src/AppInstallerCLICore/Commands/SearchCommand.cpp @@ -68,9 +68,12 @@ namespace AppInstaller::CLI void SearchCommand::ExecuteInternal(Context& context) const { + context.SetFlags(Execution::ContextFlag::TreatSourceFailuresAsWarning); + context << Workflow::OpenSource << Workflow::SearchSourceForMany << + Workflow::HandleSearchResultFailures << Workflow::EnsureMatchesFromSearchResult(false) << Workflow::ReportSearchResult; } diff --git a/src/AppInstallerCLICore/Commands/ShowCommand.cpp b/src/AppInstallerCLICore/Commands/ShowCommand.cpp @@ -51,6 +51,8 @@ namespace AppInstaller::CLI void ShowCommand::ExecuteInternal(Execution::Context& context) const { + context.SetFlags(Execution::ContextFlag::TreatSourceFailuresAsWarning); + if (context.Args.Contains(Execution::Args::Type::ListVersions)) { if (context.Args.Contains(Execution::Args::Type::Manifest)) @@ -65,6 +67,7 @@ namespace AppInstaller::CLI context << Workflow::OpenSource << Workflow::SearchSourceForSingle << + Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(false) << Workflow::ReportPackageIdentity << Workflow::ShowAppVersions; diff --git a/src/AppInstallerCLICore/Commands/UninstallCommand.cpp b/src/AppInstallerCLICore/Commands/UninstallCommand.cpp @@ -101,6 +101,8 @@ namespace AppInstaller::CLI void UninstallCommand::ExecuteInternal(Execution::Context& context) const { + context.SetFlags(Execution::ContextFlag::TreatSourceFailuresAsWarning); + // open the sources where to search for the package context << Workflow::ReportExecutionStage(ExecutionStage::Discovery) << @@ -122,6 +124,7 @@ namespace AppInstaller::CLI // search for a single package to uninstall context << Workflow::SearchSourceForSingle << + Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(true) << Workflow::ReportPackageIdentity; } diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp @@ -71,16 +71,16 @@ namespace AppInstaller::CLI } context << - Workflow::OpenSource << - Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + OpenSource << + OpenCompositeSource(Repository::PredefinedSource::Installed); switch (valueType) { case Execution::Args::Type::Query: context << - Workflow::RequireCompletionWordNonEmpty << - Workflow::SearchSourceForManyCompletion << - Workflow::CompleteWithMatchedField; + RequireCompletionWordNonEmpty << + SearchSourceForManyCompletion << + CompleteWithMatchedField; break; case Execution::Args::Type::Id: case Execution::Args::Type::Name: @@ -89,7 +89,7 @@ namespace AppInstaller::CLI case Execution::Args::Type::Channel: case Execution::Args::Type::Source: context << - Workflow::CompleteWithSingleSemanticsForValueUsingExistingSource(valueType); + CompleteWithSingleSemanticsForValueUsingExistingSource(valueType); break; } } @@ -120,24 +120,33 @@ namespace AppInstaller::CLI { context.SetFlags(Execution::ContextFlag::InstallerExecutionUseUpdate); + // Only allow for source failures when doing a list of available upgrades. + // We have to set it now to allow for source open failures to also just warn. + if (ShouldListUpgrade(context)) + { + context.SetFlags(Execution::ContextFlag::TreatSourceFailuresAsWarning); + } + context << - Workflow::ReportExecutionStage(ExecutionStage::Discovery) << - Workflow::OpenSource << - Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + ReportExecutionStage(ExecutionStage::Discovery) << + OpenSource << + OpenCompositeSource(Repository::PredefinedSource::Installed); if (ShouldListUpgrade(context)) { // Upgrade with no args list packages with updates available context << - Workflow::SearchSourceForMany << - Workflow::EnsureMatchesFromSearchResult(true) << - Workflow::ReportListResult(true); + SearchSourceForMany << + HandleSearchResultFailures << + EnsureMatchesFromSearchResult(true) << + ReportListResult(true); } else if (context.Args.Contains(Execution::Args::Type::All)) { // --all switch updates all packages found context << SearchSourceForMany << + HandleSearchResultFailures << EnsureMatchesFromSearchResult(true) << UpdateAllApplicable; } @@ -159,6 +168,7 @@ namespace AppInstaller::CLI // The remaining case: search for single installed package to update context << SearchSourceForSingle << + HandleSearchResultFailures << EnsureOneMatchFromSearchResult(true) << GetInstalledPackageVersion; diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h @@ -55,6 +55,10 @@ namespace AppInstaller::CLI::Execution InstallerHashMatched = 0x2, InstallerTrusted = 0x4, AgreementsAcceptedByCaller = 0x8, + // Allows a failure in a single source to generate a warning rather than an error. + // TODO: Remove when the source interface is refactored. + TreatSourceFailuresAsWarning = 0x10, + ShowSearchResultsOnPartialFailure = 0x20, }; DEFINE_ENUM_FLAG_OPERATORS(ContextFlag); diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h @@ -200,6 +200,10 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(RetroArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SearchCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SearchCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SearchFailureError); + WINGET_DEFINE_RESOURCE_STRINGID(SearchFailureErrorListMatches); + WINGET_DEFINE_RESOURCE_STRINGID(SearchFailureErrorNoMatches); + WINGET_DEFINE_RESOURCE_STRINGID(SearchFailureWarning); WINGET_DEFINE_RESOURCE_STRINGID(SearchId); WINGET_DEFINE_RESOURCE_STRINGID(SearchMatch); WINGET_DEFINE_RESOURCE_STRINGID(SearchName); diff --git a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp @@ -283,6 +283,7 @@ namespace AppInstaller::CLI::Workflow // Find the single version we want is available searchContext << + Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(false) << Workflow::GetManifestWithVersionFromPackage(packageRequest.VersionAndChannel) << Workflow::GetInstalledPackageVersion << diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -44,6 +44,7 @@ namespace AppInstaller::CLI::Workflow std::shared_ptr<ISource> OpenNamedSource(Execution::Context& context, std::string_view sourceName) { std::shared_ptr<Repository::ISource> source; + try { OpenSourceResult result; @@ -63,12 +64,27 @@ namespace AppInstaller::CLI::Workflow } source = result.Source; + // We'll only report the source update failure as warning and continue for (const auto& s : result.SourcesWithUpdateFailure) { context.Reporter.Warn() << Resource::String::SourceOpenWithFailedUpdate << ' ' << s.Name << std::endl; } } + catch (const wil::ResultException& re) + { + context.Reporter.Error() << Resource::String::SourceOpenFailedSuggestion << std::endl; + if (re.GetErrorCode() == APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES) + { + // Since we know there must have been multiple errors here, just fail the context rather + // than trying to get one of the exceptions back out. + AICLI_TERMINATE_CONTEXT_RETURN(APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES, {}); + } + else + { + throw; + } + } catch (...) { context.Reporter.Error() << Resource::String::SourceOpenFailedSuggestion << std::endl; @@ -224,6 +240,63 @@ namespace AppInstaller::CLI::Workflow m_func(context); } + HRESULT HandleException(Execution::Context& context, std::exception_ptr exception) + { + try + { + std::rethrow_exception(exception); + } + // Exceptions that may occur in the process of executing an arbitrary command + catch (const wil::ResultException& re) + { + // Even though they are logged at their source, log again here for completeness. + Logging::Telemetry().LogException("wil::ResultException", re.what()); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << + GetUserPresentableMessage(re) << std::endl; + return re.GetErrorCode(); + } + catch (const winrt::hresult_error& hre) + { + std::string message = GetUserPresentableMessage(hre); + Logging::Telemetry().LogException("winrt::hresult_error", message); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << + message << std::endl; + return hre.code(); + } + catch (const Settings::GroupPolicyException& e) + { + auto policy = Settings::TogglePolicy::GetPolicy(e.Policy()); + context.Reporter.Error() << Resource::String::DisabledByGroupPolicy << ": "_liv << policy.PolicyName() << std::endl; + return APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY; + } + catch (const Resource::ResourceOpenException& e) + { + Logging::Telemetry().LogException("ResourceOpenException", e.what()); + context.Reporter.Error() << GetUserPresentableMessage(e) << std::endl; + return APPINSTALLER_CLI_ERROR_MISSING_RESOURCE_FILE; + } + catch (const std::exception& e) + { + Logging::Telemetry().LogException("std::exception", e.what()); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << + GetUserPresentableMessage(e) << std::endl; + return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; + } + catch (...) + { + LOG_CAUGHT_EXCEPTION(); + Logging::Telemetry().LogException("unknown", {}); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl; + return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; + } + + return E_UNEXPECTED; + } + void OpenSource(Execution::Context& context) { std::string_view sourceName; @@ -454,6 +527,54 @@ namespace AppInstaller::CLI::Workflow } } + void HandleSearchResultFailures(Execution::Context& context) + { + const auto& searchResult = context.Get<Execution::Data::SearchResult>(); + + if (!searchResult.Failures.empty()) + { + if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::TreatSourceFailuresAsWarning)) + { + auto warn = context.Reporter.Warn(); + for (const auto& failure : searchResult.Failures) + { + warn << Resource::String::SearchFailureWarning << ' ' << failure.Source->GetDetails().Name << std::endl; + } + } + else + { + HRESULT overallHR = S_OK; + auto error = context.Reporter.Error(); + for (const auto& failure : searchResult.Failures) + { + error << Resource::String::SearchFailureError << ' ' << failure.Source->GetDetails().Name << std::endl; + HRESULT failureHR = HandleException(context, failure.Exception); + + // Just take first failure for now + if (overallHR == S_OK) + { + overallHR = failureHR; + } + } + + if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::ShowSearchResultsOnPartialFailure)) + { + if (searchResult.Matches.empty()) + { + context.Reporter.Info() << std::endl << Resource::String::SearchFailureErrorNoMatches << std::endl; + } + else + { + context.Reporter.Info() << std::endl << Resource::String::SearchFailureErrorListMatches << std::endl; + context << ReportMultiplePackageFoundResultWithSource; + } + } + + AICLI_TERMINATE_CONTEXT(overallHR); + } + } + } + void ReportMultiplePackageFoundResult(Execution::Context& context) { auto& searchResult = context.Get<Execution::Data::SearchResult>(); @@ -766,6 +887,7 @@ namespace AppInstaller::CLI::Workflow context << OpenSource << SearchSourceForSingle << + HandleSearchResultFailures << EnsureOneMatchFromSearchResult(false) << GetManifestFromPackage; } diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -56,6 +56,9 @@ namespace AppInstaller::CLI::Workflow std::string m_name; }; + // Helper to report exceptions and return the HRESULT. + HRESULT HandleException(Execution::Context& context, std::exception_ptr exception); + // Creates the source object. // Required Args: None // Inputs: None @@ -164,6 +167,12 @@ namespace AppInstaller::CLI::Workflow bool m_onlyShowUpgrades; }; + // Handles failures in the SearchResult either by warning or failing. + // Required Args: None + // Inputs: SearchResult + // Outputs: None + void HandleSearchResultFailures(Execution::Context& context); + // Outputs the search results when multiple packages found but only one expected. // Required Args: None // Inputs: SearchResult diff --git a/src/AppInstallerCLIE2ETests/BaseCommand.cs b/src/AppInstallerCLIE2ETests/BaseCommand.cs @@ -32,9 +32,10 @@ namespace AppInstallerCLIE2ETests public void ResetTestSource() { TestCommon.RunAICLICommand("source reset", "--force"); - TestCommon.RunAICLICommand("source remove", Constants.DefaultSourceName); + TestCommon.RunAICLICommand("source remove", Constants.DefaultWingetSourceName); + TestCommon.RunAICLICommand("source remove", Constants.DefaultMSStoreSourceName); TestCommon.RunAICLICommand("source add", $"{Constants.TestSourceName} {Constants.TestSourceUrl}"); - Thread.Sleep(5000); + Thread.Sleep(2000); } public void ConfigureFeature(string featureName, bool status) diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs @@ -21,8 +21,10 @@ namespace AppInstallerCLIE2ETests public const string AppInstallerTestCertThumbprint = "d03e7a688b388b1edde8476a627531c49db88017"; // Test Sources - public const string DefaultSourceName = @"winget"; - public const string DefaultSourceUrl = @"https://winget.azureedge.net/cache"; + public const string DefaultWingetSourceName = @"winget"; + public const string DefaultWingetSourceUrl = @"https://winget.azureedge.net/cache"; + public const string DefaultMSStoreSourceName = @"msstore"; + public const string DefaultMSStoreSourceUrl = @"https://storeedgefd.dsx.mp.microsoft.com/v9.0"; public const string TestSourceName = @"TestSource"; public const string TestSourceUrl = @"https://localhost:5001/TestKit"; diff --git a/src/AppInstallerCLIE2ETests/FeaturesCommand.cs b/src/AppInstallerCLIE2ETests/FeaturesCommand.cs @@ -28,7 +28,7 @@ namespace AppInstallerCLIE2ETests { var result = TestCommon.RunAICLICommand("features", ""); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); - Assert.True(result.StdOut.Contains("Microsoft Store Support")); + Assert.True(result.StdOut.Contains("Direct MSI Installation")); Assert.False(result.StdOut.Contains("Enabled")); } @@ -37,7 +37,7 @@ namespace AppInstallerCLIE2ETests { ConfigureFeature("experimentalArg", true); ConfigureFeature("experimentalCmd", true); - ConfigureFeature("experimentalMSStore", true); + ConfigureFeature("directMSI", true); var result = TestCommon.RunAICLICommand("features", ""); Assert.True(result.StdOut.Contains("Enabled")); } diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -134,6 +134,27 @@ namespace AppInstallerCLIE2ETests Assert.False(VerifyTestMsixInstalledAndCleanup()); } + [Test] + public void InstallExeWithAlternateSourceFailure() + { + TestCommon.RunAICLICommand("source add", "failSearch \"{ \"\"SearchHR\"\": \"\"0x80070002\"\" }\" Microsoft.Test.Configurable --header \"{}\""); + + try + { + var installDir = TestCommon.GetRandomTestDir(); + var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestExeInstaller --silent -l {installDir}"); + Assert.AreEqual(unchecked((int)0x80070002), result.ExitCode); + Assert.True(result.StdOut.Contains("Failed when searching source: failSearch")); + Assert.True(result.StdOut.Contains("AppInstallerTest.TestExeInstaller")); + Assert.False(result.StdOut.Contains("Successfully installed")); + Assert.False(VerifyTestExeInstalled(installDir)); + } + finally + { + ResetTestSource(); + } + } + private bool VerifyTestExeInstalled(string installDir, string expectedContent = null) { if (!File.Exists(Path.Combine(installDir, Constants.TestExeInstalledFileName))) diff --git a/src/AppInstallerCLIE2ETests/SearchCommand.cs b/src/AppInstallerCLIE2ETests/SearchCommand.cs @@ -88,5 +88,17 @@ namespace AppInstallerCLIE2ETests Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode); Assert.True(result.StdOut.Contains("No package found matching input criteria.")); } + + [Test] + public void SearchWithSingleSourceFailure() + { + TestCommon.RunAICLICommand("source add", "failSearch \"{ \"\"CreateHR\"\": \"\"0x80070002\"\" }\" Microsoft.Test.Configurable --header \"{}\""); + + var result = TestCommon.RunAICLICommand("search", "--exact AppInstallerTest.TestExampleInstaller"); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + Assert.True(result.StdOut.Contains("Failed when searching source; results will not be included: failSearch")); + Assert.True(result.StdOut.Contains("TestExampleInstaller")); + Assert.True(result.StdOut.Contains("AppInstallerTest.TestExampleInstaller")); + } } } \ No newline at end of file diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -574,7 +574,7 @@ They can be configured through the settings file 'winget settings'.</value> <value>Name of the source</value> </data> <data name="SourceOpenFailedSuggestion" xml:space="preserve"> - <value>Failed to open the source; try the 'source reset' command if the problem persists.</value> + <value>Failed when opening source(s); try the 'source reset' command if the problem persists.</value> <comment>{Locked="source reset"}</comment> </data> <data name="SourceOpenPredefinedFailedSuggestion" xml:space="preserve"> @@ -1078,6 +1078,12 @@ Do you agree to the terms?</value> <data name="ShowLabelReleaseNotesUrl" xml:space="preserve"> <value>Release Notes Url:</value> </data> + <data name="SearchFailureWarning" xml:space="preserve"> + <value>Failed when searching source; results will not be included:</value> + </data> + <data name="SearchFailureError" xml:space="preserve"> + <value>Failed when searching source:</value> + </data> <data name="FeatureDisabledByAdminSettingMessage" xml:space="preserve"> <value>This feature needs to be enabled by administrators. To enable it, run 'winget settings --enable %1' as administrator.</value> <comment>{Locked="winget settings --enable %1"} The value will be replaced with the admin setting</comment> @@ -1151,4 +1157,12 @@ Do you agree to the terms?</value> <data name="InstallerLogAvailable" xml:space="preserve"> <value>Installer log is available at:</value> </data> + <data name="SearchFailureErrorListMatches" xml:space="preserve"> + <value>The following packages were found among the working sources. +Please specify one of them using the `--source` option to proceed.</value> + <comment>{Locked="--source"}</comment> + </data> + <data name="SearchFailureErrorNoMatches" xml:space="preserve"> + <value>No packages were found among the working sources.</value> + </data> </root> \ No newline at end of file diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp @@ -646,3 +646,129 @@ TEST_CASE("CompositeSource_IsSame", "[CompositeSource]") REQUIRE(result1.Matches[0].Package->IsSame(result2.Matches[0].Package.get())); } + +TEST_CASE("CompositeSource_AvailableSearchFailure", "[CompositeSource]") +{ + HRESULT expectedHR = E_BLUETOOTH_ATT_ATTRIBUTE_NOT_FOUND; + std::string pfn = "sortof_apfn"; + + std::shared_ptr<ComponentTestSource> AvailableSucceeds = std::make_shared<ComponentTestSource>(); + AvailableSucceeds->SearchFunction = [&](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); + return result; + }; + + std::shared_ptr<ComponentTestSource> AvailableFails = std::make_shared<ComponentTestSource>(); + AvailableFails->SearchFunction = [&](const SearchRequest&) -> SearchResult { THROW_HR(expectedHR); }; + AvailableFails->Details.Name = "The one that fails"; + + CompositeSource Composite("*CompositeSource_AvailableSearchFailure"); + Composite.AddAvailableSource(AvailableSucceeds); + Composite.AddAvailableSource(AvailableFails); + + SearchResult result = Composite.Search({}); + + REQUIRE(result.Matches.size() == 1); + + auto pfns = result.Matches[0].Package->GetLatestAvailableVersion()->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); + REQUIRE(pfns.size() == 1); + REQUIRE(pfns[0] == pfn); + + REQUIRE(result.Failures.size() == 1); + REQUIRE(result.Failures[0].Source->GetDetails().Name == AvailableFails->Details.Name); + + HRESULT searchFailure = S_OK; + try + { + std::rethrow_exception(result.Failures[0].Exception); + } + catch (const wil::ResultException& re) + { + searchFailure = re.GetErrorCode(); + } + catch (...) {} + + REQUIRE(searchFailure == expectedHR); +} + +TEST_CASE("CompositeSource_InstalledToAvailableCorrelationSearchFailure", "[CompositeSource]") +{ + HRESULT expectedHR = E_BLUETOOTH_ATT_ATTRIBUTE_NOT_LONG; + std::string pfn = "sortof_apfn"; + + CompositeTestSetup setup; + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + setup.Available->Everything.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); + + std::shared_ptr<ComponentTestSource> AvailableFails = std::make_shared<ComponentTestSource>(); + AvailableFails->SearchFunction = [&](const SearchRequest&) -> SearchResult { THROW_HR(expectedHR); }; + AvailableFails->Details.Name = "The one that fails"; + + setup.Composite.AddAvailableSource(AvailableFails); + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 1); + + REQUIRE(result.Failures.size() == 1); + REQUIRE(result.Failures[0].Source->GetDetails().Name == AvailableFails->Details.Name); + + HRESULT searchFailure = S_OK; + try + { + std::rethrow_exception(result.Failures[0].Exception); + } + catch (const wil::ResultException& re) + { + searchFailure = re.GetErrorCode(); + } + catch (...) {} + + REQUIRE(searchFailure == expectedHR); +} + +TEST_CASE("CompositeSource_InstalledAvailableSearchFailure", "[CompositeSource]") +{ + HRESULT expectedHR = E_BLUETOOTH_ATT_ATTRIBUTE_NOT_LONG; + std::string pfn = "sortof_apfn"; + + CompositeTestSetup setup; + setup.Available->SearchFunction = [&](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); + return result; + }; + + std::shared_ptr<ComponentTestSource> AvailableFails = std::make_shared<ComponentTestSource>(); + AvailableFails->SearchFunction = [&](const SearchRequest&) -> SearchResult { THROW_HR(expectedHR); }; + AvailableFails->Details.Name = "The one that fails"; + + setup.Composite.AddAvailableSource(AvailableFails); + + setup.Composite.SetInstalledSource(setup.Installed, CompositeSearchBehavior::AvailablePackages); + + SearchRequest request; + request.Query = RequestMatch{ MatchType::Exact, "whatever" }; + SearchResult result = setup.Composite.Search(request); + + REQUIRE(result.Matches.size() == 1); + + REQUIRE(result.Failures.size() == 1); + REQUIRE(result.Failures[0].Source->GetDetails().Name == AvailableFails->Details.Name); + + HRESULT searchFailure = S_OK; + try + { + std::rethrow_exception(result.Failures[0].Exception); + } + catch (const wil::ResultException& re) + { + searchFailure = re.GetErrorCode(); + } + catch (...) {} + + REQUIRE(searchFailure == expectedHR); +} diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp @@ -24,6 +24,8 @@ using namespace AppInstaller::Utility; using namespace std::string_literals; using namespace std::string_view_literals; +constexpr size_t c_DefaultSourceCount = 2; + constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; constexpr std::string_view s_SourcesYaml_Source_Type = "Type"sv; @@ -35,13 +37,18 @@ constexpr std::string_view s_EmptySources = R"( Sources: )"sv; -constexpr std::string_view s_DefaultSourceTombstoned = R"( +constexpr std::string_view s_DefaultSourcesTombstoned = R"( Sources: - Name: winget Type: "" Arg: "" Data: "" IsTombstone: true + - Name: msstore + Type: "" + Arg: "" + Data: "" + IsTombstone: true )"sv; constexpr std::string_view s_SingleSource = R"( @@ -75,6 +82,11 @@ Sources: Arg: "" Data: "" IsTombstone: true + - Name: msstore + Type: "" + Arg: "" + Data: "" + IsTombstone: true )"sv; constexpr std::string_view s_ThreeSourcesMetadata = R"( @@ -156,6 +168,44 @@ namespace return result; } }; + + // Failing source for use with s_TwoSource_AggregateSourceTest + struct FailingSourcesTestSource : public TestSource + { + static constexpr HRESULT FailingHR = 0xBADDAD0D; + + FailingSourcesTestSource() = default; + FailingSourcesTestSource(const SourceDetails& details) + { + Details = details; + } + + static std::shared_ptr<ISource> CreateFailWinget(const SourceDetails& details) + { + if (details.Name == "winget") + { + THROW_HR(FailingHR); + } + + return std::shared_ptr<ISource>(new FailingSourcesTestSource(details)); + } + + static std::shared_ptr<ISource> CreateFailAll(const SourceDetails&) + { + THROW_HR(FailingHR); + } + }; + + void RequireDefaultSourcesAt(const std::vector<SourceDetails>& sources, size_t index) + { + REQUIRE(sources.size() >= index + c_DefaultSourceCount); + + for (size_t i = index; i < sources.size(); ++i) + { + INFO(i); + REQUIRE(sources[i].Origin == SourceOrigin::Default); + } + } } @@ -164,8 +214,8 @@ TEST_CASE("RepoSources_UserSettingDoesNotExist", "[sources]") RemoveSetting(Streams::UserSources); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); - REQUIRE(sources[0].Origin == SourceOrigin::Default); + REQUIRE(sources.size() == c_DefaultSourceCount); + RequireDefaultSourcesAt(sources, 0); } TEST_CASE("RepoSources_EmptySourcesList", "[sources]") @@ -173,13 +223,13 @@ TEST_CASE("RepoSources_EmptySourcesList", "[sources]") SetSetting(Streams::UserSources, s_EmptySources); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); - REQUIRE(sources[0].Origin == SourceOrigin::Default); + REQUIRE(sources.size() == c_DefaultSourceCount); + RequireDefaultSourcesAt(sources, 0); } -TEST_CASE("RepoSources_DefaultSourceTombstoned", "[sources]") +TEST_CASE("RepoSources_DefaultSourcesTombstoned", "[sources]") { - SetSetting(Streams::UserSources, s_DefaultSourceTombstoned); + SetSetting(Streams::UserSources, s_DefaultSourcesTombstoned); std::vector<SourceDetails> sources = GetSources(); REQUIRE(sources.empty()); @@ -188,9 +238,10 @@ TEST_CASE("RepoSources_DefaultSourceTombstoned", "[sources]") TEST_CASE("RepoSources_SingleSource", "[sources]") { SetSetting(Streams::UserSources, s_SingleSource); + RemoveSetting(Streams::SourcesMetadata); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[0].Name == "testName"); REQUIRE(sources[0].Type == "testType"); @@ -199,7 +250,7 @@ TEST_CASE("RepoSources_SingleSource", "[sources]") REQUIRE(sources[0].Origin == SourceOrigin::User); REQUIRE(sources[0].LastUpdateTime == ConvertUnixEpochToSystemClock(0)); - REQUIRE(sources[1].Origin == SourceOrigin::Default); + RequireDefaultSourcesAt(sources, 1); } TEST_CASE("RepoSources_ThreeSources", "[sources]") @@ -260,7 +311,7 @@ TEST_CASE("RepoSources_AddSource", "[sources]") REQUIRE(addCalledOnFactory); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[0].Name == details.Name); REQUIRE(sources[0].Type == details.Type); @@ -269,7 +320,7 @@ TEST_CASE("RepoSources_AddSource", "[sources]") REQUIRE(sources[0].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); REQUIRE(sources[0].Origin == SourceOrigin::User); - REQUIRE(sources[1].Origin == SourceOrigin::Default); + RequireDefaultSourcesAt(sources, 1); } TEST_CASE("RepoSources_AddMultipleSources", "[sources]") @@ -292,7 +343,7 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") AddSource(details, progress); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[0].Name == details.Name); REQUIRE(sources[0].Type == details.Type); @@ -301,7 +352,7 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") REQUIRE(sources[0].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); REQUIRE(sources[0].Origin == SourceOrigin::User); - REQUIRE(sources[1].Origin == SourceOrigin::Default); + RequireDefaultSourcesAt(sources, 1); SourceDetails details2; details2.Name = details.Name + suffix[1]; @@ -315,7 +366,7 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") AddSource(details2, progress); sources = GetSources(); - REQUIRE(sources.size() == 3); + REQUIRE(sources.size() == c_DefaultSourceCount + 2); for (size_t i = 0; i < 2; ++i) { @@ -328,7 +379,7 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") REQUIRE(sources[i].Origin == SourceOrigin::User); } - REQUIRE(sources[2].Origin == SourceOrigin::Default); + RequireDefaultSourcesAt(sources, 2); } TEST_CASE("RepoSources_UpdateSource", "[sources]") @@ -355,7 +406,7 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]") REQUIRE(addCalledOnFactory); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[0].Name == details.Name); REQUIRE(sources[0].Type == details.Type); @@ -364,7 +415,7 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]") REQUIRE(sources[0].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); REQUIRE(sources[0].Origin == SourceOrigin::User); - REQUIRE(sources[1].Origin == SourceOrigin::Default); + RequireDefaultSourcesAt(sources, 1); // Reset for a call to update bool updateCalledOnFactory = false; @@ -376,7 +427,7 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]") REQUIRE(updateCalledOnFactory); sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[0].Name == details.Name); REQUIRE(sources[0].Type == details.Type); @@ -443,14 +494,14 @@ TEST_CASE("RepoSources_RemoveSource", "[sources]") AddSource(details, progress); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); RemoveSource(details.Name, progress); REQUIRE(removeCalledOnFactory); sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); } TEST_CASE("RepoSources_RemoveDefaultSource", "[sources]") @@ -459,7 +510,7 @@ TEST_CASE("RepoSources_RemoveDefaultSource", "[sources]") TestHook_ClearSourceFactoryOverrides(); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); REQUIRE(sources[0].Origin == SourceOrigin::Default); bool removeCalledOnFactory = false; @@ -474,7 +525,7 @@ TEST_CASE("RepoSources_RemoveDefaultSource", "[sources]") REQUIRE(removeCalledOnFactory); sources = GetSources(); - REQUIRE(sources.empty()); + REQUIRE(sources.size() == c_DefaultSourceCount - 1); } TEST_CASE("RepoSources_UpdateOnOpen", "[sources]") @@ -501,7 +552,7 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]") REQUIRE(updateCalledOnFactory); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[0].Name == name); REQUIRE(sources[0].Type == type); @@ -547,7 +598,7 @@ TEST_CASE("RepoSources_DropAllSources", "[sources]") DropSource({}); sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); REQUIRE(sources[0].Origin == SourceOrigin::Default); } @@ -598,7 +649,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SetSetting(Streams::UserSources, s_EmptySources); auto sources = GetSources(); - REQUIRE(sources.empty()); + REQUIRE(sources.size() == c_DefaultSourceCount - 1); } SECTION("Add default source") { @@ -620,7 +671,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SetSetting(Streams::UserSources, s_DefaultSourceAsUserSource); auto sources = GetSources(); - REQUIRE(sources.empty()); + REQUIRE(sources.size() == c_DefaultSourceCount - 1); } SECTION("Add same-name source from user") { @@ -646,7 +697,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") REQUIRE(addCalledOnFactory); auto sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); REQUIRE(sources[0].Name == details.Name); REQUIRE(sources[0].Type == details.Type); @@ -662,7 +713,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SetSetting(Streams::UserSources, s_UserSourceNamedLikeDefault); auto sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); REQUIRE(sources[0].Name == "winget"); REQUIRE(sources[0].Type == "testType"); @@ -690,7 +741,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SECTION("Tombstone is overridden") { // We should ignore if the default source was already deleted. - SetSetting(Streams::UserSources, s_DefaultSourceTombstoned); + SetSetting(Streams::UserSources, s_DefaultSourcesTombstoned); auto sources = GetSources(); REQUIRE(sources.size() == 1); @@ -703,11 +754,11 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SetSetting(Streams::UserSources, s_UserSourceNamedLikeDefault); auto sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); - REQUIRE(sources[0].Name == "winget"); - REQUIRE(sources[0].Arg == "https://winget.azureedge.net/cache"); - REQUIRE(sources[0].Origin == SourceOrigin::Default); + REQUIRE(sources[1].Name == "winget"); + REQUIRE(sources[1].Arg == "https://winget.azureedge.net/cache"); + REQUIRE(sources[1].Origin == SourceOrigin::Default); } } } @@ -741,7 +792,7 @@ TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") auto sources = GetSources(); // The source list includes the default source - REQUIRE(sources.size() == policySources.size() + 1); + REQUIRE(sources.size() == policySources.size() + c_DefaultSourceCount); REQUIRE(sources.back().Origin == SourceOrigin::Default); for (size_t i = 0; i < policySources.size(); ++i) @@ -770,7 +821,7 @@ TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") auto sources = GetSources(); // The source list includes the default source - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[1].Origin == SourceOrigin::Default); REQUIRE(sources[0].Name == policySource.Name); @@ -813,7 +864,7 @@ TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") auto sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); REQUIRE(sources[0].Name == policySource.Name); REQUIRE(sources[0].Type == policySource.Type); REQUIRE(sources[0].Arg == policySource.Arg); @@ -866,7 +917,7 @@ TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") // The source list includes the default source auto sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == c_DefaultSourceCount + 1); REQUIRE(sources[1].Origin == SourceOrigin::Default); REQUIRE(sources[0].Name == policySource.Name); @@ -929,7 +980,7 @@ TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") REQUIRE_FALSE(addCalledOnFactory); auto sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); REQUIRE(sources[0].Origin == SourceOrigin::Default); } SECTION("Existing sources are ignored") @@ -937,8 +988,52 @@ TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") SetSetting(Streams::UserSources, s_SingleSource); auto sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == c_DefaultSourceCount); REQUIRE(sources[0].Origin == SourceOrigin::Default); } } } + +TEST_CASE("RepoSources_OpenMultipleWithSingleFailure", "[sources]") +{ + TestHook_ClearSourceFactoryOverrides(); + TestSourceFactory factory{ FailingSourcesTestSource::CreateFailWinget }; + TestHook_SetSourceFactoryOverride("testType", factory); + + SetSetting(Streams::UserSources, s_TwoSource_AggregateSourceTest); + + ProgressCallback progress; + auto result = OpenSource("", progress); + + REQUIRE(result.Source); + REQUIRE(result.SourcesWithUpdateFailure.empty()); + + SearchResult searchResult = result.Source->Search({}); + + REQUIRE(searchResult.Failures.size() == 1); + + HRESULT openFailure = S_OK; + try + { + std::rethrow_exception(searchResult.Failures[0].Exception); + } + catch (const wil::ResultException& re) + { + openFailure = re.GetErrorCode(); + } + catch (...) {} + + REQUIRE(openFailure == FailingSourcesTestSource::FailingHR); +} + +TEST_CASE("RepoSources_OpenMultipleWithTotalFailure", "[sources]") +{ + TestHook_ClearSourceFactoryOverrides(); + TestSourceFactory factory{ FailingSourcesTestSource::CreateFailAll }; + TestHook_SetSourceFactoryOverride("testType", factory); + + SetSetting(Streams::UserSources, s_TwoSource_AggregateSourceTest); + + ProgressCallback progress; + REQUIRE_THROWS_HR(OpenSource("", progress), APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES); +} diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj @@ -259,6 +259,11 @@ <SubSystem>Windows</SubSystem> </Link> </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(WingetDisableTestHooks)'=='true'"> + <ClCompile> + <PreprocessorDefinitions>AICLI_DISABLE_TEST_HOOKS;%(PreprocessorDefinitions)</PreprocessorDefinitions> + </ClCompile> + </ItemDefinitionGroup> <ItemGroup> <ClInclude Include="DODownloader.h" /> <ClInclude Include="Public\winget\AdminSettings.h" /> diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -221,7 +221,7 @@ namespace AppInstaller::Logging AICLI_LOG(CLI, Error, << "Terminating context: 0x" << SetHRFormat << hr << " at " << file << ":" << line); } - void TelemetryTraceLogger::LogException(std::string_view commandName, std::string_view type, std::string_view message) const noexcept + void TelemetryTraceLogger::LogException(std::string_view type, std::string_view message) const noexcept { if (IsTelemetryEnabled()) { @@ -230,7 +230,6 @@ namespace AppInstaller::Logging AICLI_TraceLoggingWriteActivity( "Exception", TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), - AICLI_TraceLoggingStringView(commandName, "Command"), AICLI_TraceLoggingStringView(type, "Type"), AICLI_TraceLoggingWStringView(anonMessage, "Message"), TraceLoggingUInt32(s_executionStage, "ExecutionStage"), diff --git a/src/AppInstallerCommonCore/ExperimentalFeature.cpp b/src/AppInstallerCommonCore/ExperimentalFeature.cpp @@ -17,14 +17,6 @@ namespace AppInstaller::Settings return true; } - // Even if all experimental features are disabled, if the store policy is enabled then override it. - if (feature == ExperimentalFeature::Feature::ExperimentalMSStore && - GroupPolicies().GetState(TogglePolicy::Policy::MSStoreSource) == PolicyState::Enabled) - { - // Force enable the feature - return true; - } - if (!GroupPolicies().IsEnabled(TogglePolicy::Policy::ExperimentalFeatures)) { AICLI_LOG(Core, Info, << @@ -41,8 +33,6 @@ namespace AppInstaller::Settings return userSettings.Get<Setting::EFExperimentalCmd>() || userSettings.Get<Setting::EFExperimentalArg>(); case ExperimentalFeature::Feature::ExperimentalArg: return userSettings.Get<Setting::EFExperimentalArg>(); - case ExperimentalFeature::Feature::ExperimentalMSStore: - return userSettings.Get<Setting::EFExperimentalMSStore>(); case ExperimentalFeature::Feature::Dependencies: return userSettings.Get<Setting::EFDependencies>(); case ExperimentalFeature::Feature::DirectMSI: @@ -73,8 +63,6 @@ namespace AppInstaller::Settings return ExperimentalFeature{ "Command Sample", "experimentalCmd", "https://aka.ms/winget-settings", Feature::ExperimentalCmd }; case Feature::ExperimentalArg: return ExperimentalFeature{ "Argument Sample", "experimentalArg", "https://aka.ms/winget-settings", Feature::ExperimentalArg }; - case Feature::ExperimentalMSStore: - return ExperimentalFeature{ "Microsoft Store Support", "experimentalMSStore", "https://aka.ms/winget-settings", Feature::ExperimentalMSStore }; case Feature::Dependencies: return ExperimentalFeature{ "Show Dependencies Information", "dependencies", "https://aka.ms/winget-settings", Feature::Dependencies }; case Feature::DirectMSI: diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -87,6 +87,7 @@ #define APPINSTALLER_CLI_ERROR_MISSING_RESOURCE_FILE ((HRESULT)0x8A150048) #define APPINSTALLER_CLI_ERROR_MSI_INSTALL_FAILED ((HRESULT)0x8A150049) #define APPINSTALLER_CLI_ERROR_INVALID_MSIEXEC_ARGUMENT ((HRESULT)0x8A15004A) +#define APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES ((HRESULT)0x8A15004B) #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) #define APPINSTALLER_CLI_ERROR_INSTALL_INSTALL_IN_PROGRESS ((HRESULT)0x8A150102) diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -58,7 +58,7 @@ namespace AppInstaller::Logging void LogCommandTermination(HRESULT hr, std::string_view file, size_t line) const noexcept; // Logs the invoked command termination. - void LogException(std::string_view commandName, std::string_view type, std::string_view message) const noexcept; + void LogException(std::string_view type, std::string_view message) const noexcept; // Logs whether the manifest used in workflow is local void LogIsManifestLocal(bool isLocalManifest) const noexcept; diff --git a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h @@ -20,10 +20,9 @@ namespace AppInstaller::Settings enum class Feature : unsigned { None = 0x0, - ExperimentalMSStore = 0x1, - Dependencies = 0x2, + Dependencies = 0x1, // Before making DirectMSI non-experimental, it should be part of manifest validation. - DirectMSI = 0x4, + DirectMSI = 0x2, Max, // This MUST always be after all experimental features // Features listed after Max will not be shown with the features command diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -67,7 +67,6 @@ namespace AppInstaller::Settings AutoUpdateTimeInMinutes, EFExperimentalCmd, EFExperimentalArg, - EFExperimentalMSStore, EFDependencies, TelemetryDisable, InstallScopePreference, @@ -114,7 +113,6 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION_POLICY(Setting::AutoUpdateTimeInMinutes, uint32_t, std::chrono::minutes, 5min, ".source.autoUpdateIntervalInMinutes"sv, ValuePolicy::SourceAutoUpdateIntervalInMinutes); SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalCmd, bool, bool, false, ".experimentalFeatures.experimentalCmd"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalArg, bool, bool, false, ".experimentalFeatures.experimentalArg"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalMSStore, bool, bool, false, ".experimentalFeatures.experimentalMSStore"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFDependencies, bool, bool, false, ".experimentalFeatures.dependencies"sv); SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, ScopePreference, ScopePreference::User, ".installBehavior.preferences.scope"sv); diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp @@ -224,7 +224,6 @@ namespace AppInstaller::Settings WINGET_VALIDATE_PASS_THROUGH(EFExperimentalCmd) WINGET_VALIDATE_PASS_THROUGH(EFExperimentalArg) - WINGET_VALIDATE_PASS_THROUGH(EFExperimentalMSStore) WINGET_VALIDATE_PASS_THROUGH(EFDependencies) WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable) WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI) diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj @@ -223,6 +223,11 @@ <SubSystem Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Windows</SubSystem> </Link> </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(WingetDisableTestHooks)'=='true'"> + <ClCompile> + <PreprocessorDefinitions>AICLI_DISABLE_TEST_HOOKS;%(PreprocessorDefinitions)</PreprocessorDefinitions> + </ClCompile> + </ItemDefinitionGroup> <ItemGroup> <ClInclude Include="CompositeSource.h" /> <ClInclude Include="ICU\SQLiteICU.h" /> @@ -259,6 +264,7 @@ <ClInclude Include="Microsoft\Schema\Version.h" /> <ClInclude Include="Microsoft\SQLiteIndex.h" /> <ClInclude Include="Microsoft\SQLiteIndexSource.h" /> + <ClInclude Include="Microsoft\ConfigurableTestSourceFactory.h" /> <ClInclude Include="pch.h" /> <ClInclude Include="Rest\RestClient.h" /> <ClInclude Include="Rest\RestSource.h" /> @@ -296,6 +302,7 @@ <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|x64'">NotUsing</PrecompiledHeader> </ClCompile> <ClCompile Include="Microsoft\ARPHelper.cpp" /> + <ClCompile Include="Microsoft\ConfigurableTestSourceFactory.cpp" /> <ClCompile Include="Microsoft\PredefinedInstalledSourceFactory.cpp" /> <ClCompile Include="Microsoft\PredefinedWriteableSourceFactory.cpp" /> <ClCompile Include="Microsoft\PreIndexedPackageSourceFactory.cpp" /> diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters @@ -231,6 +231,9 @@ <ClInclude Include="Rest\Schema\1_1\Json\SearchRequestSerializer.h"> <Filter>Rest\Schema\1_1\Json</Filter> </ClInclude> + <ClInclude Include="Microsoft\ConfigurableTestSourceFactory.h"> + <Filter>Microsoft</Filter> + </ClInclude> </ItemGroup> <ItemGroup> <ClCompile Include="pch.cpp"> @@ -356,6 +359,9 @@ <ClCompile Include="Rest\Schema\1_1\Json\SearchRequestSerializer_1_1.cpp"> <Filter>Rest\Schema\1_1\Json</Filter> </ClCompile> + <ClCompile Include="Microsoft\ConfigurableTestSourceFactory.cpp"> + <Filter>Microsoft</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -326,11 +326,32 @@ namespace AppInstaller::Repository result.Truncated = Truncated; + result.Failures = std::move(Failures); + return result; } + bool AddFailureIfSourceNotPresent(SearchResult::Failure&& failure) + { + auto itr = std::find_if(Failures.begin(), Failures.end(), + [&failure](const SearchResult::Failure& present) { + const auto& presentDetails = present.Source->GetDetails(); + const auto& incomingDetails = failure.Source->GetDetails(); + return (presentDetails.Identifier == incomingDetails.Identifier && presentDetails.Type == incomingDetails.Type); + }); + + if (itr == Failures.end()) + { + Failures.emplace_back(std::move(failure)); + return true; + } + + return false; + } + std::vector<CompositeResultMatch> Matches; bool Truncated = false; + std::vector<SearchResult::Failure> Failures; private: void AddSystemReferenceStrings(IPackageVersion* version, PackageData& data) @@ -440,7 +461,7 @@ namespace AppInstaller::Repository // only in the Installed source, but do not have an AvailableVersion. if (m_searchBehavior == CompositeSearchBehavior::AllPackages || m_searchBehavior == CompositeSearchBehavior::Installed) { - // Search installed source + // Search installed source (allow exceptions out as we own the installed source) SearchResult installedResult = m_installedSource->Search(request); result.Truncated = installedResult.Truncated; @@ -467,7 +488,32 @@ namespace AppInstaller::Repository // Search sources and add to result for (const auto& source : m_availableSources) { - SearchResult availableResult = source->Search(systemReferenceSearch); + // Do not attempt to correlate local packages against this source + if (!source->GetDetails().SupportCorrelation) + { + continue; + } + + SearchResult availableResult; + + try + { + availableResult = source->Search(systemReferenceSearch); + } + catch (...) + { + if (result.AddFailureIfSourceNotPresent({ source, std::current_exception() })) + { + LOG_CAUGHT_EXCEPTION(); + AICLI_LOG(Repo, Warning, << "Failed to search source for correlation: " << source->GetDetails().Name); + } + } + + // Move failures into the single result + for (SearchResult::Failure& failure : availableResult.Failures) + { + result.AddFailureIfSourceNotPresent(std::move(failure)); + } if (availableResult.Matches.empty()) { @@ -532,7 +578,24 @@ namespace AppInstaller::Repository // Search available sources for (const auto& source : m_availableSources) { - auto availableResult = source->Search(request); + SearchResult availableResult; + + try + { + availableResult = source->Search(request); + } + catch (...) + { + LOG_CAUGHT_EXCEPTION(); + AICLI_LOG(Repo, Warning, << "Failed to search source: " << source->GetDetails().Name); + result.AddFailureIfSourceNotPresent({ source, std::current_exception() }); + } + + // Move failures into the single result + for (auto& failure : availableResult.Failures) + { + result.AddFailureIfSourceNotPresent(std::move(failure)); + } for (auto&& match : availableResult.Matches) { @@ -551,6 +614,7 @@ namespace AppInstaller::Repository srs.AddToFilters(systemReferenceSearch.Inclusions); } + // Correlate against installed (allow exceptions out as we own the installed source) SearchResult installedCrossRef = m_installedSource->Search(systemReferenceSearch); for (auto&& crossRef : installedCrossRef.Matches) @@ -590,13 +654,22 @@ namespace AppInstaller::Repository // Search available sources for (const auto& source : m_availableSources) { - auto oneSourceResult = source->Search(request); + SearchResult oneSourceResult; - // Move all matches into the single result - for (auto&& match : oneSourceResult.Matches) + try + { + oneSourceResult = source->Search(request); + } + catch (...) { - result.Matches.emplace_back(std::move(match)); + LOG_CAUGHT_EXCEPTION(); + AICLI_LOG(Repo, Warning, << "Failed to search source: " << source->GetDetails().Name); + result.Failures.emplace_back(SearchResult::Failure{ source, std::current_exception() }); } + + // Move into the single result + std::move(oneSourceResult.Matches.begin(), oneSourceResult.Matches.end(), std::back_inserter(result.Matches)); + std::move(oneSourceResult.Failures.begin(), oneSourceResult.Failures.end(), std::back_inserter(result.Failures)); } SortResultMatches(result.Matches); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.h b/src/AppInstallerRepositoryCore/CompositeSource.h @@ -43,6 +43,9 @@ namespace AppInstaller::Repository // Adds an available source to be aggregated. void AddAvailableSource(std::shared_ptr<ISource> source); + // Checks if any available sources are present + bool HasAvailableSource() const { return !m_availableSources.empty(); } + // Sets the installed source to be composited. void SetInstalledSource(std::shared_ptr<ISource> source, CompositeSearchBehavior searchBehavior = CompositeSearchBehavior::Installed); diff --git a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "Microsoft/ConfigurableTestSourceFactory.h" + +#include <json.h> + +using namespace std::string_literals; +using namespace std::string_view_literals; + +namespace AppInstaller::Repository::Microsoft +{ + namespace + { + static constexpr std::string_view s_PreIndexedPackageSourceFactory_PackageFileName = "source.msix"sv; + + // The configuration defined for a source. + // This can be added to as new scenarios are needed for testing. + struct TestSourceConfiguration + { + TestSourceConfiguration(const std::string& config) + { + Json::Value root; + Json::CharReaderBuilder builder; + const std::unique_ptr<Json::CharReader> reader(builder.newCharReader()); + + std::string error; + + if (reader->parse(config.c_str(), config.c_str() + config.size(), &root, &error)) + { + // TODO: If this becomes more dynamic, refactor the UserSettings code to make it easier to leverage here + ParseHR(root, ".CreateHR", CreateHR); + ParseHR(root, ".SearchHR", SearchHR); + } + else + { + AICLI_LOG(Repo, Error, << "Error parsing test source config: " << error); + THROW_HR_MSG(E_INVALIDARG, "%S", error.c_str()); + } + } + + // The HR to throw on Factory::Create (if FAILED) + HRESULT CreateHR = S_OK; + + // The HR to throw on Source::Search (if FAILED) + HRESULT SearchHR = S_OK; + + private: + static void ParseHR(const Json::Value& root, const std::string& path, HRESULT& hr) + { + const Json::Path jsonPath(path); + Json::Value node = jsonPath.resolve(root); + if (!node.isNull()) + { + if (node.isIntegral()) + { + hr = node.asInt(); + } + else if (node.isString()) + { + hr = static_cast<HRESULT>(std::strtoll(node.asString().c_str(), nullptr, 0)); + } + } + } + }; + + // The configurable source itself. + struct ConfigurableTestSource : public ISource + { + ConfigurableTestSource(const SourceDetails& details, const TestSourceConfiguration& config) : + m_details(details), m_config(config) {} + + const SourceDetails& GetDetails() const override { return m_details; } + + const std::string& GetIdentifier() const override { return m_details.Identifier; } + + SearchResult Search(const SearchRequest&) const override + { + THROW_IF_FAILED(m_config.SearchHR); + return {}; + } + + private: + SourceDetails m_details; + TestSourceConfiguration m_config; + }; + + // The actual factory implementation. + struct ConfigurableTestSourceFactoryImpl : public ISourceFactory + { + std::shared_ptr<ISource> Create(const SourceDetails& details, IProgressCallback&) override final + { + // Allow the custom header to override the arg (enables `source add` with FAILED(CreateHR)) + TestSourceConfiguration config{ details.CustomHeader.value_or(details.Arg) }; + THROW_IF_FAILED(config.CreateHR); + return std::make_shared<ConfigurableTestSource>(details, config); + } + + bool Add(SourceDetails& details, IProgressCallback&) override final + { + // Attempt to parse the configuration so that we can fail at the appropriate point + TestSourceConfiguration config{ details.Arg }; + return true; + } + + bool Update(const SourceDetails&, IProgressCallback&) override final + { + return true; + } + + bool BackgroundUpdate(const SourceDetails&, IProgressCallback&) override final + { + return true; + } + + bool Remove(const SourceDetails&, IProgressCallback&) override final + { + return true; + } + }; + } + + std::unique_ptr<ISourceFactory> ConfigurableTestSourceFactory::Create() + { + return std::make_unique<ConfigurableTestSourceFactoryImpl>(); + } +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.h b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.h @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "Public/AppInstallerRepositorySource.h" +#include "SourceFactory.h" + +#include <string_view> + +namespace AppInstaller::Repository::Microsoft +{ + using namespace std::string_view_literals; + + // A source for use in manual or E2E tests that can be configured to fail as needed. + struct ConfigurableTestSourceFactory + { + // Get the type string for this source. + static constexpr std::string_view Type() + { + using namespace std::string_view_literals; + return "Microsoft.Test.Configurable"sv; + } + + // Creates a source factory for this type. + static std::unique_ptr<ISourceFactory> Create(); + }; +} diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h @@ -259,11 +259,21 @@ namespace AppInstaller::Repository // Search result data. struct SearchResult { + // Contains a failure from the Search. + struct Failure + { + std::shared_ptr<const ISource> Source; + std::exception_ptr Exception; + }; + // The full set of results from the search. std::vector<ResultMatch> Matches; // If true, the results were truncated by the given SearchRequest::MaximumResults. bool Truncated = false; + + // Present if the Search was against a composite source and one failed, but not limited to that scenario. + std::vector<Failure> Failures; }; struct UnsupportedRequestException : public wil::ResultException diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h @@ -95,14 +95,14 @@ namespace AppInstaller::Repository // The trust level of the source SourceTrustLevel TrustLevel = SourceTrustLevel::None; - // Whether the source behavior has restrictions - bool Restricted = false; - // Custom header for Rest sources std::optional<std::string> CustomHeader; // Source information containing source agreements, required/unsupported match fields. SourceInformation Information; + + // Support correlation against this source if true. + bool SupportCorrelation = true; }; // Fields that require user agreements. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -10,6 +10,10 @@ #include "Microsoft/PreIndexedPackageSourceFactory.h" #include "Rest/RestSourceFactory.h" +#ifndef AICLI_DISABLE_TEST_HOOKS +#include "Microsoft/ConfigurableTestSourceFactory.h" +#endif + #include <winget/GroupPolicy.h> namespace AppInstaller::Repository @@ -38,12 +42,7 @@ namespace AppInstaller::Repository constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; constexpr std::string_view s_Source_WingetCommunityDefault_Identifier = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Name = "msstore"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Arg = "https://winget.azureedge.net/msstore"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Data = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Identifier = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; - - constexpr std::string_view s_Source_MSStoreDefault_Name = "storepreview"sv; + constexpr std::string_view s_Source_MSStoreDefault_Name = "msstore"sv; constexpr std::string_view s_Source_MSStoreDefault_Arg = "https://storeedgefd.dsx.mp.microsoft.com/v9.0"sv; constexpr std::string_view s_Source_MSStoreDefault_Identifier = "StoreEdgeFD"sv; @@ -94,7 +93,7 @@ namespace AppInstaller::Repository details.Arg = s_Source_MSStoreDefault_Arg; details.Identifier = s_Source_MSStoreDefault_Identifier; details.TrustLevel = SourceTrustLevel::Trusted; - details.Restricted = true; + details.SupportCorrelation = false; return details; } case WellKnownSource::DesktopFrameworks: @@ -149,14 +148,9 @@ namespace AppInstaller::Repository return IsDefaultSourceEnabled(s_Source_WingetCommunityDefault_Name, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); } - bool IsWingetMSStoreDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(s_Source_WingetMSStoreDefault_Name, ExperimentalFeature::Feature::ExperimentalMSStore, onlyExplicit, TogglePolicy::Policy::MSStoreSource); - } - bool IsMSStoreDefaultSourceEnabled(bool onlyExplicit = false) { - return IsDefaultSourceEnabled(s_Source_MSStoreDefault_Name, ExperimentalFeature::Feature::ExperimentalMSStore, onlyExplicit, TogglePolicy::Policy::MSStoreSource); + return IsDefaultSourceEnabled(s_Source_MSStoreDefault_Name, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::MSStoreSource); } template<ValuePolicy P> @@ -217,11 +211,6 @@ namespace AppInstaller::Repository return TogglePolicy::Policy::DefaultSource; } - if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) - { - return TogglePolicy::Policy::MSStoreSource; - } - if (name == s_Source_MSStoreDefault_Name && IsMSStoreDefaultSourceEnabled(true)) { return TogglePolicy::Policy::MSStoreSource; @@ -242,12 +231,6 @@ namespace AppInstaller::Repository return IsWingetCommunityDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; } - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetMSStoreDefault_Arg) && - Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) - { - return IsWingetMSStoreDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; - } - if (Utility::CaseInsensitiveEquals(arg, s_Source_MSStoreDefault_Arg) && Utility::CaseInsensitiveEquals(type, Rest::RestSourceFactory::Type())) { @@ -263,12 +246,6 @@ namespace AppInstaller::Repository return TogglePolicy::Policy::DefaultSource; } - if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) - { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows a default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::MSStoreSource; - } - if (name == s_Source_MSStoreDefault_Name && IsMSStoreDefaultSourceEnabled(true)) { AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows a default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); @@ -322,7 +299,7 @@ namespace AppInstaller::Repository } if (GroupPolicies().GetState(TogglePolicy::Policy::MSStoreSource) == PolicyState::Enabled && - source.Identifier == s_Source_WingetMSStoreDefault_Identifier) + source.Identifier == s_Source_MSStoreDefault_Identifier) { throw GroupPolicyException(TogglePolicy::Policy::MSStoreSource); } @@ -488,26 +465,14 @@ namespace AppInstaller::Repository { case SourceOrigin::Default: { - if (IsWingetCommunityDefaultSourceEnabled()) - { - result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::WinGet)); - } - - if (IsWingetMSStoreDefaultSourceEnabled()) + if (IsMSStoreDefaultSourceEnabled()) { - SourceDetailsInternal details; - details.Name = s_Source_WingetMSStoreDefault_Name; - details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - details.Arg = s_Source_WingetMSStoreDefault_Arg; - details.Data = s_Source_WingetMSStoreDefault_Data; - details.Identifier = s_Source_WingetMSStoreDefault_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; - result.emplace_back(std::move(details)); + result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::MicrosoftStore)); } - if (IsMSStoreDefaultSourceEnabled()) + if (IsWingetCommunityDefaultSourceEnabled()) { - result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::MicrosoftStore)); + result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::WinGet)); } // Since we are using the tombstone trick, this is added just to have the source in the internal @@ -661,6 +626,11 @@ namespace AppInstaller::Repository { return itr->second(); } + + if (Utility::CaseInsensitiveEquals(Microsoft::ConfigurableTestSourceFactory::Type(), type)) + { + return Microsoft::ConfigurableTestSourceFactory::Create(); + } #endif // For now, enable an empty type to represent the only one we have. @@ -987,6 +957,28 @@ namespace AppInstaller::Repository } return result; } + + // Carries the exception from an OpenSource call and presents it back at search time. + struct OpenExceptionProxy : public ISource, std::enable_shared_from_this<OpenExceptionProxy> + { + OpenExceptionProxy(const SourceDetails& details, std::exception_ptr exception) : + m_details(details), m_exception(std::move(exception)) {} + + const SourceDetails& GetDetails() const override { return m_details; } + + const std::string& GetIdentifier() const override { return m_details.Identifier; } + + SearchResult Search(const SearchRequest&) const override + { + SearchResult result; + result.Failures.emplace_back(SearchResult::Failure{ shared_from_this(), m_exception }); + return result; + } + + private: + SourceDetails m_details; + std::exception_ptr m_exception; + }; } std::string_view ToString(SourceOrigin origin) @@ -1099,34 +1091,19 @@ namespace AppInstaller::Repository } else if (currentSources.size() == 1) { - // Restricted sources may not support the full set of functionality - if (currentSources[0].get().Restricted) - { - AICLI_LOG(Repo, Info, << "Default source requested, only 1 source available but not using it as it is restricted: " << currentSources[0].get().Name); - return {}; - } - else - { - AICLI_LOG(Repo, Info, << "Default source requested, only 1 source available, using the only source: " << currentSources[0].get().Name); - return OpenSource(currentSources[0].get().Name, progress); - } + AICLI_LOG(Repo, Info, << "Default source requested, only 1 source available, using the only source: " << currentSources[0].get().Name); + return OpenSource(currentSources[0].get().Name, progress); } else { AICLI_LOG(Repo, Info, << "Default source requested, multiple sources available, creating aggregated source."); auto aggregatedSource = std::make_shared<CompositeSource>("*DefaultSource"); OpenSourceResult result; + std::vector<std::shared_ptr<OpenExceptionProxy>> openExceptionProxies; bool sourceUpdated = false; for (auto& source : currentSources) { - // Restricted sources may not support the full set of functionality so they shouldn't be included in the default aggregated source. - if (source.get().Restricted) - { - AICLI_LOG(Repo, Info, << "Skipping adding to aggregated source as the current source is restricted: " << source.get().Name); - continue; - } - AICLI_LOG(Repo, Info, << "Adding to aggregated source: " << source.get().Name); if (ShouldUpdateBeforeOpen(source)) @@ -1139,12 +1116,29 @@ namespace AppInstaller::Repository } catch (...) { + LOG_CAUGHT_EXCEPTION(); AICLI_LOG(Repo, Warning, << "Failed to update source: " << source.get().Name); result.SourcesWithUpdateFailure.emplace_back(source); } } - aggregatedSource->AddAvailableSource(CreateSourceFromDetails(source, progress)); + std::shared_ptr<ISource> openedSource; + + try + { + openedSource = CreateSourceFromDetails(source, progress); + } + catch (...) + { + LOG_CAUGHT_EXCEPTION(); + AICLI_LOG(Repo, Warning, << "Failed to open source: " << source.get().Name); + openExceptionProxies.emplace_back(std::make_shared<OpenExceptionProxy>(source, std::current_exception())); + } + + if (openedSource) + { + aggregatedSource->AddAvailableSource(std::move(openedSource)); + } } if (sourceUpdated) @@ -1152,6 +1146,15 @@ namespace AppInstaller::Repository sourceList.SaveMetadata(); } + // If all sources failed to open, then throw an exception that is specific to this case. + THROW_HR_IF(APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES, !aggregatedSource->HasAvailableSource()); + + // Place all of the proxies into the source to be searched later + for (auto& proxy : openExceptionProxies) + { + aggregatedSource->AddAvailableSource(std::move(proxy)); + } + result.Source = aggregatedSource; return result; } @@ -1199,8 +1202,7 @@ namespace AppInstaller::Repository // if the details came from the same instance of the list that's being saved. // Some sources that do not need updating like the Installed source, do not have Name values. // Restricted sources don't have full functionality - if (!details.Name.empty() && - !details.Restricted) + if (!details.Name.empty()) { SourceListInternal sourceList; auto source = sourceList.GetSource(details.Name); @@ -1393,6 +1395,13 @@ namespace AppInstaller::Repository bool SupportsCustomHeader(const SourceDetails& sourceDetails) { +#ifndef AICLI_DISABLE_TEST_HOOKS + if (Utility::CaseInsensitiveEquals(Microsoft::ConfigurableTestSourceFactory::Type(), sourceDetails.Type)) + { + return true; + } +#endif + return Utility::CaseInsensitiveEquals(Rest::RestSourceFactory::Type(), sourceDetails.Type); } diff --git a/src/Microsoft.Management.Deployment/PackageCatalog.cpp b/src/Microsoft.Management.Deployment/PackageCatalog.cpp @@ -139,6 +139,14 @@ namespace winrt::Microsoft::Management::Deployment::implementation searchRequest.MaximumResults = options.ResultLimit(); auto searchResult = m_source->Search(searchRequest); + // Handle failures by just rethrowing the first one for now. + // TODO: Look into updating the COM interface to enable the single source + // failures to flow out. + if (!searchResult.Failures.empty()) + { + std::rethrow_exception(searchResult.Failures[0].Exception); + } + // Build the result object from the searchResult for (size_t i = 0; i < searchResult.Matches.size(); ++i) {