Add in verbose error message and telemetry for SetWindowsHookEx failure (#6454)

* Updated error message when SetWindowsHookEx fails to show correct error message

* Added telemetry for exception in SG, FZ and KBM

* Rename exception to error
This commit is contained in:
Arjun Balgovind
2020-09-09 14:27:40 -07:00
committed by GitHub
parent f61e9d389f
commit fb1888f01f
14 changed files with 90 additions and 45 deletions

View File

@@ -11,12 +11,6 @@
#pragma comment(lib, "advapi32.lib") #pragma comment(lib, "advapi32.lib")
#pragma comment(lib, "shlwapi.lib") #pragma comment(lib, "shlwapi.lib")
namespace localized_strings
{
const wchar_t LAST_ERROR_FORMAT_STRING[] = L"%s failed with error %d: %s";
const wchar_t LAST_ERROR_TITLE_STRING[] = L"Error";
}
std::optional<RECT> get_window_pos(HWND hwnd) std::optional<RECT> get_window_pos(HWND hwnd)
{ {
RECT window; RECT window;
@@ -91,7 +85,7 @@ std::optional<std::wstring> get_last_error_message(const DWORD dw)
return message; return message;
} }
void show_last_error_message(LPCWSTR lpszFunction, DWORD dw) void show_last_error_message(LPCWSTR lpszFunction, DWORD dw, LPCWSTR errorTitle)
{ {
const auto system_message = get_last_error_message(dw); const auto system_message = get_last_error_message(dw);
if (!system_message.has_value()) if (!system_message.has_value())
@@ -107,7 +101,7 @@ void show_last_error_message(LPCWSTR lpszFunction, DWORD dw)
lpszFunction, lpszFunction,
dw, dw,
system_message->c_str()); system_message->c_str());
MessageBoxW(NULL, (LPCTSTR)lpDisplayBuf, localized_strings::LAST_ERROR_TITLE_STRING, MB_OK); MessageBoxW(NULL, (LPCTSTR)lpDisplayBuf, errorTitle, MB_OK | MB_ICONERROR);
LocalFree(lpDisplayBuf); LocalFree(lpDisplayBuf);
} }
} }

View File

@@ -6,6 +6,13 @@
#include <memory> #include <memory>
#include <vector> #include <vector>
namespace localized_strings
{
const wchar_t LAST_ERROR_FORMAT_STRING[] = L"%s failed with error %d: %s";
const wchar_t LAST_ERROR_TITLE_STRING[] = L"Error";
}
// Gets position of given window. // Gets position of given window.
std::optional<RECT> get_window_pos(HWND hwnd); std::optional<RECT> get_window_pos(HWND hwnd);
@@ -16,7 +23,7 @@ bool is_system_window(HWND hwnd, const char* class_name);
int run_message_loop(const bool until_idle = false, const std::optional<uint32_t> timeout_seconds = {}); int run_message_loop(const bool until_idle = false, const std::optional<uint32_t> timeout_seconds = {});
std::optional<std::wstring> get_last_error_message(const DWORD dw); std::optional<std::wstring> get_last_error_message(const DWORD dw);
void show_last_error_message(LPCWSTR lpszFunction, DWORD dw); void show_last_error_message(LPCWSTR lpszFunction, DWORD dw, LPCWSTR errorTitle = localized_strings::LAST_ERROR_TITLE_STRING);
enum WindowState enum WindowState
{ {

View File

@@ -4,6 +4,7 @@
#include <msclr\marshal.h> #include <msclr\marshal.h>
#include <msclr\marshal_cppstd.h> #include <msclr\marshal_cppstd.h>
#include <common/debug_control.h> #include <common/debug_control.h>
#include <common/common.h>
using namespace interop; using namespace interop;
using namespace System::Runtime::InteropServices; using namespace System::Runtime::InteropServices;
@@ -46,7 +47,8 @@ void KeyboardHook::Start()
0); 0);
if (hookHandle == nullptr) if (hookHandle == nullptr)
{ {
throw std::exception("SetWindowsHookEx failed."); DWORD errorCode = GetLastError();
show_last_error_message(L"SetWindowsHookEx", errorCode, L"PowerToys - Interop");
} }
} }
} }

View File

@@ -82,10 +82,10 @@ public:
s_llKeyboardHook = SetWindowsHookEx(WH_KEYBOARD_LL, LowLevelKeyboardProc, GetModuleHandle(NULL), NULL); s_llKeyboardHook = SetWindowsHookEx(WH_KEYBOARD_LL, LowLevelKeyboardProc, GetModuleHandle(NULL), NULL);
if (!s_llKeyboardHook) if (!s_llKeyboardHook)
{ {
MessageBoxW(NULL, DWORD errorCode = GetLastError();
GET_RESOURCE_STRING(IDS_KEYBOARD_LISTENER_ERROR).c_str(), show_last_error_message(L"SetWindowsHookEx", errorCode, GET_RESOURCE_STRING(IDS_POWERTOYS_FANCYZONES).c_str());
GET_RESOURCE_STRING(IDS_POWERTOYS_FANCYZONES).c_str(), auto errorMessage = get_last_error_message(errorCode);
MB_OK | MB_ICONERROR); Trace::FancyZones::Error(errorCode, errorMessage.has_value() ? errorMessage.value() : L"", L"enable.SetWindowsHookEx");
} }
} }

