Skip to content

Add NotFoundPage to Router #60970

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 46 commits into from
Apr 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
11956ec
Add `NotFoundPage`.
ilonatommy Mar 18, 2025
216ec31
Fix rebase error.
ilonatommy Mar 19, 2025
3168993
Draft of template changes.
ilonatommy Mar 21, 2025
067cfd2
Typo errors.
ilonatommy Mar 21, 2025
6d22aad
NavigationManager is not needed for SSR.
ilonatommy Mar 21, 2025
6ea7082
Add BOM to new teamplate files.
ilonatommy Mar 24, 2025
25e8e66
Move instead of exclude.
ilonatommy Mar 24, 2025
1060b4d
Clean up, fix tests.
ilonatommy Mar 24, 2025
fc696c3
Fix
ilonatommy Mar 24, 2025
44e7d8e
Apply smallest possible changes to templates.
ilonatommy Mar 25, 2025
ae68011
Missing changes to baseline.
ilonatommy Mar 25, 2025
39deb2b
Prevent throwing.
ilonatommy Mar 25, 2025
6b620d5
Fix configurations without global router.
ilonatommy Mar 25, 2025
6876252
Merge branch 'main' into fix-58815
ilonatommy Mar 25, 2025
74e3eae
Fix "response started" scenarios.
ilonatommy Mar 26, 2025
e10b90c
Fix template tests.
ilonatommy Mar 26, 2025
b660d19
Fix baseline tests.
ilonatommy Mar 26, 2025
d566c4b
This is a draft of uneffective `UseStatusCodePagesWithReExecute`, cc …
ilonatommy Mar 26, 2025
d41bd5b
Update.
ilonatommy Mar 27, 2025
005f217
Fix reexecution mechanism.
ilonatommy Mar 31, 2025
8c7b6d2
Fix public API.
ilonatommy Mar 31, 2025
f876e4d
Args order.
ilonatommy Mar 31, 2025
8744e8b
Draft of test.
ilonatommy Mar 31, 2025
aee53a9
Per page interactivity test.
ilonatommy Apr 1, 2025
de91b4b
Revert unnecessary change.
ilonatommy Apr 1, 2025
7e7f1fe
Typo: we want to stop only if status pages are on.
ilonatommy Apr 2, 2025
6a10062
Remove comments.
ilonatommy Apr 2, 2025
5518812
Fix tests.
ilonatommy Apr 2, 2025
c8eb629
Feedback.
ilonatommy Apr 7, 2025
66b563c
Feedback.
ilonatommy Apr 7, 2025
1da92f9
Failing test - re-executed without a reason.
ilonatommy Apr 7, 2025
b7775ef
Add streaming test after response started.
ilonatommy Apr 8, 2025
cb32f94
Test SSR with no interactivity.
ilonatommy Apr 8, 2025
8273758
Stop the renderer regardless of `Response.HasStarted`.
ilonatommy Apr 8, 2025
c31812c
Feedback: not checking status code works as well.
ilonatommy Apr 9, 2025
b162946
Feedback: improve handling streaming-in-process case.
ilonatommy Apr 10, 2025
f57b0b7
Merge branch 'main' into fix-58815
ilonatommy Apr 10, 2025
3e77c62
Throw on NotFound without global router.
ilonatommy Apr 11, 2025
d612592
Use `IStatusCodeReExecuteFeature`.
ilonatommy Apr 11, 2025
cd5dffa
Unify "fallback" pages - check for titles only.
ilonatommy Apr 15, 2025
b416805
Fix early return condition.
ilonatommy Apr 17, 2025
fc63304
Merge branch 'main' into fix-58815
ilonatommy Apr 22, 2025
c0e7f86
Feedback.
ilonatommy Apr 22, 2025
124b470
Merge branch 'main' into fix-58815
ilonatommy Apr 22, 2025
ebdf9a9
No-op instead of exception.
ilonatommy Apr 23, 2025
fc49fe6
Solve merge conflict: hard stop in redirection and deferred stop in 404.
ilonatommy Apr 23, 2025
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
12 changes: 11 additions & 1 deletion src/Components/Components/src/NavigationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ public event EventHandler<NotFoundEventArgs> OnNotFound

private EventHandler<NotFoundEventArgs>? _notFound;

private static readonly NotFoundEventArgs _notFoundEventArgs = new NotFoundEventArgs();

// For the baseUri it's worth storing as a System.Uri so we can do operations
// on that type. System.Uri gives us access to the original string anyway.
private Uri? _baseUri;
Expand Down Expand Up @@ -203,7 +205,15 @@ public virtual void Refresh(bool forceReload = false)

private void NotFoundCore()
{
_notFound?.Invoke(this, new NotFoundEventArgs());
if (_notFound == null)
{
// global router doesn't exist, no events were registered
return;
}
else
{
_notFound.Invoke(this, _notFoundEventArgs);
}
}

/// <summary>
Expand Down
3 changes: 3 additions & 0 deletions src/Components/Components/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable

Microsoft.AspNetCore.Components.Routing.Router.NotFoundPage.get -> System.Type!
Microsoft.AspNetCore.Components.Routing.Router.NotFoundPage.set -> void
Microsoft.AspNetCore.Components.NavigationManager.OnNotFound -> System.EventHandler<Microsoft.AspNetCore.Components.Routing.NotFoundEventArgs!>!
Microsoft.AspNetCore.Components.NavigationManager.NotFound() -> void
Microsoft.AspNetCore.Components.Routing.IHostEnvironmentNavigationManager.Initialize(string! baseUri, string! uri, System.Func<string!, System.Threading.Tasks.Task!>! onNavigateTo) -> void
Expand Down
42 changes: 41 additions & 1 deletion src/Components/Components/src/Routing/Router.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@

#nullable disable warnings

using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Reflection.Metadata;
using System.Runtime.ExceptionServices;
using Microsoft.AspNetCore.Components.HotReload;
using Microsoft.AspNetCore.Components.Rendering;
using Microsoft.AspNetCore.Internal;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.DependencyInjection;

Expand Down Expand Up @@ -70,6 +72,13 @@ static readonly IReadOnlyDictionary<string, object> _emptyParametersDictionary
[Parameter]
public RenderFragment NotFound { get; set; }

/// <summary>
/// Gets or sets the page content to display when no match is found for the requested route.
/// </summary>
[Parameter]
[DynamicallyAccessedMembers(LinkerFlags.Component)]
public Type NotFoundPage { get; set; } = default!;

/// <summary>
/// Gets or sets the content to display when a match is found for the requested route.
/// </summary>
Expand Down Expand Up @@ -132,6 +141,22 @@ public async Task SetParametersAsync(ParameterView parameters)
throw new InvalidOperationException($"The {nameof(Router)} component requires a value for the parameter {nameof(Found)}.");
}

if (NotFoundPage != null)
{
if (!typeof(IComponent).IsAssignableFrom(NotFoundPage))
{
throw new InvalidOperationException($"The type {NotFoundPage.FullName} " +
$"does not implement {typeof(IComponent).FullName}.");
}

var routeAttributes = NotFoundPage.GetCustomAttributes(typeof(RouteAttribute), inherit: true);
if (routeAttributes.Length == 0)
{
throw new InvalidOperationException($"The type {NotFoundPage.FullName} " +
$"does not have a {typeof(RouteAttribute).FullName} applied to it.");
}
}

