Add ShowExceptionmethod to BaseSamplePage #275
Conversation
| { | ||
| var msg = optionalMessage ?? ex switch | ||
| { | ||
| COMException |
There was a problem hiding this comment.
IMO, we should have this code in the sample itself as this method can be generalized for all samples, not just for WCR samples. The ShowException method should be very general
There was a problem hiding this comment.
It could live in a simple static class (in a SharedCode file). The file would use the XamlRoot from the static App.Window?.Content.XamlRoot (probably) and same for RootFrame.
| @@ -15,13 +18,29 @@ internal partial class BaseSamplePage : Page | |||
| protected override async void OnNavigatedTo(NavigationEventArgs e) | |||
| { | |||
| base.OnNavigatedTo(e); | |||
| App.Current.UnhandledException += Current_UnhandledException; | |||
There was a problem hiding this comment.
Wouldn't this handle all app exception and not just the sample exception?
| "The WCL is in an unstable state.\nRebooting the machine will restart the WCL.", | ||
| _ => $"Error:\n{ex.Message}{(optionalMessage != null ? "\n" + optionalMessage : string.Empty)}" | ||
| }; | ||
| ContentDialog exceptionDialog = new() |
There was a problem hiding this comment.
We should have a copy exception details button here
| { | ||
| var msg = optionalMessage ?? ex switch | ||
| { | ||
| COMException |
There was a problem hiding this comment.
It could live in a simple static class (in a SharedCode file). The file would use the XamlRoot from the static App.Window?.Content.XamlRoot (probably) and same for RootFrame.
| Loader.Visibility = Visibility.Collapsed; | ||
| StopBtn.Visibility = Visibility.Collapsed; | ||
| _cts?.Dispose(); | ||
| _cts = null; | ||
| } |
There was a problem hiding this comment.
Shouldn't this be in the finally block, or outside the catch?
| @@ -60,6 +61,10 @@ private static ChatOptions GetDefaultChatOptions() | |||
| { | |||
| await phiSilicaClient.InitializeAsync(cancellationToken); | |||
| } | |||
| catch (COMException ex) when (ex.Message.Contains("the rpc server is unavailable", System.StringComparison.CurrentCultureIgnoreCase)) | |||
There was a problem hiding this comment.
If you are handling this here and throwing, this raises the question of if we should handle any exception here at all. I mean, why return null and not throw always, if there is an exception that is being handled elsewhere?
Fixes #175
Fixes #176