View File

@@ -213,6 +213,19 @@ void Trace::FancyZones::EditorLaunched(int value) noexcept
TraceLoggingInt32(value, EditorLaunchValueKey)); TraceLoggingInt32(value, EditorLaunchValueKey));
} }
// Log if an error occurs in FZ
void Trace::FancyZones::Error(const DWORD errorCode, std::wstring errorMessage, std::wstring methodName) noexcept
{
TraceLoggingWrite(
g_hProvider,
"FancyZones_Error",
ProjectTelemetryPrivacyDataTag(ProjectTelemetryTag_ProductAndServicePerformance),
TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE),
TraceLoggingValue(methodName.c_str(), "MethodName"),
TraceLoggingValue(errorCode, "ErrorCode"),
TraceLoggingValue(errorMessage.c_str(), "ErrorMessage"));
}
void Trace::SettingsChanged(const Settings& settings) noexcept void Trace::SettingsChanged(const Settings& settings) noexcept
{ {
const auto& editorHotkey = settings.editorHotkey; const auto& editorHotkey = settings.editorHotkey;

View File

@@ -16,6 +16,7 @@ public:
static void OnKeyDown(DWORD vkCode, bool win, bool control, bool inMoveSize) noexcept; static void OnKeyDown(DWORD vkCode, bool win, bool control, bool inMoveSize) noexcept;
static void DataChanged() noexcept; static void DataChanged() noexcept;
static void EditorLaunched(int value) noexcept; static void EditorLaunched(int value) noexcept;
static void Error(const DWORD errorCode, std::wstring errorMessage, std::wstring methodName) noexcept;
}; };
static void SettingsChanged(const Settings& settings) noexcept; static void SettingsChanged(const Settings& settings) noexcept;

View File

@@ -131,3 +131,16 @@ void Trace::ShortcutRemapInvoked(bool isShortcutToShortcut, bool isAppSpecific)
} }
} }
} }
// Log if an error occurs in KBM
void Trace::Error(const DWORD errorCode, std::wstring errorMessage, std::wstring methodName) noexcept
{
TraceLoggingWrite(
g_hProvider,
"KeyboardManager_Error",
ProjectTelemetryPrivacyDataTag(ProjectTelemetryTag_ProductAndServicePerformance),
TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE),
TraceLoggingValue(methodName.c_str(), "MethodName"),
TraceLoggingValue(errorCode, "ErrorCode"),
TraceLoggingValue(errorMessage.c_str(), "ErrorMessage"));
}

View File

@@ -23,4 +23,7 @@ public:
// Log if a shortcut remap has been invoked // Log if a shortcut remap has been invoked
static void ShortcutRemapInvoked(bool isShortcutToShortcut, bool isAppSpecific) noexcept; static void ShortcutRemapInvoked(bool isShortcutToShortcut, bool isAppSpecific) noexcept;
// Log if an error occurs in KBM
static void Error(const DWORD errorCode, std::wstring errorMessage, std::wstring methodName) noexcept;
}; };

View File

@@ -363,7 +363,10 @@ public:
hook_handle_copy = hook_handle; hook_handle_copy = hook_handle;
if (!hook_handle) if (!hook_handle)
{ {
throw std::runtime_error("Cannot install keyboard listener"); DWORD errorCode = GetLastError();
show_last_error_message(L"SetWindowsHookEx", errorCode, L"PowerToys - Keyboard Manager");
auto errorMessage = get_last_error_message(errorCode);
Trace::Error(errorCode, errorMessage.has_value() ? errorMessage.value() : L"", L"start_lowlevel_keyboard_hook.SetWindowsHookEx");
} }
} }
} }

View File

@@ -159,15 +159,6 @@ namespace Microsoft.Plugin.Folder.Properties {
} }
} }
/// <summary>
/// Looks up a localized string similar to Please select a folder link.
/// </summary>
public static string wox_plugin_folder_select_folder_link_warning {
get {
return ResourceManager.GetString("wox_plugin_folder_select_folder_link_warning", resourceCulture);
}
}
/// <summary> /// <summary>
/// Looks up a localized string similar to Could not start. /// Looks up a localized string similar to Could not start.
/// </summary> /// </summary>

View File

