From c918d72f3669e5e375776ea17b8866132c3350b3 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Mon, 22 Jul 2019 19:14:30 +0200 Subject: [PATCH] [Blazor] [Fixes #11847] Renderer.DispatchEventAsync throws null reference exception if event handler throws synchronously (#12393) [Blazor] [Fixes #11847] Renderer.DispatchEventAsync throws null reference exception if event handler throws synchronously * Returns after handling the exception. * Adds a unit test and an E2E test to validate expected behavior. --- .../Components/src/Rendering/Renderer.cs | 1 + .../Components/test/RendererTest.cs | 48 ++++++++++++++----- .../InteropReliabilityTests.cs | 23 +++++++++ .../ReliabilityComponent.razor | 4 ++ 4 files changed, 64 insertions(+), 12 deletions(-) diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index 60fe23e423..6f2a7048b4 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -235,6 +235,7 @@ namespace Microsoft.AspNetCore.Components.Rendering catch (Exception e) { HandleException(e); + return Task.CompletedTask; } finally { diff --git a/src/Components/Components/test/RendererTest.cs b/src/Components/Components/test/RendererTest.cs index 839b1a9858..c165c6e1ec 100644 --- a/src/Components/Components/test/RendererTest.cs +++ b/src/Components/Components/test/RendererTest.cs @@ -456,7 +456,7 @@ namespace Microsoft.AspNetCore.Components.Test } [Fact] - public async Task CanDispatchEventsToTopLevelComponents() + public void CanDispatchEventsToTopLevelComponents() { // Arrange: Render a component with an event handler var renderer = new TestRenderer(); @@ -482,12 +482,42 @@ namespace Microsoft.AspNetCore.Components.Test var renderTask = renderer.DispatchEventAsync(eventHandlerId, eventArgs); Assert.True(renderTask.IsCompletedSuccessfully); Assert.Same(eventArgs, receivedArgs); - - await renderTask; // Does not throw } [Fact] - public async Task CanDispatchTypedEventsToTopLevelComponents() + public void DispatchEventHandlesSynchronousExceptionsFromEventHandlers() + { + // Arrange: Render a component with an event handler + var renderer = new TestRenderer { + ShouldHandleExceptions = true + }; + + var component = new EventComponent + { + OnTest = args => throw new Exception("Error") + }; + var componentId = renderer.AssignRootComponentId(component); + component.TriggerRender(); + + var eventHandlerId = renderer.Batches.Single() + .ReferenceFrames + .First(frame => frame.AttributeValue != null) + .AttributeEventHandlerId; + + // Assert: Event not yet fired + Assert.Empty(renderer.HandledExceptions); + + // Act/Assert: Event can be fired + var eventArgs = new UIEventArgs(); + var renderTask = renderer.DispatchEventAsync(eventHandlerId, eventArgs); + Assert.True(renderTask.IsCompletedSuccessfully); + + var exception = Assert.Single(renderer.HandledExceptions); + Assert.Equal("Error", exception.Message); + } + + [Fact] + public void CanDispatchTypedEventsToTopLevelComponents() { // Arrange: Render a component with an event handler var renderer = new TestRenderer(); @@ -513,12 +543,10 @@ namespace Microsoft.AspNetCore.Components.Test var renderTask = renderer.DispatchEventAsync(eventHandlerId, eventArgs); Assert.True(renderTask.IsCompletedSuccessfully); Assert.Same(eventArgs, receivedArgs); - - await renderTask; // does not throw } [Fact] - public async Task CanDispatchActionEventsToTopLevelComponents() + public void CanDispatchActionEventsToTopLevelComponents() { // Arrange: Render a component with an event handler var renderer = new TestRenderer(); @@ -544,12 +572,10 @@ namespace Microsoft.AspNetCore.Components.Test var renderTask = renderer.DispatchEventAsync(eventHandlerId, eventArgs); Assert.True(renderTask.IsCompletedSuccessfully); Assert.NotNull(receivedArgs); - - await renderTask; // does not throw } [Fact] - public async Task CanDispatchEventsToNestedComponents() + public void CanDispatchEventsToNestedComponents() { UIEventArgs receivedArgs = null; @@ -586,8 +612,6 @@ namespace Microsoft.AspNetCore.Components.Test var renderTask = renderer.DispatchEventAsync(eventHandlerId, eventArgs); Assert.True(renderTask.IsCompletedSuccessfully); Assert.Same(eventArgs, receivedArgs); - - await renderTask; // does not throw } [Fact] diff --git a/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs b/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs index 35c41f45a7..9d9cdfc46b 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/InteropReliabilityTests.cs @@ -490,6 +490,29 @@ namespace Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests await ValidateClientKeepsWorking(Client, batches); } + + [Fact] + public async Task EventHandlerThrowsSyncExceptionTerminatesTheCircuit() + { + // Arrange + var (interopCalls, dotNetCompletions, batches) = ConfigureClient(); + 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)); + + // Act + await Client.ClickAsync("event-handler-throw-sync"); + + Assert.Contains( + logEvents, + e => LogLevel.Warning == e.logLevel && + "UnhandledExceptionInCircuit" == e.eventIdName && + "Handler threw an exception" == e.exception.Message); + + await ValidateClientKeepsWorking(Client, batches); + } + private Task ValidateClientKeepsWorking(BlazorClient Client, List<(int, int, byte[])> batches) => ValidateClientKeepsWorking(Client, () => batches.Count); diff --git a/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor b/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor index 593cb0bf3c..63be3a8b6c 100644 --- a/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor +++ b/src/Components/test/testassets/BasicTestApp/ServerReliability/ReliabilityComponent.razor @@ -19,6 +19,8 @@ + + @code @@ -33,6 +35,8 @@ currentCount++; } + void TriggerSyncException() => throw new Exception("Handler threw an exception"); + async Task TriggerJSInterop() { try