Skip to content

Commit ec717c3

Browse files
JamesNKJamesNK
committed
Change to callback
1 parent 756cde2 commit ec717c3

File tree

6 files changed

+122
-32
lines changed

6 files changed

+122
-32
lines changed

‎src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -171,18 +171,18 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
171171
context.Response.OnStarting(_clearCacheHeadersDelegate, context.Response);
172172

173173
string? handlerTag = null;
174-
var handler = Handler.None;
174+
var result = ExceptionHandlerResult.Unhandled;
175175
foreach (var exceptionHandler in _exceptionHandlers)
176176
{
177177
if (await exceptionHandler.TryHandleAsync(context, edi.SourceException, context.RequestAborted))
178178
{
179-
handler = Handler.IExceptionHandler;
179+
result = ExceptionHandlerResult.IExceptionHandler;
180180
handlerTag = exceptionHandler.GetType().FullName;
181181
break;
182182
}
183183
}
184184

185-
if (handler == Handler.None)
185+
if (result == ExceptionHandlerResult.Unhandled)
186186
{
187187
if (_options.ExceptionHandler is not null)
188188
{
@@ -191,11 +191,15 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
191191
// If the response has started, assume exception handler was successful.
192192
if (context.Response.HasStarted)
193193
{
194-
handler = Handler.ExceptionHandler;
195194
if (_options.ExceptionHandlingPath.HasValue)
196195
{
196+
result = ExceptionHandlerResult.ExceptionHandlingPath;
197197
handlerTag = _options.ExceptionHandlingPath.Value;
198198
}
199+
else
200+
{
201+
result = ExceptionHandlerResult.ExceptionHandler;
202+
}
199203
}
200204
}
201205
else
@@ -208,19 +212,29 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
208212
Exception = edi.SourceException,
209213
}))
210214
{
211-
handler = Handler.IProblemDetailsService;
215+
result = ExceptionHandlerResult.ProblemDetailsService;
212216
handlerTag = _problemDetailsService.GetType().FullName;
213217
}
214218
}
215219
}
216220

217-
if (handler != Handler.None || _options.StatusCodeSelector != null || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response)
221+
if (result != ExceptionHandlerResult.Unhandled || _options.StatusCodeSelector != null || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response)
218222
{
219-
// Customers with an IExceptionHandler that reports it handled the exception could do their own logging.
220-
// Don't log IExceptionHandler handled exceptions if SuppressLoggingIExceptionHandler is set to true.
221-
// Note: Microsoft.AspNetCore.Diagnostics.HandledException is used by AppInsights to log errors so it's also skipped.
222-
if (handler != Handler.IExceptionHandler || !_options.SuppressLoggingIExceptionHandler)
223+
var suppressLogging = false;
224+
225+
// Customers may prefer to handle the exception and perfer to do their own logging.
226+
// In that case, it can be undesirable for the middleware to log the exception at an error level.
227+
// Run the configured callback to determine if the exception logging in middleware should be suppressed.
228+
if (_options.SuppressLoggingCallback is { } suppressLoggingCallback)
223229
{
230+
var logContext = new ExceptionHandlerSuppressLoggingContext { Exception = edi.SourceException, HandlerResult = result };
231+
suppressLogging = suppressLoggingCallback(logContext);
232+
}
233+
234+
if (!suppressLogging)
235+
{
236+
// Note: Microsoft.AspNetCore.Diagnostics.HandledException is used by AppInsights to log errors.
237+
// The diagnostics event is run together with standard exception logging.
224238
const string eventName = "Microsoft.AspNetCore.Diagnostics.HandledException";
225239
if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled(eventName))
226240
{
@@ -285,12 +299,4 @@ private static Task ClearCacheHeaders(object state)
285299
headers.ETag = default;
286300
return Task.CompletedTask;
287301
}
288-
289-
private enum Handler
290-
{
291-
None,
292-
IExceptionHandler,
293-
IProblemDetailsService,
294-
ExceptionHandler
295-
}
296302
}

‎src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerOptions.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ public class ExceptionHandlerOptions
4848
public Func<Exception, int>? StatusCodeSelector { get; set; }
4949

5050
/// <summary>
51-
/// Gets or sets a value that determines if the exception handler middleware should suppress logging
52-
/// if the exception was handled by a <see cref="IExceptionHandler"/> registered in the DI container.
51+
/// Gets or sets a callback that can be used to suppress logging by <see cref="ExceptionHandlerMiddleware" />.
52+
/// This callback is only run if the exception was handled by the middleware.
53+
/// Unhandled exceptions and exceptions thrown after the response has started are always logged.
5354
/// </summary>
54-
public bool SuppressLoggingIExceptionHandler { get; set; }
55+
public Func<ExceptionHandlerSuppressLoggingContext, bool>? SuppressLoggingCallback { get; set; }
5556
}

‎src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerResult.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.AspNetCore.Diagnostics;
5+
6+
/// <summary>
7+
/// The result of executing <see cref="ExceptionHandlerMiddleware"/>.
8+
/// </summary>
9+
public enum ExceptionHandlerResult
10+
{
11+
/// <summary>
12+
/// Exception was unhandled.
13+
/// </summary>
14+
Unhandled,
15+
/// <summary>
16+
/// Exception was handled by an <see cref="Diagnostics.IExceptionHandler"/> instance registered in the DI container.
17+
/// </summary>
18+
IExceptionHandler,
19+
/// <summary>
20+
/// Exception was handled by an <see cref="Http.IProblemDetailsService"/> instance registered in the DI container.
21+
/// </summary>
22+
ProblemDetailsService,
23+
/// <summary>
24+
/// Exception was handled by by <see cref="Builder.ExceptionHandlerOptions.ExceptionHandler"/>.
25+
/// </summary>
26+
ExceptionHandler,
27+
/// <summary>
28+
/// Exception was handled by by <see cref="Builder.ExceptionHandlerOptions.ExceptionHandlingPath"/>.
29+
/// </summary>
30+
ExceptionHandlingPath
31+
}

‎src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerSuppressLoggingContext.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.AspNetCore.Diagnostics;
5+
6+
/// <summary>
7+
/// The context used to determine whether exception handler middleware should log an exception.
8+
/// </summary>
9+
public sealed class ExceptionHandlerSuppressLoggingContext
10+
{
11+
/// <summary>
12+
/// Gets the <see cref="System.Exception"/> that the exception handler middleware is processing.
13+
/// </summary>
14+
public required Exception Exception { get; init; }
15+
16+
/// <summary>
17+
/// Gets the result of the exception handler middleware.
18+
/// </summary>
19+
public required ExceptionHandlerResult HandlerResult { get; init; }
20+
}

‎src/Middleware/Diagnostics/src/PublicAPI.Unshipped.txt

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
#nullable enable
2+
Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingCallback.get -> System.Func<Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressLoggingContext!, bool>?
3+
Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingCallback.set -> void
24
Microsoft.AspNetCore.Builder.StatusCodePagesOptions.CreateScopeForErrors.get -> bool
35
Microsoft.AspNetCore.Builder.StatusCodePagesOptions.CreateScopeForErrors.set -> void
6+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult
7+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult.ExceptionHandler = 3 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult
8+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult.ExceptionHandlingPath = 4 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult
9+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult.IExceptionHandler = 1 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult
10+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult.ProblemDetailsService = 2 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult
11+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult.Unhandled = 0 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult
12+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressLoggingContext
13+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressLoggingContext.Exception.get -> System.Exception!
14+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressLoggingContext.Exception.init -> void
15+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressLoggingContext.ExceptionHandlerSuppressLoggingContext() -> void
16+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressLoggingContext.HandlerResult.get -> Microsoft.AspNetCore.Diagnostics.ExceptionHandlerResult
17+
Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressLoggingContext.HandlerResult.init -> void
418
static Microsoft.AspNetCore.Builder.StatusCodePagesExtensions.UseStatusCodePagesWithReExecute(this Microsoft.AspNetCore.Builder.IApplicationBuilder! app, string! pathFormat, bool createScopeForErrors, string? queryFormat = null) -> Microsoft.AspNetCore.Builder.IApplicationBuilder!
5-
Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingIExceptionHandler.get -> bool
6-
Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingIExceptionHandler.set -> void

‎src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public class ExceptionHandlerMiddlewareTest : LoggedTest
3434
public async Task ExceptionIsSetOnProblemDetailsContext()
3535
{
3636
// Arrange
37+
ExceptionHandlerSuppressLoggingContext loggingContext = null;
3738
using var host = new HostBuilder()
3839
.ConfigureServices(services =>
3940
{
@@ -54,7 +55,14 @@ public async Task ExceptionIsSetOnProblemDetailsContext()
5455
.UseTestServer()
5556
.Configure(app =>
5657
{
57-
app.UseExceptionHandler();
58+
app.UseExceptionHandler(new ExceptionHandlerOptions
59+
{
60+
SuppressLoggingCallback = context =>
61+
{
62+
loggingContext = context;
63+
return true;
64+
}
65+
});
5866
app.Run(context =>
5967
{
6068
throw new Exception("Test exception");
@@ -76,6 +84,9 @@ public async Task ExceptionIsSetOnProblemDetailsContext()
7684
var body = await response.Content.ReadFromJsonAsync<ProblemDetails>();
7785
var originalExceptionMessage = ((JsonElement)body.Extensions["OriginalExceptionMessage"]).GetString();
7886
Assert.Equal("Test exception", originalExceptionMessage);
87+
88+
Assert.IsType<Exception>(loggingContext.Exception);
89+
Assert.Equal(ExceptionHandlerResult.ProblemDetailsService, loggingContext.HandlerResult);
7990
}
8091

8192
[Fact]
@@ -102,8 +113,10 @@ public async Task Invoke_ExceptionThrownResultsInClearedRouteValuesAndEndpoint()
102113
Assert.Collection(sink.Writes, w => Assert.Equal("UnhandledException", w.EventId.Name));
103114
}
104115

105-
[Fact]
106-
public async Task Invoke_HasExceptionHandler_SuppressIExceptionHandlerLogging_HasLogs()
116+
[Theory]
117+
[InlineData(ExceptionHandlerResult.ExceptionHandler, false)]
118+
[InlineData(ExceptionHandlerResult.ProblemDetailsService, true)]
119+
public async Task Invoke_HasExceptionHandler_SuppressLogging_CallbackRun(ExceptionHandlerResult suppressResult, bool logged)
107120
{
108121
// Arrange
109122
var sink = new TestSink();
@@ -115,13 +128,20 @@ public async Task Invoke_HasExceptionHandler_SuppressIExceptionHandlerLogging_Ha
115128
context.Features.Set<IHttpResponseFeature>(new TestHttpResponseFeature());
116129
return Task.CompletedTask;
117130
},
118-
suppressLoggingHandlingException: true);
131+
suppressLoggingCallback: c => c.HandlerResult == suppressResult);
119132
var middleware = CreateMiddleware(_ => throw new InvalidOperationException(), optionsAccessor, loggerFactory: new TestLoggerFactory(sink, true));
120133

121134
// Act & Assert
122135
await middleware.Invoke(httpContext);
123136

124-
Assert.Collection(sink.Writes, w => Assert.Equal("UnhandledException", w.EventId.Name));
137+
if (logged)
138+
{
139+
Assert.Collection(sink.Writes, w => Assert.Equal("UnhandledException", w.EventId.Name));
140+
}
141+
else
142+
{
143+
Assert.Empty(sink.Writes);
144+
}
125145
}
126146

127147
private sealed class TestHttpResponseFeature : HttpResponseFeature
@@ -190,7 +210,7 @@ public async Task IExceptionHandlers_SuppressLogging_TestLogs(bool suppressedLog
190210
var sink = new TestSink();
191211
var httpContext = CreateHttpContext();
192212

193-
var optionsAccessor = CreateOptionsAccessor(suppressLoggingHandlingException: suppressedLogs);
213+
var optionsAccessor = CreateOptionsAccessor(suppressLoggingCallback: c => suppressedLogs);
194214

195215
var exceptionHandlers = new List<IExceptionHandler>
196216
{
@@ -584,17 +604,17 @@ private HttpContext CreateHttpContext()
584604
private IOptions<ExceptionHandlerOptions> CreateOptionsAccessor(
585605
RequestDelegate exceptionHandler = null,
586606
string exceptionHandlingPath = null,
587-
bool? suppressLoggingHandlingException = null)
607+
Func<ExceptionHandlerSuppressLoggingContext, bool> suppressLoggingCallback = null)
588608
{
589609
exceptionHandler ??= c => Task.CompletedTask;
590610
var options = new ExceptionHandlerOptions()
591611
{
592612
ExceptionHandler = exceptionHandler,
593613
ExceptionHandlingPath = exceptionHandlingPath,
594614
};
595-
if (suppressLoggingHandlingException != null)
615+
if (suppressLoggingCallback != null)
596616
{
597-
options.SuppressLoggingIExceptionHandler = suppressLoggingHandlingException.Value;
617+
options.SuppressLoggingCallback = suppressLoggingCallback;
598618
}
599619
var optionsAccessor = Mock.Of<IOptions<ExceptionHandlerOptions>>(o => o.Value == options);
600620
return optionsAccessor;

0 commit comments

Comments
 (0)