Attempt to fix and recover after taskbar widget crash and update manifest version #228
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements a crash recovery mechanism for the taskbar widget to handle edge cases where the taskbar resets, crashes, or disappears. The changes add error handling through try-catch blocks for UI automation operations and introduce automatic recovery attempts when the widget window handle is lost.
Key Changes:
- Added recovery mechanism with attempt limiting (max 5 attempts) when taskbar widget window handle becomes invalid
- Wrapped UI automation operations in try-catch blocks to handle COM exceptions and stale element references
- Updated timer interval from 2500ms to 1500ms for more responsive position updates
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| FluentFlyoutWPF/Windows/TaskbarWindow.xaml.cs | Adds recovery tracking fields, implements async recovery logic in UpdatePosition(), and adds comprehensive error handling to GetTaskbarWidgetRect() with specific handling for ElementNotAvailableException and COMException |
| FluentFlyoutWPF/MainWindow.xaml.cs | Implements RecreateTaskbarWindow() method to close and recreate the taskbar widget window during recovery |
| FluentFlyoutMSIX/Package.appxmanifest | Updates application version from 2.3.6.0 to 2.4.0.0 |
Comments suppressed due to low confidence (1)
FluentFlyoutWPF/MainWindow.xaml.cs:1219
- Poor error handling: empty catch block.
catch { }
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Dispatcher.BeginInvoke(async () => | ||
| { | ||
| await Task.Delay(1000); // delay before recovery to let taskbar stabilize | ||
| try | ||
| { | ||
| _mainWindow?.RecreateTaskbarWindow(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logger.Error(ex, "Failed to signal MainWindow to recover Taskbar Widget window"); | ||
| _recoveryAttempts++; | ||
| } |
There was a problem hiding this comment.
The recovery attempt counter is only incremented when RecreateTaskbarWindow throws an exception, not when it's successfully called. This means successful recovery attempts aren't counted toward the maximum limit. If recovery fails silently or succeeds, the counter won't increment, potentially allowing infinite recovery loops.
The counter should be incremented before calling RecreateTaskbarWindow, not just in the catch block. This ensures all recovery attempts are counted regardless of success or failure.
| { | ||
| taskbarWindow.Close(); | ||
| } | ||
| catch { } |
There was a problem hiding this comment.
Empty catch block silently swallows exceptions when attempting to close the taskbar window. Consider logging the exception or at least adding a comment explaining why the exception can be safely ignored.
| catch { } | |
| catch (Exception ex) | |
| { | |
| // Swallow the exception to continue recreating the taskbar window, | |
| // but log it for diagnostics. | |
| Logger.Warn(ex, "Failed to close existing Taskbar Widget window before recreation"); | |
| } |
| if (interop.Handle == IntPtr.Zero) // window handle lost, try to reset | ||
| { | ||
| _timer.Stop(); | ||
| Logger.Warn("Taskbar Widget window handle is zero, stopping position updates."); | ||
|
|
||
| if (_recoveryAttempts >= _maxRecoveryAttempts) | ||
| { | ||
| Logger.Warn("Taskbar Widget window handle is zero and recovery already attempted, stopping updates."); | ||
| return; // already tried recovery, don't loop | ||
| } | ||
|
|
||
| Logger.Warn("Taskbar Widget window handle is zero, attempting recovery..."); | ||
|
|
||
| Dispatcher.BeginInvoke(async () => | ||
| { | ||
| await Task.Delay(1000); // delay before recovery to let taskbar stabilize | ||
| try | ||
| { | ||
| _mainWindow?.RecreateTaskbarWindow(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logger.Error(ex, "Failed to signal MainWindow to recover Taskbar Widget window"); | ||
| _recoveryAttempts++; | ||
| } | ||
| }, DispatcherPriority.Background); |
There was a problem hiding this comment.
The recovery logic uses an asynchronous delayed operation without preventing concurrent recovery attempts. If UpdatePosition is called multiple times before the first recovery completes (though unlikely since the timer is stopped), multiple recovery operations could be scheduled, bypassing the recovery attempt limit.
Consider adding a boolean flag to track if recovery is in progress and check it before scheduling a new recovery attempt.
| private MainWindow? _mainWindow; | ||
| private bool _isPaused; | ||
| private int _recoveryAttempts = 0; | ||
| private int _maxRecoveryAttempts = 5; |
There was a problem hiding this comment.
Field '_maxRecoveryAttempts' can be 'readonly'.
| private int _maxRecoveryAttempts = 5; | |
| private readonly int _maxRecoveryAttempts = 5; |
Attempt to fix taskbar widget crashing in edge-cases where the taskbar resets, crashes or disappears. UI automations are now in try-catch blocks, and if the widget crashes, it will try to restart itself.
Fixes #226