Improve Components error handling

* Change event handlers IHandleEvent, IHandleAfterEvent to be async.
* Return faulted tasks to Renderer instead of handling exceptions in ComponentBase
* Use ILogger in RemoteRenderer, and log to console in WebAssemblyRenderer
* Cleaning up touched files

Fixes #4964

Author: pranavkm

src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs

@@ -1,13 +1,13 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
+using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components;
 using Microsoft.AspNetCore.Components.Browser;
 using Microsoft.AspNetCore.Components.Rendering;
 using Microsoft.JSInterop;
 using Mono.WebAssembly.Interop;
-using System;
-using System.Threading.Tasks;
 
 namespace Microsoft.AspNetCore.Blazor.Rendering
 {
@@ -31,9 +31,6 @@ public WebAssemblyRenderer(IServiceProvider serviceProvider): base(serviceProvid
             _webAssemblyRendererId = RendererRegistry.Current.Add(this);
         }
 
-        internal void DispatchBrowserEvent(int componentId, int eventHandlerId, UIEventArgs eventArgs)
-            => DispatchEvent(componentId, eventHandlerId, eventArgs);
-
         /// <summary>
         /// Attaches a new root component to the renderer,
         /// causing it to be displayed in the specified DOM element.
@@ -95,5 +92,24 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch)
                 throw new NotImplementedException($"{nameof(WebAssemblyRenderer)} is supported only with in-process JS runtimes.");
             }
         }
+
+        /// <inheritdoc />
+        protected override bool HandleException(int componentId, IComponent component, Exception exception)
+        {
+            Console.Error.WriteLine($"Unhandled exception executing component {component.GetType().FullName} ({componentId})");
+            if (exception is AggregateException aggregateException)
+            {
+                foreach (var innerException in aggregateException.Flatten().InnerExceptions)
+                {
+                    Console.Error.WriteLine(innerException);
+                }
+            }
+            else
+            {
+                Console.Error.WriteLine(exception);
+            }
+
+            return true;
+        }
     }
 }

src/Components/Browser/src/RendererRegistryEventDispatcher.cs

@@ -1,9 +1,10 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
+using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components.Rendering;
 using Microsoft.JSInterop;
