Skip to content

HttpNavigationManager no longer uses NavigationException #61306

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 23 commits into from
Apr 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0be84e4
Rise even instead of throwing.
ilonatommy Apr 3, 2025
2e5f7dc
Clean up, delay not needed.
ilonatommy Apr 3, 2025
5ebaaac
Fix typo + test old way of workign as well.
ilonatommy Apr 3, 2025
1ea43be
Update name to be full namespace + remove readonly.
ilonatommy Apr 4, 2025
e9bd57e
Feedback - interactive SSR updates required exposing some methods.
ilonatommy Apr 7, 2025
ebc2537
Fix missing xml.
ilonatommy Apr 7, 2025
dff3c70
Fix build of tests.
ilonatommy Apr 7, 2025
19d3fe9
Fix nullable.
ilonatommy Apr 7, 2025
def3e5c
Feedback - limit public API changes.
ilonatommy Apr 8, 2025
9f79f51
Handle the case when response started.
ilonatommy Apr 8, 2025
e647f2a
Proposal of fixing external navigation.
ilonatommy Apr 9, 2025
5e6d104
Update src/Components/test/testassets/Components.TestServer/RazorComp…
ilonatommy Apr 9, 2025
8413a40
Merge branch 'main' into fix-59451
ilonatommy Apr 10, 2025
9f7fceb
Merge branch 'fix-59451' of https://github.com/ilonatommy/aspnetcore …
ilonatommy Apr 10, 2025
aea9826
Feedback.
ilonatommy Apr 11, 2025
bafa937
More effective stopping of the renderer.
ilonatommy Apr 14, 2025
3521c8f
POST cannot safely redirect like GET does, the body should be preserved.
ilonatommy Apr 14, 2025
a3ca937
Reuse the logic from navigation exception.
ilonatommy Apr 14, 2025
5d7eb68
Editing the ongoing render batch is not possible - for non-streaming …
ilonatommy Apr 14, 2025
69426fd
Missing change for the last commit.
ilonatommy Apr 15, 2025
e0b407c
Rename switch to match http and remote navigator.
ilonatommy Apr 15, 2025
dc45f3e
Adjust test for the new behavior.
ilonatommy Apr 15, 2025
b36db8b
Fix exception - driven navigation.
ilonatommy Apr 15, 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
2 changes: 2 additions & 0 deletions src/Components/Components/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#nullable enable
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
Microsoft.AspNetCore.Components.Routing.NotFoundEventArgs
Microsoft.AspNetCore.Components.Routing.NotFoundEventArgs.NotFoundEventArgs() -> void
Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.ComponentStatePersistenceManager(Microsoft.Extensions.Logging.ILogger<Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager!>! logger, System.IServiceProvider! serviceProvider) -> void
Expand All @@ -11,3 +12,4 @@ Microsoft.AspNetCore.Components.SupplyParameterFromPersistentComponentStateAttri
Microsoft.Extensions.DependencyInjection.SupplyParameterFromPersistentComponentStateProviderServiceCollectionExtensions
static Microsoft.AspNetCore.Components.Infrastructure.RegisterPersistentComponentStateServiceCollectionExtensions.AddPersistentServiceRegistration<TService>(Microsoft.Extensions.DependencyInjection.IServiceCollection! services, Microsoft.AspNetCore.Components.IComponentRenderMode! componentRenderMode) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
static Microsoft.Extensions.DependencyInjection.SupplyParameterFromPersistentComponentStateProviderServiceCollectionExtensions.AddSupplyValueFromPersistentComponentStateProvider(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.SignalRendererToFinishRendering() -> void
19 changes: 17 additions & 2 deletions src/Components/Components/src/RenderTree/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public abstract partial class Renderer : IDisposable, IAsyncDisposable
private bool _rendererIsDisposed;

private bool _hotReloadInitialized;
private bool _rendererIsStopped;

/// <summary>
/// Allows the caller to handle exceptions from the SynchronizationContext when one is available.
Expand Down Expand Up @@ -658,6 +659,12 @@ internal void AddToRenderQueue(int componentId, RenderFragment renderFragment)
{
Dispatcher.AssertAccess();

if (_rendererIsStopped)
{
// Once we're stopped, we'll disregard further attempts to queue anything
return;
}

Comment on lines +662 to +667
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 we should do this inside https://github.com/dotnet/aspnetcore/blob/main/src/Components/Components/src/RenderTree/Renderer.cs#L736 which can be overriden by EndpointHtmlRenderer and save ourselves having the public API SignalRendererToFinishRendering

Copy link
Member Author

Choose a reason for hiding this comment

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

if we do as you propose, the queue will keep being edited during the processing (line 676 below). That was why I added a new public API, to prevent new Enqueue() when we already sent the stop signal.

var componentState = GetOptionalComponentState(componentId);
if (componentState == null)
{
Expand Down Expand Up @@ -724,14 +731,22 @@ private ComponentState GetRequiredRootComponentState(int componentId)
return componentState;
}

/// <summary>
/// Stop adding render requests to the render queue.
/// </summary>
protected virtual void SignalRendererToFinishRendering()
{
_rendererIsStopped = true;
}

/// <summary>
/// Processes pending renders requests from components if there are any.
/// </summary>
protected virtual void ProcessPendingRender()
{
if (_rendererIsDisposed)
if (_rendererIsDisposed || _rendererIsStopped)
{
// Once we're disposed, we'll disregard further attempts to render anything
// Once we're disposed or stopped, we'll disregard further attempts to render anything
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,13 @@ public interface IHostEnvironmentNavigationManager
/// <param name="baseUri">The base URI.</param>
/// <param name="uri">The absolute URI.</param>
void Initialize(string baseUri, string uri);

/// <summary>
/// Initializes the <see cref="NavigationManager" />.
/// </summary>
/// <param name="baseUri">The base URI.</param>
/// <param name="uri">The absolute URI.</param>
/// <param name="onNavigateTo">A delegate that points to a method handling navigation events. </param>
void Initialize(string baseUri, string uri, Func<string, Task> onNavigateTo) =>
Initialize(baseUri, uri);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,40 @@ namespace Microsoft.AspNetCore.Components.Endpoints;

internal sealed class HttpNavigationManager : NavigationManager, IHostEnvironmentNavigationManager
{
private const string _enableThrowNavigationException = "Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException";

private static bool _throwNavigationException =>
AppContext.TryGetSwitch(_enableThrowNavigationException, out var switchValue) && switchValue;

private Func<string, Task>? _onNavigateTo;

void IHostEnvironmentNavigationManager.Initialize(string baseUri, string uri) => Initialize(baseUri, uri);

void IHostEnvironmentNavigationManager.Initialize(string baseUri, string uri, Func<string, Task> onNavigateTo)
{
_onNavigateTo = onNavigateTo;
Initialize(baseUri, uri);
}

protected override void NavigateToCore(string uri, NavigationOptions options)
{
var absoluteUriString = ToAbsoluteUri(uri).AbsoluteUri;
throw new NavigationException(absoluteUriString);
if (_throwNavigationException)
{
throw new NavigationException(absoluteUriString);
}
else
{
_ = PerformNavigationAsync();
}

async Task PerformNavigationAsync()
{
if (_onNavigateTo == null)
{
throw new InvalidOperationException($"'{GetType().Name}' method for endpoint-based navigation has not been initialized.");
}
await _onNavigateTo(absoluteUriString);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Components.Endpoints.Rendering;
using Microsoft.AspNetCore.Components.Rendering;
using Microsoft.AspNetCore.Components.RenderTree;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.WebUtilities;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using System.Buffers;
using System.Globalization;
using System.Linq;
using System.Text;
Expand Down Expand Up @@ -84,6 +87,23 @@ private void SetNotFoundResponse(object? sender, EventArgs args)
SignalRendererToFinishRendering();
}

private async Task OnNavigateTo(string uri)
{
if (_httpContext.Response.HasStarted)
{
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);
HandleNavigationAfterResponseStarted(bufferWriter, _httpContext, uri);
await bufferWriter.FlushAsync();
}
else
{
await HandleNavigationBeforeResponseStarted(_httpContext, uri);
}
SignalRendererToFinishRendering();
}

private void UpdateNamedSubmitEvents(in RenderBatch renderBatch)
{
if (renderBatch.NamedEventChanges is { } changes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,20 +206,28 @@ public static ValueTask<PrerenderedComponentHtmlContent> HandleNavigationExcepti
"response and avoid using features like FlushAsync() before all components on the page have been rendered to prevent failed navigation commands.",
navigationException);
}
else if (IsPossibleExternalDestination(httpContext.Request, navigationException.Location)
else
{
return HandleNavigationBeforeResponseStarted(httpContext, navigationException.Location);
}
}

private static ValueTask<PrerenderedComponentHtmlContent> HandleNavigationBeforeResponseStarted(HttpContext httpContext, string destinationLocation)
{
if (IsPossibleExternalDestination(httpContext.Request, destinationLocation)
&& IsProgressivelyEnhancedNavigation(httpContext.Request))
{
// For progressively-enhanced nav, we prefer to use opaque redirections for external URLs rather than
// forcing the request to be retried, since that allows post-redirect-get to work, plus avoids a
// duplicated request. The client can't rely on receiving this header, though, since non-Blazor endpoints
// wouldn't return it.
httpContext.Response.Headers.Add("blazor-enhanced-nav-redirect-location",
OpaqueRedirection.CreateProtectedRedirectionUrl(httpContext, navigationException.Location));
OpaqueRedirection.CreateProtectedRedirectionUrl(httpContext, destinationLocation));
return new ValueTask<PrerenderedComponentHtmlContent>(PrerenderedComponentHtmlContent.Empty);
}
else
{
httpContext.Response.Redirect(navigationException.Location);
httpContext.Response.Redirect(destinationLocation);
return new ValueTask<PrerenderedComponentHtmlContent>(PrerenderedComponentHtmlContent.Empty);
}
}
Expand Down
17 changes: 3 additions & 14 deletions src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ internal async Task InitializeStandardComponentServicesAsync(
IFormCollection? form = null)
{
var navigationManager = httpContext.RequestServices.GetRequiredService<NavigationManager>();
((IHostEnvironmentNavigationManager)navigationManager)?.Initialize(GetContextBaseUri(httpContext.Request), GetFullUri(httpContext.Request));
((IHostEnvironmentNavigationManager)navigationManager)?.Initialize(GetContextBaseUri(httpContext.Request), GetFullUri(httpContext.Request), OnNavigateTo);

if (navigationManager != null)
{
Expand Down Expand Up @@ -176,21 +176,10 @@ protected override void AddPendingTask(ComponentState? componentState, Task task
base.AddPendingTask(componentState, task);
}

private void SignalRendererToFinishRendering()
protected override void SignalRendererToFinishRendering()
{
_rendererIsStopped = true;
}

protected override void ProcessPendingRender()
{
if (_rendererIsStopped)
{
// 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.
return;
}
base.ProcessPendingRender();
base.SignalRendererToFinishRendering();
}

// For tests only
Expand Down
55 changes: 41 additions & 14 deletions src/Components/Endpoints/test/EndpointHtmlRendererTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -828,35 +828,62 @@ public async Task Rendering_ComponentWithJsInteropThrows()
exception.Message);
}

[Fact]
public async Task UriHelperRedirect_ThrowsInvalidOperationException_WhenResponseHasAlreadyStarted()
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task UriHelperRedirect_ThrowsInvalidOperationException_WhenResponseHasAlreadyStarted(bool expectException)
{
AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException", isEnabled: expectException);
// Arrange
var ctx = new DefaultHttpContext();
ctx.Request.Scheme = "http";
ctx.Request.Host = new HostString("localhost");
ctx.Request.PathBase = "/base";
ctx.Request.Path = "/path";
ctx.Request.QueryString = new QueryString("?query=value");
ctx.Response.Body = new MemoryStream();
var responseMock = new Mock<IHttpResponseFeature>();
responseMock.Setup(r => r.HasStarted).Returns(true);
ctx.Features.Set(responseMock.Object);
var httpContext = GetHttpContext(ctx);
string redirectUri = "http://localhost/redirect";

// Act
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () => await renderer.PrerenderComponentAsync(
httpContext,
typeof(RedirectComponent),
null,
ParameterView.FromDictionary(new Dictionary<string, object>
{
{ "RedirectUri", "http://localhost/redirect" }
})));
if (expectException)
{
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () => await renderer.PrerenderComponentAsync(
httpContext,
typeof(RedirectComponent),
null,
ParameterView.FromDictionary(new Dictionary<string, object>
{
{ "RedirectUri", redirectUri }
})));

Assert.Equal("A navigation command was attempted during prerendering after the server already started sending the response. " +
"Navigation commands can not be issued during server-side prerendering after the response from the server has started. Applications must buffer the" +
"response and avoid using features like FlushAsync() before all components on the page have been rendered to prevent failed navigation commands.",
exception.Message);
Assert.Equal("A navigation command was attempted during prerendering after the server already started sending the response. " +
"Navigation commands can not be issued during server-side prerendering after the response from the server has started. Applications must buffer the" +
"response and avoid using features like FlushAsync() before all components on the page have been rendered to prevent failed navigation commands.",
exception.Message);
}
else
{
await renderer.PrerenderComponentAsync(
httpContext,
typeof(RedirectComponent),
null,
ParameterView.FromDictionary(new Dictionary<string, object>
{
{ "RedirectUri", redirectUri }
}));
// read the custom element from the response body
httpContext.Response.Body.Position = 0;
var reader = new StreamReader(httpContext.Response.Body);
var output = await reader.ReadToEndAsync();

// Assert that the output contains expected navigation instructions.
var pattern = "^<blazor-ssr><template type=\"redirection\".*>.*<\\/template><blazor-ssr-end><\\/blazor-ssr-end><\\/blazor-ssr>$";
Assert.Matches(pattern, output);
}
}

[Fact]
Expand Down
39 changes: 37 additions & 2 deletions src/Components/Server/src/Circuits/RemoteNavigationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ internal sealed partial class RemoteNavigationManager : NavigationManager, IHost
private readonly ILogger<RemoteNavigationManager> _logger;
private IJSRuntime _jsRuntime;
private bool? _navigationLockStateBeforeJsRuntimeAttached;
private const string _enableThrowNavigationException = "Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException";
private static bool _throwNavigationException =>
AppContext.TryGetSwitch(_enableThrowNavigationException, out var switchValue) && switchValue;
private Func<string, Task>? _onNavigateTo;

public event EventHandler<Exception>? UnhandledException;

Expand Down Expand Up @@ -45,6 +49,19 @@ public RemoteNavigationManager(ILogger<RemoteNavigationManager> logger)
NotifyLocationChanged(isInterceptedLink: false);
}

/// <summary>
/// Initializes the <see cref="NavigationManager" />.
/// </summary>
/// <param name="baseUri">The base URI.</param>
/// <param name="uri">The absolute URI.</param>
/// <param name="onNavigateTo">A delegate that points to a method handling navigation events. </param>
public void Initialize(string baseUri, string uri, Func<string, Task> onNavigateTo)
{
_onNavigateTo += onNavigateTo;
base.Initialize(baseUri, uri);
NotifyLocationChanged(isInterceptedLink: false);
}

/// <summary>
/// Initializes the <see cref="RemoteNavigationManager"/>.
/// </summary>
Expand Down Expand Up @@ -88,7 +105,16 @@ protected override void NavigateToCore(string uri, NavigationOptions options)
if (_jsRuntime == null)
{
var absoluteUriString = ToAbsoluteUri(uri).AbsoluteUri;
throw new NavigationException(absoluteUriString);
if (_throwNavigationException)
{
throw new NavigationException(absoluteUriString);
}
if (_onNavigateTo == null)
{
throw new InvalidOperationException($"'{GetType().Name}' method for endpoint-based navigation has not been initialized.");
}
_ = _onNavigateTo(absoluteUriString);
return;
}

_ = PerformNavigationAsync();
Expand Down Expand Up @@ -129,7 +155,16 @@ public override void Refresh(bool forceReload = false)
if (_jsRuntime == null)
{
var absoluteUriString = ToAbsoluteUri(Uri).AbsoluteUri;
throw new NavigationException(absoluteUriString);
if (_throwNavigationException)
{
throw new NavigationException(absoluteUriString);
}
if (_onNavigateTo == null)
{
throw new InvalidOperationException($"'{GetType().Name}' method for endpoint-based navigation has not been initialized.");
}
_ = _onNavigateTo(absoluteUriString);
return;
}

_ = RefreshAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1399,4 +1399,16 @@ public void CanPersistMultiplePrerenderedStateDeclaratively_Auto_PersistsOnWebAs
Browser.Equal("restored 2", () => Browser.FindElement(By.Id("auto-2")).Text);
Browser.Equal("WebAssembly", () => Browser.FindElement(By.Id("render-mode-auto-2")).Text);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void NavigatesWithInteractivityByRequestRedirection(bool controlFlowByException)
{
AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException", isEnabled: controlFlowByException);
Navigate($"{ServerPathBase}/routing/ssr-navigate-to");
Browser.Equal("Click submit to navigate to home", () => Browser.Exists(By.Id("test-info")).Text);
Browser.Click(By.Id("redirectButton"));
Browser.Equal("Routing test cases", () => Browser.Exists(By.Id("test-info")).Text);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,18 @@ public void CanUseServerAuthenticationStateByDefault()
Browser.Equal("True", () => Browser.FindElement(By.Id("is-in-test-role-1")).Text);
Browser.Equal("True", () => Browser.FindElement(By.Id("is-in-test-role-2")).Text);
}

[Theory]
[InlineData(true, true)]
[InlineData(true, false)]
[InlineData(false, true)]
public void NavigatesWithoutInteractivityByRequestRedirection(bool controlFlowByException, bool isStreaming)
{
AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException", isEnabled: controlFlowByException);
string streaming = isStreaming ? $"streaming-" : "";
Navigate($"{ServerPathBase}/routing/ssr-{streaming}navigate-to");
Browser.Equal("Click submit to navigate to home", () => Browser.Exists(By.Id("test-info")).Text);
Browser.Click(By.Id("redirectButton"));
Browser.Equal("Routing test cases", () => Browser.Exists(By.Id("test-info")).Text);
}
}
Loading
Loading