Browse Source

Fix for Unobserved `NavigationException` in Blazor SSR (#62554)

* Add test that reproduces the issue.

* Make sure we always run this test with navigation exception flow.

* @campersau's feedback: unify the catching approach.
Ilona Tomkowicz 8 months ago
parent
commit
c3fa2bb749

+ 10 - 3
src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.Prerendering.cs

@@ -229,9 +229,16 @@ internal partial class EndpointHtmlRenderer
                 // Clear all pending work.
                 _nonStreamingPendingTasks.Clear();
 
-                // new work might be added before we check again as a result of waiting for all
-                // the child components to finish executing SetParametersAsync
-                await pendingWork;
+                try
+                {
+                    // new work might be added before we check again as a result of waiting for all
+                    // the child components to finish executing SetParametersAsync
+                    await pendingWork;
+                }
+                catch (NavigationException navigationException)
+                {
+                    await HandleNavigationException(_httpContext, navigationException);
+                }
             }
         }
     }

+ 12 - 0
src/Components/test/E2ETest/ServerRenderingTests/RedirectionTest.cs

@@ -221,6 +221,18 @@ public class RedirectionTest : ServerTestBase<BasicTestAppServerSiteFixture<Razo
         Assert.EndsWith("/subdir/redirect", Browser.Url);
     }
 
+    [Fact]
+    public void NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException()
+    {
+        AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.DisableThrowNavigationException", false);
+
+        // Navigate to the page that triggers the circular redirect.
+        Navigate($"{ServerPathBase}/redirect/circular");
+
+        // The component will stop redirecting after 3 attempts and render the exception count.
+        Browser.Equal("0", () => Browser.FindElement(By.Id("unobserved-exceptions-count")).Text);
+    }
+
     private void AssertElementRemoved(IWebElement element)
     {
         Browser.True(() =>

+ 11 - 2
src/Components/test/testassets/Components.TestServer/Program.cs

@@ -81,8 +81,16 @@ public class Program
 
     public static IHost BuildWebHost(string[] args) => BuildWebHost<Startup>(args);
 
-    public static IHost BuildWebHost<TStartup>(string[] args) where TStartup : class =>
-        Host.CreateDefaultBuilder(args)
+    public static IHost BuildWebHost<TStartup>(string[] args) where TStartup : class
+    {
+        var unobservedTaskExceptionObserver = new UnobservedTaskExceptionObserver();
+        TaskScheduler.UnobservedTaskException += unobservedTaskExceptionObserver.OnUnobservedTaskException;
+
+        return Host.CreateDefaultBuilder(args)
+            .ConfigureServices(services =>
+            {
+                services.AddSingleton(unobservedTaskExceptionObserver);
+            })
             .ConfigureLogging((ctx, lb) =>
             {
                 TestSink sink = new TestSink();
@@ -98,6 +106,7 @@ public class Program
                 webHostBuilder.UseStaticWebAssets();
             })
             .Build();
+    }
 
     private static int GetNextChildAppPortNumber()
     {

+ 53 - 0
src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Redirections/CircularRedirection.razor

@@ -0,0 +1,53 @@
+@page "/redirect/circular"
+@using System.Collections.Concurrent
+@inject NavigationManager Nav
+@inject UnobservedTaskExceptionObserver Observer
+
+<h1>Hello, world!</h1>
+
+@if (_shouldStopRedirecting)
+{
+    <p id="unobserved-exceptions-count">@_unobservedExceptions.Count</p>
+
+    @if (_unobservedExceptions.Any())
+    {
+        <h2>Unobserved Exceptions (for debugging):</h2>
+        <ul>
+            @foreach (var ex in _unobservedExceptions)
+            {
+                <li>@ex.ToString()</li>
+            }
+        </ul>
+    }
+}
+
+@code {
+    private bool _shouldStopRedirecting;
+    private IReadOnlyCollection<Exception> _unobservedExceptions = Array.Empty<Exception>();
+
+    protected override async Task OnInitializedAsync()
+    {
+		int visits = Observer.GetCircularRedirectCount();
+		if (visits == 0)
+		{
+			// make sure we start with clean logs
+			Observer.Clear();
+		}
+
+        // Force GC collection to trigger finalizers - this is what causes the issue
+        GC.Collect();
+        GC.WaitForPendingFinalizers();
+        GC.Collect();
+        await Task.Yield();
+
+        if (Observer.GetAndIncrementCircularRedirectCount() < 3)
+        {
+            Nav.NavigateTo("redirect/circular");
+        }
+        else
+        {
+            _shouldStopRedirecting = true;
+            _unobservedExceptions = Observer.GetExceptions();
+        }
+    }
+}

+ 39 - 0
src/Components/test/testassets/Components.TestServer/UnobservedTaskExceptionObserver.cs

@@ -0,0 +1,39 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Concurrent;
+using System.Threading;
+
+namespace TestServer;
+
+public class UnobservedTaskExceptionObserver
+{
+    private readonly ConcurrentQueue<Exception> _exceptions = new();
+    private int _circularRedirectCount;
+
+    public void OnUnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e)
+    {
+        _exceptions.Enqueue(e.Exception);
+        e.SetObserved(); // Mark as observed to prevent the process from crashing during tests
+    }
+
+    public bool HasExceptions => !_exceptions.IsEmpty;
+
+    public IReadOnlyCollection<Exception> GetExceptions() => _exceptions.ToArray();
+
+    public void Clear()
+    {
+        _exceptions.Clear();
+        _circularRedirectCount = 0;
+    }
+
+    public int GetCircularRedirectCount()
+    {
+        return _circularRedirectCount;
+    }
+
+    public int GetAndIncrementCircularRedirectCount()
+    {
+        return Interlocked.Increment(ref _circularRedirectCount) - 1;
+    }
+}