From 134f12a56151d3791d677c599e25f817a8624ce7 Mon Sep 17 00:00:00 2001 From: Tucker Burns Date: Tue, 31 Mar 2026 19:00:39 -0700 Subject: [PATCH] Add defensive error handling for Settings navigation and search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the root cause chain behind the NullReferenceException crash in Frame_NavigationFailed. Crash dump analysis with WinDbg revealed that Frame.Navigate() can throw native WinUI ABI exceptions that propagate unhandled through multiple code paths. NavigationService.Navigate: - Wrap Frame.Navigate() in try-catch to gracefully handle native WinUI ABI failures (e.g. page XAML parse errors, InitializeComponent failures) - Log the exception details for diagnostics instead of letting it propagate - Return false on failure so callers can handle navigation failure gracefully NavigationService.EnsurePageIsSelected: - Add matching try-catch for consistency — this method also calls Frame.Navigate() without protection ShellViewModel.Frame_NavigationFailed: - Replace 'throw e.Exception' with 'e.Handled = true' and Logger.LogError - The original code threw a NullReferenceException when e.Exception was null (native errors don't always marshal to managed Exception objects) - Mark the event handled to prevent the framework from propagating the error ShellPage.SearchBox_QuerySubmitted: - Add try-catch around the entire async void method body - Prevents unhandled exceptions from crashing the process when search indexing (Task.Run) or navigation to SearchResultsPage fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Settings.UI/Services/NavigationService.cs | 28 ++++++++++--- .../SettingsXAML/Views/ShellPage.xaml.cs | 41 +++++++++++-------- .../Settings.UI/ViewModels/ShellViewModel.cs | 4 +- 3 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/settings-ui/Settings.UI/Services/NavigationService.cs b/src/settings-ui/Settings.UI/Services/NavigationService.cs index d7c408208b90..c8f6854dcf19 100644 --- a/src/settings-ui/Settings.UI/Services/NavigationService.cs +++ b/src/settings-ui/Settings.UI/Services/NavigationService.cs @@ -4,6 +4,7 @@ using System; +using ManagedCommon; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Media.Animation; @@ -57,13 +58,21 @@ public static bool Navigate(Type pageType, object parameter = null, NavigationTr // Don't open the same page multiple times if (Frame.Content?.GetType() != pageType || (parameter != null && !parameter.Equals(lastParamUsed))) { - var navigationResult = Frame.Navigate(pageType, parameter, infoOverride); - if (navigationResult) + try { - lastParamUsed = parameter; - } + var navigationResult = Frame.Navigate(pageType, parameter, infoOverride); + if (navigationResult) + { + lastParamUsed = parameter; + } - return navigationResult; + return navigationResult; + } + catch (Exception ex) + { + Logger.LogError($"Navigation to {pageType?.Name} failed with exception", ex); + return false; + } } else { @@ -101,7 +110,14 @@ internal static void EnsurePageIsSelected(Type pageType) { if (Frame.Content == null) { - Frame.Navigate(pageType); + try + { + Frame.Navigate(pageType); + } + catch (Exception ex) + { + Logger.LogError($"EnsurePageIsSelected failed for {pageType?.Name}", ex); + } } } } diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/ShellPage.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/Views/ShellPage.xaml.cs index 7515cad863fc..a78406913571 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/ShellPage.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/ShellPage.xaml.cs @@ -599,27 +599,34 @@ private List BuildSuggestionItems(string query, List(); - return; - } + var queryText = (args.QueryText ?? _lastQueryText)?.Trim(); + if (string.IsNullOrWhiteSpace(queryText)) + { + NavigationService.Navigate(); + return; + } - // Prefer cached results (from live search); if empty, perform a fresh search - var matched = _lastSearchResults?.Count > 0 && string.Equals(_lastQueryText, queryText, StringComparison.Ordinal) - ? _lastSearchResults - : await Task.Run(() => SearchIndexService.Search(queryText)); + // Prefer cached results (from live search); if empty, perform a fresh search + var matched = _lastSearchResults?.Count > 0 && string.Equals(_lastQueryText, queryText, StringComparison.Ordinal) + ? _lastSearchResults + : await Task.Run(() => SearchIndexService.Search(queryText)); - var searchParams = new SearchResultsNavigationParams(queryText, matched); - NavigationService.Navigate(searchParams); + var searchParams = new SearchResultsNavigationParams(queryText, matched); + NavigationService.Navigate(searchParams); + } + catch (Exception ex) + { + Logger.LogError("Search query submission failed", ex); + } } public void Dispose() diff --git a/src/settings-ui/Settings.UI/ViewModels/ShellViewModel.cs b/src/settings-ui/Settings.UI/ViewModels/ShellViewModel.cs index aa0e8855f2de..c04afdf39510 100644 --- a/src/settings-ui/Settings.UI/ViewModels/ShellViewModel.cs +++ b/src/settings-ui/Settings.UI/ViewModels/ShellViewModel.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using System.Windows.Input; +using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Helpers; using Microsoft.PowerToys.Settings.UI.Library; using Microsoft.PowerToys.Settings.UI.Library.Helpers; @@ -135,7 +136,8 @@ private void OnBackRequested(NavigationView sender, NavigationViewBackRequestedE private void Frame_NavigationFailed(object sender, NavigationFailedEventArgs e) { - throw e.Exception; + e.Handled = true; + Logger.LogError($"Failed to load page '{e.SourcePageType?.FullName}'", e.Exception); } private void Frame_Navigated(object sender, NavigationEventArgs e)