Skip to content

Commit be19faf

Browse files
authored
[release/9.0] Prevent unnecessary debugger stops for user-unhandled exceptions in Blazor apps with Just My Code enabled (#58573)
* Add [DebuggerDisableUserUnhandledExceptions] closer to user code * Remove [DebuggerDisableUserUnhandledExceptions] from developer exception page middleware
1 parent 74972b5 commit be19faf

File tree

5 files changed

+41
-28
lines changed

5 files changed

+41
-28
lines changed

src/Components/Components/src/ComponentBase.cs

+32-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Diagnostics;
45
using Microsoft.AspNetCore.Components.Rendering;
56

67
namespace Microsoft.AspNetCore.Components;
@@ -271,10 +272,22 @@ public virtual Task SetParametersAsync(ParameterView parameters)
271272
}
272273
}
273274

275+
// We do not want the debugger to consider NavigationExceptions caught by this method as user-unhandled.
276+
[DebuggerDisableUserUnhandledExceptions]
274277
private async Task RunInitAndSetParametersAsync()
275278
{
276-
OnInitialized();
277-
var task = OnInitializedAsync();
279+
Task task;
280+
281+
try
282+
{
283+
OnInitialized();
284+
task = OnInitializedAsync();
285+
}
286+
catch (Exception ex) when (ex is not NavigationException)
287+
{
288+
Debugger.BreakForUserUnhandledException(ex);
289+
throw;
290+
}
278291

279292
if (task.Status != TaskStatus.RanToCompletion && task.Status != TaskStatus.Canceled)
280293
{
@@ -307,10 +320,23 @@ private async Task RunInitAndSetParametersAsync()
307320
await CallOnParametersSetAsync();
308321
}
309322

323+
// We do not want the debugger to consider NavigationExceptions caught by this method as user-unhandled.
324+
[DebuggerDisableUserUnhandledExceptions]
310325
private Task CallOnParametersSetAsync()
311326
{
312-
OnParametersSet();
313-
var task = OnParametersSetAsync();
327+
Task task;
328+
329+
try
330+
{
331+
OnParametersSet();
332+
task = OnParametersSetAsync();
333+
}
334+
catch (Exception ex) when (ex is not NavigationException)
335+
{
336+
Debugger.BreakForUserUnhandledException(ex);
337+
throw;
338+
}
339+
314340
// If no async work is to be performed, i.e. the task has already ran to completion
315341
// or was canceled by the time we got to inspect it, avoid going async and re-invoking
316342
// StateHasChanged at the culmination of the async work.
@@ -326,6 +352,8 @@ private Task CallOnParametersSetAsync()
326352
Task.CompletedTask;
327353
}
328354

355+
// We do not want the debugger to stop more than once per user-unhandled exception.
356+
[DebuggerDisableUserUnhandledExceptions]
329357
private async Task CallStateHasChangedOnAsyncCompletion(Task task)
330358
{
331359
try

src/Components/Components/src/RenderTree/Renderer.cs

+2
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,8 @@ async Task ContinueAfterTask(ArrayRange<ulong> eventHandlerIds, Task afterTaskIg
10391039
}
10401040
}
10411041

1042+
// We do not want the debugger to stop more than once per user-unhandled exception.
1043+
[DebuggerDisableUserUnhandledExceptions]
10421044
private async Task GetErrorHandledTask(Task taskToHandle, ComponentState? owningComponentState)
10431045
{
10441046
try

src/Components/Components/src/Rendering/ComponentState.cs

+7
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ public ComponentState(Renderer renderer, int componentId, IComponent component,
8787

8888
internal Renderer Renderer => _renderer;
8989

90+
// We do not want the debugger to consider NavigationExceptions caught by this method as user-unhandled.
91+
[DebuggerDisableUserUnhandledExceptions]
9092
internal void RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment renderFragment, out Exception? renderFragmentException)
9193
{
9294
renderFragmentException = null;
@@ -106,6 +108,11 @@ internal void RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment re
106108
}
107109
catch (Exception ex)
108110
{
111+
if (ex is not NavigationException)
112+
{
113+
Debugger.BreakForUserUnhandledException(ex);
114+
}
115+
109116
// If an exception occurs in the render fragment delegate, we won't process the diff in any way, so child components,
110117
// event handlers, etc., will all be left untouched as if this component didn't re-render at all. The Renderer will
111118
// then forcibly clear the descendant subtree by rendering an empty fragment for this component.

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.Streaming.cs

-4
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilT
7777
}
7878
catch (Exception ex)
7979
{
80-
// Rethrowing also informs the debugger that this exception should be considered user-unhandled unlike NavigationExceptions,
81-
// but calling BreakForUserUnhandledException here allows the debugger to break before we modify the HttpContext.
82-
Debugger.BreakForUserUnhandledException(ex);
83-
8480
// Theoretically it might be possible to let the error middleware run, capture the output,
8581
// then emit it in a special format so the JS code can display the error page. However
8682
// for now we're not going to support that and will simply emit a message.

src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs

-20
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,8 @@ private static void SetExceptionHandlerFeatures(ErrorContext errorContext)
102102
/// </summary>
103103
/// <param name="context"></param>
104104
/// <returns></returns>
105-
[DebuggerDisableUserUnhandledExceptions]
106105
public async Task Invoke(HttpContext context)
107106
{
108-
// We want to avoid treating exceptions as user-unhandled if an exception filter like the DatabaseDeveloperPageExceptionFilter
109-
// handles the exception rather than letting it flow to the default DisplayException method. This is because we don't want to stop the
110-
// debugger if the developer shouldn't be handling the exception and instead just needs to do something like click a link to run a
111-
// database migration.
112107
try
113108
{
114109
await _next(context);
@@ -127,11 +122,6 @@ public async Task Invoke(HttpContext context)
127122
context.Response.StatusCode = StatusCodes.Status499ClientClosedRequest;
128123
}
129124

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

@@ -141,8 +131,6 @@ public async Task Invoke(HttpContext context)
141131
{
142132
_logger.ResponseStartedErrorPageMiddleware();
143133
_metrics.RequestException(exceptionName, ExceptionResult.Skipped, handler: null);
144-
145-
// Rethrowing informs the debugger that this exception should be considered user-unhandled.
146134
throw;
147135
}
148136

@@ -173,16 +161,11 @@ public async Task Invoke(HttpContext context)
173161
}
174162
catch (Exception ex2)
175163
{
176-
// It might make sense to call BreakForUserUnhandledException for ex2 after we do the same for the original exception.
177-
// But for now, considering the rarity of user-defined IDeveloperPageExceptionFilters, we're not for simplicity.
178-
179164
// If there's a Exception while generating the error page, re-throw the original exception.
180165
_logger.DisplayErrorPageException(ex2);
181166
}
182167

183168
_metrics.RequestException(exceptionName, ExceptionResult.Unhandled, handler: null);
184-
185-
// Rethrowing informs the debugger that this exception should be considered user-unhandled.
186169
throw;
187170
}
188171

@@ -195,9 +178,6 @@ public async Task Invoke(HttpContext context)
195178
// Assumes the response headers have not been sent. If they have, still attempt to write to the body.
196179
private Task DisplayException(ErrorContext errorContext)
197180
{
198-
// We need to inform the debugger that this exception should be considered user-unhandled since it wasn't fully handled by an exception filter.
199-
Debugger.BreakForUserUnhandledException(errorContext.Exception);
200-
201181
var httpContext = errorContext.HttpContext;
202182
var headers = httpContext.Request.GetTypedHeaders();
203183
var acceptHeader = headers.Accept;

0 commit comments

Comments
 (0)