Merge pull request #5365 from MDoerner/AdditionalTopLevelExceptionHandling

Additional top level exception handling and better status message sequencing

Author: retailcoder

Rubberduck.Core/UI/Command/MenuItems/CommandBars/AppCommandBarBase.cs

@@ -37,7 +37,13 @@ protected ICommandMenuItem FindChildByTag(string tag)
             }
             catch (COMException exception)
             {
-                Logger.Error(exception,$"COMException while finding child with tag '{tag}'.");
+                Logger.Error(exception, $"COMException while finding child with tag '{tag}' in the command bar.");
+            }
+            catch (InvalidCastException exception)
+            {
+                //This exception will be encountered whenever the registration of the command bar control not correct in the system.
+                //See issue #5349 at https://github.com/rubberduck-vba/Rubberduck/issues/5349
+                Logger.Error(exception, $"Invalid cast exception while finding child with tag '{tag}' in the command bar.");
             }
             return null;
         }

Rubberduck.Core/UI/Command/MenuItems/CommandBars/RubberduckCommandBar.cs

@@ -2,8 +2,8 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
+using System.Runtime.InteropServices;
 using Rubberduck.Resources;
-using Rubberduck.Parsing;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.UIContext;
 using Rubberduck.Parsing.VBA;
@@ -14,18 +14,18 @@ namespace Rubberduck.UI.Command.MenuItems.CommandBars
     public class RubberduckCommandBar : AppCommandBarBase, IDisposable
     {
         private readonly IContextFormatter _formatter;
-        private readonly IParseCoordinator _parser;
+        private readonly RubberduckParserState _state;
         private readonly ISelectionChangeService _selectionService;
 
-        public RubberduckCommandBar(IParseCoordinator parser, IEnumerable<ICommandMenuItem> items, IContextFormatter formatter, ISelectionChangeService selectionService, IUiDispatcher uiDispatcher) 
+        public RubberduckCommandBar(RubberduckParserState state, IEnumerable<ICommandMenuItem> items, IContextFormatter formatter, ISelectionChangeService selectionService, IUiDispatcher uiDispatcher) 
             : base("Rubberduck", CommandBarPosition.Top, items, uiDispatcher)
         {
-            _parser = parser;
+            _state = state;
             _formatter = formatter;
             _selectionService = selectionService;
 
-            _parser.State.StateChanged += OnParserStateChanged;
-            _parser.State.StatusMessageUpdate += OnParserStatusMessageUpdate;
+            _state.StateChangedHighPriority += OnParserStateChanged;
+            _state.StatusMessageUpdate += OnParserStatusMessageUpdate;
             _selectionService.SelectionChanged += OnSelectionChange;
         }
 
@@ -35,14 +35,14 @@ public override void Initialize()
         {
             base.Initialize();
             SetStatusLabelCaption(ParserState.Pending);
-            EvaluateCanExecute(_parser.State);
+            EvaluateCanExecute(_state);
         }
 
         private Declaration _lastDeclaration;
         private ParserState _lastStatus = ParserState.None;
         private void EvaluateCanExecute(RubberduckParserState state, Declaration selected)
         {
-            var currentStatus = _parser.State.Status;
+            var currentStatus = _state.Status;
             if (_lastStatus == currentStatus && 
                 (selected == null || selected.Equals(_lastDeclaration)) &&
                 (selected != null || _lastDeclaration == null))
@@ -68,7 +68,7 @@ private void OnSelectionChange(object sender, DeclarationChangedEventArgs e)
             var description = e.Declaration?.DescriptionString ?? string.Empty;
             //& renders the next character as if it was an accelerator.
             SetContextSelectionCaption(caption?.Replace("&", "&&"), refCount, description);
-            EvaluateCanExecute(_parser.State, e.Declaration);
+            EvaluateCanExecute(_state, e.Declaration);
         }
 
 
@@ -81,14 +81,14 @@ private void OnParserStatusMessageUpdate(object sender, RubberduckStatusMessageE
                 message = RubberduckUI.ParserState_LoadingReference;
             }
 
-            SetStatusLabelCaption(message, _parser.State.ModuleExceptions.Count);            
+            SetStatusLabelCaption(message, _state.ModuleExceptions.Count);            
         }
 
         private void OnParserStateChanged(object sender, EventArgs e)
         {
-            _lastStatus = _parser.State.Status;
-            EvaluateCanExecute(_parser.State);    
-            SetStatusLabelCaption(_parser.State.Status, _parser.State.ModuleExceptions.Count);                 
+            _lastStatus = _state.Status;
+            EvaluateCanExecute(_state);    
+            SetStatusLabelCaption(_state.Status, _state.ModuleExceptions.Count);                 
         }
 
         public void SetStatusLabelCaption(ParserState state, int? errorCount = null)
