Skip to content
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

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

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

halter73
Copy link
Member

@halter73 halter73 commented Oct 22, 2024

Prevent unnecessary debugger stops for user-unhandled exceptions in Blazor apps with Just My Code enabled

Improve usage of the new [DebuggerDisableUserUnhandledExceptions] and Debugger.BreakForUserUnhandledException(ex) APIs to prevent unnecessarily repeated debugger stops for the same user-unhandled exceptions in Blazor apps with Just My Code enabled.

Description

This is a follow up to #57148 which I've made after testing Visual Studio's treatment of NavigationExceptions when debugging with just my code enabled. The intent was to ensure that NavigationExceptions that get handled properly by Blazor do not get treated as user-unhandled exceptions. Unfortunately, this didn't fully work because the debugger does not follow exceptions through multiple try-catch blocks. For example:

Sample code demonstrating what the debugger does and does not stop for
// ExternalAssembly.dll
public class Class1
{
    // This attribute works, but only for Funcs that return faulted Tasks.
    [DebuggerDisableUserUnhandledExceptions]
    public static async Task RunNestedAsync(Func<Task> callback)
    {
        try
        {
            await Nested(callback);
        }
        catch
        {
        }
    }

    private static async Task Nested(Func<Task> callback)
    {
        await callback();
    }


    // This attribute works for Funcs that return faulted Tasks or throw directly.
    [DebuggerDisableUserUnhandledExceptions]
    public static async Task RunNestedPassThroughAsync(Func<Task> callback)
    {
        try
        {
            await NestedPassThrough(callback);
        }
        catch
        {
        }
    }

    private static Task NestedPassThrough(Func<Task> callback) => callback();

    // This attribute does nothing due to the try/catch block in NestedTryCatch.
    [DebuggerDisableUserUnhandledExceptions]
    public static async Task RunNestedTryCatchAsync(Func<Task> callback)
    {
        try
        {
            await NestedTryCatch(callback);
        }
        catch
        {
        }
    }

    private static async Task NestedTryCatch(Func<Task> callback)
    {
        try
        {
            await callback();
        }
        catch
        {
            throw;
        }
    }
}

// Program.cs in another assembly
app.MapGet("/nested", async () =>
{
    await Class1.RunNestedAsync(async () =>
    {
        // This is successfully ignored as a user-unhandled exception despite async nested method.
        throw new Exception();
    });
});

app.MapGet("/nested-non-async", async () =>
{
    await Class1.RunNestedAsync(() =>
    {
        // However, throwing from a non-"async" callback makes the debugger treat it as user-unhandled again.
        throw new Exception();
    });
});

app.MapGet("/nested-pass-through-non-async", async () =>
{
    await Class1.RunNestedPassThroughAsync(() =>
    {
        // This is successfully ignored as a user-unhandled exception again despite the non-async callback
        // now that the nested method is non-async.
        throw new Exception();
    });
});

app.MapGet("/nested-try-catch", async () =>
{
    await Class1.RunNestedTryCatchAsync(async () =>
    {
        // A nested method with a try/catch block makes the debugger treat all thrown exceptions 
        // as user-unhandled again regardless of the asyncness of the callback.
        throw new Exception();
    });
});

This caused the debugger to treat most NavigationException's as user-unhandled exceptions, because the method with [DebuggerDisableUserUnhandledExceptions] was too far down the stack with multiple try/catch blocks in between. And in some cases, the debugger would stop for exceptions at a try/catch boundary and then extraneously stop again for the same exception because of calls Debugger.BreakForUserUnhandledException(ex).

I used the Redirect*.razor pages in https://github.com/halter73/DebuggerDisableUserUnhandledExceptions/tree/nav/BlazorApp1/Components/Pages and https://github.com/halter73/DebuggerDisableUserUnhandledExceptions/tree/throw/BlazorApp1/Components/Pages to test these changes.

@gregg-miskelly

Customer Impact

Without this change, Blazor developers with Just My Code enabled will sometimes see the debugger stop repeatedly for the same user-unhandled exception. They might also see the debugger stop for some NavigationExceptions which are a normal part of Blazor's operation when the NavigationManager is used during prerendering.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Previously, the debugger would not stop at all for these user-unhandled exceptions. Breaking for Async User-Unhandled exceptions in the Visual Studio Debugger in async code is new in .NET 9.

If exceptions happen to be common in a given developer's workflow, they can easily disable breaking for user-unhandled exceptions either in general or on a per exception type basis to get the old behavior of never stopping for user-unhandled exceptions.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Oct 22, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0.x milestone Oct 22, 2024
@halter73 halter73 marked this pull request as ready for review October 22, 2024 23:07
@halter73 halter73 requested review from BrennanConroy and a team as code owners October 22, 2024 23:07
@halter73 halter73 changed the title Improve usage of [DebuggerDisableUserUnhandledExceptions] [release/9.0] Prevent unnecessary debugger stops for user unhandled exceptions in Blazor apps with Just My Code enabled Oct 22, 2024
@halter73 halter73 added the Servicing-consider Shiproom approval is required for the issue label Oct 22, 2024
@halter73 halter73 changed the title [release/9.0] Prevent unnecessary debugger stops for user unhandled exceptions in Blazor apps with Just My Code enabled [release/9.0] Prevent unnecessary debugger stops for user-unhandled exceptions in Blazor apps with Just My Code enabled Oct 22, 2024
@wtgodbe wtgodbe modified the milestones: 9.0.x, 9.0.1 Oct 23, 2024
@wtgodbe
Copy link
Member

wtgodbe commented Oct 23, 2024

Hold for 9.0.1

@rbhanda rbhanda added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Oct 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Oct 31, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Nov 13, 2024
@wtgodbe wtgodbe merged commit be19faf into release/9.0 Nov 13, 2024
25 checks passed
@wtgodbe wtgodbe deleted the halter73/unhandled-9.0 branch November 13, 2024 21:45
@dotnet-policy-service dotnet-policy-service bot modified the milestone: 9.0.1 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants