Format WinINet HRESULT messages#6321
Conversation
Decode HRESULT_FROM_WIN32 values before asking std::system_category for user-facing text so WinINet errors do not render as unknown error. Adds a regression test for ERROR_INTERNET_CANNOT_CONNECT.
|
@microsoft-github-policy-service agree |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR updates WinGet’s shared error formatting so that HRESULTs created via HRESULT_FROM_WIN32(...) are decoded back to their underlying Win32 error codes before retrieving a system error message, preventing “unknown error” output for Win32-facility HRESULTs. It also adds a regression test to validate the new behavior for a WinINet/Win32 error wrapped as an HRESULT.
Changes:
- Decode
FACILITY_WIN32HRESULTs usingHRESULT_CODE(hr)before callingstd::system_category().message(...)inGetUserPresentableMessageForHR. - Add a Catch2 regression test for
HRESULT_FROM_WIN32(ERROR_INTERNET_CANNOT_CONNECT)message formatting.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/AppInstallerSharedLib/Errors.cpp |
Adjusts user-presentable HRESULT formatting to use Win32 codes when facility is Win32. |
src/AppInstallerCLITests/Errors.cpp |
Adds a regression test ensuring decoded Win32 messages appear for HRESULT_FROM_WIN32(...). |
| strstr << std::system_category().message( | ||
| HRESULT_FACILITY(hr) == FACILITY_WIN32 ? HRESULT_CODE(hr) : hr); | ||
| } |
| TEST_CASE("Win32HResultMessageUsesWin32Code", "[errors]") | ||
| { | ||
| constexpr HRESULT internetCannotConnect = HRESULT_FROM_WIN32(ERROR_INTERNET_CANNOT_CONNECT); | ||
| const std::string message = GetUserPresentableMessage(internetCannotConnect); | ||
| const std::string expectedSystemMessage = std::system_category().message(ERROR_INTERNET_CANNOT_CONNECT); | ||
| const std::string hresultSystemMessage = std::system_category().message(internetCannotConnect); | ||
|
|
||
| INFO(message); | ||
| INFO(expectedSystemMessage); | ||
| INFO(hresultSystemMessage); | ||
| REQUIRE(message.find("0x80072efd") != std::string::npos); | ||
| REQUIRE(message.find(expectedSystemMessage) != std::string::npos); | ||
| REQUIRE(message.find(hresultSystemMessage) == std::string::npos); | ||
| } No newline at end of file |
|
Follow-up: I found the cause of the previous x64 and x86 test failures. On MSVC, This update retrieves the localized message from Local validation: |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 6321 in repo microsoft/winget-cli |
|
Azure reports that I do not have permission to queue the x64/x86 pipeline for the updated head eefe8e7. @florelis, could you please rerun it when convenient? Evidence: the prior test failure was caused by the system-category assumption. This head now reads the WinINet message table and covers both user-facing formatting paths. |
Summary
Fixes #6319.
HRESULT_FROM_WIN32(ERROR_INTERNET_CANNOT_CONNECT)is a WinINet error. On MSVC,std::system_category().message(12029)andstd::system_category().message(0x80072efd)both returnunknown error, so decoding the HRESULT alone cannot provide the useful WinINet diagnostic.This change decodes
FACILITY_WIN32HRESULTs and retrieves WinINet-range messages fromwininet.dll, falling back tostd::system_categoryfor all other errors.Changes
GetUserPresentableMessageandHResultInformation::Find(...)->GetDescription().Validation
git diff --checkpassed.unknown errorfor both values while the WinINet message table returns the localized connection-failure message.AppInstallerCLITestsbecause the first-time vcpkg registry fetch was interrupted before the required historical port trees were available. The updated PR CI is queued for the x64 and x86 test validation.