@@ -99,11 +99,19 @@ public void SetStatusLabelCaption(ParserState state, int? errorCount = null)
 
         private void SetStatusLabelCaption(string caption, int? errorCount = null)
         {
-            var reparseCommandButton = FindChildByTag(typeof(ReparseCommandMenuItem).FullName) as ReparseCommandMenuItem;
-            if (reparseCommandButton == null) { return; }
+            var reparseCommandButton =
+                FindChildByTag(typeof(ReparseCommandMenuItem).FullName) as ReparseCommandMenuItem;
+            if (reparseCommandButton == null)
+            {
+                return;
+            }
 
-            var showErrorsCommandButton = FindChildByTag(typeof(ShowParserErrorsCommandMenuItem).FullName) as ShowParserErrorsCommandMenuItem;
-            if (showErrorsCommandButton == null) { return; }
+            var showErrorsCommandButton =
+                FindChildByTag(typeof(ShowParserErrorsCommandMenuItem).FullName) as ShowParserErrorsCommandMenuItem;
+            if (showErrorsCommandButton == null)
+            {
+                return;
+            }
 
             _uiDispatcher.Invoke(() =>
             {
@@ -119,7 +127,8 @@ private void SetStatusLabelCaption(string caption, int? errorCount = null)
                 }
                 catch (Exception exception)
                 {
-                    Logger.Error(exception, "Exception thrown trying to set the status label caption on the UI thread.");
+                    Logger.Error(exception,
+                        "Exception thrown trying to set the status label caption on the UI thread.");
                 }
             });
             Localize();
@@ -162,8 +171,8 @@ protected virtual void Dispose(bool disposing)
             }
 
             _selectionService.SelectionChanged -= OnSelectionChange;
-            _parser.State.StateChanged -= OnParserStateChanged;
-            _parser.State.StatusMessageUpdate -= OnParserStatusMessageUpdate;
+            _state.StateChangedHighPriority -= OnParserStateChanged;
+            _state.StatusMessageUpdate -= OnParserStatusMessageUpdate;
 
             RemoveCommandBar();
 

Rubberduck.Core/UI/Inspections/InspectionResultsViewModel.cs

@@ -428,6 +428,20 @@ private void HandleStateChanged(object sender, ParserStateEventArgs e)
         }
 
         private async void RefreshInspections(CancellationToken token)
+        {
+            //We have to catch all exceptions here since this method is a fire-and-forget async action.
+            //Accordingly, any exception bubbling out of this method will likely take down the runtime and, thus, crash the host.
+            try
+            {
+                await RefreshInspections_Internal(token);
+            }
+            catch (Exception ex)
+            {
+                Logger.Error(ex,"Unhandled exception when refreshing inspection results.");
+            }
+        }
+
+        private async Task RefreshInspections_Internal(CancellationToken token)
         {
             var stopwatch = Stopwatch.StartNew();
             IsBusy = true;

Rubberduck.Parsing/VBA/RubberduckParserState.cs

@@ -359,6 +359,7 @@ public IReadOnlyList<Tuple<QualifiedModuleName, SyntaxErrorException>> ModuleExc
             }
         }
 
+        public event EventHandler<ParserStateEventArgs> StateChangedHighPriority;
         public event EventHandler<ParserStateEventArgs> StateChanged;
 
         private int _stateChangedInvocations;
@@ -367,6 +368,27 @@ private void OnStateChanged(object requestor, CancellationToken token, ParserSta
             Interlocked.Increment(ref _stateChangedInvocations);
 
             Logger.Info($"{nameof(RubberduckParserState)} ({_stateChangedInvocations}) is invoking {nameof(StateChanged)} ({Status})");
+
+            var highPriorityHandler = StateChangedHighPriority;
+            if (highPriorityHandler != null && !token.IsCancellationRequested)
+            {
+                try
+                {
+                    var args = new ParserStateEventArgs(state, oldStatus, token);
+                    highPriorityHandler.Invoke(requestor, args);
+                }
+                catch (OperationCanceledException cancellation)
+                {
+                    throw;
+                }
+                catch (Exception e)
+                {
+                    // Error state, because this implies consumers are not exception-safe!
+                    // this behaviour could leave us in a state where some consumers have correctly updated and some have not
+                    Logger.Error(e, "An exception occurred when notifying consumers of updated parser state.");
+                }
+            }
+
             var handler = StateChanged;
             if (handler != null && !token.IsCancellationRequested)
             {