fix(windows): handle hwnd==0 gracefully for UAC/secure desktop#118
Conversation
When the foreground window handle is 0 (e.g. during UAC prompts, lock screen, or secure desktop), Win32 API calls like OpenProcess fail with "The parameter is incorrect" (error 87). This caused continuous error spam in logs. Fix: - Return None early when hwnd==0, letting heartbeat_loop skip the poll cycle (it already handles None gracefully) - Wrap WMI fallback in try/except so if both win32api and WMI fail for elevated processes, we degrade to "unknown" instead of crashing Fixes ActivityWatch#90
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 0723a64 in 11 seconds. Click for details.
- Reviewed
28lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_ugiXQEsElxrUhE24
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Greptile SummaryAdds defensive handling for scenarios where no foreground window exists (UAC prompts, lock screen, secure desktop). The fix prevents error spam by returning early when Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 0723a64 |
| window_handle = windows.get_active_window_handle() | ||
|
|
||
| # hwnd 0 means no foreground window (e.g. during UAC prompt, lock screen, | ||
| # or secure desktop). Return None so heartbeat_loop skips this poll cycle. |
There was a problem hiding this comment.
Should it skip poll cycle or lead to unknown?
There was a problem hiding this comment.
Good question — I'd argue skip the poll cycle is the right behavior here, for the same reason we already do it for non-Windows (the get_current_window functions return None and heartbeat_loop treats it as 'no window, try again').
The distinction:
hwnd == 0: There is literally no foreground window (lock screen, UAC prompt, secure desktop). Nothing to record.app = None(from failed OpenProcess / WMI): There is a window, we just can't identify it → falls back to"unknown".
Recording "unknown" during a UAC prompt or lock screen would create misleading activity — the user isn't actually in any app. Skipping the heartbeat leaves a gap (or the previous heartbeat's pulsetime covers it), which is semantically correct.
That said, if you'd prefer "unknown" for continuity, happy to change it — it's a one-liner swap.
Summary
hwnd == 0(no foreground window), which occurs during UAC prompts, lock screen, and secure desktopProblem
When the foreground window handle is 0 (e.g. during UAC prompts, lock screen, or secure desktop),
OpenProcessfails withpywintypes.error: (87, 'OpenProcess', 'The parameter is incorrect.'). This causes continuous error logging every poll cycle.Additionally, when the primary
get_app_name()fails for elevated/admin processes (error 5: Access denied), the WMI fallback could also fail and propagate exceptions upward, causing more error spam.Fix
Early return for
hwnd == 0:get_active_window_handle()returns 0 when there's no foreground window. We now returnNoneearly, whichheartbeat_loopalready handles gracefully (logs debug, skips heartbeat, tries again next poll).Wrapped WMI fallback: If both
get_app_name()andget_app_name_wmi()fail, we now setapp = Nonewhich resolves to"unknown"instead of propagating the exception.Test plan
heartbeat_loopalready handlesNonereturn fromget_current_window()(line 144-145 in main.py)Fixes #90
Important
Handles
hwnd == 0gracefully inget_current_window_windows()and wraps WMI fallback to prevent error spam inlib.py.get_current_window_windows()inlib.pyreturnsNoneifwindow_handleis 0, skipping the poll cycle during UAC prompts or secure desktop.get_current_window_windows()to setapp = Noneif bothget_app_name()andget_app_name_wmi()fail, resolving to "unknown" instead of propagating exceptions.lib.py.This description was created by
for 0723a64. You can customize this summary. It will automatically update as commits are pushed.