if (!_onNavigateCalled)
{
_onNavigateCalled = true;
Expand Down Expand Up @@ -327,7 +352,22 @@ private void OnNotFound(object sender, EventArgs args)
if (_renderHandle.IsInitialized)
{
Log.DisplayingNotFound(_logger);
_renderHandle.Render(NotFound ?? DefaultNotFoundContent);
_renderHandle.Render(builder =>
{
if (NotFoundPage != null)
{
builder.OpenComponent(0, NotFoundPage);
builder.CloseComponent();
}
else if (NotFound != null)
{
NotFound(builder);
}
else
{
DefaultNotFoundContent(builder);
}
});
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
<Reference Include="Microsoft.AspNetCore.Antiforgery" />
<Reference Include="Microsoft.AspNetCore.Components.Authorization" />
<Reference Include="Microsoft.AspNetCore.Components.Web" />
<Reference Include="Microsoft.AspNetCore.Diagnostics" />
<Reference Include="Microsoft.AspNetCore.Diagnostics.Abstractions" />
<Reference Include="Microsoft.AspNetCore.DataProtection.Extensions" />
<Reference Include="Microsoft.AspNetCore.Hosting.Abstractions" />
Expand Down
28 changes: 22 additions & 6 deletions src/Components/Endpoints/src/RazorComponentEndpointInvoker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,18 @@ private async Task RenderComponentCore(HttpContext context)
{
context.Response.ContentType = RazorComponentResultExecutor.DefaultContentType;
var isErrorHandler = context.Features.Get<IExceptionHandlerFeature>() is not null;
var hasStatusCodePage = context.Features.Get<IStatusCodePagesFeature>() is not null;
Copy link
Member

Choose a reason for hiding this comment

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

I think the interface we are looking for is IStatusCodeReExecuteFeature not IStatusCodePagesFeature. The latter is added all the time to the middleware, while the first one is only added by the handler when it's dealing with the re-execution code path, which I think is what we want (and is the equivalent of isErrorHandler)

Copy link
Member Author

Choose a reason for hiding this comment

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

This complicates the logic a bit. We need a way to avoid writing the response (so return from RenderComponentCore method before flushing) if we expect re-execution. By "expect" re-executon I mean: status code page was set, we used to have a 200 but now we have 404. At the place where we would like to detect it, IStatusCodeReExecuteFeature is unset and we have no way of preventing starting the response.

We could set IStatusCodeReExecuteFeature for in the handler subscribed to OnNotFound event: in SetNotFoundResponseAsync. This forces us to add <Reference Include="Microsoft.AspNetCore.Diagnostics" /> to Microsoft.AspNetCore.Components.Endpoints.csproj. Is this acceptable?

Otherwise, we will return too early to handle re-execution, again this condition:

Copy link
Member Author

Choose a reason for hiding this comment

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

Relying on IStatusCodeReExecuteFeature fails - the result is that we always flush the response and we never have a chance to re-execute.
The problem here is that we would like to detect the page that is going to re-execute, to prevent writing to its contents, not a page that already was re-executed.

Copy link
Member Author

Choose a reason for hiding this comment

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

In error handler we don't have such a problem (we don't have to detect to return early) because the thrown exception does it for us. We never execute any code after RenderEndpointComponent in EH case. For 404 we have to return early based on a condition.
The most reasonable is probably IStatusCodePagesFeature->set & IStatusCodeReExecuteFeature->unset & responseCode->404.

Copy link
Member

Choose a reason for hiding this comment

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

I don't fully follow, but I don't think we need to block on this

var isReExecuted = context.Features.Get<IStatusCodeReExecuteFeature>() is not null;
if (isErrorHandler)
{
Log.InteractivityDisabledForErrorHandling(_logger);
}
_renderer.InitializeStreamingRenderingFraming(context, isErrorHandler);
EndpointHtmlRenderer.MarkAsAllowingEnhancedNavigation(context);
_renderer.InitializeStreamingRenderingFraming(context, isErrorHandler, isReExecuted);
if (!isReExecuted)
{
// re-executed pages have Headers already set up
EndpointHtmlRenderer.MarkAsAllowingEnhancedNavigation(context);
}

var endpoint = context.GetEndpoint() ?? throw new InvalidOperationException($"An endpoint must be set on the '{nameof(HttpContext)}'.");

Expand Down Expand Up @@ -85,14 +91,23 @@ await _renderer.InitializeStandardComponentServicesAsync(
await using var writer = new HttpResponseStreamWriter(context.Response.Body, Encoding.UTF8, defaultBufferSize, ArrayPool<byte>.Shared, ArrayPool<char>.Shared);
using var bufferWriter = new BufferedTextWriter(writer);

bool isErrorHandlerOrReExecuted = isErrorHandler || isReExecuted;

// Note that we always use Static rendering mode for the top-level output from a RazorComponentResult,
// because you never want to serialize the invocation of RazorComponentResultHost. Instead, that host
// component takes care of switching into your desired render mode when it produces its own output.
var htmlContent = await _renderer.RenderEndpointComponent(
context,
rootComponent,
ParameterView.Empty,
waitForQuiescence: result.IsPost || isErrorHandler);
waitForQuiescence: result.IsPost || isErrorHandlerOrReExecuted);

bool avoidStartingResponse = hasStatusCodePage && !isReExecuted && context.Response.StatusCode == StatusCodes.Status404NotFound;
if (avoidStartingResponse)
{
// the request is going to be re-executed, we should avoid writing to the response
return;
}

Task quiesceTask;
if (!result.IsPost)
Expand Down Expand Up @@ -145,7 +160,7 @@ await _renderer.InitializeStandardComponentServicesAsync(
}

// Emit comment containing state.
if (!isErrorHandler)
if (!isErrorHandlerOrReExecuted)
{
var componentStateHtmlContent = await _renderer.PrerenderPersistedStateAsync(context);
componentStateHtmlContent.WriteTo(bufferWriter, HtmlEncoder.Default);
Expand All @@ -160,10 +175,11 @@ await _renderer.InitializeStandardComponentServicesAsync(
private async Task<RequestValidationState> ValidateRequestAsync(HttpContext context, IAntiforgery? antiforgery)
{
var processPost = HttpMethods.IsPost(context.Request.Method) &&
// Disable POST functionality during exception handling.
// Disable POST functionality during exception handling and reexecution.
// The exception handler middleware will not update the request method, and we don't
// want to run the form handling logic against the error page.
context.Features.Get<IExceptionHandlerFeature>() == null;
context.Features.Get<IExceptionHandlerFeature>() == null &&
context.Features.Get<IStatusCodePagesFeature>() == null;

if (processPost)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,27 @@ private Task ReturnErrorResponse(string detailedMessage)
: Task.CompletedTask;
}

private void SetNotFoundResponse(object? sender, EventArgs args)
private async Task SetNotFoundResponseAsync(string baseUri)
{
if (_httpContext.Response.HasStarted)
{
throw new InvalidOperationException("Cannot set a NotFound response after the response has already started.");
var defaultBufferSize = 16 * 1024;
await using var writer = new HttpResponseStreamWriter(_httpContext.Response.Body, Encoding.UTF8, defaultBufferSize, ArrayPool<byte>.Shared, ArrayPool<char>.Shared);
using var bufferWriter = new BufferedTextWriter(writer);
var notFoundUri = $"{baseUri}not-found";
HandleNavigationAfterResponseStarted(bufferWriter, _httpContext, notFoundUri);
await bufferWriter.FlushAsync();
}
_httpContext.Response.StatusCode = StatusCodes.Status404NotFound;
SignalRendererToFinishRendering();
else
{
_httpContext.Response.StatusCode = StatusCodes.Status404NotFound;
_httpContext.Response.ContentType = null;
}

// When the application triggers a NotFound event, we continue rendering the current batch.
// However, after completing this batch, we do not want to process any further UI updates,
// as we are going to return a 404 status and discard the UI updates generated so far.
SignalRendererToFinishRenderingAfterCurrentBatch();
}

private async Task OnNavigateTo(string uri)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.AspNetCore.Components.Web.HtmlRendering;
using Microsoft.AspNetCore.Html;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

namespace Microsoft.AspNetCore.Components.Endpoints;
Expand All @@ -18,7 +19,7 @@ internal partial class EndpointHtmlRenderer

protected override IComponent ResolveComponentForRenderMode([DynamicallyAccessedMembers(Component)] Type componentType, int? parentComponentId, IComponentActivator componentActivator, IComponentRenderMode renderMode)
{
if (_isHandlingErrors)
if (_isHandlingErrors || _isReExecuted)
{
// Ignore the render mode boundary in error scenarios.
return componentActivator.CreateInstance(componentType);
Expand Down Expand Up @@ -166,7 +167,50 @@ private async Task WaitForResultReady(bool waitForQuiescence, PrerenderedCompone
}
else if (_nonStreamingPendingTasks.Count > 0)
{
await WaitForNonStreamingPendingTasks();
if (_isReExecuted)
{
HandleNonStreamingTasks();
}
else
{
await WaitForNonStreamingPendingTasks();
}
}
}

public void HandleNonStreamingTasks()
{
if (NonStreamingPendingTasksCompletion == null)
{
foreach (var task in _nonStreamingPendingTasks)
{
_ = GetErrorHandledTask(task);
}

// Clear the pending tasks since we are handling them
_nonStreamingPendingTasks.Clear();

NonStreamingPendingTasksCompletion = Task.CompletedTask;
}
}

private async Task GetErrorHandledTask(Task taskToHandle)
{
try
{
await taskToHandle;
}
catch (Exception ex)
{
// Ignore errors due to task cancellations.
if (!taskToHandle.IsCanceled)
{
_logger.LogError(
ex,
"An exception occurred during non-streaming rendering. " +
"This exception will be ignored because the response " +
"is being discarded and the request is being re-executed.");
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ internal partial class EndpointHtmlRenderer
private HashSet<int>? _visitedComponentIdsInCurrentStreamingBatch;
private string? _ssrFramingCommentMarkup;
private bool _isHandlingErrors;
private bool _isReExecuted;

public void InitializeStreamingRenderingFraming(HttpContext httpContext, bool isErrorHandler)
public void InitializeStreamingRenderingFraming(HttpContext httpContext, bool isErrorHandler, bool isReExecuted)
{
_isHandlingErrors = isErrorHandler;
if (IsProgressivelyEnhancedNavigation(httpContext.Request))
_isReExecuted = isReExecuted;
if (!isReExecuted && IsProgressivelyEnhancedNavigation(httpContext.Request))
{
var id = Guid.NewGuid().ToString();
httpContext.Response.Headers.Add(_streamingRenderingFramingHeaderName, id);
Expand Down
27 changes: 25 additions & 2 deletions src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ internal partial class EndpointHtmlRenderer : StaticHtmlRenderer, IComponentPrer
private HttpContext _httpContext = default!; // Always set at the start of an inbound call
private ResourceAssetCollection? _resourceCollection;
private bool _rendererIsStopped;
private readonly ILogger _logger;

// The underlying Renderer always tracks the pending tasks representing *full* quiescence, i.e.,
// when everything (regardless of streaming SSR) is fully complete. In this subclass we also track
Expand All @@ -56,6 +57,7 @@ public EndpointHtmlRenderer(IServiceProvider serviceProvider, ILoggerFactory log
{
_services = serviceProvider;
_options = serviceProvider.GetRequiredService<IOptions<RazorComponentsServiceOptions>>().Value;
_logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Components.RenderTree.Renderer");
}

internal HttpContext? HttpContext => _httpContext;
Expand Down Expand Up @@ -83,7 +85,7 @@ internal async Task InitializeStandardComponentServicesAsync(

if (navigationManager != null)
{
navigationManager.OnNotFound += SetNotFoundResponse;
navigationManager.OnNotFound += async (sender, args) => await SetNotFoundResponseAsync(navigationManager.BaseUri);
}

var authenticationStateProvider = httpContext.RequestServices.GetService<AuthenticationStateProvider>();
Expand Down Expand Up @@ -163,6 +165,11 @@ protected override ComponentState CreateComponentState(int componentId, ICompone

protected override void AddPendingTask(ComponentState? componentState, Task task)
{
if (_isReExecuted)
{
return;
}

var streamRendering = componentState is null
? false
: ((EndpointComponentState)componentState).StreamRendering;
Expand All @@ -176,12 +183,28 @@ protected override void AddPendingTask(ComponentState? componentState, Task task
base.AddPendingTask(componentState, task);
}

protected override void SignalRendererToFinishRendering()
private void SignalRendererToFinishRenderingAfterCurrentBatch()
{
// sets a deferred stop on the renderer, which will have an effect after the current batch is completed
_rendererIsStopped = true;
}

protected override void SignalRendererToFinishRendering()
{
SignalRendererToFinishRenderingAfterCurrentBatch();
// sets a hard stop on the renderer, which will have an effect immediately
base.SignalRendererToFinishRendering();
}

protected override void ProcessPendingRender()
{
if (_rendererIsStopped)
{
return;
}
base.ProcessPendingRender();
}

// For tests only
internal Task? NonStreamingPendingTasksCompletion;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ private static Task RenderComponentToResponse(
return endpointHtmlRenderer.Dispatcher.InvokeAsync(async () =>
{
var isErrorHandler = httpContext.Features.Get<IExceptionHandlerFeature>() is not null;
endpointHtmlRenderer.InitializeStreamingRenderingFraming(httpContext, isErrorHandler);
var isReExecuted = httpContext.Features.Get<IStatusCodeReExecuteFeature>() is not null;
endpointHtmlRenderer.InitializeStreamingRenderingFraming(httpContext, isErrorHandler, isReExecuted);
EndpointHtmlRenderer.MarkAsAllowingEnhancedNavigation(httpContext);

// We could pool these dictionary instances if we wanted, and possibly even the ParameterView
Expand Down
Loading
Loading