@@ -58,24 +58,6 @@ namespace PowerLauncher.Properties {
set { set {
resourceCulture = value; resourceCulture = value;
} }
}
/// <summary>
/// Looks up a localized string similar to Settings will be reset to default and program will continue to function..
/// </summary>
public static string deseralization_error_message {
get {
return ResourceManager.GetString("deseralization_error_message", resourceCulture);
}
}
/// <summary>
/// Looks up a localized string similar to Powertoys Run deserialization error.
/// </summary>
public static string deseralization_error_title {
get {
return ResourceManager.GetString("deseralization_error_title", resourceCulture);
}
} }
/// <summary> /// <summary>
@@ -132,6 +114,24 @@ namespace PowerLauncher.Properties {
} }
} }
/// <summary>
/// Looks up a localized string similar to Settings will be reset to default and program will continue to function..
/// </summary>
public static string deseralization_error_message {
get {
return ResourceManager.GetString("deseralization_error_message", resourceCulture);
}
}
/// <summary>
/// Looks up a localized string similar to Powertoys Run deserialization error.
/// </summary>
public static string deseralization_error_title {
get {
return ResourceManager.GetString("deseralization_error_title", resourceCulture);
}
}
/// <summary> /// <summary>
/// Looks up a localized string similar to Execute query: {0}. /// Looks up a localized string similar to Execute query: {0}.
/// </summary> /// </summary>

View File

@@ -6,6 +6,7 @@
#include <common/common.h> #include <common/common.h>
#include <common/settings_objects.h> #include <common/settings_objects.h>
#include <common/debug_control.h> #include <common/debug_control.h>
#include <sstream>
extern "C" IMAGE_DOS_HEADER __ImageBase; extern "C" IMAGE_DOS_HEADER __ImageBase;
@@ -84,8 +85,8 @@ namespace
// For now, since ShortcutGuide can only disable entire "Windows Controls" // For now, since ShortcutGuide can only disable entire "Windows Controls"
// group, we require that the window supports all the options. // group, we require that the window supports all the options.
result.snappable = ((style & WS_MAXIMIZEBOX) == WS_MAXIMIZEBOX) && result.snappable = ((style & WS_MAXIMIZEBOX) == WS_MAXIMIZEBOX) &&
((style & WS_MINIMIZEBOX) == WS_MINIMIZEBOX) && ((style & WS_MINIMIZEBOX) == WS_MINIMIZEBOX) &&
((style & WS_THICKFRAME) == WS_THICKFRAME); ((style & WS_THICKFRAME) == WS_THICKFRAME);
return result; return result;
} }
} }
@@ -224,7 +225,10 @@ void OverlayWindow::enable()
hook_handle = SetWindowsHookEx(WH_KEYBOARD_LL, LowLevelKeyboardProc, GetModuleHandle(NULL), NULL); hook_handle = SetWindowsHookEx(WH_KEYBOARD_LL, LowLevelKeyboardProc, GetModuleHandle(NULL), NULL);
if (!hook_handle) if (!hook_handle)
{ {
MessageBoxW(NULL, L"Cannot install keyboard listener.", L"PowerToys - Shortcut Guide", MB_OK | MB_ICONERROR); DWORD errorCode = GetLastError();
show_last_error_message(L"SetWindowsHookEx", errorCode, L"PowerToys - Shortcut Guide");
auto errorMessage = get_last_error_message(errorCode);
Trace::Error(errorCode, errorMessage.has_value() ? errorMessage.value() : L"", L"OverlayWindow.enable.SetWindowsHookEx");
} }
} }
RegisterHotKey(winkey_popup->get_window_handle(), alternative_switch_hotkey_id, alternative_switch_modifier_mask, alternative_switch_vk_code); RegisterHotKey(winkey_popup->get_window_handle(), alternative_switch_hotkey_id, alternative_switch_modifier_mask, alternative_switch_vk_code);

View File

@@ -65,3 +65,16 @@ void Trace::SettingsChanged(const int press_delay_time, const int overlay_opacit
TraceLoggingBoolean(TRUE, "UTCReplace_AppSessionGuid"), TraceLoggingBoolean(TRUE, "UTCReplace_AppSessionGuid"),
TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE)); TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE));
} }
// Log if an error occurs in Shortcut Guide
void Trace::Error(const DWORD errorCode, std::wstring errorMessage, std::wstring methodName) noexcept
{
TraceLoggingWrite(
g_hProvider,
"ShortcutGuide_Error",
ProjectTelemetryPrivacyDataTag(ProjectTelemetryTag_ProductAndServicePerformance),
TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE),
TraceLoggingValue(methodName.c_str(), "MethodName"),
TraceLoggingValue(errorCode, "ErrorCode"),
TraceLoggingValue(errorMessage.c_str(), "ErrorMessage"));
}

View File

@@ -8,4 +8,5 @@ public:
static void HideGuide(const __int64 duration_ms, std::vector<int>& key_pressed) noexcept; static void HideGuide(const __int64 duration_ms, std::vector<int>& key_pressed) noexcept;
static void EnableShortcutGuide(const bool enabled) noexcept; static void EnableShortcutGuide(const bool enabled) noexcept;
static void SettingsChanged(const int press_delay_time, const int overlay_opacity, const std::wstring& theme) noexcept; static void SettingsChanged(const int press_delay_time, const int overlay_opacity, const std::wstring& theme) noexcept;
static void Error(const DWORD errorCode, std::wstring errorMessage, std::wstring methodName) noexcept;
}; };