-using System;
 
 namespace Microsoft.AspNetCore.Components.Browser
 {
@@ -16,12 +17,13 @@ public static class RendererRegistryEventDispatcher
         /// For framework use only.
         /// </summary>
         [JSInvokable(nameof(DispatchEvent))]
-        public static void DispatchEvent(
+        public static Task DispatchEvent(
             BrowserEventDescriptor eventDescriptor, string eventArgsJson)
         {
             var eventArgs = ParseEventArgsJson(eventDescriptor.EventArgsType, eventArgsJson);
             var renderer = RendererRegistry.Current.Find(eventDescriptor.BrowserRendererId);
-            renderer.DispatchEvent(
+
+            return renderer.DispatchEventAsync(
                 eventDescriptor.ComponentId,
                 eventDescriptor.EventHandlerId,
                 eventArgs);

src/Components/Components/src/ComponentBase.cs

@@ -32,7 +32,7 @@ public abstract class ComponentBase : IComponent, IHandleEvent, IHandleAfterRend
 
         private readonly RenderFragment _renderFragment;
         private RenderHandle _renderHandle;
-        private bool _hasCalledInit;
+        private bool _initialized;
         private bool _hasNeverRendered = true;
         private bool _hasPendingQueuedRender;
 
@@ -177,188 +177,93 @@ void IComponent.Configure(RenderHandle renderHandle)
         public virtual Task SetParametersAsync(ParameterCollection parameters)
         {
             parameters.SetParameterProperties(this);
-            if (!_hasCalledInit)
+            if (!_initialized)
             {
-                return RunInitAndSetParameters();
+                _initialized = true;
+
+                return RunInitAndSetParametersAsync();
             }
             else
             {
-                OnParametersSet();
-                // If you override OnInitAsync or OnParametersSetAsync and return a noncompleted task,
-                // then by default we automatically re-render once each of those tasks completes.
-                var isAsync = false;
-                Task parametersTask = null;
-                (isAsync, parametersTask) = ProcessLifeCycletask(OnParametersSetAsync());
-                StateHasChanged();
-                // We call StateHasChanged here so that we render after OnParametersSet and after the
-                // synchronous part of OnParametersSetAsync has run, and in case there is async work
-                // we trigger another render.
-                if (isAsync)
-                {
-                    return parametersTask;
-                }
-
-                return Task.CompletedTask;
+                return SetParametersAsyncCore();
             }
         }
 
-        private async Task RunInitAndSetParameters()
+        private async Task RunInitAndSetParametersAsync()
         {
-            _hasCalledInit = true;
-            var initIsAsync = false;
-
             OnInit();
-            Task initTask = null;
-            (initIsAsync, initTask) = ProcessLifeCycletask(OnInitAsync());
-            if (initIsAsync)
+            var task = OnInitAsync();
+
+            if (task.Status != TaskStatus.RanToCompletion && task.Status != TaskStatus.Canceled)
             {
                 // Call state has changed here so that we render after the sync part of OnInitAsync has run
                 // and wait for it to finish before we continue. If no async work has been done yet, we want
                 // to defer calling StateHasChanged up until the first bit of async code happens or until
-                // the end.
+                // the end. Additionally, we want to avoid calling StateHasChanged if no
+                // async work is to be performed.
                 StateHasChanged();
-                await initTask;
-            }
 
-            OnParametersSet();
-            Task parametersTask = null;
-            var setParametersIsAsync = false;
-            (setParametersIsAsync, parametersTask) = ProcessLifeCycletask(OnParametersSetAsync());
-            // We always call StateHasChanged here as we want to trigger a rerender after OnParametersSet and
-            // the synchronous part of OnParametersSetAsync has run, triggering another re-render in case there
-            // is additional async work.
-            StateHasChanged();
-            if (setParametersIsAsync)
-            {
-                await parametersTask;
+                await ProcessLifecycleTaskAsync(task);
             }
+
+            await SetParametersAsyncCore();
         }
 
-        private (bool isAsync, Task asyncTask) ProcessLifeCycletask(Task task)
+        private Task SetParametersAsyncCore()
         {
-            if (task == null)
-            {
-                throw new ArgumentNullException(nameof(task));
-            }
+            OnParametersSet();
+            var task = OnParametersSetAsync();
+            // If no aync work is to be performed, i.e. the task has already ran to completion
+            // or was canceled by the time we got to inspect it, avoid going async and re-invoking
+            // StateHasChanged at the culmination of the async work.
+            var shouldAwaitTask = task.Status != TaskStatus.RanToCompletion &&
+                task.Status != TaskStatus.Canceled;
 
-            switch (task.Status)
-            {
-                // If it's already completed synchronously, no need to await and no
-                // need to issue a further render (we already rerender synchronously).
-                // Just need to make sure we propagate any errors.
-                case TaskStatus.RanToCompletion:
-                case TaskStatus.Canceled:
-                    return (false, null);
-                case TaskStatus.Faulted:
-                    HandleException(task.Exception);
-                    return (false, null);
-                // For incomplete tasks, automatically re-render on successful completion
-                default:
-                    return (true, ReRenderAsyncTask(task));
-            }
+            // We always call StateHasChanged here as we want to trigger a rerender after OnParametersSet and
+            // the synchronous part of OnParametersSetAsync has run.
+            StateHasChanged();
+
+            return shouldAwaitTask ?
+                ProcessLifecycleTaskAsync(task) :
+                Task.CompletedTask;
         }
 
-        private async Task ReRenderAsyncTask(Task task)
+        private async Task ProcessLifecycleTaskAsync(Task task)
         {
             try
             {
                 await task;
-                StateHasChanged();
             }
-            catch (Exception ex)
+            catch when (task.IsCanceled)
             {
-                // Either the task failed, or it was cancelled, or StateHasChanged threw.
-                // We want to report task failure or StateHasChanged exceptions only.
-                if (!task.IsCanceled)
-                {
-                    HandleException(ex);
-                }
-            }
-        }
-
-        private async void ContinueAfterLifecycleTask(Task task)
-        {
-            switch (task == null ? TaskStatus.RanToCompletion : task.Status)
-            {
-                // If it's already completed synchronously, no need to await and no
-                // need to issue a further render (we already rerender synchronously).
-                // Just need to make sure we propagate any errors.
-                case TaskStatus.RanToCompletion:
-                case TaskStatus.Canceled:
-                    break;
-                case TaskStatus.Faulted:
-                    HandleException(task.Exception);
-                    break;
-
-                // For incomplete tasks, automatically re-render on successful completion
-                default:
-                    try
-                    {
-                        await task;
-                        StateHasChanged();
-                    }
-                    catch (Exception ex)
-                    {
-                        // Either the task failed, or it was cancelled, or StateHasChanged threw.
-                        // We want to report task failure or StateHasChanged exceptions only.
-                        if (!task.IsCanceled)
-                        {
-                            HandleException(ex);
-                        }
-                    }
-
-                    break;
-            }
-        }
-
-        private static void HandleException(Exception ex)
-        {
-            if (ex is AggregateException && ex.InnerException != null)
-            {
-                ex = ex.InnerException; // It's more useful
+                // Ignore task cancelation but don't bother issuing a state change.
+                return;
             }
 
-            // TODO: Need better global exception handling
-            Console.Error.WriteLine($"[{ex.GetType().FullName}] {ex.Message}\n{ex.StackTrace}");
+            StateHasChanged();
         }
 
-        void IHandleEvent.HandleEvent(EventHandlerInvoker binding, UIEventArgs args)
+        async Task IHandleEvent.HandleEventAsync(EventHandlerInvoker binding, UIEventArgs args)
         {
-            var task = binding.Invoke(args);
-            ContinueAfterLifecycleTask(task);
+            await binding.Invoke(args);
 
             // After each event, we synchronously re-render (unless !ShouldRender())
             // This just saves the developer the trouble of putting "StateHasChanged();"
             // at the end of every event callback.
             StateHasChanged();
         }
 
-        void IHandleAfterRender.OnAfterRender()
+        Task IHandleAfterRender.OnAfterRenderAsync()
         {
             OnAfterRender();
 
-            var onAfterRenderTask = OnAfterRenderAsync();
-            if (onAfterRenderTask != null && onAfterRenderTask.Status != TaskStatus.RanToCompletion)
-            {
-                // Note that we don't call StateHasChanged to trigger a render after
-                // handling this, because that would be an infinite loop. The only
-                // reason we have OnAfterRenderAsync is so that the developer doesn't
-                // have to use "async void" and do their own exception handling in
-                // the case where they want to start an async task.
-                var taskWithHandledException = HandleAfterRenderException(onAfterRenderTask);
-            }
-        }
+            return OnAfterRenderAsync();
 
-        private async Task HandleAfterRenderException(Task parentTask)
-        {
-            try
-            {
-                await parentTask;
-            }
-            catch (Exception e)
-            {
-                HandleException(e);
-            }
+            // Note that we don't call StateHasChanged to trigger a render after
+            // handling this, because that would be an infinite loop. The only
+            // reason we have OnAfterRenderAsync is so that the developer doesn't
+            // have to use "async void" and do their own exception handling in
+            // the case where they want to start an async task.
         }
     }
 }

src/Components/Components/src/IHandleAfterRender.cs

@@ -1,6 +1,8 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Threading.Tasks;
+
 namespace Microsoft.AspNetCore.Components
 {
     /// <summary>
@@ -11,6 +13,7 @@ public interface IHandleAfterRender
         /// <summary>
         /// Notifies the component that it has been rendered.
         /// </summary>
-        void OnAfterRender();
+        /// <returns>A <see cref="Task"/> that represents the asynchronous event handling operation.</returns>
+        Task OnAfterRenderAsync();
     }
 }

src/Components/Components/src/IHandleEvent.cs

@@ -1,6 +1,8 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Threading.Tasks;
+
 namespace Microsoft.AspNetCore.Components
 {
     /// <summary>
@@ -13,6 +15,7 @@ public interface IHandleEvent
         /// </summary>
         /// <param name="binding">The event binding.</param>
         /// <param name="args">Arguments for the event handler.</param>
-        void HandleEvent(EventHandlerInvoker binding, UIEventArgs args);
+        /// <returns>A <see cref="Task"/> that represents the asynchronous event handling operation.</returns>
+        Task HandleEventAsync(EventHandlerInvoker binding, UIEventArgs args);
     }
 }