Skip to content

Commit

Permalink
Forms PRG and error handling fixes (#49472)
Browse files Browse the repository at this point in the history
* Add some E2E test app code that demonstrates problems (no actual E2E test scripts yet though)

* Fix one of the problems (streaming SSR wasn't returning synchronous errors)

* Fix event dispatch error handling

* E2E cases

* Fix and test for P/R/G

* Renames from other PR review
  • Loading branch information
SteveSandersonMS authored Jul 17, 2023
1 parent e5bb36d commit c69cb31
Show file tree
Hide file tree
Showing 17 changed files with 354 additions and 59 deletions.
2 changes: 1 addition & 1 deletion src/Components/Components/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ static Microsoft.Extensions.DependencyInjection.CascadingValueServiceCollectionE
virtual Microsoft.AspNetCore.Components.Rendering.ComponentState.DisposeAsync() -> System.Threading.Tasks.ValueTask
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.AddPendingTask(Microsoft.AspNetCore.Components.Rendering.ComponentState? componentState, System.Threading.Tasks.Task! task) -> void
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.CreateComponentState(int componentId, Microsoft.AspNetCore.Components.IComponent! component, Microsoft.AspNetCore.Components.Rendering.ComponentState? parentComponentState) -> Microsoft.AspNetCore.Components.Rendering.ComponentState!
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.DispatchEventAsync(ulong eventHandlerId, Microsoft.AspNetCore.Components.RenderTree.EventFieldInfo? fieldInfo, System.EventArgs! eventArgs, bool quiesce) -> System.Threading.Tasks.Task!
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.DispatchEventAsync(ulong eventHandlerId, Microsoft.AspNetCore.Components.RenderTree.EventFieldInfo? fieldInfo, System.EventArgs! eventArgs, bool waitForQuiescence) -> System.Threading.Tasks.Task!
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.ResolveComponentForRenderMode(System.Type! componentType, int? parentComponentId, Microsoft.AspNetCore.Components.IComponentActivator! componentActivator, Microsoft.AspNetCore.Components.IComponentRenderMode! renderMode) -> Microsoft.AspNetCore.Components.IComponent!
~Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame.ComponentRenderMode.get -> Microsoft.AspNetCore.Components.IComponentRenderMode
~Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame.NamedEventAssignedName.get -> string
Expand Down
40 changes: 18 additions & 22 deletions src/Components/Components/src/RenderTree/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ protected virtual ComponentState CreateComponentState(int componentId, IComponen
/// </returns>
public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? fieldInfo, EventArgs eventArgs)
{
return DispatchEventAsync(eventHandlerId, fieldInfo, eventArgs, quiesce: false);
return DispatchEventAsync(eventHandlerId, fieldInfo, eventArgs, waitForQuiescence: false);
}

/// <summary>
Expand All @@ -389,15 +389,20 @@ public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? fie
/// <param name="eventHandlerId">The <see cref="RenderTreeFrame.AttributeEventHandlerId"/> value from the original event attribute.</param>
/// <param name="eventArgs">Arguments to be passed to the event handler.</param>
/// <param name="fieldInfo">Information that the renderer can use to update the state of the existing render tree to match the UI.</param>
/// <param name="quiesce">Whether to wait for quiescence or not.</param>
/// <param name="waitForQuiescence">A flag indicating whether to wait for quiescence.</param>
/// <returns>
/// A <see cref="Task"/> which will complete once all asynchronous processing related to the event
/// has completed.
/// </returns>
public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? fieldInfo, EventArgs eventArgs, bool quiesce)
public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? fieldInfo, EventArgs eventArgs, bool waitForQuiescence)
{
Dispatcher.AssertAccess();

if (waitForQuiescence)
{
_pendingTasks ??= new();
}

var callback = GetRequiredEventCallback(eventHandlerId);
Log.HandlingEvent(_logger, eventHandlerId, eventArgs);

Expand Down Expand Up @@ -425,22 +430,9 @@ public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? fie
_isBatchInProgress = true;

task = callback.InvokeAsync(eventArgs);
if (quiesce)
{
// If we are waiting for quiescence, the quiescence task will capture any async exception.
// If the exception is thrown synchronously, we just want it to flow to the callers, and
// not go through the ErrorBoundary.
_pendingTasks ??= new();
AddPendingTask(receiverComponentState, task);
}
}
catch (Exception e)
{
if (quiesce)
{
// Exception filters are not AoT friendly.
throw;
}
HandleExceptionViaErrorBoundary(e, receiverComponentState);
return Task.CompletedTask;
}
Expand All @@ -453,15 +445,19 @@ public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? fie
ProcessPendingRender();
}

if (quiesce)
// Task completed synchronously or is still running. We already processed all of the rendering
// work that was queued so let our error handler deal with it.
var errorHandledTask = GetErrorHandledTask(task, receiverComponentState);

if (waitForQuiescence)
{
AddPendingTask(receiverComponentState, errorHandledTask);
return WaitForQuiescence();
}

// Task completed synchronously or is still running. We already processed all of the rendering
// work that was queued so let our error handler deal with it.
var result = GetErrorHandledTask(task, receiverComponentState);
return result;
else
{
return errorHandledTask;
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private static bool MatchesScope(string incomingScopeQualifiedFormName, string c
public void Map(FormValueMappingContext context)
{
// This will func to a proper binder
if (!CanMap(context.ValueType, context.MappingScopeName, context.RestrictToFormName))
if (!CanMap(context.ValueType, context.AcceptMappingScopeName, context.AcceptFormName))
{
context.SetResult(null);
}
Expand Down
29 changes: 21 additions & 8 deletions src/Components/Endpoints/src/RazorComponentEndpointInvoker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,29 @@ await EndpointHtmlRenderer.InitializeStandardComponentServicesAsync(
ParameterView.Empty,
waitForQuiescence: isPost);

var isBadRequest = false;
var quiesceTask = isPost ? _renderer.DispatchSubmitEventAsync(handler, out isBadRequest) : htmlContent.QuiescenceTask;
if (isBadRequest)
Task quiesceTask;
if (!isPost)
{
return;
quiesceTask = htmlContent.QuiescenceTask;
}

if (isPost)
else
{
await Task.WhenAll(_renderer.NonStreamingPendingTasks);
try
{
var isBadRequest = false;
quiesceTask = _renderer.DispatchSubmitEventAsync(handler, out isBadRequest);
if (isBadRequest)
{
return;
}

await Task.WhenAll(_renderer.NonStreamingPendingTasks);
}
catch (NavigationException ex)
{
await EndpointHtmlRenderer.HandleNavigationException(_context, ex);
quiesceTask = Task.CompletedTask;
}
}

// Importantly, we must not yield this thread (which holds exclusive access to the renderer sync context)
Expand All @@ -95,7 +108,7 @@ await EndpointHtmlRenderer.InitializeStandardComponentServicesAsync(
// renderer sync context and cause a batch that would get missed.
htmlContent.WriteTo(writer, HtmlEncoder.Default); // Don't use WriteToAsync, as per the comment above

if (!quiesceTask.IsCompleted)
if (!quiesceTask.IsCompletedSuccessfully)
{
await _renderer.SendStreamingUpdatesAsync(_context, quiesceTask, writer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal Task DispatchSubmitEventAsync(string? handlerName, out bool isBadReques
var frameLocation = locationsForName.Single();
var eventHandlerId = FindEventHandlerIdForNamedEvent("onsubmit", frameLocation.ComponentId, frameLocation.FrameIndex);
return eventHandlerId.HasValue
? DispatchEventAsync(eventHandlerId.Value, null, EventArgs.Empty, quiesce: true)
? DispatchEventAsync(eventHandlerId.Value, null, EventArgs.Empty, waitForQuiescence: true)
: Task.CompletedTask;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private async Task WaitForResultReady(bool waitForQuiescence, PrerenderedCompone
}
}

private static ValueTask<PrerenderedComponentHtmlContent> HandleNavigationException(HttpContext httpContext, NavigationException navigationException)
public static ValueTask<PrerenderedComponentHtmlContent> HandleNavigationException(HttpContext httpContext, NavigationException navigationException)
{
if (httpContext.Response.HasStarted)
{
Expand Down
18 changes: 9 additions & 9 deletions src/Components/Web/src/Forms/Mapping/FormValueMappingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,31 @@ public class FormValueMappingContext
/// <summary>
/// Initializes a new instance of <see cref="FormValueMappingContext"/>.
/// </summary>
/// <param name="mappingScopeName">The name of the current <see cref="FormMappingScope"/>. Values will only be mapped if the incoming data corresponds to this scope name.</param>
/// <param name="restrictToFormName">If set, indicates that the mapping should only receive values if the incoming form matches this name. If null, the mapping should receive data from any form in the mapping scope.</param>
/// <param name="acceptMappingScopeName">The name of a <see cref="FormMappingScope"/>. Values will only be mapped if the incoming data corresponds to this scope name.</param>
/// <param name="acceptFormName">If set, indicates that the mapping should only receive values if the incoming form matches this name. If null, the mapping should receive data from any form in the mapping scope.</param>
/// <param name="valueType">The <see cref="Type"/> of the value to map.</param>
/// <param name="parameterName">The name of the parameter to map data to.</param>
public FormValueMappingContext(string mappingScopeName, string? restrictToFormName, Type valueType, string parameterName)
public FormValueMappingContext(string acceptMappingScopeName, string? acceptFormName, Type valueType, string parameterName)
{
ArgumentNullException.ThrowIfNull(mappingScopeName, nameof(mappingScopeName));
ArgumentNullException.ThrowIfNull(acceptMappingScopeName, nameof(acceptMappingScopeName));
ArgumentNullException.ThrowIfNull(valueType, nameof(valueType));
ArgumentNullException.ThrowIfNull(parameterName, nameof(parameterName));

MappingScopeName = mappingScopeName;
RestrictToFormName = restrictToFormName;
AcceptMappingScopeName = acceptMappingScopeName;
AcceptFormName = acceptFormName;
ParameterName = parameterName;
ValueType = valueType;
}

/// <summary>
/// Gets the name of the current <see cref="FormMappingScope"/>.
/// Gets the name of <see cref="FormMappingScope"/> that is allowed to supply data in this context.
/// </summary>
public string MappingScopeName { get; }
public string AcceptMappingScopeName { get; }

/// <summary>
/// If set, indicates that the mapping should only receive values if the incoming form matches this name. If null, the mapping should receive data from any form in the mapping scope.
/// </summary>
public string? RestrictToFormName { get; }
public string? AcceptFormName { get; }

/// <summary>
/// Gets the name of the parameter to map data to.
Expand Down
6 changes: 3 additions & 3 deletions src/Components/Web/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ Microsoft.AspNetCore.Components.Forms.Mapping.FormMappingError.ErrorMessages.get
Microsoft.AspNetCore.Components.Forms.Mapping.FormMappingError.Name.get -> string!
Microsoft.AspNetCore.Components.Forms.Mapping.FormMappingError.Path.get -> string!
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.FormValueMappingContext(string! mappingScopeName, string? restrictToFormName, System.Type! valueType, string! parameterName) -> void
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.AcceptFormName.get -> string?
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.AcceptMappingScopeName.get -> string!
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.FormValueMappingContext(string! acceptMappingScopeName, string? acceptFormName, System.Type! valueType, string! parameterName) -> void
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.MapErrorToContainer.get -> System.Action<string!, object!>?
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.MapErrorToContainer.set -> void
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.MappingScopeName.get -> string!
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.OnError.get -> System.Action<string!, System.FormattableString!, string?>?
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.OnError.set -> void
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.ParameterName.get -> string!
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.RestrictToFormName.get -> string?
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.Result.get -> object?
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.SetResult(object? result) -> void
Microsoft.AspNetCore.Components.Forms.Mapping.FormValueMappingContext.ValueType.get -> System.Type!
Expand Down
Loading

0 comments on commit c69cb31

Please sign in to comment.