Skip to content

Forms PRG and error handling fixes #49472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These unrelated renames are follow-up on code-review feedback from an earlier PR. Just trying to clear up!

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