Add in verbose error message and telemetry for SetWindowsHookEx failu…

…re (#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
microsoft · Sep 9, 2020 · fb1888f · fb1888f
1 parent f61e9d3
commit fb1888f
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 45 deletions.
diff --git a/src/common/common.cpp b/src/common/common.cpp
@@ -11,12 +11,6 @@
 #pragma comment(lib, "advapi32.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)
 {
     RECT window;
@@ -91,7 +85,7 @@ std::optional<std::wstring> get_last_error_message(const DWORD dw)
     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);
     if (!system_message.has_value())
@@ -107,7 +101,7 @@ void show_last_error_message(LPCWSTR lpszFunction, DWORD dw)
                          lpszFunction,
                          dw,
                          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);
     }
 }

diff --git a/src/common/common.h b/src/common/common.h
@@ -6,6 +6,13 @@
 #include <memory>
 #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.
 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 = {});
 
 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
 {

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

diff --git a/src/modules/fancyzones/dll/dllmain.cpp b/src/modules/fancyzones/dll/dllmain.cpp
@@ -82,10 +82,10 @@ class FancyZonesModule : public PowertoyModuleIface
                 s_llKeyboardHook = SetWindowsHookEx(WH_KEYBOARD_LL, LowLevelKeyboardProc, GetModuleHandle(NULL), NULL);
                 if (!s_llKeyboardHook)
                 {
-                    MessageBoxW(NULL,
-                                GET_RESOURCE_STRING(IDS_KEYBOARD_LISTENER_ERROR).c_str(),
-                                GET_RESOURCE_STRING(IDS_POWERTOYS_FANCYZONES).c_str(),
-                                MB_OK | MB_ICONERROR);
+                    DWORD errorCode = GetLastError();
+                    show_last_error_message(L"SetWindowsHookEx", errorCode, GET_RESOURCE_STRING(IDS_POWERTOYS_FANCYZONES).c_str());
+                    auto errorMessage = get_last_error_message(errorCode);
+                    Trace::FancyZones::Error(errorCode, errorMessage.has_value() ? errorMessage.value() : L"", L"enable.SetWindowsHookEx");
                 }
             }
 

diff --git a/src/modules/fancyzones/lib/trace.cpp b/src/modules/fancyzones/lib/trace.cpp
@@ -213,6 +213,19 @@ void Trace::FancyZones::EditorLaunched(int value) noexcept
         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
 {
     const auto& editorHotkey = settings.editorHotkey;

diff --git a/src/modules/fancyzones/lib/trace.h b/src/modules/fancyzones/lib/trace.h
@@ -16,6 +16,7 @@ class Trace
         static void OnKeyDown(DWORD vkCode, bool win, bool control, bool inMoveSize) noexcept;
         static void DataChanged() 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;

diff --git a/src/modules/keyboardmanager/common/trace.cpp b/src/modules/keyboardmanager/common/trace.cpp
@@ -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"));
+}
diff --git a/src/modules/keyboardmanager/common/trace.h b/src/modules/keyboardmanager/common/trace.h
@@ -23,4 +23,7 @@ class Trace
 
     // Log if a shortcut remap has been invoked
     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;
 };
diff --git a/src/modules/keyboardmanager/dll/dllmain.cpp b/src/modules/keyboardmanager/dll/dllmain.cpp
@@ -363,7 +363,10 @@ class KeyboardManager : public PowertoyModuleIface
             hook_handle_copy = 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");
             }
         }
     }

diff --git a/src/modules/launcher/Plugins/Microsoft.Plugin.Folder/Properties/Resources.Designer.cs b/src/modules/launcher/Plugins/Microsoft.Plugin.Folder/Properties/Resources.Designer.cs
diff --git a/src/modules/launcher/PowerLauncher/Properties/Resources.Designer.cs b/src/modules/launcher/PowerLauncher/Properties/Resources.Designer.cs
diff --git a/src/modules/shortcut_guide/shortcut_guide.cpp b/src/modules/shortcut_guide/shortcut_guide.cpp
@@ -6,6 +6,7 @@
 #include <common/common.h>
 #include <common/settings_objects.h>
 #include <common/debug_control.h>
+#include <sstream>
 
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 
@@ -84,8 +85,8 @@ namespace
         // For now, since ShortcutGuide can only disable entire "Windows Controls"
         // group, we require that the window supports all the options.
         result.snappable = ((style & WS_MAXIMIZEBOX) == WS_MAXIMIZEBOX) &&
-                            ((style & WS_MINIMIZEBOX) == WS_MINIMIZEBOX) &&
-                            ((style & WS_THICKFRAME) == WS_THICKFRAME);
+                           ((style & WS_MINIMIZEBOX) == WS_MINIMIZEBOX) &&
+                           ((style & WS_THICKFRAME) == WS_THICKFRAME);
         return result;
     }
 }
@@ -224,7 +225,10 @@ void OverlayWindow::enable()
             hook_handle = SetWindowsHookEx(WH_KEYBOARD_LL, LowLevelKeyboardProc, GetModuleHandle(NULL), NULL);
             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);

diff --git a/src/modules/shortcut_guide/trace.cpp b/src/modules/shortcut_guide/trace.cpp
@@ -65,3 +65,16 @@ void Trace::SettingsChanged(const int press_delay_time, const int overlay_opacit
         TraceLoggingBoolean(TRUE, "UTCReplace_AppSessionGuid"),
         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"));
+}
diff --git a/src/modules/shortcut_guide/trace.h b/src/modules/shortcut_guide/trace.h
@@ -8,4 +8,5 @@ class Trace
     static void HideGuide(const __int64 duration_ms, std::vector<int>& key_pressed) 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 Error(const DWORD errorCode, std::wstring errorMessage, std::wstring methodName) noexcept;
 };