From 129d5aa811a6ed409d6ea3044075fa514b850339 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Jun 2025 10:06:10 +0000 Subject: [PATCH 1/8] Initial plan for issue From 5129adc45a53128f69202dba16027914110a727b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Jun 2025 10:18:00 +0000 Subject: [PATCH 2/8] Wrap OnNavigateTo callback with GetErrorHandledTask for proper exception logging Co-authored-by: ilonatommy <32700855+ilonatommy@users.noreply.github.com> --- .../Endpoints/src/Rendering/EndpointHtmlRenderer.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs b/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs index f5d0699e1efe..3617c84b3956 100644 --- a/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs +++ b/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs @@ -83,7 +83,11 @@ internal async Task InitializeStandardComponentServicesAsync( IFormCollection? form = null) { var navigationManager = httpContext.RequestServices.GetRequiredService(); - ((IHostEnvironmentNavigationManager)navigationManager)?.Initialize(GetContextBaseUri(httpContext.Request), GetFullUri(httpContext.Request), OnNavigateTo); + ((IHostEnvironmentNavigationManager)navigationManager)?.Initialize(GetContextBaseUri(httpContext.Request), GetFullUri(httpContext.Request), uri => + { + _ = GetErrorHandledTask(OnNavigateTo(uri)); + return Task.CompletedTask; + }); navigationManager?.OnNotFound += (sender, args) => { From 949150a9fd409c754b39f18d9e820dfda7afe747 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Jun 2025 10:39:58 +0000 Subject: [PATCH 3/8] Add test for OnNavigateTo callback exception handling in NavigationManagerTest.cs Co-authored-by: ilonatommy <32700855+ilonatommy@users.noreply.github.com> --- .../Components/test/NavigationManagerTest.cs | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/Components/Components/test/NavigationManagerTest.cs b/src/Components/Components/test/NavigationManagerTest.cs index 3f6e680d0c8b..73ba7405f792 100644 --- a/src/Components/Components/test/NavigationManagerTest.cs +++ b/src/Components/Components/test/NavigationManagerTest.cs @@ -886,6 +886,38 @@ public void OnNotFoundSubscriptionIsTriggeredWhenNotFoundCalled() // Assert Assert.True(notFoundTriggered, "The OnNotFound event was not triggered as expected."); } + + [Fact] + public void OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() + { + // Arrange + var baseUri = "scheme://host/"; + var uri = "scheme://host/test"; + var testNavManager = new TestNavigationManagerWithCallback(); + var exceptionThrown = false; + var expectedException = new InvalidOperationException("Test exception from OnNavigateTo"); + + // Configure the onNavigateTo callback to throw an exception + testNavManager.ConfigureOnNavigateToCallback(throwingUri => + { + exceptionThrown = true; + throw expectedException; + }); + + // Act + // Initialize the navigation manager with the callback + testNavManager.Initialize(baseUri, uri, testNavManager.GetOnNavigateToCallback()); + + // Assert + Assert.True(testNavManager.IsInitialized); + + // When navigation is triggered, the exception should be handled gracefully + var thrownException = testNavManager.TriggerOnNavigateToCallback(uri); + + // Assert + Assert.True(exceptionThrown, "The OnNavigateTo callback should have been called and thrown an exception."); + Assert.Same(expectedException, thrownException); + } private class TestNavigationManager : NavigationManager { @@ -932,4 +964,64 @@ protected override void HandleLocationChangingHandlerException(Exception ex, Loc _exceptionsThrownFromLocationChangingHandlers.Add(ex); } } + + private class TestNavigationManagerWithCallback : TestNavigationManager, IHostEnvironmentNavigationManager + { + private Func _onNavigateToCallback; + + public TestNavigationManagerWithCallback() + { + } + + public void Initialize(string baseUri, string uri, Func onNavigateTo) + { + _onNavigateToCallback = onNavigateTo; + base.Initialize(baseUri, uri); + } + + public void ConfigureOnNavigateToCallback(Func callback) + { + _onNavigateToCallback = callback; + } + + public Func GetOnNavigateToCallback() + { + return _onNavigateToCallback; + } + + public Exception TriggerOnNavigateToCallback(string uri) + { + if (_onNavigateToCallback == null) + { + return null; + } + + try + { + // Simulate the fire-and-forget pattern used in RemoteNavigationManager + _ = _onNavigateToCallback(uri); + return null; + } + catch (Exception ex) + { + return ex; + } + } + + public bool IsInitialized + { + get + { + try + { + _ = BaseUri; // This will throw if not initialized + return true; + } + catch (InvalidOperationException) + { + return false; + } + } + } + } } From 80512e6b6d3692a9c285dab94cfbfda032ca6494 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Jun 2025 10:49:47 +0000 Subject: [PATCH 4/8] Use correct Initialize method signature for OnNavigateTo callback Co-authored-by: ilonatommy <32700855+ilonatommy@users.noreply.github.com> --- .../Endpoints/src/Rendering/EndpointHtmlRenderer.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs b/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs index 3617c84b3956..3fb8c965ad27 100644 --- a/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs +++ b/src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs @@ -83,11 +83,10 @@ internal async Task InitializeStandardComponentServicesAsync( IFormCollection? form = null) { var navigationManager = httpContext.RequestServices.GetRequiredService(); - ((IHostEnvironmentNavigationManager)navigationManager)?.Initialize(GetContextBaseUri(httpContext.Request), GetFullUri(httpContext.Request), uri => - { - _ = GetErrorHandledTask(OnNavigateTo(uri)); - return Task.CompletedTask; - }); + ((IHostEnvironmentNavigationManager)navigationManager)?.Initialize( + GetContextBaseUri(httpContext.Request), + GetFullUri(httpContext.Request), + uri => GetErrorHandledTask(OnNavigateTo(uri))); navigationManager?.OnNotFound += (sender, args) => { From ae9db6909d98c9b0dff2bd8157cfee9f9f76045b Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Wed, 2 Jul 2025 12:28:28 +0200 Subject: [PATCH 5/8] Finish the test. --- .../Components/test/NavigationManagerTest.cs | 93 +++++++++---------- 1 file changed, 43 insertions(+), 50 deletions(-) diff --git a/src/Components/Components/test/NavigationManagerTest.cs b/src/Components/Components/test/NavigationManagerTest.cs index 73ba7405f792..fe8e23c161fe 100644 --- a/src/Components/Components/test/NavigationManagerTest.cs +++ b/src/Components/Components/test/NavigationManagerTest.cs @@ -8,6 +8,8 @@ using Microsoft.AspNetCore.Components.Routing; using Microsoft.AspNetCore.InternalTesting; +#nullable enable + namespace Microsoft.AspNetCore.Components; public class NavigationManagerTest @@ -893,32 +895,30 @@ public void OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() // Arrange var baseUri = "scheme://host/"; var uri = "scheme://host/test"; - var testNavManager = new TestNavigationManagerWithCallback(); - var exceptionThrown = false; + var testNavManager = new TestNavigationManagerWithExceptionHandling(baseUri); var expectedException = new InvalidOperationException("Test exception from OnNavigateTo"); - // Configure the onNavigateTo callback to throw an exception - testNavManager.ConfigureOnNavigateToCallback(throwingUri => - { - exceptionThrown = true; - throw expectedException; - }); + // First test: Initialize with a callback that throws exceptions + testNavManager.Initialize(baseUri, uri, uri => testNavManager.GetErrorHandledTask(ThrowingMethod(uri))); - // Act - // Initialize the navigation manager with the callback - testNavManager.Initialize(baseUri, uri, testNavManager.GetOnNavigateToCallback()); + // Act & Assert + // Verify that the wrapped callback handles the exception gracefully + var wrappedException = testNavManager.TriggerOnNavigateToCallback(uri); - // Assert - Assert.True(testNavManager.IsInitialized); + // Should be null because the exception was handled gracefully + Assert.Null(wrappedException); - // When navigation is triggered, the exception should be handled gracefully - var thrownException = testNavManager.TriggerOnNavigateToCallback(uri); + // Verify that the exception was logged + Assert.Single(testNavManager.HandledExceptions); + Assert.Same(expectedException, testNavManager.HandledExceptions[0]); - // Assert - Assert.True(exceptionThrown, "The OnNavigateTo callback should have been called and thrown an exception."); - Assert.Same(expectedException, thrownException); + async Task ThrowingMethod(string param) + { + await Task.Yield(); + throw expectedException; + } } - + private class TestNavigationManager : NavigationManager { public TestNavigationManager() @@ -965,40 +965,27 @@ protected override void HandleLocationChangingHandlerException(Exception ex, Loc } } - private class TestNavigationManagerWithCallback : TestNavigationManager, IHostEnvironmentNavigationManager + private class TestNavigationManagerWithExceptionHandling : TestNavigationManager, IHostEnvironmentNavigationManager { private Func _onNavigateToCallback; - public TestNavigationManagerWithCallback() + public List HandledExceptions { get; } = new(); + + public TestNavigationManagerWithExceptionHandling(string baseUri = null, string uri = null) + : base(baseUri, uri) { } public void Initialize(string baseUri, string uri, Func onNavigateTo) { _onNavigateToCallback = onNavigateTo; - base.Initialize(baseUri, uri); - } - - public void ConfigureOnNavigateToCallback(Func callback) - { - _onNavigateToCallback = callback; - } - - public Func GetOnNavigateToCallback() - { - return _onNavigateToCallback; } - public Exception TriggerOnNavigateToCallback(string uri) + public Exception? TriggerOnNavigateToCallback(string uri) { - if (_onNavigateToCallback == null) - { - return null; - } - try { - // Simulate the fire-and-forget pattern used in RemoteNavigationManager + // Simulate the fire-and-forget pattern of RemoteNavigationManager _ = _onNavigateToCallback(uri); return null; } @@ -1008,19 +995,25 @@ public Exception TriggerOnNavigateToCallback(string uri) } } - public bool IsInitialized + protected override void NavigateToCore(string uri, bool forceLoad) { - get + // Simulate the behavior where NavigateToCore calls the onNavigateTo callback + // in a fire-and-forget manner when JSRuntime is not available + if (_onNavigateToCallback is not null) { - try - { - _ = BaseUri; // This will throw if not initialized - return true; - } - catch (InvalidOperationException) - { - return false; - } + _ = _onNavigateToCallback(uri); + } + } + + public async Task GetErrorHandledTask(Task taskToHandle) + { + try + { + await taskToHandle; + } + catch (Exception ex) + { + HandledExceptions.Add(ex); } } } From 3983ad0bfcd8f8ebde28622b63e7cc5c2adc1043 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Wed, 2 Jul 2025 14:13:07 +0200 Subject: [PATCH 6/8] Fix nullability. --- src/Components/Components/test/NavigationManagerTest.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Components/Components/test/NavigationManagerTest.cs b/src/Components/Components/test/NavigationManagerTest.cs index fe8e23c161fe..80c5741b2535 100644 --- a/src/Components/Components/test/NavigationManagerTest.cs +++ b/src/Components/Components/test/NavigationManagerTest.cs @@ -8,8 +8,6 @@ using Microsoft.AspNetCore.Components.Routing; using Microsoft.AspNetCore.InternalTesting; -#nullable enable - namespace Microsoft.AspNetCore.Components; public class NavigationManagerTest @@ -892,6 +890,7 @@ public void OnNotFoundSubscriptionIsTriggeredWhenNotFoundCalled() [Fact] public void OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() { +#nullable enable // Arrange var baseUri = "scheme://host/"; var uri = "scheme://host/test"; @@ -917,6 +916,7 @@ async Task ThrowingMethod(string param) await Task.Yield(); throw expectedException; } +#nullable restore } private class TestNavigationManager : NavigationManager @@ -981,7 +981,9 @@ public void Initialize(string baseUri, string uri, Func onNavigate _onNavigateToCallback = onNavigateTo; } +#nullable enable public Exception? TriggerOnNavigateToCallback(string uri) +#nullable restore { try { From 2ebe7c86d0ad1ba37b7d089573eaf736e6738d9f Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Wed, 2 Jul 2025 16:07:08 +0200 Subject: [PATCH 7/8] Make deterministic. --- .../Components/test/NavigationManagerTest.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Components/Components/test/NavigationManagerTest.cs b/src/Components/Components/test/NavigationManagerTest.cs index 80c5741b2535..988e42f6d9a7 100644 --- a/src/Components/Components/test/NavigationManagerTest.cs +++ b/src/Components/Components/test/NavigationManagerTest.cs @@ -888,7 +888,7 @@ public void OnNotFoundSubscriptionIsTriggeredWhenNotFoundCalled() } [Fact] - public void OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() + public async Task OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() { #nullable enable // Arrange @@ -896,6 +896,7 @@ public void OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() var uri = "scheme://host/test"; var testNavManager = new TestNavigationManagerWithExceptionHandling(baseUri); var expectedException = new InvalidOperationException("Test exception from OnNavigateTo"); + var tcs = new TaskCompletionSource(); // First test: Initialize with a callback that throws exceptions testNavManager.Initialize(baseUri, uri, uri => testNavManager.GetErrorHandledTask(ThrowingMethod(uri))); @@ -907,14 +908,23 @@ public void OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() // Should be null because the exception was handled gracefully Assert.Null(wrappedException); + await tcs.Task.WaitAsync(Timeout); + // Verify that the exception was logged Assert.Single(testNavManager.HandledExceptions); Assert.Same(expectedException, testNavManager.HandledExceptions[0]); async Task ThrowingMethod(string param) { - await Task.Yield(); - throw expectedException; + try + { + await Task.Yield(); + throw expectedException; + } + finally + { + tcs.SetResult(); + } } #nullable restore } From 3702281b8367e8477d99bad411a032ed4279c270 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Thu, 3 Jul 2025 10:37:21 +0200 Subject: [PATCH 8/8] Even more deterministic. --- .../Components/test/NavigationManagerTest.cs | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/Components/Components/test/NavigationManagerTest.cs b/src/Components/Components/test/NavigationManagerTest.cs index 988e42f6d9a7..081165aa81b1 100644 --- a/src/Components/Components/test/NavigationManagerTest.cs +++ b/src/Components/Components/test/NavigationManagerTest.cs @@ -896,10 +896,10 @@ public async Task OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() var uri = "scheme://host/test"; var testNavManager = new TestNavigationManagerWithExceptionHandling(baseUri); var expectedException = new InvalidOperationException("Test exception from OnNavigateTo"); - var tcs = new TaskCompletionSource(); + var exceptionHandledTcs = new TaskCompletionSource(); // First test: Initialize with a callback that throws exceptions - testNavManager.Initialize(baseUri, uri, uri => testNavManager.GetErrorHandledTask(ThrowingMethod(uri))); + testNavManager.Initialize(baseUri, uri, uri => testNavManager.GetErrorHandledTask(ThrowingMethod(uri), exceptionHandledTcs)); // Act & Assert // Verify that the wrapped callback handles the exception gracefully @@ -908,7 +908,7 @@ public async Task OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() // Should be null because the exception was handled gracefully Assert.Null(wrappedException); - await tcs.Task.WaitAsync(Timeout); + await exceptionHandledTcs.Task; // Verify that the exception was logged Assert.Single(testNavManager.HandledExceptions); @@ -916,15 +916,8 @@ public async Task OnNavigateToCallback_WhenThrows_ShouldBeHandledGracefully() async Task ThrowingMethod(string param) { - try - { - await Task.Yield(); - throw expectedException; - } - finally - { - tcs.SetResult(); - } + await Task.Yield(); + throw expectedException; } #nullable restore } @@ -1017,7 +1010,7 @@ protected override void NavigateToCore(string uri, bool forceLoad) } } - public async Task GetErrorHandledTask(Task taskToHandle) + public async Task GetErrorHandledTask(Task taskToHandle, TaskCompletionSource completionSource = null) { try { @@ -1027,6 +1020,10 @@ public async Task GetErrorHandledTask(Task taskToHandle) { HandledExceptions.Add(ex); } + finally + { + completionSource?.SetResult(); + } } } }