Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

arjunbalgovind
Copy link
Contributor

@arjunbalgovind arjunbalgovind commented Sep 8, 2020

Summary of the Pull Request

What is this about?
In #5414 the exceptions that users reported was related to the SetWindowsHookEx API failing. As per the docs and after the reaching out to the respective team, the suggestion was to use the GetLastError API to get a more detailed error code/message. In this PR I have modified the Message box that we were earlier displaying to show the error code as well as the error message as well, rather than the generic "Cannot install keyboard listener". This will allow us to get more details when users post the issue, since we do not have enough information to figure out the reason right now. The PR also adds telemetry to log this exception for KBM, FZ and SG.

PR Checklist

Info on Pull Request

What does this include?

  • show_last_error_message in common.h was modified to allow a different message box title, with default set to the earlier value.
  • In KBM, FZ, SG and interop, when the SetWindowsHookEx return value is NULL, GetLastError is called along with show_last_error_message.
  • A telemetry event was added for KBM, FZ and SG, which logs the error code, error message and method name. This is called after show_last_error_message, with the method name as the parent function name followed by SetWindowsHookEx (for example for SG it is OverlayWindow.enable.SetWindowsHookEx).
  • As per this comment Added telemetry to ImageResizer Shell Extension code (dev/imageResizer) #1272 (comment) we can't have a generic telemetry event across all modules

Validation Steps Performed

How does someone test & validate?

  • Build and run PowerToys
  • Modify one of the SetWindowsHookEx checks to check non-null and validate that the message box appears.

@arjunbalgovind arjunbalgovind requested a review from a team September 8, 2020 22:19
@arjunbalgovind arjunbalgovind changed the title User/arbalgov/hook api fail Add in verbose error message and telemetry for SetWindowsHookEx failure Sep 8, 2020
src/modules/fancyzones/dll/dllmain.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@enricogior enricogior left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants