winget-cli

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | README | LICENSE

commit 2e3fc154e3fd149846224159dc9922bd3784a606
parent fc20f3237ad6eec0e2cfb15ee0258ddc2c6e84d6
Author: JohnMcPMS <johnmcp@microsoft.com>
Date:   Wed, 19 May 2021 19:22:22 -0700

Fixes for a few crashes, change DO timeout to 60 seconds default, update version that I forgot (#973)

Fixes a few crashes that were happening, increase the default DO timeout for no progress to 60 seconds, and bump the minor version for the previous command releases.
Diffstat:
Msrc/AppInstallerCLICore/Workflows/InstallFlow.cpp | 29++++++++++++++++++-----------
Msrc/AppInstallerCLITests/main.cpp | 4++++
Msrc/AppInstallerCommonCore/AppInstallerTelemetry.cpp | 13+++++++++++--
Msrc/AppInstallerCommonCore/Public/AppInstallerTelemetry.h | 1+
Msrc/AppInstallerCommonCore/Public/winget/UserSettings.h | 2+-
Msrc/AppInstallerRepositoryCore/CompositeSource.cpp | 6+++++-
Msrc/binver/binver/version.h | 2+-
7 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -461,10 +461,13 @@ namespace AppInstaller::CLI::Workflow for (const auto& entry : arpSource->Search({}).Matches) { auto installed = entry.Package->GetInstalledVersion(); - entries.emplace_back(std::make_tuple( - entry.Package->GetProperty(PackageProperty::Id), - installed->GetProperty(PackageVersionProperty::Version), - installed->GetProperty(PackageVersionProperty::Channel))); + if (installed) + { + entries.emplace_back(std::make_tuple( + entry.Package->GetProperty(PackageProperty::Id), + installed->GetProperty(PackageVersionProperty::Version), + installed->GetProperty(PackageVersionProperty::Channel))); + } } std::sort(entries.begin(), entries.end()); @@ -492,15 +495,19 @@ namespace AppInstaller::CLI::Workflow for (auto& entry : arpSource->Search({}).Matches) { auto installed = entry.Package->GetInstalledVersion(); - auto entryKey = std::make_tuple( - entry.Package->GetProperty(PackageProperty::Id), - installed->GetProperty(PackageVersionProperty::Version), - installed->GetProperty(PackageVersionProperty::Channel)); - auto itr = std::lower_bound(entries.begin(), entries.end(), entryKey); - if (itr == entries.end() || *itr != entryKey) + if (installed) { - changes.emplace_back(std::move(entry)); + auto entryKey = std::make_tuple( + entry.Package->GetProperty(PackageProperty::Id), + installed->GetProperty(PackageVersionProperty::Version), + installed->GetProperty(PackageVersionProperty::Channel)); + + auto itr = std::lower_bound(entries.begin(), entries.end(), entryKey); + if (itr == entries.end() || *itr != entryKey) + { + changes.emplace_back(std::move(entry)); + } } } diff --git a/src/AppInstallerCLITests/main.cpp b/src/AppInstallerCLITests/main.cpp @@ -9,6 +9,7 @@ #include <Public/AppInstallerLogging.h> #include <Public/AppInstallerTelemetry.h> +#include <Telemetry/TraceLogging.h> #include "TestCommon.h" #include "TestHooks.h" @@ -122,6 +123,9 @@ int main(int argc, char** argv) Logging::Log().SetLevel(Logging::Level::Verbose); Logging::EnableWilFailureTelemetry(); + // Forcibly enable event writing to catch bugs in that code + g_IsTelemetryProviderEnabled = true; + // Force all tests to run against settings inside this container. // This prevents test runs from trashing the users actual settings. Runtime::TestHook_SetPathOverride(Runtime::PathName::LocalState, Runtime::GetPathTo(Runtime::PathName::LocalState) / "Tests"); diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -95,13 +95,15 @@ namespace AppInstaller::Logging { if (IsTelemetryEnabled()) { + auto anonMessage = AnonymizeString(failure.pszMessage); + TraceLoggingWriteActivity(g_hTelemetryProvider, "FailureInfo", GetActivityId(), nullptr, TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), TraceLoggingHResult(failure.hr, "HResult"), - AICLI_TraceLoggingWStringView(AnonymizeString(failure.pszMessage), "Message"), + AICLI_TraceLoggingWStringView(anonMessage, "Message"), TraceLoggingString(failure.pszModule, "Module"), TraceLoggingUInt32(failure.threadId, "ThreadId"), TraceLoggingUInt32(static_cast<uint32_t>(failure.type), "Type"), @@ -206,6 +208,8 @@ namespace AppInstaller::Logging { if (IsTelemetryEnabled()) { + auto anonMessage = AnonymizeString(Utility::ConvertToUTF16(message)); + TraceLoggingWriteActivity(g_hTelemetryProvider, "Exception", GetActivityId(), @@ -213,7 +217,7 @@ namespace AppInstaller::Logging TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(commandName, "Command"), AICLI_TraceLoggingStringView(type, "Type"), - AICLI_TraceLoggingWStringView(AnonymizeString(Utility::ConvertToUTF16(message)), "Message"), + AICLI_TraceLoggingWStringView(anonMessage, "Message"), TraceLoggingUInt32(s_executionStage, "ExecutionStage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); @@ -515,6 +519,11 @@ namespace AppInstaller::Logging return g_IsTelemetryProviderEnabled && m_isSettingEnabled && m_isRuntimeEnabled; } + std::wstring TelemetryTraceLogger::AnonymizeString(const wchar_t* input) const noexcept + { + return input ? AnonymizeString(std::wstring_view{ input }) : std::wstring{}; + } + std::wstring TelemetryTraceLogger::AnonymizeString(std::wstring_view input) const noexcept try { return Utility::ReplaceWhileCopying(input, m_userProfile, s_UserProfileReplacement); diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -121,6 +121,7 @@ namespace AppInstaller::Logging // Used to anonymize a string to the best of our ability. // Should primarily be used on failure messages or paths if needed. + std::wstring AnonymizeString(const wchar_t* input) const noexcept; std::wstring AnonymizeString(std::wstring_view input) const noexcept; bool m_isSettingEnabled = true; diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -117,7 +117,7 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, ScopePreference, ScopePreference::User, ".installBehavior.preferences.scope"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopeRequirement, std::string, ScopePreference, ScopePreference::None, ".installBehavior.requirements.scope"sv); SETTINGMAPPING_SPECIALIZATION(Setting::NetworkDownloader, std::string, InstallerDownloader, InstallerDownloader::Default, ".network.downloader"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::NetworkDOProgressTimeoutInSeconds, uint32_t, std::chrono::seconds, 20s, ".network.doProgressTimeoutInSeconds"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::NetworkDOProgressTimeoutInSeconds, uint32_t, std::chrono::seconds, 60s, ".network.doProgressTimeoutInSeconds"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallLocalePreference, std::vector<std::string>, std::vector<std::string>, {}, ".installBehavior.preferences.locale"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallLocaleRequirement, std::vector<std::string>, std::vector<std::string>, {}, ".installBehavior.requirements.locale"sv); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -39,7 +39,11 @@ namespace AppInstaller::Repository // Grab the installed version's channel to allow for filtering in calls to get available info. if (m_installedPackage) { - m_installedChannel = m_installedPackage->GetInstalledVersion()->GetProperty(PackageVersionProperty::Channel); + auto installedVersion = m_installedPackage->GetInstalledVersion(); + if (installedVersion) + { + m_installedChannel = installedVersion->GetProperty(PackageVersionProperty::Channel); + } } } diff --git a/src/binver/binver/version.h b/src/binver/binver/version.h @@ -2,7 +2,7 @@ #define STRINGIZE(s) STRINGIZE2(s) #define VERSION_MAJOR 0 -#define VERSION_MINOR 3 +#define VERSION_MINOR 4 #define VERSION_BUILD 0 #define VERSION_REVISION 0