Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/AppInstallerCLITests/Errors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "pch.h"
#include "TestCommon.h"
#include <AppInstallerErrors.h>
#include <system_error>

using namespace AppInstaller;
using namespace AppInstaller::Utility;
Expand All @@ -17,3 +18,22 @@ TEST_CASE("EnsureSortedErrorList", "[errors]")
REQUIRE(errors[i]->Value() > errors[i - 1]->Value());
}
}

TEST_CASE("WinInetHResultMessageUsesWinInetMessage", "[errors]")
{
constexpr HRESULT internetCannotConnect = HRESULT_FROM_WIN32(ERROR_INTERNET_CANNOT_CONNECT);
const std::string message = GetUserPresentableMessage(internetCannotConnect);
const std::string fallbackSystemMessage = std::system_category().message(internetCannotConnect);

INFO(message);
INFO(fallbackSystemMessage);
REQUIRE(message.find("0x80072efd") != std::string::npos);
REQUIRE(message.find(fallbackSystemMessage) == std::string::npos);

auto hresultInfo = Errors::HResultInformation::Find(internetCannotConnect);
REQUIRE(hresultInfo);

const auto description = hresultInfo->GetDescription();
INFO(description);
REQUIRE(description.get().find(fallbackSystemMessage) == std::string::npos);
}
54 changes: 52 additions & 2 deletions src/AppInstallerSharedLib/Errors.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#include "pch.h"
#include <wininet.h>
#include "Public/AppInstallerErrors.h"
#include "Public/AppInstallerLogging.h"
#include "Public/AppInstallerStrings.h"
Expand Down Expand Up @@ -343,6 +344,55 @@ namespace AppInstaller
UnknownHResultInformation(hr).GetDescription();
}

int GetSystemErrorCode(HRESULT hr)
{
return static_cast<int>(HRESULT_FACILITY(hr) == FACILITY_WIN32 ? HRESULT_CODE(hr) : hr);
}

// WinINet errors are not present in the system message table.
std::optional<std::string> GetWinInetErrorMessage(int errorCode)
{
if (errorCode < ERROR_INTERNET_OUT_OF_HANDLES || errorCode > INTERNET_ERROR_LAST)
{
return std::nullopt;
}

wil::unique_hmodule module{ LoadLibraryExW(L"wininet.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should specify LOAD_LIBRARY_AS_DATAFILE since we are only extracting a message.

if (!module)
{
return std::nullopt;
}

LPWSTR buffer = nullptr;
const DWORD messageLength = FormatMessageW(
FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_HMODULE | FORMAT_MESSAGE_IGNORE_INSERTS,
module.get(), errorCode, 0, reinterpret_cast<LPWSTR>(&buffer), 0, nullptr);
if (!messageLength)
{
return std::nullopt;
}

auto freeBuffer = wil::scope_exit([&]() { LocalFree(buffer); });
std::string message = Utility::ConvertToUTF8(std::wstring_view{ buffer, messageLength });
Utility::Trim(message);
return message.empty() ? std::nullopt : std::optional<std::string>{ std::move(message) };
}

std::string GetSystemErrorMessage(HRESULT hr)
{
const int errorCode = GetSystemErrorCode(hr);
if (HRESULT_FACILITY(hr) == FACILITY_WIN32)
{
auto winInetMessage = GetWinInetErrorMessage(errorCode);
if (winInetMessage)
{
return std::move(winInetMessage).value();
}
}

return std::system_category().message(errorCode);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you intentionally change the Win32 errors to look up via code only? I don't know that it is required, and it would make the code above cleaner to simply extract the code when needed rather than unconditionally.

}

void GetUserPresentableMessageForHR(std::ostringstream& strstr, HRESULT hr)
{
strstr << "0x" << Logging::SetHRFormat << hr << " : ";
Expand All @@ -361,7 +411,7 @@ namespace AppInstaller
}
else
{
strstr << std::system_category().message(hr);
strstr << GetSystemErrorMessage(hr);
}
}
}
Expand Down Expand Up @@ -437,7 +487,7 @@ namespace AppInstaller
}
else
{
return Utility::LocIndString{ std::system_category().message(m_value) };
return Utility::LocIndString{ GetSystemErrorMessage(m_value) };
}
}

Expand Down
Loading