commit 8f6b6f8dfbebc119b193dfbdb8d73d22396726a0
parent 8a31bfdf3393e6962e917b07e642907a0e4673fe
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Mon, 7 Oct 2024 20:06:34 -0700
Add extended error codes to results without them (#4858)
## Change
Adds `HRESULT ExtendedErrorCode{ get; };` to the `FindPackagesResult`
and `ConnectResult` classes that did not have them. They work just like
the other cases, exposing the raw `HRESULT` value from their respective
operations.
Diffstat:
9 files changed, 159 insertions(+), 105 deletions(-)
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
@@ -388,7 +388,7 @@ namespace AppInstaller::CLI::Workflow
return authArgs;
}
- HRESULT HandleException(Execution::Context& context, std::exception_ptr exception)
+ HRESULT HandleException(Execution::Context* context, std::exception_ptr exception)
{
try
{
@@ -399,47 +399,67 @@ namespace AppInstaller::CLI::Workflow
{
// Even though they are logged at their source, log again here for completeness.
Logging::Telemetry().LogException(Logging::FailureTypeEnum::ResultException, re.what());
- context.Reporter.Error() <<
- Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
- GetUserPresentableMessage(re) << std::endl;
+ if (context)
+ {
+ 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(Logging::FailureTypeEnum::WinrtHResultError, message);
- context.Reporter.Error() <<
- Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
- message << std::endl;
+ if (context)
+ {
+ 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());
- auto policyNameId = policy.PolicyName();
- context.Reporter.Error() << Resource::String::DisabledByGroupPolicy(policyNameId) << std::endl;
+ if (context)
+ {
+ auto policy = Settings::TogglePolicy::GetPolicy(e.Policy());
+ auto policyNameId = policy.PolicyName();
+ context->Reporter.Error() << Resource::String::DisabledByGroupPolicy(policyNameId) << std::endl;
+ }
return APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY;
}
catch (const std::exception& e)
{
Logging::Telemetry().LogException(Logging::FailureTypeEnum::StdException, e.what());
- context.Reporter.Error() <<
- Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl <<
- GetUserPresentableMessage(e) << std::endl;
+ if (context)
+ {
+ 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(Logging::FailureTypeEnum::Unknown, {});
- context.Reporter.Error() <<
- Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl;
+ if (context)
+ {
+ context->Reporter.Error() <<
+ Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl;
+ }
return APPINSTALLER_CLI_ERROR_COMMAND_FAILED;
}
return E_UNEXPECTED;
}
+ HRESULT HandleException(Execution::Context& context, std::exception_ptr exception)
+ {
+ return HandleException(&context, exception);
+ }
+
void OpenSource::operator()(Execution::Context& context) const
{
std::string_view sourceName;
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h
@@ -85,6 +85,10 @@ namespace AppInstaller::CLI::Workflow
Authentication::AuthenticationArguments GetAuthenticationArguments(const Execution::Context& context);
// Helper to report exceptions and return the HRESULT.
+ // If context is null, no output will be attempted.
+ HRESULT HandleException(Execution::Context* context, std::exception_ptr exception);
+
+ // Helper to report exceptions and return the HRESULT.
HRESULT HandleException(Execution::Context& context, std::exception_ptr exception);
// Creates the source object.
diff --git a/src/Microsoft.Management.Deployment/ConnectResult.cpp b/src/Microsoft.Management.Deployment/ConnectResult.cpp
@@ -6,10 +6,11 @@
namespace winrt::Microsoft::Management::Deployment::implementation
{
- void ConnectResult::Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus status, winrt::Microsoft::Management::Deployment::PackageCatalog packageCatalog)
+ void ConnectResult::Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus status, winrt::Microsoft::Management::Deployment::PackageCatalog packageCatalog, winrt::hresult extendedErrorCode)
{
m_status = status;
- m_packageCatalog = packageCatalog;
+ m_packageCatalog = packageCatalog;
+ m_extendedErrorCode = extendedErrorCode;
}
winrt::Microsoft::Management::Deployment::ConnectResultStatus ConnectResult::Status()
{
@@ -18,5 +19,10 @@ namespace winrt::Microsoft::Management::Deployment::implementation
winrt::Microsoft::Management::Deployment::PackageCatalog ConnectResult::PackageCatalog()
{
return m_packageCatalog;
+ }
+
+ winrt::hresult ConnectResult::ExtendedErrorCode()
+ {
+ return m_extendedErrorCode;
}
}
diff --git a/src/Microsoft.Management.Deployment/ConnectResult.h b/src/Microsoft.Management.Deployment/ConnectResult.h
@@ -10,16 +10,18 @@ namespace winrt::Microsoft::Management::Deployment::implementation
ConnectResult() = default;
#if !defined(INCLUDE_ONLY_INTERFACE_METHODS)
- void Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus status, winrt::Microsoft::Management::Deployment::PackageCatalog packageCatalog);
+ void Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus status, winrt::Microsoft::Management::Deployment::PackageCatalog packageCatalog, winrt::hresult extendedErrorCode);
#endif
winrt::Microsoft::Management::Deployment::ConnectResultStatus Status();
winrt::Microsoft::Management::Deployment::PackageCatalog PackageCatalog();
+ winrt::hresult ExtendedErrorCode();
#if !defined(INCLUDE_ONLY_INTERFACE_METHODS)
private:
winrt::Microsoft::Management::Deployment::ConnectResultStatus m_status = winrt::Microsoft::Management::Deployment::ConnectResultStatus::Ok;
winrt::Microsoft::Management::Deployment::PackageCatalog m_packageCatalog{ nullptr };
+ winrt::hresult m_extendedErrorCode = S_OK;
#endif
};
}
diff --git a/src/Microsoft.Management.Deployment/FindPackagesResult.cpp b/src/Microsoft.Management.Deployment/FindPackagesResult.cpp
@@ -10,11 +10,13 @@ namespace winrt::Microsoft::Management::Deployment::implementation
void FindPackagesResult::Initialize(
winrt::Microsoft::Management::Deployment::FindPackagesResultStatus status,
bool wasLimitExceeded,
- Windows::Foundation::Collections::IVector<Microsoft::Management::Deployment::MatchResult> matches)
+ Windows::Foundation::Collections::IVector<Microsoft::Management::Deployment::MatchResult> matches,
+ winrt::hresult extendedErrorCode)
{
m_status = status;
m_matches = matches;
m_wasLimitExceeded = wasLimitExceeded;
+ m_extendedErrorCode = extendedErrorCode;
}
winrt::Microsoft::Management::Deployment::FindPackagesResultStatus FindPackagesResult::Status()
{
@@ -28,4 +30,9 @@ namespace winrt::Microsoft::Management::Deployment::implementation
{
return m_wasLimitExceeded;
}
+
+ winrt::hresult FindPackagesResult::ExtendedErrorCode()
+ {
+ return m_extendedErrorCode;
+ }
}
diff --git a/src/Microsoft.Management.Deployment/FindPackagesResult.h b/src/Microsoft.Management.Deployment/FindPackagesResult.h
@@ -14,12 +14,14 @@ namespace winrt::Microsoft::Management::Deployment::implementation
void Initialize(
winrt::Microsoft::Management::Deployment::FindPackagesResultStatus status,
bool wasLimitExceeded,
- Windows::Foundation::Collections::IVector<winrt::Microsoft::Management::Deployment::MatchResult> matches);
+ Windows::Foundation::Collections::IVector<winrt::Microsoft::Management::Deployment::MatchResult> matches,
+ winrt::hresult extendedErrorCode);
#endif
winrt::Microsoft::Management::Deployment::FindPackagesResultStatus Status();
winrt::Windows::Foundation::Collections::IVectorView<winrt::Microsoft::Management::Deployment::MatchResult> Matches();
bool WasLimitExceeded();
+ winrt::hresult ExtendedErrorCode();
#if !defined(INCLUDE_ONLY_INTERFACE_METHODS)
private:
@@ -27,6 +29,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation
Windows::Foundation::Collections::IVector<winrt::Microsoft::Management::Deployment::MatchResult> m_matches{
winrt::single_threaded_vector<winrt::Microsoft::Management::Deployment::MatchResult>() };
bool m_wasLimitExceeded = false;
+ winrt::hresult m_extendedErrorCode = S_OK;
#endif
};
}
diff --git a/src/Microsoft.Management.Deployment/PackageCatalog.cpp b/src/Microsoft.Management.Deployment/PackageCatalog.cpp
@@ -115,7 +115,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation
winrt::Microsoft::Management::Deployment::implementation::FindPackagesResult>>();
// TODO: Add search timeout and error code.
winrt::Microsoft::Management::Deployment::FindPackagesResultStatus status = FindPackagesResultStatus(hr);
- findPackagesResult->Initialize(status, isTruncated, matches);
+ findPackagesResult->Initialize(status, isTruncated, matches, hr);
return *findPackagesResult;
}
diff --git a/src/Microsoft.Management.Deployment/PackageCatalogReference.cpp b/src/Microsoft.Management.Deployment/PackageCatalogReference.cpp
@@ -60,124 +60,124 @@ namespace winrt::Microsoft::Management::Deployment::implementation
{
co_return Connect();
}
- winrt::Microsoft::Management::Deployment::ConnectResult GetConnectCatalogErrorResult()
+ winrt::Microsoft::Management::Deployment::ConnectResult GetConnectCatalogErrorResult(hresult hr)
{
auto connectResult = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::ConnectResult>>();
- connectResult->Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus::CatalogError, nullptr);
+ connectResult->Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus::CatalogError, nullptr, hr);
return *connectResult;
}
winrt::Microsoft::Management::Deployment::ConnectResult GetConnectSourceAgreementsNotAcceptedErrorResult()
{
auto connectResult = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::ConnectResult>>();
- connectResult->Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus::SourceAgreementsNotAccepted, nullptr);
+ connectResult->Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus::SourceAgreementsNotAccepted, nullptr, APPINSTALLER_CLI_ERROR_SOURCE_AGREEMENTS_NOT_ACCEPTED);
return *connectResult;
}
- winrt::Microsoft::Management::Deployment::ConnectResult PackageCatalogReference::Connect()
+ winrt::Microsoft::Management::Deployment::ConnectResult PackageCatalogReference::Connect() try
{
- try
+ HRESULT hr = EnsureComCallerHasCapability(Capability::PackageQuery);
+ if (FAILED(hr))
{
- if (FAILED(EnsureComCallerHasCapability(Capability::PackageQuery)))
- {
- // TODO: When more error codes are added, this should go back as something other than CatalogError.
- return GetConnectCatalogErrorResult();
- }
+ // TODO: When more error codes are added, this should go back as something other than CatalogError.
+ return GetConnectCatalogErrorResult(hr);
+ }
- std::string callerName = GetCallerName();
+ std::string callerName = GetCallerName();
- ::AppInstaller::ProgressCallback progress;
- ::AppInstaller::Repository::Source source;
- if (m_compositePackageCatalogOptions)
- {
- std::vector<::AppInstaller::Repository::Source> remoteSources;
+ ::AppInstaller::ProgressCallback progress;
+ ::AppInstaller::Repository::Source source;
+ if (m_compositePackageCatalogOptions)
+ {
+ std::vector<::AppInstaller::Repository::Source> remoteSources;
- for (uint32_t i = 0; i < m_compositePackageCatalogOptions.Catalogs().Size(); ++i)
+ for (uint32_t i = 0; i < m_compositePackageCatalogOptions.Catalogs().Size(); ++i)
+ {
+ auto catalog = m_compositePackageCatalogOptions.Catalogs().GetAt(i);
+ if (!catalog.AcceptSourceAgreements() && catalog.SourceAgreements().Size() != 0)
{
- auto catalog = m_compositePackageCatalogOptions.Catalogs().GetAt(i);
- if (!catalog.AcceptSourceAgreements() && catalog.SourceAgreements().Size() != 0)
- {
- return GetConnectSourceAgreementsNotAcceptedErrorResult();
- }
+ return GetConnectSourceAgreementsNotAcceptedErrorResult();
+ }
- winrt::Microsoft::Management::Deployment::implementation::PackageCatalogReference* catalogImpl = get_self<winrt::Microsoft::Management::Deployment::implementation::PackageCatalogReference>(catalog);
- auto copy = catalogImpl->m_sourceReference;
- copy.SetCaller(callerName);
- copy.SetBackgroundUpdateInterval(catalog.PackageCatalogBackgroundUpdateInterval());
- copy.InstalledPackageInformationOnly(catalog.InstalledPackageInformationOnly());
- if (catalog.AuthenticationInfo().AuthenticationType() != winrt::Microsoft::Management::Deployment::AuthenticationType::None)
- {
- copy.SetAuthenticationArguments(GetAuthenticationArguments(catalog.AuthenticationArguments()));
- }
- copy.Open(progress);
- remoteSources.emplace_back(std::move(copy));
+ winrt::Microsoft::Management::Deployment::implementation::PackageCatalogReference* catalogImpl = get_self<winrt::Microsoft::Management::Deployment::implementation::PackageCatalogReference>(catalog);
+ auto copy = catalogImpl->m_sourceReference;
+ copy.SetCaller(callerName);
+ copy.SetBackgroundUpdateInterval(catalog.PackageCatalogBackgroundUpdateInterval());
+ copy.InstalledPackageInformationOnly(catalog.InstalledPackageInformationOnly());
+ if (catalog.AuthenticationInfo().AuthenticationType() != winrt::Microsoft::Management::Deployment::AuthenticationType::None)
+ {
+ copy.SetAuthenticationArguments(GetAuthenticationArguments(catalog.AuthenticationArguments()));
}
+ copy.Open(progress);
+ remoteSources.emplace_back(std::move(copy));
+ }
- // Create the aggregated source.
- source = ::AppInstaller::Repository::Source{ remoteSources };
+ // Create the aggregated source.
+ source = ::AppInstaller::Repository::Source{ remoteSources };
- // Create composite with installed source if needed.
- ::AppInstaller::Repository::CompositeSearchBehavior searchBehavior = GetRepositoryCompositeSearchBehavior(m_compositePackageCatalogOptions.CompositeSearchBehavior());
+ // Create composite with installed source if needed.
+ ::AppInstaller::Repository::CompositeSearchBehavior searchBehavior = GetRepositoryCompositeSearchBehavior(m_compositePackageCatalogOptions.CompositeSearchBehavior());
- // Check if search behavior indicates that the caller does not want to do local correlation.
- if (m_compositePackageCatalogOptions.CompositeSearchBehavior() != Microsoft::Management::Deployment::CompositeSearchBehavior::RemotePackagesFromRemoteCatalogs)
+ // Check if search behavior indicates that the caller does not want to do local correlation.
+ if (m_compositePackageCatalogOptions.CompositeSearchBehavior() != Microsoft::Management::Deployment::CompositeSearchBehavior::RemotePackagesFromRemoteCatalogs)
+ {
+ ::AppInstaller::Repository::Source installedSource;
+ auto manifestInstalledScope = GetManifestScope(m_compositePackageCatalogOptions.InstalledScope()).first;
+ if (manifestInstalledScope == ::AppInstaller::Manifest::ScopeEnum::User)
{
- ::AppInstaller::Repository::Source installedSource;
- auto manifestInstalledScope = GetManifestScope(m_compositePackageCatalogOptions.InstalledScope()).first;
- if (manifestInstalledScope == ::AppInstaller::Manifest::ScopeEnum::User)
- {
- installedSource = ::AppInstaller::Repository::Source{ ::AppInstaller::Repository::PredefinedSource::InstalledUser };
- }
- else if (manifestInstalledScope == ::AppInstaller::Manifest::ScopeEnum::Machine)
- {
- installedSource = ::AppInstaller::Repository::Source{ ::AppInstaller::Repository::PredefinedSource::InstalledMachine };
- }
- else
- {
- installedSource = ::AppInstaller::Repository::Source{ ::AppInstaller::Repository::PredefinedSource::Installed };
- }
-
- installedSource.Open(progress);
- source = ::AppInstaller::Repository::Source{ installedSource, source, searchBehavior };
+ installedSource = ::AppInstaller::Repository::Source{ ::AppInstaller::Repository::PredefinedSource::InstalledUser };
}
- }
- else
- {
- if (!AcceptSourceAgreements() && SourceAgreements().Size() != 0)
+ else if (manifestInstalledScope == ::AppInstaller::Manifest::ScopeEnum::Machine)
{
- return GetConnectSourceAgreementsNotAcceptedErrorResult();
+ installedSource = ::AppInstaller::Repository::Source{ ::AppInstaller::Repository::PredefinedSource::InstalledMachine };
}
-
- source = m_sourceReference;
- source.SetCaller(callerName);
- source.SetBackgroundUpdateInterval(PackageCatalogBackgroundUpdateInterval());
- source.InstalledPackageInformationOnly(m_installedPackageInformationOnly);
- if (AuthenticationInfo().AuthenticationType() != winrt::Microsoft::Management::Deployment::AuthenticationType::None)
+ else
{
- source.SetAuthenticationArguments(GetAuthenticationArguments(m_authenticationArguments));
+ installedSource = ::AppInstaller::Repository::Source{ ::AppInstaller::Repository::PredefinedSource::Installed };
}
- source.Open(progress);
- }
- if (!source)
+ installedSource.Open(progress);
+ source = ::AppInstaller::Repository::Source{ installedSource, source, searchBehavior };
+ }
+ }
+ else
+ {
+ if (!AcceptSourceAgreements() && SourceAgreements().Size() != 0)
{
- // If source is null, return the error. There's no way to get the hresult that caused the error right now.
- return GetConnectCatalogErrorResult();
+ return GetConnectSourceAgreementsNotAcceptedErrorResult();
}
- // Have to make another package catalog info because source->GetDetails has more fields than m_info does.
- // Specifically, Rest sources do not have the Ids filled in m_info since they only get the id from the rest server after being Opened.
- auto packageCatalogInfo = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::PackageCatalogInfo>>();
- packageCatalogInfo->Initialize(source.GetDetails());
- auto connectResult = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::ConnectResult>>();
- auto packageCatalog = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::PackageCatalog>>();
- packageCatalog->Initialize(*packageCatalogInfo, source, (m_compositePackageCatalogOptions != nullptr));
- connectResult->Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus::Ok, *packageCatalog);
- return *connectResult;
+ source = m_sourceReference;
+ source.SetCaller(callerName);
+ source.SetBackgroundUpdateInterval(PackageCatalogBackgroundUpdateInterval());
+ source.InstalledPackageInformationOnly(m_installedPackageInformationOnly);
+ if (AuthenticationInfo().AuthenticationType() != winrt::Microsoft::Management::Deployment::AuthenticationType::None)
+ {
+ source.SetAuthenticationArguments(GetAuthenticationArguments(m_authenticationArguments));
+ }
+ source.Open(progress);
}
- catch (...)
+
+ if (!source)
{
+ // We call `Open` on each individual source above, meaning that they should throw any error that occurs.
+ // If the source is still not open at this point it is a bug.
+ return GetConnectCatalogErrorResult(E_UNEXPECTED);
}
- return GetConnectCatalogErrorResult();
+
+ // Have to make another package catalog info because source->GetDetails has more fields than m_info does.
+ // Specifically, Rest sources do not have the Ids filled in m_info since they only get the id from the rest server after being Opened.
+ auto packageCatalogInfo = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::PackageCatalogInfo>>();
+ packageCatalogInfo->Initialize(source.GetDetails());
+ auto connectResult = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::ConnectResult>>();
+ auto packageCatalog = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::PackageCatalog>>();
+ packageCatalog->Initialize(*packageCatalogInfo, source, (m_compositePackageCatalogOptions != nullptr));
+ connectResult->Initialize(winrt::Microsoft::Management::Deployment::ConnectResultStatus::Ok, *packageCatalog, S_OK);
+ return *connectResult;
}
+ catch (...)
+ {
+ return GetConnectCatalogErrorResult(AppInstaller::CLI::Workflow::HandleException(nullptr, std::current_exception()));
+ }
+
winrt::Windows::Foundation::Collections::IVectorView<winrt::Microsoft::Management::Deployment::SourceAgreement> PackageCatalogReference::SourceAgreements()
{
std::call_once(m_sourceAgreementsOnceFlag,
diff --git a/src/Microsoft.Management.Deployment/PackageManager.idl b/src/Microsoft.Management.Deployment/PackageManager.idl
@@ -730,6 +730,12 @@ namespace Microsoft.Management.Deployment
/// USAGE NOTE: Windows Package Manager does not support result pagination, there is no way to continue
/// getting more results.
Boolean WasLimitExceeded { get; };
+
+ [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 11)]
+ {
+ /// The error code of the operation.
+ HRESULT ExtendedErrorCode{ get; };
+ }
}
/// Options for FindPackages
@@ -849,6 +855,12 @@ namespace Microsoft.Management.Deployment
ConnectResultStatus Status { get; };
PackageCatalog PackageCatalog { get; };
+
+ [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 11)]
+ {
+ /// The error code of the operation.
+ HRESULT ExtendedErrorCode{ get; };
+ }
}
/// A reference to a catalog that callers can try to Connect.
