diff --git a/src/Components/Components/src/Rendering/ComponentState.cs b/src/Components/Components/src/Rendering/ComponentState.cs index a0609b9331..1b999f30fa 100644 --- a/src/Components/Components/src/Rendering/ComponentState.cs +++ b/src/Components/Components/src/Rendering/ComponentState.cs @@ -75,16 +75,24 @@ namespace Microsoft.AspNetCore.Components.Rendering batchBuilder.UpdatedComponentDiffs.Append(diff); } - public void DisposeInBatch(RenderBatchBuilder batchBuilder) + public bool TryDisposeInBatch(RenderBatchBuilder batchBuilder, out Exception exception) { _componentWasDisposed = true; + exception = null; - // TODO: Handle components throwing during dispose. Shouldn't break the whole render batch. - if (Component is IDisposable disposable) + try { - disposable.Dispose(); + if (Component is IDisposable disposable) + { + disposable.Dispose(); + } + } + catch (Exception ex) + { + exception = ex; } + // We don't expect these things to throw. RenderTreeDiffBuilder.DisposeFrames(batchBuilder, CurrentRenderTree.GetFrames()); if (_hasAnyCascadingParameterSubscriptions) @@ -93,13 +101,27 @@ namespace Microsoft.AspNetCore.Components.Rendering } DisposeBuffers(); + + return exception == null; } + // Callers expect this method to always return a faulted task. public Task NotifyRenderCompletedAsync() { if (Component is IHandleAfterRender handlerAfterRender) { - return handlerAfterRender.OnAfterRenderAsync(); + try + { + return handlerAfterRender.OnAfterRenderAsync(); + } + catch (OperationCanceledException cex) + { + return Task.FromCanceled(cex.CancellationToken); + } + catch (Exception ex) + { + return Task.FromException(ex); + } } return Task.CompletedTask; diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index ed3d075f0a..1ca8636192 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -587,16 +587,31 @@ namespace Microsoft.AspNetCore.Components.Rendering Log.RenderingComponent(_logger, componentState); componentState.RenderIntoBatch(_batchBuilder, renderQueueEntry.RenderFragment); + List exceptions = null; + // Process disposal queue now in case it causes further component renders to be enqueued while (_batchBuilder.ComponentDisposalQueue.Count > 0) { var disposeComponentId = _batchBuilder.ComponentDisposalQueue.Dequeue(); var disposeComponentState = GetRequiredComponentState(disposeComponentId); Log.DisposingComponent(_logger, disposeComponentState); - disposeComponentState.DisposeInBatch(_batchBuilder); + if (!disposeComponentState.TryDisposeInBatch(_batchBuilder, out var exception)) + { + exceptions ??= new List(); + exceptions.Add(exception); + } _componentStateById.Remove(disposeComponentId); _batchBuilder.DisposedComponentIds.Append(disposeComponentId); } + + if (exceptions?.Count > 1) + { + HandleException(new AggregateException("Exceptions were encountered while disposing components.", exceptions)); + } + else if (exceptions?.Count == 1) + { + HandleException(exceptions[0]); + } } private void RemoveEventHandlerIds(ArrayRange eventHandlerIds, Task afterTaskIgnoreErrors) @@ -681,6 +696,9 @@ namespace Microsoft.AspNetCore.Components.Rendering /// if this method is being invoked by , otherwise . protected virtual void Dispose(bool disposing) { + // It's important that we handle all exceptions here before reporting any of them. + // This way we can dispose all components before an error handler kicks in. + List exceptions = null; foreach (var componentState in _componentStateById.Values) { Log.DisposingComponent(_logger, componentState); @@ -693,11 +711,21 @@ namespace Microsoft.AspNetCore.Components.Rendering } catch (Exception exception) { - HandleException(exception); + exceptions ??= new List(); + exceptions.Add(exception); } } + } - _batchBuilder.Dispose(); + _batchBuilder.Dispose(); + + if (exceptions?.Count > 1) + { + HandleException(new AggregateException("Exceptions were encountered while disposing components.", exceptions)); + } + else if (exceptions?.Count == 1) + { + HandleException(exceptions[0]); } } diff --git a/src/Components/Components/test/RendererTest.cs b/src/Components/Components/test/RendererTest.cs index 8d3ce23570..677b9fb952 100644 --- a/src/Components/Components/test/RendererTest.cs +++ b/src/Components/Components/test/RendererTest.cs @@ -2046,6 +2046,127 @@ namespace Microsoft.AspNetCore.Components.Test Assert.Equal(2, renderer.Batches.Count); } + [Fact] + public void RenderBatch_HandlesExceptionsFromAllDisposedComponents() + { + // Arrange + var renderer = new TestRenderer { ShouldHandleExceptions = true }; + var exception1 = new Exception(); + var exception2 = new Exception(); + + var firstRender = true; + var component = new TestComponent(builder => + { + if (firstRender) + { + builder.AddContent(0, "Hello"); + builder.OpenComponent(1); + builder.AddAttribute(1, nameof(DisposableComponent.DisposeAction), (Action)(() => throw exception1)); + builder.CloseComponent(); + + builder.OpenComponent(2); + builder.AddAttribute(1, nameof(DisposableComponent.DisposeAction), (Action)(() => throw exception2)); + builder.CloseComponent(); + } + }); + var componentId = renderer.AssignRootComponentId(component); + component.TriggerRender(); + + // Act: Second render + firstRender = false; + component.TriggerRender(); + + // Assert: Applicable children are included in disposal list + Assert.Equal(2, renderer.Batches.Count); + Assert.Equal(new[] { 1, 2 }, renderer.Batches[1].DisposedComponentIDs); + + // Outer component is still alive and not disposed. + Assert.False(component.Disposed); + var aex = Assert.IsType(Assert.Single(renderer.HandledExceptions)); + Assert.Contains(exception1, aex.InnerExceptions); + Assert.Contains(exception2, aex.InnerExceptions); + } + + [Fact] + public void RenderBatch_DoesNotDisposeComponentMultipleTimes() + { + // Arrange + var renderer = new TestRenderer { ShouldHandleExceptions = true }; + var exception1 = new Exception(); + var exception2 = new Exception(); + + var count1 = 0; + var count2 = 0; + var count3 = 0; + var count4 = 0; + var count5 = 0; + + var firstRender = true; + var component = new TestComponent(builder => + { + if (firstRender) + { + builder.AddContent(0, "Hello"); + builder.OpenComponent(1); + builder.AddAttribute(1, nameof(DisposableComponent.DisposeAction), (Action)(() => { count1++; })); + builder.CloseComponent(); + + builder.OpenComponent(2); + builder.AddAttribute(1, nameof(DisposableComponent.DisposeAction), (Action)(() => { count2++; throw exception1; })); + builder.CloseComponent(); + + builder.OpenComponent(3); + builder.AddAttribute(1, nameof(DisposableComponent.DisposeAction), (Action)(() => { count3++; })); + builder.CloseComponent(); + } + + builder.OpenComponent(4); + builder.AddAttribute(1, nameof(DisposableComponent.DisposeAction), (Action)(() => { count4++; throw exception2; })); + builder.CloseComponent(); + + builder.OpenComponent(5); + builder.AddAttribute(1, nameof(DisposableComponent.DisposeAction), (Action)(() => { count5++; })); + builder.CloseComponent(); + }); + var componentId = renderer.AssignRootComponentId(component); + component.TriggerRender(); + + // Act: Second render + firstRender = false; + component.TriggerRender(); + + // Assert: Applicable children are included in disposal list + Assert.Equal(2, renderer.Batches.Count); + Assert.Equal(new[] { 1, 2, 3 }, renderer.Batches[1].DisposedComponentIDs); + + // Components "disposed" in the batch were all disposed, components that are still live were not disposed + Assert.Equal(1, count1); + Assert.Equal(1, count2); + Assert.Equal(1, count3); + Assert.Equal(0, count4); + Assert.Equal(0, count5); + + // Outer component is still alive and not disposed. + Assert.False(component.Disposed); + var ex = Assert.IsType(Assert.Single(renderer.HandledExceptions)); + Assert.Same(exception1, ex); + + // Act: Dispose renderer + renderer.Dispose(); + + Assert.Equal(2, renderer.HandledExceptions.Count); + ex = renderer.HandledExceptions[1]; + Assert.Same(exception2, ex); + + // Assert: Everything was disposed once. + Assert.Equal(1, count1); + Assert.Equal(1, count2); + Assert.Equal(1, count3); + Assert.Equal(1, count4); + Assert.Equal(1, count5); + Assert.True(component.Disposed); + } + [Fact] public async Task DisposesEventHandlersWhenAttributeValueChanged() { @@ -3059,7 +3180,7 @@ namespace Microsoft.AspNetCore.Components.Test } [Fact] - public async Task ExceptionsThrownFromHandleAfterRender_AreHandled() + public async Task ExceptionsThrownFromHandleAfterRender_Sync_AreHandled() { // Arrange var renderer = new TestRenderer { ShouldHandleExceptions = true }; @@ -3078,7 +3199,7 @@ namespace Microsoft.AspNetCore.Components.Test { new NestedAsyncComponent.ExecutionAction { - Event = NestedAsyncComponent.EventType.OnAfterRenderAsync, + Event = NestedAsyncComponent.EventType.OnAfterRenderAsyncSync, EventAction = () => { throw exception; @@ -3089,12 +3210,11 @@ namespace Microsoft.AspNetCore.Components.Test { new NestedAsyncComponent.ExecutionAction { - Event = NestedAsyncComponent.EventType.OnAfterRenderAsync, - EventAction = async () => + Event = NestedAsyncComponent.EventType.OnAfterRenderAsyncSync, + EventAction = () => { - await Task.Yield(); taskCompletionSource.TrySetResult(0); - return (1, NestedAsyncComponent.EventType.OnAfterRenderAsync); + return Task.FromResult((1, NestedAsyncComponent.EventType.OnAfterRenderAsyncSync)); }, } } @@ -3113,6 +3233,137 @@ namespace Microsoft.AspNetCore.Components.Test Assert.Same(exception, Assert.Single(renderer.HandledExceptions).GetBaseException()); } + [Fact] + public async Task ExceptionsThrownFromHandleAfterRender_Async_AreHandled() + { + // Arrange + var renderer = new TestRenderer { ShouldHandleExceptions = true }; + var component = new NestedAsyncComponent(); + var exception = new InvalidTimeZoneException(); + + var taskCompletionSource = new TaskCompletionSource(); + + // Act/Assert + var componentId = renderer.AssignRootComponentId(component); + var renderTask = renderer.RenderRootComponentAsync(componentId, ParameterView.FromDictionary(new Dictionary + { + [nameof(NestedAsyncComponent.EventActions)] = new Dictionary> + { + [0] = new[] + { + new NestedAsyncComponent.ExecutionAction + { + Event = NestedAsyncComponent.EventType.OnAfterRenderAsyncAsync, + EventAction = async () => + { + await Task.Yield(); + throw exception; + }, + } + }, + [1] = new[] + { + new NestedAsyncComponent.ExecutionAction + { + Event = NestedAsyncComponent.EventType.OnAfterRenderAsyncAsync, + EventAction = async () => + { + await Task.Yield(); + taskCompletionSource.TrySetResult(0); + return (1, NestedAsyncComponent.EventType.OnAfterRenderAsyncAsync); + }, + } + } + }, + [nameof(NestedAsyncComponent.WhatToRender)] = new Dictionary> + { + [0] = CreateRenderFactory(new[] { 1 }), + [1] = CreateRenderFactory(Array.Empty()), + }, + })); + + Assert.True(renderTask.IsCompletedSuccessfully); + + // OnAfterRenderAsync happens in the background. Make it more predictable, by gating it until we're ready to capture exceptions. + await taskCompletionSource.Task.TimeoutAfter(TimeSpan.FromSeconds(10)); + Assert.Same(exception, Assert.Single(renderer.HandledExceptions).GetBaseException()); + } + + [Fact] + public async Task ExceptionThrownFromConstructor() + { + // Arrange + var renderer = new TestRenderer { ShouldHandleExceptions = true }; + var component = new TestComponent(builder => + { + builder.OpenComponent(0); + builder.CloseComponent(); + }); + + // Act/Assert + var componentId = renderer.AssignRootComponentId(component); + var renderTask = renderer.RenderRootComponentAsync(componentId); + + await renderTask; + Assert.True(renderTask.IsCompletedSuccessfully); + Assert.Same(ConstructorThrowingComponent.Exception, Assert.Single(renderer.HandledExceptions).GetBaseException()); + } + + private class ConstructorThrowingComponent : IComponent + { + public static readonly Exception Exception = new InvalidTimeZoneException(); + + public ConstructorThrowingComponent() + { + throw Exception; + } + + public void Attach(RenderHandle renderHandle) + { + throw new NotImplementedException(); + } + + public Task SetParametersAsync(ParameterView parameters) + { + throw new NotImplementedException(); + } + } + + [Fact] + public async Task ExceptionThrownFromAttach() + { + // Arrange + var renderer = new TestRenderer { ShouldHandleExceptions = true }; + var component = new TestComponent(builder => + { + builder.OpenComponent(0); + builder.CloseComponent(); + }); + + // Act/Assert + var componentId = renderer.AssignRootComponentId(component); + var renderTask = renderer.RenderRootComponentAsync(componentId); + + await renderTask; + Assert.True(renderTask.IsCompletedSuccessfully); + Assert.Same(AttachThrowingComponent.Exception, Assert.Single(renderer.HandledExceptions).GetBaseException()); + } + + private class AttachThrowingComponent : IComponent + { + public static readonly Exception Exception = new InvalidTimeZoneException(); + + public void Attach(RenderHandle renderHandle) + { + throw Exception; + } + + public Task SetParametersAsync(ParameterView parameters) + { + throw new NotImplementedException(); + } + } + [Fact] public void SynchronousCancelledTasks_HandleAfterRender_Works() { @@ -3132,7 +3383,7 @@ namespace Microsoft.AspNetCore.Components.Test { new NestedAsyncComponent.ExecutionAction { - Event = NestedAsyncComponent.EventType.OnAfterRenderAsync, + Event = NestedAsyncComponent.EventType.OnAfterRenderAsyncAsync, EventAction = () => tcs.Task, } }, @@ -3166,7 +3417,7 @@ namespace Microsoft.AspNetCore.Components.Test { new NestedAsyncComponent.ExecutionAction { - Event = NestedAsyncComponent.EventType.OnAfterRenderAsync, + Event = NestedAsyncComponent.EventType.OnAfterRenderAsyncAsync, EventAction = () => tcs.Task, } }, @@ -3203,7 +3454,7 @@ namespace Microsoft.AspNetCore.Components.Test { new NestedAsyncComponent.ExecutionAction { - Event = NestedAsyncComponent.EventType.OnAfterRenderAsync, + Event = NestedAsyncComponent.EventType.OnAfterRenderAsyncSync, EventAction = () => { taskCompletionSource.TrySetResult(0); @@ -3291,9 +3542,9 @@ namespace Microsoft.AspNetCore.Components.Test // All components must be disposed even if some throw as part of being diposed. Assert.True(component.Disposed); - Assert.Equal(2, renderer.HandledExceptions.Count); - Assert.Contains(exception1, renderer.HandledExceptions); - Assert.Contains(exception2, renderer.HandledExceptions); + var aex = Assert.IsType(Assert.Single(renderer.HandledExceptions)); + Assert.Contains(exception1, aex.InnerExceptions); + Assert.Contains(exception2, aex.InnerExceptions); } [Theory] @@ -3941,6 +4192,7 @@ namespace Microsoft.AspNetCore.Components.Test if (TryGetEntry(EventType.OnInit, out var entry)) { var result = entry.EventAction(); + Assert.True(result.IsCompleted, "Task must complete synchronously."); LogResult(result.Result); } } @@ -3949,8 +4201,9 @@ namespace Microsoft.AspNetCore.Components.Test { if (TryGetEntry(EventType.OnInitAsyncSync, out var entrySync)) { - var result = await entrySync.EventAction(); - LogResult(result); + var result = entrySync.EventAction(); + Assert.True(result.IsCompleted, "Task must complete synchronously."); + LogResult(result.Result); } else if (TryGetEntry(EventType.OnInitAsyncAsync, out var entryAsync)) { @@ -3964,6 +4217,7 @@ namespace Microsoft.AspNetCore.Components.Test if (TryGetEntry(EventType.OnParametersSet, out var entry)) { var result = entry.EventAction(); + Assert.True(result.IsCompleted, "Task must complete synchronously."); LogResult(result.Result); } base.OnParametersSet(); @@ -3973,10 +4227,9 @@ namespace Microsoft.AspNetCore.Components.Test { if (TryGetEntry(EventType.OnParametersSetAsyncSync, out var entrySync)) { - var result = await entrySync.EventAction(); - LogResult(result); - - await entrySync.EventAction(); + var result = entrySync.EventAction(); + Assert.True(result.IsCompleted, "Task must complete synchronously."); + LogResult(result.Result); } else if (TryGetEntry(EventType.OnParametersSetAsyncAsync, out var entryAsync)) { @@ -3993,9 +4246,15 @@ namespace Microsoft.AspNetCore.Components.Test protected override async Task OnAfterRenderAsync() { - if (TryGetEntry(EventType.OnAfterRenderAsync, out var entry)) + if (TryGetEntry(EventType.OnAfterRenderAsyncSync, out var entrySync)) { - var result = await entry.EventAction(); + var result = entrySync.EventAction(); + Assert.True(result.IsCompleted, "Task must complete synchronously."); + LogResult(result.Result); + } + if (TryGetEntry(EventType.OnAfterRenderAsyncAsync, out var entryAsync)) + { + var result = await entryAsync.EventAction(); LogResult(result); } } @@ -4054,7 +4313,8 @@ namespace Microsoft.AspNetCore.Components.Test OnParametersSet, OnParametersSetAsyncSync, OnParametersSetAsyncAsync, - OnAfterRenderAsync, + OnAfterRenderAsyncSync, + OnAfterRenderAsyncAsync, } } diff --git a/src/Components/Server/src/CircuitDisconnectMiddleware.cs b/src/Components/Server/src/CircuitDisconnectMiddleware.cs index ecacf511d7..0792e009d3 100644 --- a/src/Components/Server/src/CircuitDisconnectMiddleware.cs +++ b/src/Components/Server/src/CircuitDisconnectMiddleware.cs @@ -78,7 +78,7 @@ namespace Microsoft.AspNetCore.Components.Server { try { - await Registry.Terminate(circuitId); + await Registry.TerminateAsync(circuitId); Log.CircuitTerminatedGracefully(Logger, circuitId); } catch (Exception e) diff --git a/src/Components/Server/src/Circuits/CircuitHandle.cs b/src/Components/Server/src/Circuits/CircuitHandle.cs new file mode 100644 index 0000000000..88493453d0 --- /dev/null +++ b/src/Components/Server/src/Circuits/CircuitHandle.cs @@ -0,0 +1,14 @@ +// 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. + +namespace Microsoft.AspNetCore.Components.Server.Circuits +{ + // Used to isolate a circuit from a CircuitHost. + // + // We can't refer to Hub.Items from a CircuitHost - but we want need to be + // able to break the link between Hub.Items and a CircuitHost. + internal class CircuitHandle + { + public CircuitHost CircuitHost { get; set; } + } +} diff --git a/src/Components/Server/src/Circuits/CircuitHost.cs b/src/Components/Server/src/Circuits/CircuitHost.cs index 92a2365971..8a5fd717af 100644 --- a/src/Components/Server/src/Circuits/CircuitHost.cs +++ b/src/Components/Server/src/Circuits/CircuitHost.cs @@ -8,6 +8,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Web; using Microsoft.AspNetCore.Components.Web.Rendering; +using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.JSInterop; @@ -18,9 +19,11 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits { private readonly SemaphoreSlim HandlerLock = new SemaphoreSlim(1); private readonly IServiceScope _scope; + private readonly CircuitOptions _options; private readonly CircuitHandler[] _circuitHandlers; private readonly ILogger _logger; private bool _initialized; + private bool _disposed; /// /// Sets the current . @@ -42,11 +45,18 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits JSInterop.JSRuntime.SetCurrentJSRuntime(circuitHost.JSRuntime); } + // This event is fired when there's an unrecoverable exception coming from the circuit, and + // it need so be torn down. The registry listens to this even so that the circuit can + // be torn down even when a client is not connected. + // + // We don't expect the registry to do anything with the exception. We only provide it here + // for testability. public event UnhandledExceptionEventHandler UnhandledException; public CircuitHost( string circuitId, IServiceScope scope, + CircuitOptions options, CircuitClientProxy client, RemoteRenderer renderer, IReadOnlyList descriptors, @@ -54,23 +64,28 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits CircuitHandler[] circuitHandlers, ILogger logger) { - CircuitId = circuitId; + CircuitId = circuitId ?? throw new ArgumentNullException(nameof(circuitId)); + _scope = scope ?? throw new ArgumentNullException(nameof(scope)); - Client = client; - Descriptors = descriptors ?? throw new ArgumentNullException(nameof(descriptors)); + _options = options ?? throw new ArgumentNullException(nameof(options)); + Client = client ?? throw new ArgumentNullException(nameof(client)); Renderer = renderer ?? throw new ArgumentNullException(nameof(renderer)); + Descriptors = descriptors ?? throw new ArgumentNullException(nameof(descriptors)); JSRuntime = jsRuntime ?? throw new ArgumentNullException(nameof(jsRuntime)); - _logger = logger; + _circuitHandlers = circuitHandlers ?? throw new ArgumentNullException(nameof(circuitHandlers)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); Services = scope.ServiceProvider; Circuit = new Circuit(this); - _circuitHandlers = circuitHandlers; + Handle = new CircuitHandle() { CircuitHost = this, }; Renderer.UnhandledException += Renderer_UnhandledException; Renderer.UnhandledSynchronizationException += SynchronizationContext_UnhandledException; } + public CircuitHandle Handle { get; } + public string CircuitId { get; } public Circuit Circuit { get; } @@ -85,88 +100,19 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits public IServiceProvider Services { get; } - public void SetCircuitUser(ClaimsPrincipal user) + // InitializeAsync is used in a fire-and-forget context, so it's responsible for its own + // error handling. + public Task InitializeAsync(CancellationToken cancellationToken) { - var authenticationStateProvider = Services.GetService() as IHostEnvironmentAuthenticationStateProvider; - if (authenticationStateProvider != null) + Log.InitializationStarted(_logger); + + return Renderer.Dispatcher.InvokeAsync(async () => { - var authenticationState = new AuthenticationState(user); - authenticationStateProvider.SetAuthenticationState(Task.FromResult(authenticationState)); - } - } - - internal void SendPendingBatches() - { - // Dispatch any buffered renders we accumulated during a disconnect. - // Note that while the rendering is async, we cannot await it here. The Task returned by ProcessBufferedRenderBatches relies on - // OnRenderCompleted to be invoked to complete, and SignalR does not allow concurrent hub method invocations. - _ = Renderer.Dispatcher.InvokeAsync(() => Renderer.ProcessBufferedRenderBatches()); - } - - public async Task EndInvokeJSFromDotNet(long asyncCall, bool succeded, string arguments) - { - try - { - AssertInitialized(); - - await Renderer.Dispatcher.InvokeAsync(() => + if (_initialized) { - SetCurrentCircuitHost(this); - if (!succeded) - { - // We can log the arguments here because it is simply the JS error with the call stack. - Log.EndInvokeJSFailed(_logger, asyncCall, arguments); - } - else - { - Log.EndInvokeJSSucceeded(_logger, asyncCall); - } + throw new InvalidOperationException("The circuit host is already initialized."); + } - DotNetDispatcher.EndInvoke(arguments); - }); - } - catch (Exception ex) - { - Log.EndInvokeDispatchException(_logger, ex); - } - } - - public async Task DispatchEvent(string eventDescriptorJson, string eventArgsJson) - { - WebEventData webEventData; - try - { - AssertInitialized(); - webEventData = WebEventData.Parse(eventDescriptorJson, eventArgsJson); - } - catch (Exception ex) - { - Log.DispatchEventFailedToParseEventData(_logger, ex); - return; - } - - try - { - await Renderer.Dispatcher.InvokeAsync(() => - { - SetCurrentCircuitHost(this); - return Renderer.DispatchEventAsync( - webEventData.EventHandlerId, - webEventData.EventFieldInfo, - webEventData.EventArgs); - }); - } - catch (Exception ex) - { - Log.DispatchEventFailedToDispatchEvent(_logger, webEventData.EventHandlerId.ToString(), ex); - UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); - } - } - - public async Task InitializeAsync(CancellationToken cancellationToken) - { - await Renderer.Dispatcher.InvokeAsync(async () => - { try { SetCurrentCircuitHost(this); @@ -185,92 +131,117 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits var (componentType, domElementSelector) = Descriptors[i]; await Renderer.AddComponentAsync(componentType, domElementSelector); } + + Log.InitializationSucceeded(_logger); } catch (Exception ex) { - // We have to handle all our own errors here, because the upstream caller - // has to fire-and-forget this - Renderer_UnhandledException(this, ex); + // Report errors asynchronously. InitializeAsync is designed not to throw. + Log.InitializationFailed(_logger, ex); + UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); } }); } - public async Task BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson) + // We handle errors in DisposeAsync because there's no real value in letting it propagate. + // We run user code here (CircuitHandlers) and it's reasonable to expect some might throw, however, + // there isn't anything better to do than log when one of these exceptions happens - because the + // client is already gone. + public async ValueTask DisposeAsync() { - try - { - AssertInitialized(); + Log.DisposeStarted(_logger, CircuitId); - await Renderer.Dispatcher.InvokeAsync(() => - { - SetCurrentCircuitHost(this); - Log.BeginInvokeDotNet(_logger, callId, assemblyName, methodIdentifier, dotNetObjectId); - DotNetDispatcher.BeginInvoke(callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson); - }); - } - catch (Exception ex) + await Renderer.Dispatcher.InvokeAsync(async () => { - // We don't expect any of this code to actually throw, because DotNetDispatcher.BeginInvoke doesn't throw - // however, we still want this to get logged if we do. - UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); - } - } - - public async Task OnLocationChangedAsync(string uri, bool intercepted) - { - try - { - AssertInitialized(); - await Renderer.Dispatcher.InvokeAsync(() => - { - SetCurrentCircuitHost(this); - Log.LocationChange(_logger, CircuitId, uri); - var navigationManager = (RemoteNavigationManager)Services.GetRequiredService(); - navigationManager.NotifyLocationChanged(uri, intercepted); - Log.LocationChangeSucceeded(_logger, CircuitId, uri); - }); - } - catch (Exception ex) - { - // It's up to the NavigationManager implementation to validate the URI. - // - // Note that it's also possible that setting the URI could cause a failure in code that listens - // to NavigationManager.LocationChanged. - // - // In either case, a well-behaved client will not send invalid URIs, and we don't really - // want to continue processing with the circuit if setting the URI failed inside application - // code. The safest thing to do is consider it a critical failure since URI is global state, - // and a failure means that an update to global state was partially applied. - Log.LocationChangeFailed(_logger, CircuitId, uri, ex); - UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); - } + if (_disposed) + { + return; + } + + // Make sure that no hub or connection can refer to this circuit anymore now that it's shutting down. + Handle.CircuitHost = null; + _disposed = true; + + try + { + await OnConnectionDownAsync(CancellationToken.None); + } + catch + { + // Individual exceptions logged as part of OnConnectionDownAsync - nothing to do here + // since we're already shutting down. + } + + try + { + await OnCircuitDownAsync(CancellationToken.None); + } + catch + { + // Individual exceptions logged as part of OnCircuitDownAsync - nothing to do here + // since we're already shutting down. + } + + try + { + Renderer.Dispose(); + _scope.Dispose(); + Log.DisposeSucceeded(_logger, CircuitId); + } + catch (Exception ex) + { + Log.DisposeFailed(_logger, CircuitId, ex); + } + }); } + // Note: we log exceptions and re-throw while running handlers, because there may be multiple + // exceptions. private async Task OnCircuitOpenedAsync(CancellationToken cancellationToken) { Log.CircuitOpened(_logger, Circuit.Id); - for (var i = 0; i < _circuitHandlers.Length; i++) + await HandlerLock.WaitAsync(cancellationToken); + + try { - var circuitHandler = _circuitHandlers[i]; - try + List exceptions = null; + + for (var i = 0; i < _circuitHandlers.Length; i++) { - await circuitHandler.OnCircuitOpenedAsync(Circuit, cancellationToken); + var circuitHandler = _circuitHandlers[i]; + try + { + await circuitHandler.OnCircuitOpenedAsync(Circuit, cancellationToken); + } + catch (Exception ex) + { + Log.CircuitHandlerFailed(_logger, circuitHandler, nameof(CircuitHandler.OnCircuitOpenedAsync), ex); + exceptions ??= new List(); + exceptions.Add(ex); + } } - catch (Exception ex) + + if (exceptions != null) { - OnHandlerError(circuitHandler, nameof(CircuitHandler.OnCircuitOpenedAsync), ex); + throw new AggregateException("Encountered exceptions while executing circuit handlers.", exceptions); } } + finally + { + HandlerLock.Release(); + } } public async Task OnConnectionUpAsync(CancellationToken cancellationToken) { Log.ConnectionUp(_logger, Circuit.Id, Client.ConnectionId); + await HandlerLock.WaitAsync(cancellationToken); + try { - await HandlerLock.WaitAsync(cancellationToken); + List exceptions = null; for (var i = 0; i < _circuitHandlers.Length; i++) { @@ -281,9 +252,16 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits } catch (Exception ex) { - OnHandlerError(circuitHandler, nameof(CircuitHandler.OnConnectionUpAsync), ex); + Log.CircuitHandlerFailed(_logger, circuitHandler, nameof(CircuitHandler.OnConnectionUpAsync), ex); + exceptions ??= new List(); + exceptions.Add(ex); } } + + if (exceptions != null) + { + throw new AggregateException("Encountered exceptions while executing circuit handlers.", exceptions); + } } finally { @@ -295,9 +273,11 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits { Log.ConnectionDown(_logger, Circuit.Id, Client.ConnectionId); + await HandlerLock.WaitAsync(cancellationToken); + try { - await HandlerLock.WaitAsync(cancellationToken); + List exceptions = null; for (var i = 0; i < _circuitHandlers.Length; i++) { @@ -308,9 +288,16 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits } catch (Exception ex) { - OnHandlerError(circuitHandler, nameof(CircuitHandler.OnConnectionDownAsync), ex); + Log.CircuitHandlerFailed(_logger, circuitHandler, nameof(CircuitHandler.OnConnectionDownAsync), ex); + exceptions ??= new List(); + exceptions.Add(ex); } } + + if (exceptions != null) + { + throw new AggregateException("Encountered exceptions while executing circuit handlers.", exceptions); + } } finally { @@ -318,46 +305,229 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits } } - protected virtual void OnHandlerError(CircuitHandler circuitHandler, string handlerMethod, Exception ex) - { - Log.UnhandledExceptionInvokingCircuitHandler(_logger, circuitHandler, handlerMethod, ex); - } - - private async Task OnCircuitDownAsync() + private async Task OnCircuitDownAsync(CancellationToken cancellationToken) { Log.CircuitClosed(_logger, Circuit.Id); - for (var i = 0; i < _circuitHandlers.Length; i++) + await HandlerLock.WaitAsync(cancellationToken); + + try { - var circuitHandler = _circuitHandlers[i]; - try + List exceptions = null; + + for (var i = 0; i < _circuitHandlers.Length; i++) { - await circuitHandler.OnCircuitClosedAsync(Circuit, default); + var circuitHandler = _circuitHandlers[i]; + try + { + await circuitHandler.OnCircuitClosedAsync(Circuit, cancellationToken); + } + catch (Exception ex) + { + Log.CircuitHandlerFailed(_logger, circuitHandler, nameof(CircuitHandler.OnCircuitClosedAsync), ex); + exceptions ??= new List(); + exceptions.Add(ex); + } } - catch (Exception ex) + + if (exceptions != null) { - OnHandlerError(circuitHandler, nameof(CircuitHandler.OnCircuitClosedAsync), ex); + throw new AggregateException("Encountered exceptions while executing circuit handlers.", exceptions); } } + finally + { + HandlerLock.Release(); + } } - public async ValueTask DisposeAsync() + // BeginInvokeDotNetFromJS is used in a fire-and-forget context, so it's responsible for its own + // error handling. + public async Task BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson) { - Log.DisposingCircuit(_logger, CircuitId); + AssertInitialized(); + AssertNotDisposed(); - await Renderer.Dispatcher.InvokeAsync(async () => + try { - try + await Renderer.Dispatcher.InvokeAsync(() => { - await OnConnectionDownAsync(CancellationToken.None); - await OnCircuitDownAsync(); - } - finally + SetCurrentCircuitHost(this); + Log.BeginInvokeDotNet(_logger, callId, assemblyName, methodIdentifier, dotNetObjectId); + DotNetDispatcher.BeginInvoke(callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson); + }); + } + catch (Exception ex) + { + // We don't expect any of this code to actually throw, because DotNetDispatcher.BeginInvoke doesn't throw + // however, we still want this to get logged if we do. + Log.BeginInvokeDotNetFailed(_logger, callId, assemblyName, methodIdentifier, dotNetObjectId, ex); + if (Client.Connected) { - Renderer.Dispose(); - _scope.Dispose(); + await NotifyClientError(Client, "Interop call failed."); } - }); + UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); + } + } + + // EndInvokeJSFromDotNet is used in a fire-and-forget context, so it's responsible for its own + // error handling. + public async Task EndInvokeJSFromDotNet(long asyncCall, bool succeded, string arguments) + { + AssertInitialized(); + AssertNotDisposed(); + + try + { + await Renderer.Dispatcher.InvokeAsync(() => + { + SetCurrentCircuitHost(this); + if (!succeded) + { + // We can log the arguments here because it is simply the JS error with the call stack. + Log.EndInvokeJSFailed(_logger, asyncCall, arguments); + } + else + { + Log.EndInvokeJSSucceeded(_logger, asyncCall); + } + + DotNetDispatcher.EndInvoke(arguments); + }); + } + catch (Exception ex) + { + // An error completing JS interop means that the user sent invalid data, a well-behaved + // client won't do this. + Log.EndInvokeDispatchException(_logger, ex); + if (Client.Connected) + { + await NotifyClientError(Client, "Invalid interop arguments."); + } + UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); + } + } + + // DispatchEvent is used in a fire-and-forget context, so it's responsible for its own + // error handling. + public async Task DispatchEvent(string eventDescriptorJson, string eventArgsJson) + { + AssertInitialized(); + AssertNotDisposed(); + + WebEventData webEventData; + try + { + AssertInitialized(); + webEventData = WebEventData.Parse(eventDescriptorJson, eventArgsJson); + } + catch (Exception ex) + { + // Invalid event data is fatal. We expect a well-behaved client to send valid JSON. + Log.DispatchEventFailedToParseEventData(_logger, ex); + if (Client.Connected) + { + await NotifyClientError(Client, "Invalid event data."); + } + UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); + return; + } + + try + { + await Renderer.Dispatcher.InvokeAsync(() => + { + SetCurrentCircuitHost(this); + return Renderer.DispatchEventAsync( + webEventData.EventHandlerId, + webEventData.EventFieldInfo, + webEventData.EventArgs); + }); + } + catch (Exception ex) + { + // A failure in dispatching an event means that it was an attempt to use an invalid event id. + // A well-behaved client won't do this. + Log.DispatchEventFailedToDispatchEvent(_logger, webEventData.EventHandlerId.ToString(), ex); + if (Client.Connected) + { + await NotifyClientError(Client, "Failed to dispatch event."); + } + UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); + } + } + + // OnLocationChangedAsync is used in a fire-and-forget context, so it's responsible for its own + // error handling. + public async Task OnLocationChangedAsync(string uri, bool intercepted) + { + AssertInitialized(); + AssertNotDisposed(); + + try + { + await Renderer.Dispatcher.InvokeAsync(() => + { + SetCurrentCircuitHost(this); + Log.LocationChange(_logger, uri, CircuitId); + var navigationManager = (RemoteNavigationManager)Services.GetRequiredService(); + navigationManager.NotifyLocationChanged(uri, intercepted); + Log.LocationChangeSucceeded(_logger, uri, CircuitId); + }); + } + + // It's up to the NavigationManager implementation to validate the URI. + // + // Note that it's also possible that setting the URI could cause a failure in code that listens + // to NavigationManager.LocationChanged. + // + // In either case, a well-behaved client will not send invalid URIs, and we don't really + // want to continue processing with the circuit if setting the URI failed inside application + // code. The safest thing to do is consider it a critical failure since URI is global state, + // and a failure means that an update to global state was partially applied. + catch (LocationChangeException nex) + { + // LocationChangeException means that it failed in user-code. Treat this like an unhandled + // exception in user-code. + Log.LocationChangeFailedInCircuit(_logger, uri, CircuitId, nex); + await ReportUnhandledException(nex); + UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(nex, isTerminating: false)); + } + catch (Exception ex) + { + // Any other exception means that it failed validation, or inside the NavigationManager. Treat + // this like bad data. + Log.LocationChangeFailed(_logger, uri, CircuitId, ex); + if (Client.Connected) + { + await NotifyClientError(Client, $"Location change to {uri} failed."); + } + UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false)); + } + } + + public void SetCircuitUser(ClaimsPrincipal user) + { + // This can be called before the circuit is initialized. + AssertNotDisposed(); + + var authenticationStateProvider = Services.GetService() as IHostEnvironmentAuthenticationStateProvider; + if (authenticationStateProvider != null) + { + var authenticationState = new AuthenticationState(user); + authenticationStateProvider.SetAuthenticationState(Task.FromResult(authenticationState)); + } + } + + public void SendPendingBatches() + { + AssertInitialized(); + AssertNotDisposed(); + + // Dispatch any buffered renders we accumulated during a disconnect. + // Note that while the rendering is async, we cannot await it here. The Task returned by ProcessBufferedRenderBatches relies on + // OnRenderCompleted to be invoked to complete, and SignalR does not allow concurrent hub method invocations. + _ = Renderer.Dispatcher.InvokeAsync(() => Renderer.ProcessBufferedRenderBatches()); } private void AssertInitialized() @@ -368,26 +538,86 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits } } - private void Renderer_UnhandledException(object sender, Exception e) + private void AssertNotDisposed() { + if (_disposed) + { + throw new ObjectDisposedException(objectName: null); + } + } + + // An unhandled exception from the renderer is always fatal because it came from user code. + // We want to notify the client if it's still connected, and then tear-down the circuit. + private async void Renderer_UnhandledException(object sender, Exception e) + { + await ReportUnhandledException(e); UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(e, isTerminating: false)); } - private void SynchronizationContext_UnhandledException(object sender, UnhandledExceptionEventArgs e) + // An unhandled exception from the renderer is always fatal because it came from user code. + // We want to notify the client if it's still connected, and then tear-down the circuit. + private async void SynchronizationContext_UnhandledException(object sender, UnhandledExceptionEventArgs e) { + await ReportUnhandledException((Exception)e.ExceptionObject); UnhandledException?.Invoke(this, e); } + private async Task ReportUnhandledException(Exception exception) + { + Log.CircuitUnhandledException(_logger, CircuitId, exception); + if (!Client.Connected) + { + _logger.LogDebug("Client is disconnected YO."); + return; + } + + try + { + if (_options.DetailedErrors) + { + await NotifyClientError(Client, exception.ToString()); + } + else + { + var message = + $"There was an unhandled exception on the current circuit, so this circuit will be terminated. For more details turn on " + + $"detailed exceptions in '{typeof(CircuitOptions).Name}.{nameof(CircuitOptions.DetailedErrors)}'"; + await NotifyClientError(Client, message); + } + } + catch (Exception ex) + { + Log.CircuitUnhandledExceptionFailed(_logger, CircuitId, ex); + } + } + + private async Task NotifyClientError(IClientProxy client, string error) + { + _logger.LogDebug("About to notify of an error"); + await client.SendAsync("JS.Error", error); + _logger.LogDebug("Completed notify of an error"); + } + private static class Log { - private static readonly Action _unhandledExceptionInvokingCircuitHandler; - private static readonly Action _disposingCircuit; + private static readonly Action _intializationStarted; + private static readonly Action _intializationSucceded; + private static readonly Action _intializationFailed; + private static readonly Action _disposeStarted; + private static readonly Action _disposeSucceded; + private static readonly Action _disposeFailed; private static readonly Action _onCircuitOpened; private static readonly Action _onConnectionUp; private static readonly Action _onConnectionDown; private static readonly Action _onCircuitClosed; + private static readonly Action _circuitHandlerFailed; + private static readonly Action _circuitUnhandledException; + private static readonly Action _circuitUnhandledExceptionFailed; + private static readonly Action _beginInvokeDotNetStatic; private static readonly Action _beginInvokeDotNetInstance; + private static readonly Action _beginInvokeDotNetStaticFailed; + private static readonly Action _beginInvokeDotNetInstanceFailed; private static readonly Action _endInvokeDispatchException; private static readonly Action _endInvokeJSFailed; private static readonly Action _endInvokeJSSucceeded; @@ -396,39 +626,71 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits private static readonly Action _locationChange; private static readonly Action _locationChangeSucceeded; private static readonly Action _locationChangeFailed; + private static readonly Action _locationChangeFailedInCircuit; private static class EventIds { - public static readonly EventId ExceptionInvokingCircuitHandlerMethod = new EventId(100, "ExceptionInvokingCircuitHandlerMethod"); - public static readonly EventId DisposingCircuit = new EventId(101, "DisposingCircuitHost"); - public static readonly EventId OnCircuitOpened = new EventId(102, "OnCircuitOpened"); - public static readonly EventId OnConnectionUp = new EventId(103, "OnConnectionUp"); - public static readonly EventId OnConnectionDown = new EventId(104, "OnConnectionDown"); - public static readonly EventId OnCircuitClosed = new EventId(105, "OnCircuitClosed"); - public static readonly EventId InvalidBrowserEventFormat = new EventId(106, "InvalidBrowserEventFormat"); - public static readonly EventId DispatchEventFailedToParseEventData = new EventId(107, "DispatchEventFailedToParseEventData"); - public static readonly EventId DispatchEventFailedToDispatchEvent = new EventId(108, "DispatchEventFailedToDispatchEvent"); - public static readonly EventId BeginInvokeDotNet = new EventId(109, "BeginInvokeDotNet"); - public static readonly EventId EndInvokeDispatchException = new EventId(110, "EndInvokeDispatchException"); - public static readonly EventId EndInvokeJSFailed = new EventId(111, "EndInvokeJSFailed"); - public static readonly EventId EndInvokeJSSucceeded = new EventId(112, "EndInvokeJSSucceeded"); - public static readonly EventId DispatchEventThroughJSInterop = new EventId(113, "DispatchEventThroughJSInterop"); - public static readonly EventId LocationChange = new EventId(114, "LocationChange"); - public static readonly EventId LocationChangeSucceded = new EventId(115, "LocationChangeSucceeded"); - public static readonly EventId LocationChangeFailed = new EventId(116, "LocationChangeFailed"); + // 100s used for lifecycle stuff + public static readonly EventId InitializationStarted = new EventId(100, "InitializationStarted"); + public static readonly EventId InitializationSucceeded = new EventId(101, "InitializationSucceeded"); + public static readonly EventId InitializationFailed = new EventId(102, "InitializationFailed"); + public static readonly EventId DisposeStarted = new EventId(103, "DisposeStarted"); + public static readonly EventId DisposeSucceeded = new EventId(104, "DisposeSucceeded"); + public static readonly EventId DisposeFailed = new EventId(105, "DisposeFailed"); + public static readonly EventId OnCircuitOpened = new EventId(106, "OnCircuitOpened"); + public static readonly EventId OnConnectionUp = new EventId(107, "OnConnectionUp"); + public static readonly EventId OnConnectionDown = new EventId(108, "OnConnectionDown"); + public static readonly EventId OnCircuitClosed = new EventId(109, "OnCircuitClosed"); + public static readonly EventId CircuitHandlerFailed = new EventId(110, "CircuitHandlerFailed"); + public static readonly EventId CircuitUnhandledException = new EventId(111, "CircuitUnhandledException"); + public static readonly EventId CircuitUnhandledExceptionFailed = new EventId(112, "CircuitUnhandledExceptionFailed"); + + // 200s used for interactive stuff + public static readonly EventId DispatchEventFailedToParseEventData = new EventId(200, "DispatchEventFailedToParseEventData"); + public static readonly EventId DispatchEventFailedToDispatchEvent = new EventId(201, "DispatchEventFailedToDispatchEvent"); + public static readonly EventId BeginInvokeDotNet = new EventId(202, "BeginInvokeDotNet"); + public static readonly EventId BeginInvokeDotNetFailed = new EventId(203, "BeginInvokeDotNetFailed"); + public static readonly EventId EndInvokeDispatchException = new EventId(204, "EndInvokeDispatchException"); + public static readonly EventId EndInvokeJSFailed = new EventId(205, "EndInvokeJSFailed"); + public static readonly EventId EndInvokeJSSucceeded = new EventId(206, "EndInvokeJSSucceeded"); + public static readonly EventId DispatchEventThroughJSInterop = new EventId(207, "DispatchEventThroughJSInterop"); + public static readonly EventId LocationChange = new EventId(208, "LocationChange"); + public static readonly EventId LocationChangeSucceded = new EventId(209, "LocationChangeSucceeded"); + public static readonly EventId LocationChangeFailed = new EventId(210, "LocationChangeFailed"); + public static readonly EventId LocationChangeFailedInCircuit = new EventId(211, "LocationChangeFailedInCircuit"); } static Log() { - _unhandledExceptionInvokingCircuitHandler = LoggerMessage.Define( - LogLevel.Error, - EventIds.ExceptionInvokingCircuitHandlerMethod, - "Unhandled error invoking circuit handler type {handlerType}.{handlerMethod}: {Message}"); - - _disposingCircuit = LoggerMessage.Define( + _intializationStarted = LoggerMessage.Define( LogLevel.Debug, - EventIds.DisposingCircuit, - "Disposing circuit with identifier {CircuitId}"); + EventIds.InitializationFailed, + "Circuit initialization started"); + + _intializationSucceded = LoggerMessage.Define( + LogLevel.Debug, + EventIds.InitializationFailed, + "Circuit initialization succeeded"); + + _intializationFailed = LoggerMessage.Define( + LogLevel.Debug, + EventIds.InitializationFailed, + "Circuit initialization failed"); + + _disposeStarted = LoggerMessage.Define( + LogLevel.Debug, + EventIds.DisposeStarted, + "Disposing circuit {CircuitId} started"); + + _disposeSucceded = LoggerMessage.Define( + LogLevel.Debug, + EventIds.DisposeSucceeded, + "Disposing circuit {CircuitId} succeded"); + + _disposeFailed = LoggerMessage.Define( + LogLevel.Debug, + EventIds.DisposeFailed, + "Disposing circuit {CircuitId} failed"); _onCircuitOpened = LoggerMessage.Define( LogLevel.Debug, @@ -450,6 +712,21 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits EventIds.OnCircuitClosed, "Closing circuit with id {CircuitId}."); + _circuitHandlerFailed = LoggerMessage.Define( + LogLevel.Error, + EventIds.CircuitHandlerFailed, + "Unhandled error invoking circuit handler type {handlerType}.{handlerMethod}: {Message}"); + + _circuitUnhandledException = LoggerMessage.Define( + LogLevel.Error, + EventIds.CircuitUnhandledException, + "Unhandled exception in circuit {CircuitId}"); + + _circuitUnhandledExceptionFailed = LoggerMessage.Define( + LogLevel.Debug, + EventIds.CircuitUnhandledExceptionFailed, + "Failed to transmit exception to client in circuit {CircuitId}"); + _beginInvokeDotNetStatic = LoggerMessage.Define( LogLevel.Debug, EventIds.BeginInvokeDotNet, @@ -460,6 +737,16 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits EventIds.BeginInvokeDotNet, "Invoking instance method '{MethodIdentifier}' on instance '{DotNetObjectId}' with callback id '{CallId}'"); + _beginInvokeDotNetStaticFailed = LoggerMessage.Define( + LogLevel.Debug, + EventIds.BeginInvokeDotNetFailed, + "Failed to invoke static method with identifier '{MethodIdentifier}' on assembly '{Assembly}' with callback id '{CallId}'"); + + _beginInvokeDotNetInstanceFailed = LoggerMessage.Define( + LogLevel.Debug, + EventIds.BeginInvokeDotNetFailed, + "Failed to invoke instance method '{MethodIdentifier}' on instance '{DotNetObjectId}' with callback id '{CallId}'"); + _endInvokeDispatchException = LoggerMessage.Define( LogLevel.Debug, EventIds.EndInvokeDispatchException, @@ -488,22 +775,38 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits _locationChange = LoggerMessage.Define( LogLevel.Debug, EventIds.LocationChange, - "Location changing to {URI} in {CircuitId}."); + "Location changing to {URI} in circuit {CircuitId}."); _locationChangeSucceeded = LoggerMessage.Define( LogLevel.Debug, EventIds.LocationChangeSucceded, - "Location change to {URI} in {CircuitId} succeded."); + "Location change to {URI} in circuit {CircuitId} succeded."); _locationChangeFailed = LoggerMessage.Define( LogLevel.Debug, EventIds.LocationChangeFailed, - "Location change to {URI} in {CircuitId} failed."); + "Location change to {URI} in circuit {CircuitId} failed."); + + _locationChangeFailedInCircuit = LoggerMessage.Define( + LogLevel.Error, + EventIds.LocationChangeFailed, + "Location change to {URI} in circuit {CircuitId} failed."); } - public static void UnhandledExceptionInvokingCircuitHandler(ILogger logger, CircuitHandler handler, string handlerMethod, Exception exception) + public static void InitializationStarted(ILogger logger) =>_intializationStarted(logger, null); + public static void InitializationSucceeded(ILogger logger) => _intializationSucceded(logger, null); + public static void InitializationFailed(ILogger logger, Exception exception) => _intializationFailed(logger, exception); + public static void DisposeStarted(ILogger logger, string circuitId) => _disposeStarted(logger, circuitId, null); + public static void DisposeSucceeded(ILogger logger, string circuitId) => _disposeSucceded(logger, circuitId, null); + public static void DisposeFailed(ILogger logger, string circuitId, Exception exception) => _disposeFailed(logger, circuitId, exception); + public static void CircuitOpened(ILogger logger, string circuitId) => _onCircuitOpened(logger, circuitId, null); + public static void ConnectionUp(ILogger logger, string circuitId, string connectionId) => _onConnectionUp(logger, circuitId, connectionId, null); + public static void ConnectionDown(ILogger logger, string circuitId, string connectionId) => _onConnectionDown(logger, circuitId, connectionId, null); + public static void CircuitClosed(ILogger logger, string circuitId) => _onCircuitClosed(logger, circuitId, null); + + public static void CircuitHandlerFailed(ILogger logger, CircuitHandler handler, string handlerMethod, Exception exception) { - _unhandledExceptionInvokingCircuitHandler( + _circuitHandlerFailed( logger, handler.GetType(), handlerMethod, @@ -511,27 +814,12 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits exception); } - public static void DisposingCircuit(ILogger logger, string circuitId) => _disposingCircuit(logger, circuitId, null); - - public static void CircuitOpened(ILogger logger, string circuitId) => _onCircuitOpened(logger, circuitId, null); - - public static void ConnectionUp(ILogger logger, string circuitId, string connectionId) => - _onConnectionUp(logger, circuitId, connectionId, null); - - public static void ConnectionDown(ILogger logger, string circuitId, string connectionId) => - _onConnectionDown(logger, circuitId, connectionId, null); - - public static void CircuitClosed(ILogger logger, string circuitId) => - _onCircuitClosed(logger, circuitId, null); - + public static void CircuitUnhandledException(ILogger logger, string circuitId, Exception exception) => _circuitUnhandledException(logger, circuitId, exception); + public static void CircuitUnhandledExceptionFailed(ILogger logger, string circuitId, Exception exception) => _circuitUnhandledExceptionFailed(logger, circuitId, exception); public static void EndInvokeDispatchException(ILogger logger, Exception ex) => _endInvokeDispatchException(logger, ex); - public static void EndInvokeJSFailed(ILogger logger, long asyncHandle, string arguments) => _endInvokeJSFailed(logger, asyncHandle, arguments, null); - public static void EndInvokeJSSucceeded(ILogger logger, long asyncCall) => _endInvokeJSSucceeded(logger, asyncCall, null); - public static void DispatchEventFailedToParseEventData(ILogger logger, Exception ex) => _dispatchEventFailedToParseEventData(logger, ex); - public static void DispatchEventFailedToDispatchEvent(ILogger logger, string eventHandlerId, Exception ex) => _dispatchEventFailedToDispatchEvent(logger, eventHandlerId ?? "", ex); public static void BeginInvokeDotNet(ILogger logger, string callId, string assemblyName, string methodIdentifier, long dotNetObjectId) @@ -546,11 +834,22 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits } } - public static void LocationChange(ILogger logger, string circuitId, string uri) => _locationChange(logger, circuitId, uri, null); + public static void BeginInvokeDotNetFailed(ILogger logger, string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, Exception exception) + { + if (assemblyName != null) + { + _beginInvokeDotNetStaticFailed(logger, methodIdentifier, assemblyName, callId, null); + } + else + { + _beginInvokeDotNetInstanceFailed(logger, methodIdentifier, dotNetObjectId, callId, null); + } + } - public static void LocationChangeSucceeded(ILogger logger, string circuitId, string uri) => _locationChangeSucceeded(logger, circuitId, uri, null); - - public static void LocationChangeFailed(ILogger logger, string circuitId, string uri, Exception exception) => _locationChangeFailed(logger, circuitId, uri, exception); + public static void LocationChange(ILogger logger, string uri, string circuitId) => _locationChange(logger, uri, circuitId, null); + public static void LocationChangeSucceeded(ILogger logger, string uri, string circuitId) => _locationChangeSucceeded(logger, uri, circuitId, null); + public static void LocationChangeFailed(ILogger logger, string uri, string circuitId, Exception exception) => _locationChangeFailed(logger, uri, circuitId, exception); + public static void LocationChangeFailedInCircuit(ILogger logger, string uri, string circuitId, Exception exception) => _locationChangeFailedInCircuit(logger, uri, circuitId, exception); } } } diff --git a/src/Components/Server/src/Circuits/CircuitRegistry.cs b/src/Components/Server/src/Circuits/CircuitRegistry.cs index a5c970c4af..3e3461a7dc 100644 --- a/src/Components/Server/src/Circuits/CircuitRegistry.cs +++ b/src/Components/Server/src/Circuits/CircuitRegistry.cs @@ -79,6 +79,10 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits // This will likely never happen, except perhaps in unit tests, since CircuitIds are unique. throw new ArgumentException($"Circuit with identity {circuitHost.CircuitId} is already registered."); } + + // Register for unhandled exceptions from the circuit. The registry is responsible for tearing + // down the circuit on errors. + circuitHost.UnhandledException += CircuitHost_UnhandledException; } public virtual Task DisconnectAsync(CircuitHost circuitHost, string connectionId) @@ -154,6 +158,16 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits DisconnectedCircuits.Set(circuitHost.CircuitId, entry, entryOptions); } + // ConnectAsync is called from the CircuitHub - but the error handling story is a little bit complicated. + // We return the circuit from this method, but need to clean up the circuit on failure. So we don't want to + // throw from this method because we don't want to return a *failed* circuit. + // + // The solution is to handle exceptions here, and then return null to represent failure. + // + // 1. If the circuit id is invalue return null + // 2. If the circuit is not found return null + // 3. If the circuit is found, but fails to connect, we need to dispose it here and return null + // 4. If everything goes well, return the circuit. public virtual async Task ConnectAsync(string circuitId, IClientProxy clientProxy, string connectionId, CancellationToken cancellationToken) { Log.CircuitConnectStarted(_logger, circuitId); @@ -169,6 +183,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits Task circuitHandlerTask; + // We don't expect any of the logic inside the lock to throw, or run user code. lock (CircuitRegistryLock) { // Transition the host from disconnected to connected if it's available. In this critical section, we return @@ -188,7 +203,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits // b) out of order connection-up \ connection-down events e.g. a client that disconnects as soon it finishes reconnecting. // Dispatch the circuit handlers inside the sync context to ensure the order of execution. CircuitHost executes circuit handlers inside of - // + // the sync context. circuitHandlerTask = circuitHost.Renderer.Dispatcher.InvokeAsync(async () => { if (previouslyConnected) @@ -200,13 +215,22 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits await circuitHost.OnConnectionUpAsync(cancellationToken); }); - - Log.ReconnectionSucceeded(_logger, circuitId); } - await circuitHandlerTask; + try + { + await circuitHandlerTask; + Log.ReconnectionSucceeded(_logger, circuitId); + return circuitHost; + } + catch (Exception ex) + { + Log.FailedToReconnectToCircuit(_logger, circuitId, ex); + await TerminateAsync(circuitId); - return circuitHost; + // Return null on failure, because we need to clean up the circuit. + return null; + } } protected virtual (CircuitHost circuitHost, bool previouslyConnected) ConnectCore(string circuitId, IClientProxy clientProxy, string connectionId) @@ -268,6 +292,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits try { + entry.CircuitHost.UnhandledException -= CircuitHost_UnhandledException; await entry.CircuitHost.DisposeAsync(); } catch (Exception ex) @@ -288,7 +313,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits } } - public ValueTask Terminate(string circuitId) + public ValueTask TerminateAsync(string circuitId) { CircuitHost circuitHost; DisconnectedCircuitEntry entry = default; @@ -302,13 +327,33 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits Log.CircuitDisconnectedPermanently(_logger, circuitHost.CircuitId); circuitHost.Client.SetDisconnected(); } - else - { - return default; - } } - return circuitHost?.DisposeAsync() ?? default; + if (circuitHost != null) + { + circuitHost.UnhandledException -= CircuitHost_UnhandledException; + return circuitHost.DisposeAsync(); + } + + return default; + } + + // We don't need to do anything with the exception here, logging and sending exceptions to the client + // is done inside the circuit host. + private async void CircuitHost_UnhandledException(object sender, UnhandledExceptionEventArgs e) + { + var circuitHost = (CircuitHost)sender; + + try + { + // This will dispose the circuit and remove it from the registry. + await TerminateAsync(circuitHost.CircuitId); + } + catch (Exception ex) + { + // We don't expect TerminateAsync to throw, but we want exceptions here for completeness. + Log.CircuitExceptionHandlerFailed(_logger, circuitHost.CircuitId, ex); + } } private readonly struct DisconnectedCircuitEntry @@ -339,6 +384,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits private static readonly Action _circuitMarkedDisconnected; private static readonly Action _circuitDisconnectedPermanently; private static readonly Action _circuitEvicted; + private static readonly Action _circuitExceptionHandlerFailed; private static class EventIds { @@ -355,6 +401,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits public static readonly EventId CircuitMarkedDisconnected = new EventId(110, "CircuitMarkedDisconnected"); public static readonly EventId CircuitEvicted = new EventId(111, "CircuitEvicted"); public static readonly EventId CircuitDisconnectedPermanently = new EventId(112, "CircuitDisconnectedPermanently"); + public static readonly EventId CircuitExceptionHandlerFailed = new EventId(113, "CircuitExceptionHandlerFailed"); } static Log() @@ -428,6 +475,11 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits LogLevel.Debug, EventIds.CircuitEvicted, "Circuit with id {CircuitId} evicted due to {EvictionReason}."); + + _circuitExceptionHandlerFailed = LoggerMessage.Define( + LogLevel.Error, + EventIds.CircuitExceptionHandlerFailed, + "Exception handler for {CircuitId} failed."); } public static void UnhandledExceptionDisposingCircuitHost(ILogger logger, Exception exception) => @@ -448,8 +500,8 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits public static void ConnectingToDisconnectedCircuit(ILogger logger, string circuitId, string connectionId) => _connectingToDisconnectedCircuit(logger, circuitId, connectionId, null); - public static void FailedToReconnectToCircuit(ILogger logger, string circuitId) => - _failedToReconnectToCircuit(logger, circuitId, null); + public static void FailedToReconnectToCircuit(ILogger logger, string circuitId, Exception exception = null) => + _failedToReconnectToCircuit(logger, circuitId, exception); public static void ReconnectionSucceeded(ILogger logger, string circuitId) => _reconnectionSucceeded(logger, circuitId, null); @@ -471,6 +523,9 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits public static void CircuitEvicted(ILogger logger, string circuitId, EvictionReason evictionReason) => _circuitEvicted(logger, circuitId, evictionReason, null); + + public static void CircuitExceptionHandlerFailed(ILogger logger, string circuitId, Exception exception) => + _circuitExceptionHandlerFailed(logger, circuitId, exception); } } } diff --git a/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs b/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs index 43af5cc8f4..b5c14b0911 100644 --- a/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs +++ b/src/Components/Server/src/Circuits/DefaultCircuitFactory.cs @@ -6,12 +6,9 @@ using System.Collections.Generic; using System.Linq; using System.Security.Claims; using System.Text.Encodings.Web; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Components.Web; -using Microsoft.AspNetCore.Components.Web.Rendering; using Microsoft.AspNetCore.Components.Routing; +using Microsoft.AspNetCore.Components.Web.Rendering; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -23,9 +20,9 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits { private readonly IServiceScopeFactory _scopeFactory; private readonly ILoggerFactory _loggerFactory; - private readonly ILogger _logger; private readonly CircuitIdFactory _circuitIdFactory; private readonly CircuitOptions _options; + private readonly ILogger _logger; public DefaultCircuitFactory( IServiceScopeFactory scopeFactory, @@ -34,10 +31,11 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits IOptions options) { _scopeFactory = scopeFactory ?? throw new ArgumentNullException(nameof(scopeFactory)); - _loggerFactory = loggerFactory; - _logger = _loggerFactory.CreateLogger(); + _loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); _circuitIdFactory = circuitIdFactory ?? throw new ArgumentNullException(nameof(circuitIdFactory)); - _options = options.Value; + _options = options?.Value ?? throw new ArgumentNullException(nameof(options)); + + _logger = _loggerFactory.CreateLogger(); } public override CircuitHost CreateCircuitHost( @@ -49,7 +47,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits { // We do as much intialization as possible eagerly in this method, which makes the error handling // story much simpler. If we throw from here, it's handled inside the initial hub method. - var components = ResolveComponentMetadata(httpContext, client); + var components = ResolveComponentMetadata(httpContext); var scope = _scopeFactory.CreateScope(); var encoder = scope.ServiceProvider.GetRequiredService(); @@ -88,6 +86,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits var circuitHost = new CircuitHost( _circuitIdFactory.CreateCircuitId(), scope, + _options, client, renderer, components, @@ -103,28 +102,17 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits return circuitHost; } - internal static List ResolveComponentMetadata(HttpContext httpContext, CircuitClientProxy client) + public static IReadOnlyList ResolveComponentMetadata(HttpContext httpContext) { - if (!client.Connected) + var endpoint = httpContext.GetEndpoint(); + if (endpoint == null) { - // This is the prerendering case. Descriptors will be registered by the prerenderer. - return new List(); + throw new InvalidOperationException( + $"{nameof(ComponentHub)} doesn't have an associated endpoint. " + + "Use 'app.UseEndpoints(endpoints => endpoints.MapBlazorHub(\"app\"))' to register your hub."); } - else - { - var endpointFeature = httpContext.Features.Get(); - var endpoint = endpointFeature?.Endpoint; - if (endpoint == null) - { - throw new InvalidOperationException( - $"{nameof(ComponentHub)} doesn't have an associated endpoint. " + - "Use 'app.UseEndpoints(endpoints => endpoints.MapBlazorHub(\"app\"))' to register your hub."); - } - var componentsMetadata = endpoint.Metadata.OfType().ToList(); - - return componentsMetadata; - } + return endpoint.Metadata.GetOrderedMetadata(); } private static class Log diff --git a/src/Components/Server/src/ComponentHub.cs b/src/Components/Server/src/ComponentHub.cs index f83fb5e210..a2ddc909ca 100644 --- a/src/Components/Server/src/ComponentHub.cs +++ b/src/Components/Server/src/ComponentHub.cs @@ -7,15 +7,35 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Server.Circuits; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Components.Server { - /// - /// A SignalR hub that accepts connections to an ASP.NET Core Components application. - /// + // Some notes about our expectations for error handling: + // + // In general, we need to prevent any client from interacting with a circuit that's in an unpredictable + // state. This means that when a circuit throws an unhandled exception our top priority is to + // unregister and dispose the circuit. This will prevent any new dispatches from the client + // from making it into application code. + // + // As part of this process, we also notify the client (if there is one) of the error, and we + // *expect* a well-behaved client to disconnect. A malicious client can't be expected to disconnect, + // but since we've unregistered the circuit they won't be able to access it anyway. When a call + // comes into any hub method and the circuit has been disassociated, we will abort the connection. + // It's safe to assume that's the result of a race condition or misbehaving client. + // + // Now it's important to remember that we can only abort a connection as part of a hub method call. + // We can dispose a circuit in the background, but we have to deal with a possible race condition + // any time we try to acquire access to the circuit - because it could have gone away in the + // background - outside of the scope of a hub method. + // + // In general we author our Hub methods as async methods, but we fire-and-forget anything that + // needs access to the circuit/application state to unblock the message loop. Using async in our + // Hub methods allows us to ensure message delivery to the client before we abort the connection + // in error cases. internal sealed class ComponentHub : Hub { private static readonly object CircuitKey = new object(); @@ -24,10 +44,6 @@ namespace Microsoft.AspNetCore.Components.Server private readonly CircuitOptions _options; private readonly ILogger _logger; - /// - /// Intended for framework use only. Applications should not instantiate - /// this class directly. - /// public ComponentHub( CircuitFactory circuitFactory, CircuitRegistry circuitRegistry, @@ -45,24 +61,11 @@ namespace Microsoft.AspNetCore.Components.Server /// public static PathString DefaultPath { get; } = "/_blazor"; - /// - /// For unit testing only. - /// - // We store the circuit host in Context.Items which is tied to the lifetime of the underlying - // SignalR connection. There's no need to clean this up, it's a non-owning reference and it - // will go away when the connection does. - internal CircuitHost CircuitHost - { - get => (CircuitHost)Context.Items[CircuitKey]; - private set => Context.Items[CircuitKey] = value; - } - - /// - /// Intended for framework use only. Applications should not call this method directly. - /// public override Task OnDisconnectedAsync(Exception exception) { - var circuitHost = CircuitHost; + // If the CircuitHost is gone now this isn't an error. This could happen if the disconnect + // if the result of well behaving client hanging up after an unhandled exception. + var circuitHost = GetCircuit(); if (circuitHost == null) { return Task.CompletedTask; @@ -71,15 +74,16 @@ namespace Microsoft.AspNetCore.Components.Server return _circuitRegistry.DisconnectAsync(circuitHost, Context.ConnectionId); } - /// - /// Intended for framework use only. Applications should not call this method directly. - /// - public string StartCircuit(string baseUri, string uri) + public async ValueTask StartCircuit(string baseUri, string uri) { - if (CircuitHost != null) + var circuitHost = GetCircuit(); + if (circuitHost != null) { - Log.CircuitAlreadyInitialized(_logger, CircuitHost.CircuitId); - NotifyClientError(Clients.Caller, $"The circuit host '{CircuitHost.CircuitId}' has already been initialized."); + // This is an error condition and an attempt to bind multiple circuits to a single connection. + // We can reject this and terminate the connection. + Log.CircuitAlreadyInitialized(_logger, circuitHost.CircuitId); + await NotifyClientError(Clients.Caller, $"The circuit host '{circuitHost.CircuitId}' has already been initialized."); + Context.Abort(); return null; } @@ -90,34 +94,33 @@ namespace Microsoft.AspNetCore.Components.Server { // We do some really minimal validation here to prevent obviously wrong data from getting in // without duplicating too much logic. + // + // This is an error condition attempting to initialize the circuit in a way that would fail. + // We can reject this and terminate the connection. Log.InvalidInputData(_logger); - _ = NotifyClientError(Clients.Caller, $"The uris provided are invalid."); + await NotifyClientError(Clients.Caller, $"The uris provided are invalid."); + Context.Abort(); return null; } - var circuitClient = new CircuitClientProxy(Clients.Caller, Context.ConnectionId); - if (DefaultCircuitFactory.ResolveComponentMetadata(Context.GetHttpContext(), circuitClient).Count == 0) + // From this point, we can try to actually initialize the circuit. + if (DefaultCircuitFactory.ResolveComponentMetadata(Context.GetHttpContext()).Count == 0) { - var endpointFeature = Context.GetHttpContext().Features.Get(); - var endpoint = endpointFeature?.Endpoint; - - Log.NoComponentsRegisteredInEndpoint(_logger, endpoint.DisplayName); - // No components preregistered so return. This is totally normal if the components were prerendered. + Log.NoComponentsRegisteredInEndpoint(_logger, Context.GetHttpContext().GetEndpoint()?.DisplayName); return null; } try { - var circuitHost = _circuitFactory.CreateCircuitHost( + var circuitClient = new CircuitClientProxy(Clients.Caller, Context.ConnectionId); + circuitHost = _circuitFactory.CreateCircuitHost( Context.GetHttpContext(), circuitClient, baseUri, uri, Context.User); - circuitHost.UnhandledException += CircuitHost_UnhandledException; - // Fire-and-forget the initialization process, because we can't block the // SignalR message loop (we'd get a deadlock if any of the initialization // logic relied on receiving a subsequent message from SignalR), and it will @@ -127,141 +130,136 @@ namespace Microsoft.AspNetCore.Components.Server // It's safe to *publish* the circuit now because nothing will be able // to run inside it until after InitializeAsync completes. _circuitRegistry.Register(circuitHost); - CircuitHost = circuitHost; + SetCircuit(circuitHost); return circuitHost.CircuitId; } catch (Exception ex) { + // If the circuit fails to initialize synchronously we can notify the client immediately + // and shut down the connection. Log.CircuitInitializationFailed(_logger, ex); - NotifyClientError(Clients.Caller, "The circuit failed to initialize."); + await NotifyClientError(Clients.Caller, "The circuit failed to initialize."); + Context.Abort(); return null; } } - /// - /// Intended for framework use only. Applications should not call this method directly. - /// - public async Task ConnectCircuit(string circuitId) + public async ValueTask ConnectCircuit(string circuitId) { + // ConnectionAsync will not throw. var circuitHost = await _circuitRegistry.ConnectAsync(circuitId, Clients.Caller, Context.ConnectionId, Context.ConnectionAborted); if (circuitHost != null) { - CircuitHost = circuitHost; - CircuitHost.UnhandledException += CircuitHost_UnhandledException; - + SetCircuit(circuitHost); circuitHost.SetCircuitUser(Context.User); circuitHost.SendPendingBatches(); return true; } + // If we get here the circuit does not exist anymore. This is something that's valid for a client to + // recover from, and the client is not holding any resources right now other than the connection. return false; } - /// - /// Intended for framework use only. Applications should not call this method directly. - /// - public void BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson) + public async ValueTask BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson) { - if (CircuitHost == null) + var circuitHost = await GetActiveCircuitAsync(); + if (circuitHost == null) { - Log.CircuitHostNotInitialized(_logger); - _ = NotifyClientError(Clients.Caller, "Circuit not initialized."); return; } - _ = CircuitHost.BeginInvokeDotNetFromJS(callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson); + _ = circuitHost.BeginInvokeDotNetFromJS(callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson); } - /// - /// Intended for framework use only. Applications should not call this method directly. - /// - public void EndInvokeJSFromDotNet(long asyncHandle, bool succeeded, string arguments) + public async ValueTask EndInvokeJSFromDotNet(long asyncHandle, bool succeeded, string arguments) { - if (CircuitHost == null) + var circuitHost = await GetActiveCircuitAsync(); + if (circuitHost == null) { - Log.CircuitHostNotInitialized(_logger); - _ = NotifyClientError(Clients.Caller, "Circuit not initialized."); return; } - _ = CircuitHost.EndInvokeJSFromDotNet(asyncHandle, succeeded, arguments); + _ = circuitHost.EndInvokeJSFromDotNet(asyncHandle, succeeded, arguments); } - /// - /// Intended for framework use only. Applications should not call this method directly. - /// - public void DispatchBrowserEvent(string eventDescriptor, string eventArgs) + public async ValueTask DispatchBrowserEvent(string eventDescriptor, string eventArgs) { - if (CircuitHost == null) + var circuitHost = await GetActiveCircuitAsync(); + if (circuitHost == null) { - Log.CircuitHostNotInitialized(_logger); - _ = NotifyClientError(Clients.Caller, "Circuit not initialized."); return; } - _ = CircuitHost.DispatchEvent(eventDescriptor, eventArgs); + _ = circuitHost.DispatchEvent(eventDescriptor, eventArgs); } - /// - /// Intended for framework use only. Applications should not call this method directly. - /// - public void OnRenderCompleted(long renderId, string errorMessageOrNull) + public async ValueTask OnRenderCompleted(long renderId, string errorMessageOrNull) { - if (CircuitHost == null) + var circuitHost = await GetActiveCircuitAsync(); + if (circuitHost == null) { - Log.CircuitHostNotInitialized(_logger); - NotifyClientError(Clients.Caller, "Circuit not initialized."); return; } Log.ReceivedConfirmationForBatch(_logger, renderId); - _ = CircuitHost.Renderer.OnRenderCompleted(renderId, errorMessageOrNull); + _ = circuitHost.Renderer.OnRenderCompleted(renderId, errorMessageOrNull); } - public void OnLocationChanged(string uri, bool intercepted) + public async ValueTask OnLocationChanged(string uri, bool intercepted) { - if (CircuitHost == null) + var circuitHost = await GetActiveCircuitAsync(); + if (circuitHost == null) { - Log.CircuitHostNotInitialized(_logger); - NotifyClientError(Clients.Caller, "Circuit not initialized."); return; } - _ = CircuitHost.OnLocationChangedAsync(uri, intercepted); + _ = circuitHost.OnLocationChangedAsync(uri, intercepted); } - private async void CircuitHost_UnhandledException(object sender, UnhandledExceptionEventArgs e) + // We store the CircuitHost through a *handle* here because Context.Items is tied to the lifetime + // of the connection. It's possible that a misbehaving client could cause disposal of a CircuitHost + // but keep a connection open indefinitely, preventing GC of the Circuit and related application state. + // Using a handle allows the CircuitHost to clear this reference in the background. + // + // See comment on error handling on the class definition. + private async ValueTask GetActiveCircuitAsync([CallerMemberName] string callSite = "") { - var circuitHost = (CircuitHost)sender; - var circuitId = circuitHost?.CircuitId; - - try + var handle = (CircuitHandle)Context.Items[CircuitKey]; + var circuitHost = handle?.CircuitHost; + if (handle != null && circuitHost == null) { - Log.UnhandledExceptionInCircuit(_logger, circuitId, (Exception)e.ExceptionObject); - if (_options.DetailedErrors) - { - await NotifyClientError(circuitHost.Client, e.ExceptionObject.ToString()); - } - else - { - var message = $"There was an unhandled exception on the current circuit, so this circuit will be terminated. For more details turn on " + - $"detailed exceptions in '{typeof(CircuitOptions).Name}.{nameof(CircuitOptions.DetailedErrors)}'"; - - await NotifyClientError(circuitHost.Client, message); - } - - // We generally can't abort the connection here since this is an async - // callback. The Hub has already been torn down. We'll rely on the - // client to abort the connection if we successfully transmit an error. + // This can occur when a circuit host does not exist anymore due to an unhandled exception. + // We can reject this and terminate the connection. + Log.CircuitHostShutdown(_logger, callSite); + await NotifyClientError(Clients.Caller, "Circuit has been shut down due to error."); + Context.Abort(); + return null; } - catch (Exception ex) + else if (circuitHost == null) { - Log.FailedToTransmitException(_logger, circuitId, ex); + // This can occur when a circuit host does not exist anymore due to an unhandled exception. + // We can reject this and terminate the connection. + Log.CircuitHostNotInitialized(_logger, callSite); + await NotifyClientError(Clients.Caller, "Circuit not initialized."); + Context.Abort(); + return null; } + + return circuitHost; } - private static Task NotifyClientError(IClientProxy client, string error) => - client.SendAsync("JS.Error", error); + private CircuitHost GetCircuit() + { + return ((CircuitHandle)Context.Items[CircuitKey])?.CircuitHost; + } + + private void SetCircuit(CircuitHost circuitHost) + { + Context.Items[CircuitKey] = circuitHost?.Handle; + } + + private static Task NotifyClientError(IClientProxy client, string error) => client.SendAsync("JS.Error", error); private static class Log { @@ -274,14 +272,14 @@ namespace Microsoft.AspNetCore.Components.Server private static readonly Action _unhandledExceptionInCircuit = LoggerMessage.Define(LogLevel.Warning, new EventId(3, "UnhandledExceptionInCircuit"), "Unhandled exception in circuit {CircuitId}"); - private static readonly Action _failedToTransmitException = - LoggerMessage.Define(LogLevel.Debug, new EventId(4, "FailedToTransmitException"), "Failed to transmit exception to client in circuit {CircuitId}"); - private static readonly Action _circuitAlreadyInitialized = - LoggerMessage.Define(LogLevel.Debug, new EventId(5, "CircuitAlreadyInitialized"), "The circuit host '{CircuitId}' has already been initialized"); + LoggerMessage.Define(LogLevel.Debug, new EventId(4, "CircuitAlreadyInitialized"), "The circuit host '{CircuitId}' has already been initialized"); private static readonly Action _circuitHostNotInitialized = - LoggerMessage.Define(LogLevel.Debug, new EventId(6, "CircuitHostNotInitialized"), "Call to '{CallSite}' received before the circuit host initialization"); + LoggerMessage.Define(LogLevel.Debug, new EventId(5, "CircuitHostNotInitialized"), "Call to '{CallSite}' received before the circuit host initialization"); + + private static readonly Action _circuitHostShutdown = + LoggerMessage.Define(LogLevel.Debug, new EventId(6, "CircuitHostShutdown"), "Call to '{CallSite}' received after the circuit was shut down"); private static readonly Action _circuitTerminatedGracefully = LoggerMessage.Define(LogLevel.Debug, new EventId(7, "CircuitTerminatedGracefully"), "Circuit '{CircuitId}' terminated gracefully"); @@ -307,15 +305,12 @@ namespace Microsoft.AspNetCore.Components.Server _unhandledExceptionInCircuit(logger, circuitId, exception); } - public static void FailedToTransmitException(ILogger logger, string circuitId, Exception transmissionException) - { - _failedToTransmitException(logger, circuitId, transmissionException); - } - public static void CircuitAlreadyInitialized(ILogger logger, string circuitId) => _circuitAlreadyInitialized(logger, circuitId, null); public static void CircuitHostNotInitialized(ILogger logger, [CallerMemberName] string callSite = "") => _circuitHostNotInitialized(logger, callSite, null); + public static void CircuitHostShutdown(ILogger logger, [CallerMemberName] string callSite = "") => _circuitHostShutdown(logger, callSite, null); + public static void CircuitTerminatedGracefully(ILogger logger, string circuitId) => _circuitTerminatedGracefully(logger, circuitId, null); public static void InvalidInputData(ILogger logger, [CallerMemberName] string callSite = "") => _invalidInputData(logger, callSite, null); diff --git a/src/Components/Server/test/Circuits/CircuitHostTest.cs b/src/Components/Server/test/Circuits/CircuitHostTest.cs index 3c2ca28992..9a1973e2f4 100644 --- a/src/Components/Server/test/Circuits/CircuitHostTest.cs +++ b/src/Components/Server/test/Circuits/CircuitHostTest.cs @@ -38,10 +38,11 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits // Assert serviceScope.Verify(s => s.Dispose(), Times.Once()); Assert.True(remoteRenderer.Disposed); + Assert.Null(circuitHost.Handle.CircuitHost); } [Fact] - public async Task DisposeAsync_DisposesResourcesEvenIfCircuitHandlerOrComponentThrows() + public async Task DisposeAsync_DisposesResourcesAndSilencesException() { // Arrange var serviceScope = new Mock(); @@ -60,10 +61,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits circuitHost.Renderer.AssignRootComponentId(throwOnDisposeComponent); // Act - await Assert.ThrowsAsync(async () => - { - await circuitHost.DisposeAsync(); - }); + await circuitHost.DisposeAsync(); // Does not throw // Assert Assert.True(throwOnDisposeComponent.DidCallDispose); @@ -181,7 +179,8 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits await initializeAsyncTask; // Assert: The async exception was reported via the side-channel - Assert.Same(ex, reportedErrors.Single().ExceptionObject); + var aex = Assert.IsType(reportedErrors.Single().ExceptionObject); + Assert.Same(ex, aex.InnerExceptions.Single()); Assert.False(reportedErrors.Single().IsTerminating); } diff --git a/src/Components/Server/test/Circuits/CircuitRegistryTest.cs b/src/Components/Server/test/Circuits/CircuitRegistryTest.cs index eb7828a570..6efdf212a0 100644 --- a/src/Components/Server/test/Circuits/CircuitRegistryTest.cs +++ b/src/Components/Server/test/Circuits/CircuitRegistryTest.cs @@ -133,6 +133,30 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits handler.Verify(v => v.OnCircuitClosedAsync(It.IsAny(), It.IsAny()), Times.Never()); } + [Fact] + public async Task ConnectAsync_InvokesCircuitHandlers_DisposesCircuitOnFailure() + { + // Arrange + var circuitIdFactory = TestCircuitIdFactory.CreateTestFactory(); + var registry = CreateRegistry(circuitIdFactory); + var handler = new Mock { CallBase = true }; + handler.Setup(h => h.OnConnectionUpAsync(It.IsAny(), It.IsAny())).Throws(new InvalidTimeZoneException()); + var circuitHost = TestCircuitHost.Create(circuitIdFactory.CreateCircuitId(), handlers: new[] { handler.Object }); + registry.Register(circuitHost); + + var newClient = Mock.Of(); + var newConnectionId = "new-id"; + + // Act + var result = await registry.ConnectAsync(circuitHost.CircuitId, newClient, newConnectionId, default); + + // Assert + Assert.Null(result); + Assert.Null(circuitHost.Handle.CircuitHost); // Will be null if disposed. + Assert.Empty(registry.ConnectedCircuits); + Assert.Equal(0, registry.DisconnectedCircuits.Count); + } + [Fact] public async Task DisconnectAsync_DoesNothing_IfCircuitIsInactive() { @@ -409,7 +433,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits protected override void OnEntryEvicted(object key, object value, EvictionReason reason, object state) { base.OnEntryEvicted(key, value, reason, state); - OnAfterEntryEvicted(); + OnAfterEntryEvicted?.Invoke(); } } diff --git a/src/Components/Server/test/Circuits/TestCircuitHost.cs b/src/Components/Server/test/Circuits/TestCircuitHost.cs index 3e9896e9df..8e04f6ca2d 100644 --- a/src/Components/Server/test/Circuits/TestCircuitHost.cs +++ b/src/Components/Server/test/Circuits/TestCircuitHost.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; -using System.Runtime.ExceptionServices; using System.Text.Encodings.Web; -using Microsoft.AspNetCore.Components.Web; using Microsoft.AspNetCore.Components.Web.Rendering; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.DependencyInjection; @@ -18,16 +16,11 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits { internal class TestCircuitHost : CircuitHost { - private TestCircuitHost(string circuitId, IServiceScope scope, CircuitClientProxy client, RemoteRenderer renderer, IReadOnlyList descriptors, RemoteJSRuntime jsRuntime, CircuitHandler[] circuitHandlers, ILogger logger) - : base(circuitId, scope, client, renderer, descriptors, jsRuntime, circuitHandlers, logger) + private TestCircuitHost(string circuitId, IServiceScope scope, CircuitOptions options, CircuitClientProxy client, RemoteRenderer renderer, IReadOnlyList descriptors, RemoteJSRuntime jsRuntime, CircuitHandler[] circuitHandlers, ILogger logger) + : base(circuitId, scope, options, client, renderer, descriptors, jsRuntime, circuitHandlers, logger) { } - protected override void OnHandlerError(CircuitHandler circuitHandler, string handlerMethod, Exception ex) - { - ExceptionDispatchInfo.Capture(ex).Throw(); - } - public static CircuitHost Create( string circuitId = null, IServiceScope serviceScope = null, @@ -55,6 +48,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits return new TestCircuitHost( circuitId ?? Guid.NewGuid().ToString(), serviceScope, + new CircuitOptions(), clientProxy, remoteRenderer, new List(), diff --git a/src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj b/src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj index fc3a9c6dfa..29f54b1df1 100644 --- a/src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj +++ b/src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj @@ -16,9 +16,10 @@ - + false + @@ -29,6 +30,7 @@ + diff --git a/src/Components/test/E2ETest/ServerExecutionTests/ComponentHubReliabilityTest.cs b/src/Components/test/E2ETest/ServerExecutionTests/ComponentHubReliabilityTest.cs index 3984f32d43..2c579568c0 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/ComponentHubReliabilityTest.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/ComponentHubReliabilityTest.cs @@ -2,7 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; +using System.Data; +using System.Text.RegularExpressions; using System.Threading.Tasks; using Ignitor; using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; @@ -11,26 +14,29 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using Xunit; +using Xunit.Abstractions; namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests { public class ComponentHubReliabilityTest : IClassFixture, IDisposable { - private static readonly TimeSpan DefaultLatencyTimeout = TimeSpan.FromMilliseconds(500); + private static readonly TimeSpan DefaultLatencyTimeout = TimeSpan.FromSeconds(10); private readonly AspNetSiteServerFixture _serverFixture; - public ComponentHubReliabilityTest(AspNetSiteServerFixture serverFixture) + public ComponentHubReliabilityTest(AspNetSiteServerFixture serverFixture, ITestOutputHelper output) { - serverFixture.BuildWebHostMethod = TestServer.Program.BuildWebHost; _serverFixture = serverFixture; + Output = output; + + serverFixture.BuildWebHostMethod = TestServer.Program.BuildWebHost; CreateDefaultConfiguration(); } public BlazorClient Client { get; set; } - + public ITestOutputHelper Output { get; set; } private IList Batches { get; set; } = new List(); private IList Errors { get; set; } = new List(); - private IList Logs { get; set; } = new List(); + private ConcurrentQueue Logs { get; set; } = new ConcurrentQueue(); public TestSink TestSink { get; set; } @@ -39,13 +45,24 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Client = new BlazorClient() { DefaultLatencyTimeout = DefaultLatencyTimeout }; Client.RenderBatchReceived += (id, data) => Batches.Add(new Batch(id, data)); Client.OnCircuitError += (error) => Errors.Add(error); + Client.LoggerProvider = new XunitLoggerProvider(Output); + Client.FormatError = (error) => + { + var logs = string.Join(Environment.NewLine, Logs); + return new Exception(error + Environment.NewLine + logs); + }; _ = _serverFixture.RootUri; // this is needed for the side-effects of getting the URI. TestSink = _serverFixture.Host.Services.GetRequiredService(); TestSink.MessageLogged += LogMessages; } - private void LogMessages(WriteContext context) => Logs.Add(new LogMessage(context.LogLevel, context.Message, context.Exception)); + private void LogMessages(WriteContext context) + { + var log = new LogMessage(context.LogLevel, context.Message, context.Exception); + Logs.Enqueue(log); + Output.WriteLine(log.ToString()); + } [Fact] public async Task CannotStartMultipleCircuits() @@ -58,7 +75,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Single(Batches); // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync( "StartCircuit", baseUri, baseUri + "/home")); @@ -79,25 +96,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.True(await Client.ConnectAsync(uri, prerendered: false, connectAutomatically: false), "Couldn't connect to the app"); // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync("StartCircuit", null, null)); - - // Assert - var actualError = Assert.Single(Errors); - Assert.Matches(expectedError, actualError); - Assert.DoesNotContain(Logs, l => l.LogLevel > LogLevel.Information); - } - - [Fact] - public async Task CannotStartCircuitWithInvalidUris() - { - // Arrange - var expectedError = "The uris provided are invalid."; - var rootUri = _serverFixture.RootUri; - var uri = new Uri(rootUri, "/subdir"); - Assert.True(await Client.ConnectAsync(uri, prerendered: false, connectAutomatically: false), "Couldn't connect to the app"); - - // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync("StartCircuit", uri.AbsoluteUri, "/foo")); + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync("StartCircuit", null, null)); // Assert var actualError = Assert.Single(Errors); @@ -119,7 +118,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests // Act // // These are valid URIs by the BaseUri doesn't contain the Uri - so it fails to initialize. - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync("StartCircuit", uri, "http://example.com"), TimeSpan.FromHours(1)); + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync("StartCircuit", uri, "http://example.com")); // Assert var actualError = Assert.Single(Errors); @@ -138,7 +137,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Empty(Batches); // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync( "BeginInvokeDotNetFromJS", "", "", @@ -164,7 +163,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Empty(Batches); // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync( "EndInvokeJSFromDotNet", 3, true, @@ -188,7 +187,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Empty(Batches); // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync( "DispatchBrowserEvent", "", "")); @@ -201,7 +200,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests } [Fact] - public async Task CannotInvokeOnRenderCompletedInitialization() + public async Task CannotInvokeOnRenderCompletedBeforeInitialization() { // Arrange var expectedError = "Circuit not initialized."; @@ -211,7 +210,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Empty(Batches); // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync( "OnRenderCompleted", 5, null)); @@ -234,7 +233,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Empty(Batches); // Act - await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + await Client.ExpectCircuitErrorAndDisconnect(() => Client.HubConnection.SendAsync( "OnLocationChanged", baseUri.AbsoluteUri, false)); @@ -246,6 +245,129 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Contains(Logs, l => (l.LogLevel, l.Message) == (LogLevel.Debug, "Call to 'OnLocationChanged' received before the circuit host initialization")); } + [Fact] + public async Task OnLocationChanged_ReportsDebugForExceptionInValidation() + { + // Arrange + var expectedError = "Location change to http://example.com failed."; + var rootUri = _serverFixture.RootUri; + var baseUri = new Uri(rootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + // Act + await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + "OnLocationChanged", + "http://example.com", + false)); + + // Assert + var actualError = Assert.Single(Errors); + Assert.Equal(expectedError, actualError); + Assert.DoesNotContain(Logs, l => l.LogLevel > LogLevel.Information); + Assert.Contains(Logs, l => + { + return l.LogLevel == LogLevel.Debug && Regex.IsMatch(l.Message, "Location change to http://example.com in circuit .* failed."); + }); + } + + [Fact] + public async Task OnLocationChanged_ReportsErrorForExceptionInUserCode() + { + // Arrange + var expectedError = "There was an unhandled exception .?"; + var rootUri = _serverFixture.RootUri; + var baseUri = new Uri(rootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + await Client.SelectAsync("test-selector-select", "BasicTestApp.NavigationFailureComponent"); + + // Act + await Client.ExpectCircuitError(() => Client.HubConnection.SendAsync( + "OnLocationChanged", + new Uri(baseUri, "/test").AbsoluteUri, + false)); + + // Assert + var actualError = Assert.Single(Errors); + Assert.Matches(expectedError, actualError); + Assert.Contains(Logs, l => + { + return l.LogLevel == LogLevel.Error && Regex.IsMatch(l.Message, "Unhandled exception in circuit .*"); + }); + } + + [Theory] + [InlineData("constructor-throw")] + [InlineData("attach-throw")] + [InlineData("setparameters-sync-throw")] + [InlineData("setparameters-async-throw")] + [InlineData("render-throw")] + [InlineData("afterrender-sync-throw")] + [InlineData("afterrender-async-throw")] + public async Task ComponentLifecycleMethodThrowsExceptionTerminatesTheCircuit(string id) + { + // Arrange + var expectedError = "Unhandled exception in circuit .*"; + var rootUri = _serverFixture.RootUri; + var baseUri = new Uri(rootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + await Client.SelectAsync("test-selector-select", "BasicTestApp.ReliabilityComponent"); + + // Act + await Client.ExpectCircuitError(async () => + { + await Client.ClickAsync(id, expectRenderBatch: false); + }); + + // Now if you try to click again, you will get *forcibly* disconnected for trying to talk to + // a circuit that's gone. + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(async () => await Client.ClickAsync(id, expectRenderBatch: false)); + }); + + // Checking logs at the end to avoid race condition. + Assert.Contains( + Logs, + e => LogLevel.Error == e.LogLevel && Regex.IsMatch(e.Message, expectedError)); + } + + [Fact] + public async Task ComponentDisposeMethodThrowsExceptionTerminatesTheCircuit() + { + // Arrange + var expectedError = "Unhandled exception in circuit .*"; + var rootUri = _serverFixture.RootUri; + var baseUri = new Uri(rootUri, "/subdir"); + Assert.True(await Client.ConnectAsync(baseUri, prerendered: false), "Couldn't connect to the app"); + Assert.Single(Batches); + + await Client.SelectAsync("test-selector-select", "BasicTestApp.ReliabilityComponent"); + + // Act - show then hide + await Client.ClickAsync("dispose-throw"); + await Client.ExpectCircuitError(async () => + { + await Client.ClickAsync("dispose-throw", expectRenderBatch: false); + }); + + // Now if you try to click again, you will get *forcibly* disconnected for trying to talk to + // a circuit that's gone. + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(async () => await Client.ClickAsync("dispose-throw", expectRenderBatch: false)); + }); + + // Checking logs at the end to avoid race condition. + Assert.Contains( + Logs, + e => LogLevel.Error == e.LogLevel && Regex.IsMatch(e.Message, expectedError)); + } + public void Dispose() { TestSink.MessageLogged -= LogMessages; @@ -263,6 +385,11 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests public LogLevel LogLevel { get; set; } public string Message { get; set; } public Exception Exception { get; set; } + + public override string ToString() + { + return $"{LogLevel}: {Message}{(Exception != null ? Environment.NewLine : "")}{Exception}"; + } } private class Batch diff --git a/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs b/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs index ee4f4a00ed..3d589988b8 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Text.Json; @@ -10,28 +11,63 @@ using Ignitor; using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; using Microsoft.AspNetCore.Components.Web; using Microsoft.AspNetCore.SignalR.Client; -using Microsoft.AspNetCore.Testing; -using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using Xunit; +using Xunit.Abstractions; namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests { - [Flaky("https://github.com/aspnet/AspNetCore/issues/12940", FlakyOn.All)] - public class InteropReliabilityTests : IClassFixture + public class InteropReliabilityTests : IClassFixture, IDisposable { - private static readonly TimeSpan DefaultLatencyTimeout = TimeSpan.FromSeconds(5); + private static readonly TimeSpan DefaultLatencyTimeout = TimeSpan.FromSeconds(30); private readonly AspNetSiteServerFixture _serverFixture; - public InteropReliabilityTests(AspNetSiteServerFixture serverFixture) + public InteropReliabilityTests(AspNetSiteServerFixture serverFixture, ITestOutputHelper output) { - serverFixture.BuildWebHostMethod = TestServer.Program.BuildWebHost; _serverFixture = serverFixture; + Output = output; + + serverFixture.BuildWebHostMethod = TestServer.Program.BuildWebHost; + CreateDefaultConfiguration(); } - public BlazorClient Client { get; set; } = new BlazorClient() { DefaultLatencyTimeout = DefaultLatencyTimeout }; + public BlazorClient Client { get; set; } + public ITestOutputHelper Output { get; set; } + private IList Batches { get; set; } = new List(); + private List DotNetCompletions = new List(); + private List JSInteropCalls = new List(); + private IList Errors { get; set; } = new List(); + private ConcurrentQueue Logs { get; set; } = new ConcurrentQueue(); + + public TestSink TestSink { get; set; } + + private void CreateDefaultConfiguration() + { + Client = new BlazorClient() { DefaultLatencyTimeout = DefaultLatencyTimeout }; + Client.RenderBatchReceived += (id, data) => Batches.Add(new Batch(id, data)); + Client.DotNetInteropCompletion += (method) => DotNetCompletions.Add(new DotNetCompletion(method)); + Client.JSInterop += (asyncHandle, identifier, argsJson) => JSInteropCalls.Add(new JSInteropCall(asyncHandle, identifier, argsJson)); + Client.OnCircuitError += (error) => Errors.Add(error); + Client.LoggerProvider = new XunitLoggerProvider(Output); + Client.FormatError = (error) => + { + var logs = string.Join(Environment.NewLine, Logs); + return new Exception(error + Environment.NewLine + logs); + }; + + _ = _serverFixture.RootUri; // this is needed for the side-effects of getting the URI. + TestSink = _serverFixture.Host.Services.GetRequiredService(); + TestSink.MessageLogged += LogMessages; + } + + private void LogMessages(WriteContext context) + { + var log = new LogMessage(context.LogLevel, context.Message, context.Exception); + Logs.Enqueue(log); + Output.WriteLine(log.ToString()); + } [Fact] public async Task CannotInvokeNonJSInvokableMethods() @@ -40,8 +76,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests var expectedError = "[\"1\"," + "false," + "\"There was an exception invoking \\u0027WriteAllText\\u0027 on assembly \\u0027System.IO.FileSystem\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -52,9 +87,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests JsonSerializer.Serialize(new[] { ".\\log.txt", "log" })); // Assert - Assert.Single(dotNetCompletions, expectedError); - - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] @@ -64,8 +98,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests var expectedError = "[\"1\"," + "false," + "\"There was an exception invoking \\u0027MadeUpMethod\\u0027 on assembly \\u0027BasicTestApp\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -76,19 +110,19 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests JsonSerializer.Serialize(new[] { ".\\log.txt", "log" })); // Assert - Assert.Single(dotNetCompletions, expectedError); - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } - [Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/12940")] + [Fact] public async Task CannotInvokeJSInvokableMethodsWithWrongNumberOfArguments() { // Arrange var expectedError = "[\"1\"," + "false," + "\"There was an exception invoking \\u0027NotifyLocationChanged\\u0027 on assembly \\u0027Microsoft.AspNetCore.Components.Server\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -99,9 +133,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests JsonSerializer.Serialize(new[] { _serverFixture.RootUri })); // Assert - Assert.Single(dotNetCompletions, expectedError); - - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] @@ -111,8 +144,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests var expectedError = "[\"1\"," + "false," + "\"There was an exception invoking \\u0027NotifyLocationChanged\\u0027 on assembly \\u0027\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -123,9 +156,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests JsonSerializer.Serialize(new object[] { _serverFixture.RootUri + "counter", false })); // Assert - Assert.Single(dotNetCompletions, expectedError); - - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] @@ -135,8 +167,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests var expectedError = "[\"1\"," + "false," + "\"There was an exception invoking \\u0027\\u0027 on assembly \\u0027Microsoft.AspNetCore.Components.Server\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -147,9 +179,9 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests JsonSerializer.Serialize(new object[] { _serverFixture.RootUri + "counter", false })); // Assert - Assert.Single(dotNetCompletions, expectedError); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); - await ValidateClientKeepsWorking(Client, batches); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] @@ -160,8 +192,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests var expectedError = "[\"1\"," + "false," + "\"There was an exception invoking \\u0027Reverse\\u0027 on assembly \\u0027\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -171,7 +203,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests null, JsonSerializer.Serialize(Array.Empty())); - Assert.Single(dotNetCompletions, expectedDotNetObjectRef); + Assert.Single(DotNetCompletions, c => c.Message == expectedDotNetObjectRef); await Client.InvokeDotNetMethod( "1", @@ -181,7 +213,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests JsonSerializer.Serialize(Array.Empty())); // Assert - Assert.Single(dotNetCompletions, "[\"1\",true,\"tnatropmI\"]"); + Assert.Single(DotNetCompletions, c => c.Message == "[\"1\",true,\"tnatropmI\"]"); await Client.InvokeDotNetMethod( "1", @@ -190,9 +222,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests 3, // non existing ref JsonSerializer.Serialize(Array.Empty())); - Assert.Single(dotNetCompletions, expectedError); - - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] @@ -204,8 +235,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests "false," + "\"There was an exception invoking \\u0027ReceiveTrivial\\u0027 on assembly \\u0027BasicTestApp\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); await Client.InvokeDotNetMethod( "1", @@ -214,7 +244,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests null, JsonSerializer.Serialize(Array.Empty())); - Assert.Single(dotNetCompletions, expectedImportantDotNetObjectRef); + Assert.Single(DotNetCompletions, c => c.Message == expectedImportantDotNetObjectRef); // Act await Client.InvokeDotNetMethod( @@ -225,9 +255,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests JsonSerializer.Serialize(new object[] { new { __dotNetObject = 1 } })); // Assert - Assert.Single(dotNetCompletions, expectedError); - - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] @@ -236,16 +265,15 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests // Arrange var expectedError = "An exception occurred executing JS interop: The JSON value could not be converted to System.Int32. Path: $ | LineNumber: 0 | BytePositionInLine: 3.. See InnerException for more details."; - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); // Act await Client.ClickAsync("triggerjsinterop-malformed"); - var call = interopCalls.FirstOrDefault(call => call.identifier == "sendMalformedCallbackReturn"); + var call = JSInteropCalls.FirstOrDefault(call => call.Identifier == "sendMalformedCallbackReturn"); Assert.NotEqual(default, call); - var id = call.id; + var id = call.AsyncHandle; await Client.HubConnection.InvokeAsync( "EndInvokeJSFromDotNet", id, @@ -256,41 +284,25 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Client.FindElementById("errormessage-malformed").Children.OfType(), e => expectedError == e.TextContent); - await ValidateClientKeepsWorking(Client, batches); + await ValidateClientKeepsWorking(Client, Batches); } - [Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/12940")] - public async Task LogsJSInteropCompletionsCallbacksAndContinuesWorkingInAllSituations() + [Fact] + public async Task JSInteropCompletionSuccess() { // Arrange - - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); var sink = _serverFixture.Host.Services.GetRequiredService(); var logEvents = new List<(LogLevel logLevel, string)>(); sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name)); // Act - await Client.ClickAsync("triggerjsinterop-malformed"); + await Client.ClickAsync("triggerjsinterop-success"); - var call = interopCalls.FirstOrDefault(call => call.identifier == "sendMalformedCallbackReturn"); + var call = JSInteropCalls.FirstOrDefault(call => call.Identifier == "sendSuccessCallbackReturn"); Assert.NotEqual(default, call); - var id = call.id; - await Client.HubConnection.InvokeAsync( - "EndInvokeJSFromDotNet", - id, - true, - $"[{id}, true, }}"); - - // A completely malformed payload like the one above never gets to the application. - Assert.Single( - Client.FindElementById("errormessage-malformed").Children.OfType(), - e => "" == e.TextContent); - - Assert.Contains((LogLevel.Debug, "EndInvokeDispatchException"), logEvents); - - await Client.ClickAsync("triggerjsinterop-success"); + var id = call.AsyncHandle; await Client.HubConnection.InvokeAsync( "EndInvokeJSFromDotNet", id++, @@ -302,13 +314,32 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests e => "" == e.TextContent); Assert.Contains((LogLevel.Debug, "EndInvokeJSSucceeded"), logEvents); + } + [Fact] + public async Task JSInteropThrowsInUserCode() + { + // Arrange + await GoToTestComponent(Batches); + var sink = _serverFixture.Host.Services.GetRequiredService(); + var logEvents = new List<(LogLevel logLevel, string)>(); + sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name)); + + // Act await Client.ClickAsync("triggerjsinterop-failure"); - await Client.HubConnection.InvokeAsync( - "EndInvokeJSFromDotNet", - id++, - false, - $"[{id}, false, \"There was an error invoking sendFailureCallbackReturn\"]"); + + var call = JSInteropCalls.FirstOrDefault(call => call.Identifier == "sendFailureCallbackReturn"); + Assert.NotEqual(default, call); + + var id = call.AsyncHandle; + await Client.ExpectRenderBatch(async () => + { + await Client.HubConnection.InvokeAsync( + "EndInvokeJSFromDotNet", + id, + false, + $"[{id}, false, \"There was an error invoking sendFailureCallbackReturn\"]"); + }); Assert.Single( Client.FindElementById("errormessage-failure").Children.OfType(), @@ -318,7 +349,45 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.DoesNotContain(logEvents, m => m.logLevel > LogLevel.Information); - await ValidateClientKeepsWorking(Client, batches); + await ValidateClientKeepsWorking(Client, Batches); + } + + [Fact] + public async Task MalformedJSInteropCallbackDisposesCircuit() + { + // Arrange + await GoToTestComponent(Batches); + var sink = _serverFixture.Host.Services.GetRequiredService(); + var logEvents = new List<(LogLevel logLevel, string)>(); + sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name)); + + // Act + await Client.ClickAsync("triggerjsinterop-malformed"); + + var call = JSInteropCalls.FirstOrDefault(call => call.Identifier == "sendMalformedCallbackReturn"); + Assert.NotEqual(default, call); + + var id = call.AsyncHandle; + await Client.ExpectCircuitError(async () => + { + await Client.HubConnection.InvokeAsync( + "EndInvokeJSFromDotNet", + id, + true, + $"[{id}, true, }}"); + }); + + // A completely malformed payload like the one above never gets to the application. + Assert.Single( + Client.FindElementById("errormessage-malformed").Children.OfType(), + e => "" == e.TextContent); + + Assert.Contains((LogLevel.Debug, "EndInvokeDispatchException"), logEvents); + + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(() => Client.ClickAsync("event-handler-throw-sync", expectRenderBatch: true)); + }); } [Fact] @@ -329,8 +398,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests "false," + "\"There was an exception invoking \\u0027NotifyLocationChanged\\u0027 on assembly \\u0027Microsoft.AspNetCore.Components.Server\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -341,8 +409,8 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests "[ \"invalidPayload\"}"); // Assert - Assert.Single(dotNetCompletions, expectedError); - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] @@ -353,8 +421,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests "false," + "\"There was an exception invoking \\u0027ReceiveTrivial\\u0027 on assembly \\u0027BasicTestApp\\u0027. For more details turn on detailed exceptions in \\u0027CircuitOptions.DetailedErrors\\u0027\"]"; - var (_, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); // Act await Client.InvokeDotNetMethod( @@ -365,62 +432,73 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests "[ { \"data\": {\"}} ]"); // Assert - Assert.Single(dotNetCompletions, expectedError); - await ValidateClientKeepsWorking(Client, batches); + Assert.Single(DotNetCompletions, c => c.Message == expectedError); + await ValidateClientKeepsWorking(Client, Batches); } [Fact] - public async Task DispatchingEventsWithInvalidPayloadsDoesNotCrashTheCircuit() + public async Task DispatchingEventsWithInvalidPayloadsShutsDownCircuitGracefully() { // Arrange - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); var sink = _serverFixture.Host.Services.GetRequiredService(); var logEvents = new List<(LogLevel logLevel, string)>(); sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name)); // Act - await Client.HubConnection.InvokeAsync( + await Client.ExpectCircuitError(async () => + { + await Client.HubConnection.InvokeAsync( "DispatchBrowserEvent", null, null); + }); Assert.Contains( (LogLevel.Debug, "DispatchEventFailedToParseEventData"), logEvents); - await ValidateClientKeepsWorking(Client, batches); + // Taking any other action will fail because the circuit is disposed. + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(() => Client.ClickAsync("event-handler-throw-sync", expectRenderBatch: true)); + }); } [Fact] public async Task DispatchingEventsWithInvalidEventDescriptor() { // Arrange - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); var sink = _serverFixture.Host.Services.GetRequiredService(); var logEvents = new List<(LogLevel logLevel, string)>(); sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name)); // Act - await Client.HubConnection.InvokeAsync( + await Client.ExpectCircuitError(async () => + { + await Client.HubConnection.InvokeAsync( "DispatchBrowserEvent", "{Invalid:{\"payload}", "{}"); + }); Assert.Contains( (LogLevel.Debug, "DispatchEventFailedToParseEventData"), logEvents); - await ValidateClientKeepsWorking(Client, batches); + // Taking any other action will fail because the circuit is disposed. + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(() => Client.ClickAsync("event-handler-throw-sync", expectRenderBatch: true)); + }); } [Fact] public async Task DispatchingEventsWithInvalidEventArgs() { // Arrange - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); var sink = _serverFixture.Host.Services.GetRequiredService(); var logEvents = new List<(LogLevel logLevel, string)>(); sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name)); @@ -433,24 +511,30 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests EventArgsType = "mouse", }; - await Client.HubConnection.InvokeAsync( - "DispatchBrowserEvent", - JsonSerializer.Serialize(browserDescriptor, TestJsonSerializerOptionsProvider.Options), - "{Invalid:{\"payload}"); + await Client.ExpectCircuitError(async () => + { + await Client.HubConnection.InvokeAsync( + "DispatchBrowserEvent", + JsonSerializer.Serialize(browserDescriptor, TestJsonSerializerOptionsProvider.Options), + "{Invalid:{\"payload}"); + }); Assert.Contains( (LogLevel.Debug, "DispatchEventFailedToParseEventData"), logEvents); - await ValidateClientKeepsWorking(Client, batches); + // Taking any other action will fail because the circuit is disposed. + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(() => Client.ClickAsync("event-handler-throw-sync", expectRenderBatch: true)); + }); } - [Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/12940")] + [Fact] public async Task DispatchingEventsWithInvalidEventHandlerId() { // Arrange - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); var sink = _serverFixture.Host.Services.GetRequiredService(); var logEvents = new List<(LogLevel logLevel, string eventIdName, Exception exception)>(); sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name, wc.Exception)); @@ -468,25 +552,31 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests EventArgsType = "mouse", }; - await Client.HubConnection.InvokeAsync( + await Client.ExpectCircuitError(async () => + { + await Client.HubConnection.InvokeAsync( "DispatchBrowserEvent", JsonSerializer.Serialize(browserDescriptor, TestJsonSerializerOptionsProvider.Options), JsonSerializer.Serialize(mouseEventArgs, TestJsonSerializerOptionsProvider.Options)); + }); Assert.Contains( logEvents, e => e.eventIdName == "DispatchEventFailedToDispatchEvent" && e.logLevel == LogLevel.Debug && e.exception is ArgumentException ae && ae.Message.Contains("There is no event handler with ID 1")); - await ValidateClientKeepsWorking(Client, batches); + // Taking any other action will fail because the circuit is disposed. + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(() => Client.ClickAsync("event-handler-throw-sync", expectRenderBatch: true)); + }); } [Fact] public async Task EventHandlerThrowsSyncExceptionTerminatesTheCircuit() { // Arrange - var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); - await GoToTestComponent(batches); + await GoToTestComponent(Batches); var sink = _serverFixture.Host.Services.GetRequiredService(); var logEvents = new List<(LogLevel logLevel, string eventIdName, Exception exception)>(); sink.MessageLogged += (wc) => logEvents.Add((wc.LogLevel, wc.EventId.Name, wc.Exception)); @@ -496,12 +586,19 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Contains( logEvents, - e => LogLevel.Warning == e.logLevel && - "UnhandledExceptionInCircuit" == e.eventIdName && + e => LogLevel.Error == e.logLevel && + "CircuitUnhandledException" == e.eventIdName && "Handler threw an exception" == e.exception.Message); + + // Now if you try to click again, you will get *forcibly* disconnected for trying to talk to + // a circuit that's gone. + await Client.ExpectCircuitErrorAndDisconnect(async () => + { + await Assert.ThrowsAsync(() => Client.ClickAsync("event-handler-throw-sync", expectRenderBatch: true)); + }); } - private Task ValidateClientKeepsWorking(BlazorClient Client, List<(int, byte[])> batches) => + private Task ValidateClientKeepsWorking(BlazorClient Client, IList batches) => ValidateClientKeepsWorking(Client, () => batches.Count); private async Task ValidateClientKeepsWorking(BlazorClient Client, Func countAccessor) @@ -512,7 +609,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Equal(currentBatches + 1, countAccessor()); } - private async Task GoToTestComponent(List<(int, byte[])> batches) + private async Task GoToTestComponent(IList batches) { var rootUri = _serverFixture.RootUri; Assert.True(await Client.ConnectAsync(new Uri(rootUri, "/subdir"), prerendered: false), "Couldn't connect to the app"); @@ -522,15 +619,64 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests Assert.Equal(2, batches.Count); } - private (List<(int id, string identifier, string args)>, List, List<(int, byte[])>) ConfigureClient() + public void Dispose() { - var interopCalls = new List<(int, string, string)>(); - Client.JSInterop += (int arg1, string arg2, string arg3) => interopCalls.Add((arg1, arg2, arg3)); - var batches = new List<(int, byte[])>(); - Client.RenderBatchReceived += (renderer, data) => batches.Add((renderer, data)); - var endInvokeDotNetCompletions = new List(); - Client.DotNetInteropCompletion += (completion) => endInvokeDotNetCompletions.Add(completion); - return (interopCalls, endInvokeDotNetCompletions, batches); + TestSink.MessageLogged -= LogMessages; + } + + private class LogMessage + { + public LogMessage(LogLevel logLevel, string message, Exception exception) + { + LogLevel = logLevel; + Message = message; + Exception = exception; + } + + public LogLevel LogLevel { get; set; } + public string Message { get; set; } + public Exception Exception { get; set; } + + public override string ToString() + { + return $"{LogLevel}: {Message}{(Exception != null ? Environment.NewLine : "")}{Exception}"; + } + } + + private class Batch + { + public Batch(int id, byte[] data) + { + Id = id; + Data = data; + } + + public int Id { get; } + public byte[] Data { get; } + } + + private class DotNetCompletion + { + public DotNetCompletion(string message) + { + Message = message; + } + + public string Message { get; } + } + + private class JSInteropCall + { + public JSInteropCall(int asyncHandle, string identifier, string argsJson) + { + AsyncHandle = asyncHandle; + Identifier = identifier; + ArgsJson = argsJson; + } + + public int AsyncHandle { get; } + public string Identifier { get; } + public string ArgsJson { get; } } } } diff --git a/src/Components/test/testassets/BasicTestApp/Index.razor b/src/Components/test/testassets/BasicTestApp/Index.razor index 2d125580ab..5acfe91fa0 100644 --- a/src/Components/test/testassets/BasicTestApp/Index.razor +++ b/src/Components/test/testassets/BasicTestApp/Index.razor @@ -51,6 +51,7 @@ + diff --git a/src/Components/test/testassets/BasicTestApp/NavigationFailureComponent.razor b/src/Components/test/testassets/BasicTestApp/NavigationFailureComponent.razor new file mode 100644 index 0000000000..f62cb87eb2 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/NavigationFailureComponent.razor @@ -0,0 +1,28 @@ +@implements IDisposable +@inject NavigationManager Navigation +@using Microsoft.AspNetCore.Components.Routing +

+ This component is used to test the behaviour when you attach to NavigationManager.LocationChanged + and throw an exception. We have a special code path to recognize this case and treat it as a failure + in user code rather than invalid input. + + This component is used headless tests for error handling. Markup that's provided here is for manually + testing this case in the browser. +

+ +Click here for some fireworks. + +@code { + + protected override void OnInitialized() + { + Navigation.LocationChanged += NavigationManager_LocationChanged; + } + + private void NavigationManager_LocationChanged(object sender, LocationChangedEventArgs e) + { + throw new InvalidTimeZoneException(); + } + + void IDisposable.Dispose() => Navigation.LocationChanged -= NavigationManager_LocationChanged; +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor b/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor index 63be3a8b6c..8557fa95c0 100644 --- a/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor @@ -1,6 +1,8 @@ @using Microsoft.JSInterop @inject IJSRuntime JSRuntime @namespace BasicTestApp +@using BasicTestApp.ServerReliability +

Server reliability

This component is used on the server-side execution model to validate that the circuit is resilient to failures, intentional or not. @@ -23,12 +25,68 @@ + +@if (showConstructorThrow) +{ + +} + + +@if (showAttachThrow) +{ + +} + + +@if (showSetParametersSyncThrow) +{ + +} + + +@if (showSetParametersAsyncThrow) +{ + +} + + +@if (showRenderThrow) +{ + +} + + +@if (showOnAfterRenderSyncThrow) +{ + +} + + +@if (showOnAfterRenderAsyncThrow) +{ + +} + + +@if (showDisposeThrow) +{ + +} + @code { int currentCount = 0; string errorMalformed = ""; string errorSuccess = ""; string errorFailure = ""; + bool showConstructorThrow; + bool showAttachThrow; + bool showSetParametersSyncThrow; + bool showSetParametersAsyncThrow; + bool showRenderThrow; + bool showOnAfterRenderSyncThrow; + bool showOnAfterRenderAsyncThrow; + bool showDisposeThrow; void IncrementCount() { diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingAttachComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingAttachComponent.cs new file mode 100644 index 0000000000..598ce7aadc --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingAttachComponent.cs @@ -0,0 +1,19 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingAttachComponent : IComponent + { + public void Attach(RenderHandle renderHandle) + { + throw new InvalidTimeZoneException(); + } + + public Task SetParametersAsync(ParameterView parameters) + { + throw new NotImplementedException(); + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingConstructorComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingConstructorComponent.cs new file mode 100644 index 0000000000..0101296c76 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingConstructorComponent.cs @@ -0,0 +1,24 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingConstructorComponent : IComponent + { + public ThrowingConstructorComponent() + { + throw new InvalidTimeZoneException(); + } + + public void Attach(RenderHandle renderHandle) + { + throw new NotImplementedException(); + } + + public Task SetParametersAsync(ParameterView parameters) + { + throw new NotImplementedException(); + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingDisposeComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingDisposeComponent.cs new file mode 100644 index 0000000000..1d52b03fa7 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingDisposeComponent.cs @@ -0,0 +1,27 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingDisposeComponent : IComponent, IDisposable + { + public void Attach(RenderHandle renderHandle) + { + renderHandle.Render(builder => + { + // Do nothing. + }); + } + + public void Dispose() + { + throw new InvalidTimeZoneException(); + } + + public Task SetParametersAsync(ParameterView parameters) + { + return Task.CompletedTask; + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingOnAfterRenderAsyncComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingOnAfterRenderAsyncComponent.cs new file mode 100644 index 0000000000..ab1c454779 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingOnAfterRenderAsyncComponent.cs @@ -0,0 +1,28 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingOnAfterRenderAsyncComponent : IComponent, IHandleAfterRender + { + public void Attach(RenderHandle renderHandle) + { + renderHandle.Render(builder => + { + // Do nothing. + }); + } + + public async Task OnAfterRenderAsync() + { + await Task.Yield(); + throw new InvalidTimeZoneException(); + } + + public Task SetParametersAsync(ParameterView parameters) + { + return Task.CompletedTask; + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingOnAfterRenderSyncComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingOnAfterRenderSyncComponent.cs new file mode 100644 index 0000000000..3ddcd2ddf3 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingOnAfterRenderSyncComponent.cs @@ -0,0 +1,27 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingOnAfterRenderSyncComponent : IComponent, IHandleAfterRender + { + public void Attach(RenderHandle renderHandle) + { + renderHandle.Render(builder => + { + // Do nothing. + }); + } + + public Task OnAfterRenderAsync() + { + throw new InvalidTimeZoneException(); + } + + public Task SetParametersAsync(ParameterView parameters) + { + return Task.CompletedTask; + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingRenderComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingRenderComponent.cs new file mode 100644 index 0000000000..9dcb1056ec --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingRenderComponent.cs @@ -0,0 +1,22 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingRenderComponent : IComponent + { + public void Attach(RenderHandle renderHandle) + { + renderHandle.Render(builder => + { + throw new InvalidTimeZoneException(); + }); + } + + public Task SetParametersAsync(ParameterView parameters) + { + return Task.CompletedTask; + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingSetParametersAsyncComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingSetParametersAsyncComponent.cs new file mode 100644 index 0000000000..09cd9269fb --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingSetParametersAsyncComponent.cs @@ -0,0 +1,19 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingSetParametersAsyncComponent : IComponent + { + public void Attach(RenderHandle renderHandle) + { + } + + public async Task SetParametersAsync(ParameterView parameters) + { + await Task.Yield(); + throw new InvalidTimeZoneException(); + } + } +} diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingSetParametersSyncComponent.cs b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingSetParametersSyncComponent.cs new file mode 100644 index 0000000000..83f46d522a --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ThrowingSetParametersSyncComponent.cs @@ -0,0 +1,18 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Components; + +namespace BasicTestApp.ServerReliability +{ + public class ThrowingSetParametersSyncComponent : IComponent + { + public void Attach(RenderHandle renderHandle) + { + } + + public Task SetParametersAsync(ParameterView parameters) + { + throw new InvalidTimeZoneException(); + } + } +} diff --git a/src/Components/test/testassets/Ignitor/BlazorClient.cs b/src/Components/test/testassets/Ignitor/BlazorClient.cs index e6867ef73b..fa4e45f248 100644 --- a/src/Components/test/testassets/Ignitor/BlazorClient.cs +++ b/src/Components/test/testassets/Ignitor/BlazorClient.cs @@ -26,12 +26,12 @@ namespace Ignitor { TaskCompletionSource.TrySetCanceled(); }); - - ImplicitWait = DefaultLatencyTimeout != null; } public TimeSpan? DefaultLatencyTimeout { get; set; } = TimeSpan.FromMilliseconds(500); + public Func FormatError { get; set; } + private CancellationTokenSource CancellationTokenSource { get; } private CancellationToken CancellationToken => CancellationTokenSource.Token; @@ -42,10 +42,14 @@ namespace Ignitor private CancellableOperation NextErrorReceived { get; set; } + private CancellableOperation NextDisconnect { get; set; } + private CancellableOperation NextJSInteropReceived { get; set; } private CancellableOperation NextDotNetInteropCompletionReceived { get; set; } + public ILoggerProvider LoggerProvider { get; set; } + public bool ConfirmRenderBatch { get; set; } = true; public event Action JSInterop; @@ -60,7 +64,7 @@ namespace Ignitor public ElementHive Hive { get; set; } = new ElementHive(); - public bool ImplicitWait { get; set; } + public bool ImplicitWait => DefaultLatencyTimeout != null; public HubConnection HubConnection { get; set; } @@ -112,6 +116,18 @@ namespace Ignitor return NextErrorReceived.Completion.Task; } + public Task PrepareForNextDisconnect(TimeSpan? timeout) + { + if (NextDisconnect?.Completion != null) + { + throw new InvalidOperationException("Invalid state previous task not completed"); + } + + NextDisconnect = new CancellableOperation(timeout); + + return NextDisconnect.Completion.Task; + } + public Task ClickAsync(string elementId, bool expectRenderBatch = true) { if (!Hive.TryFindElementById(elementId, out var elementNode)) @@ -128,14 +144,14 @@ namespace Ignitor } } - public async Task SelectAsync(string elementId, string value) + public Task SelectAsync(string elementId, string value) { if (!Hive.TryFindElementById(elementId, out var elementNode)) { throw new InvalidOperationException($"Could not find element with id {elementId}."); } - await ExpectRenderBatch(() => elementNode.SelectAsync(HubConnection, value)); + return ExpectRenderBatch(() => elementNode.SelectAsync(HubConnection, value)); } public async Task ExpectRenderBatch(Func action, TimeSpan? timeout = null) @@ -166,6 +182,22 @@ namespace Ignitor await task; } + public async Task ExpectCircuitErrorAndDisconnect(Func action, TimeSpan? timeout = null) + { + // NOTE: timeout is used for each operation individually. + await ExpectDisconnect(async () => + { + await ExpectCircuitError(action, timeout); + }, timeout); + } + + public async Task ExpectDisconnect(Func action, TimeSpan? timeout = null) + { + var task = WaitForDisconnect(timeout); + await action(); + await task; + } + private Task WaitForRenderBatch(TimeSpan? timeout = null) { if (ImplicitWait) @@ -220,12 +252,32 @@ namespace Ignitor } } + private async Task WaitForDisconnect(TimeSpan? timeout = null) + { + if (ImplicitWait) + { + if (DefaultLatencyTimeout == null && timeout == null) + { + throw new InvalidOperationException("Implicit wait without DefaultLatencyTimeout is not allowed."); + } + } + + await PrepareForNextDisconnect(timeout ?? DefaultLatencyTimeout); + } + public async Task ConnectAsync(Uri uri, bool prerendered, bool connectAutomatically = true) { var builder = new HubConnectionBuilder(); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.WithUrl(GetHubUrl(uri)); - builder.ConfigureLogging(l => l.AddConsole().SetMinimumLevel(LogLevel.Trace)); + builder.ConfigureLogging(l => + { + l.SetMinimumLevel(LogLevel.Trace); + if (LoggerProvider != null) + { + l.AddProvider(LoggerProvider); + } + }); HubConnection = builder.Build(); await HubConnection.StartAsync(CancellationToken); @@ -260,53 +312,32 @@ namespace Ignitor private void OnEndInvokeDotNet(string completion) { - try - { - DotNetInteropCompletion?.Invoke(completion); + DotNetInteropCompletion?.Invoke(completion); - NextDotNetInteropCompletionReceived?.Completion?.TrySetResult(null); - } - catch (Exception e) - { - NextDotNetInteropCompletionReceived?.Completion?.TrySetException(e); - } + NextDotNetInteropCompletionReceived?.Completion?.TrySetResult(null); } private void OnBeginInvokeJS(int asyncHandle, string identifier, string argsJson) { - try - { - JSInterop?.Invoke(asyncHandle, identifier, argsJson); + JSInterop?.Invoke(asyncHandle, identifier, argsJson); - NextJSInteropReceived?.Completion?.TrySetResult(null); - } - catch (Exception e) - { - NextJSInteropReceived?.Completion?.TrySetException(e); - } + NextJSInteropReceived?.Completion?.TrySetResult(null); } private void OnRenderBatch(int batchId, byte[] batchData) { - try + RenderBatchReceived?.Invoke(batchId, batchData); + + var batch = RenderBatchReader.Read(batchData); + + Hive.Update(batch); + + if (ConfirmRenderBatch) { - RenderBatchReceived?.Invoke(batchId, batchData); - - var batch = RenderBatchReader.Read(batchData); - - Hive.Update(batch); - - if (ConfirmRenderBatch) - { - _ = ConfirmBatch(batchId); - } - - NextBatchReceived?.Completion?.TrySetResult(null); - } - catch (Exception e) - { - NextBatchReceived?.Completion?.TrySetResult(e); + _ = ConfirmBatch(batchId); } + + NextBatchReceived?.Completion?.TrySetResult(null); } public Task ConfirmBatch(int batchId, string error = null) @@ -316,20 +347,19 @@ namespace Ignitor private void OnError(string error) { - try - { - OnCircuitError?.Invoke(error); + OnCircuitError?.Invoke(error); - NextErrorReceived?.Completion?.TrySetResult(null); - } - catch (Exception e) - { - NextErrorReceived?.Completion?.TrySetResult(e); - } + var exception = FormatError?.Invoke(error) ?? new Exception(error); + NextBatchReceived?.Completion?.TrySetException(exception); + NextDotNetInteropCompletionReceived?.Completion.TrySetException(exception); + NextJSInteropReceived?.Completion.TrySetException(exception); + NextErrorReceived?.Completion?.TrySetResult(null); } private Task OnClosedAsync(Exception ex) { + NextDisconnect?.Completion?.TrySetResult(null); + if (ex == null) { TaskCompletionSource.TrySetResult(null);