Skip to content

[release/9.0] Prevent unnecessary debugger stops for user-unhandled exceptions in Blazor apps with Just My Code enabled #58573

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 2 commits into from
Nov 13, 2024
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
36 changes: 32 additions & 4 deletions src/Components/Components/src/ComponentBase.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using Microsoft.AspNetCore.Components.Rendering;

namespace Microsoft.AspNetCore.Components;
Expand Down Expand Up @@ -271,10 +272,22 @@ public virtual Task SetParametersAsync(ParameterView parameters)
}
}

// We do not want the debugger to consider NavigationExceptions caught by this method as user-unhandled.
[DebuggerDisableUserUnhandledExceptions]
private async Task RunInitAndSetParametersAsync()
{
OnInitialized();
var task = OnInitializedAsync();
Task task;

try
{
OnInitialized();
task = OnInitializedAsync();
}
catch (Exception ex) when (ex is not NavigationException)
{
Debugger.BreakForUserUnhandledException(ex);
throw;
}

if (task.Status != TaskStatus.RanToCompletion && task.Status != TaskStatus.Canceled)
{
Expand Down Expand Up @@ -307,10 +320,23 @@ private async Task RunInitAndSetParametersAsync()
await CallOnParametersSetAsync();
}

// We do not want the debugger to consider NavigationExceptions caught by this method as user-unhandled.
[DebuggerDisableUserUnhandledExceptions]
private Task CallOnParametersSetAsync()
{
OnParametersSet();
var task = OnParametersSetAsync();
Task task;

try
{
OnParametersSet();
task = OnParametersSetAsync();
}
catch (Exception ex) when (ex is not NavigationException)
{
Debugger.BreakForUserUnhandledException(ex);
throw;
}

// If no async work is to be performed, i.e. the task has already ran to completion
// or was canceled by the time we got to inspect it, avoid going async and re-invoking
// StateHasChanged at the culmination of the async work.
Expand All @@ -326,6 +352,8 @@ private Task CallOnParametersSetAsync()
Task.CompletedTask;
}

// We do not want the debugger to stop more than once per user-unhandled exception.
[DebuggerDisableUserUnhandledExceptions]
private async Task CallStateHasChangedOnAsyncCompletion(Task task)
{
try
Expand Down
2 changes: 2 additions & 0 deletions src/Components/Components/src/RenderTree/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,8 @@ async Task ContinueAfterTask(ArrayRange<ulong> eventHandlerIds, Task afterTaskIg
}
}

// We do not want the debugger to stop more than once per user-unhandled exception.
[DebuggerDisableUserUnhandledExceptions]
private async Task GetErrorHandledTask(Task taskToHandle, ComponentState? owningComponentState)
{
try
Expand Down
7 changes: 7 additions & 0 deletions src/Components/Components/src/Rendering/ComponentState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public ComponentState(Renderer renderer, int componentId, IComponent component,

internal Renderer Renderer => _renderer;

// We do not want the debugger to consider NavigationExceptions caught by this method as user-unhandled.
[DebuggerDisableUserUnhandledExceptions]
internal void RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment renderFragment, out Exception? renderFragmentException)
{
renderFragmentException = null;
Expand All @@ -106,6 +108,11 @@ internal void RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment re
}
catch (Exception ex)
{
if (ex is not NavigationException)
{
Debugger.BreakForUserUnhandledException(ex);
}

// If an exception occurs in the render fragment delegate, we won't process the diff in any way, so child components,
// event handlers, etc., will all be left untouched as if this component didn't re-render at all. The Renderer will
// then forcibly clear the descendant subtree by rendering an empty fragment for this component.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilT
}
catch (Exception ex)
{
// Rethrowing also informs the debugger that this exception should be considered user-unhandled unlike NavigationExceptions,
// but calling BreakForUserUnhandledException here allows the debugger to break before we modify the HttpContext.
Debugger.BreakForUserUnhandledException(ex);

// Theoretically it might be possible to let the error middleware run, capture the output,
// then emit it in a special format so the JS code can display the error page. However
// for now we're not going to support that and will simply emit a message.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,8 @@ private static void SetExceptionHandlerFeatures(ErrorContext errorContext)
/// </summary>
/// <param name="context"></param>
/// <returns></returns>
[DebuggerDisableUserUnhandledExceptions]
public async Task Invoke(HttpContext context)
{
// We want to avoid treating exceptions as user-unhandled if an exception filter like the DatabaseDeveloperPageExceptionFilter
// handles the exception rather than letting it flow to the default DisplayException method. This is because we don't want to stop the
// debugger if the developer shouldn't be handling the exception and instead just needs to do something like click a link to run a
// database migration.
try
{
await _next(context);
Expand All @@ -127,11 +122,6 @@ public async Task Invoke(HttpContext context)
context.Response.StatusCode = StatusCodes.Status499ClientClosedRequest;
}

// Generally speaking, we do not expect application code to handle things like IOExceptions during a request
// body read due to a client disconnect. But aborted requests should be rare in development, and developers
// might be surprised if an IOException propagating through their code was not considered user-unhandled.
// That said, if developers complain, we consider removing the following line.
Debugger.BreakForUserUnhandledException(ex);
return;
}

Expand All @@ -141,8 +131,6 @@ public async Task Invoke(HttpContext context)
{
_logger.ResponseStartedErrorPageMiddleware();
_metrics.RequestException(exceptionName, ExceptionResult.Skipped, handler: null);

// Rethrowing informs the debugger that this exception should be considered user-unhandled.
throw;
}

Expand Down Expand Up @@ -173,16 +161,11 @@ public async Task Invoke(HttpContext context)
}
catch (Exception ex2)
{
// It might make sense to call BreakForUserUnhandledException for ex2 after we do the same for the original exception.
// But for now, considering the rarity of user-defined IDeveloperPageExceptionFilters, we're not for simplicity.

// If there's a Exception while generating the error page, re-throw the original exception.
_logger.DisplayErrorPageException(ex2);
}

_metrics.RequestException(exceptionName, ExceptionResult.Unhandled, handler: null);

// Rethrowing informs the debugger that this exception should be considered user-unhandled.
throw;
}

Expand All @@ -195,9 +178,6 @@ public async Task Invoke(HttpContext context)
// Assumes the response headers have not been sent. If they have, still attempt to write to the body.
private Task DisplayException(ErrorContext errorContext)
{
// We need to inform the debugger that this exception should be considered user-unhandled since it wasn't fully handled by an exception filter.
Debugger.BreakForUserUnhandledException(errorContext.Exception);

var httpContext = errorContext.HttpContext;
var headers = httpContext.Request.GetTypedHeaders();
var acceptHeader = headers.Accept;
Expand Down
Loading