diff --git a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs index b14b8d60b0e9..9b473ac08547 100644 --- a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs @@ -150,7 +150,8 @@ public void RequestEnd(HttpContext httpContext, Exception? exception, HostingApp if (context.MetricsEnabled) { - var route = httpContext.GetEndpoint()?.Metadata.GetMetadata()?.Route; + var endpoint = HttpExtensions.GetOriginalEndpoint(httpContext); + var route = endpoint?.Metadata.GetMetadata()?.Route; var customTags = context.MetricsTagsFeature?.TagsList; _metrics.RequestEnd( diff --git a/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj b/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj index 49974f5104af..c898aed6af58 100644 --- a/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj +++ b/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj @@ -34,6 +34,7 @@ + diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs index e8f40e9b3fab..d0579a121b94 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs @@ -247,12 +247,7 @@ private static void ClearHttpContext(HttpContext context) // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset // the endpoint and route values to ensure things are re-calculated. - context.SetEndpoint(endpoint: null); - var routeValuesFeature = context.Features.Get(); - if (routeValuesFeature != null) - { - routeValuesFeature.RouteValues = null!; - } + HttpExtensions.ClearEndpoint(context); } private static Task ClearCacheHeaders(object state) diff --git a/src/Middleware/Diagnostics/src/Microsoft.AspNetCore.Diagnostics.csproj b/src/Middleware/Diagnostics/src/Microsoft.AspNetCore.Diagnostics.csproj index c83395d96d9b..4657b64cbf7b 100644 --- a/src/Middleware/Diagnostics/src/Microsoft.AspNetCore.Diagnostics.csproj +++ b/src/Middleware/Diagnostics/src/Microsoft.AspNetCore.Diagnostics.csproj @@ -16,6 +16,7 @@ + diff --git a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs index 5cb8c981b280..a431f35582f8 100644 --- a/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs +++ b/src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs @@ -190,11 +190,7 @@ private static Func CreateHandler(string pathFormat, st // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset // the endpoint and route values to ensure things are re-calculated. - context.HttpContext.SetEndpoint(endpoint: null); - if (routeValuesFeature != null) - { - routeValuesFeature.RouteValues = null!; - } + HttpExtensions.ClearEndpoint(context.HttpContext); context.HttpContext.Request.Path = newPath; context.HttpContext.Request.QueryString = newQueryString; diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs index 31c3fa12f5df..b5513d9cf0e1 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.Diagnostics.Metrics; +using System.Net; using System.Net.Http; using System.Net.Http.Headers; using System.Net.Http.Json; @@ -11,20 +12,22 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.TestHost; -using Microsoft.AspNetCore.InternalTesting; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Diagnostics.Metrics; using Microsoft.Extensions.Diagnostics.Metrics.Testing; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; namespace Microsoft.AspNetCore.Diagnostics; -public class ExceptionHandlerMiddlewareTest +public class ExceptionHandlerMiddlewareTest : LoggedTest { [Fact] public async Task ExceptionIsSetOnProblemDetailsContext() @@ -291,6 +294,133 @@ public async Task Metrics_ExceptionThrown_DefaultSettings_Handled_Reported() m => AssertRequestException(m, "System.InvalidOperationException", "handled", null)); } + [Fact] + public async Task Metrics_ExceptionThrown_Handled_UseOriginalRoute() + { + // Arrange + var originalEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/path"), 0); + var originalEndpoint = originalEndpointBuilder.Build(); + + var meterFactory = new TestMeterFactory(); + using var requestDurationCollector = new MetricCollector(meterFactory, "Microsoft.AspNetCore.Hosting", "http.server.request.duration"); + using var requestExceptionCollector = new MetricCollector(meterFactory, DiagnosticsMetrics.MeterName, "aspnetcore.diagnostics.exceptions"); + + using var host = new HostBuilder() + .ConfigureServices(s => + { + s.AddSingleton(meterFactory); + s.AddSingleton(LoggerFactory); + }) + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .Configure(app => + { + app.UseExceptionHandler(new ExceptionHandlerOptions + { + ExceptionHandler = (c) => Task.CompletedTask + }); + app.Run(context => + { + context.SetEndpoint(originalEndpoint); + throw new Exception("Test exception"); + }); + + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + // Act + var response = await server.CreateClient().GetAsync("/path"); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + + await requestDurationCollector.WaitForMeasurementsAsync(minCount: 1).DefaultTimeout(); + + // Assert + Assert.Collection( + requestDurationCollector.GetMeasurementSnapshot(), + m => + { + Assert.True(m.Value > 0); + Assert.Equal(500, (int)m.Tags["http.response.status_code"]); + Assert.Equal("System.Exception", (string)m.Tags["error.type"]); + Assert.Equal("/path", (string)m.Tags["http.route"]); + }); + Assert.Collection(requestExceptionCollector.GetMeasurementSnapshot(), + m => AssertRequestException(m, "System.Exception", "handled")); + } + + [Fact] + public async Task Metrics_ExceptionThrown_Handled_UseNewRoute() + { + // Arrange + var originalEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/path"), 0); + var originalEndpoint = originalEndpointBuilder.Build(); + + var newEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/new"), 0); + var newEndpoint = newEndpointBuilder.Build(); + + var meterFactory = new TestMeterFactory(); + using var requestDurationCollector = new MetricCollector(meterFactory, "Microsoft.AspNetCore.Hosting", "http.server.request.duration"); + using var requestExceptionCollector = new MetricCollector(meterFactory, DiagnosticsMetrics.MeterName, "aspnetcore.diagnostics.exceptions"); + + using var host = new HostBuilder() + .ConfigureServices(s => + { + s.AddSingleton(meterFactory); + s.AddSingleton(LoggerFactory); + }) + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .Configure(app => + { + app.UseExceptionHandler(new ExceptionHandlerOptions + { + ExceptionHandler = (c) => + { + c.SetEndpoint(newEndpoint); + return Task.CompletedTask; + } + }); + app.Run(context => + { + context.SetEndpoint(originalEndpoint); + throw new Exception("Test exception"); + }); + + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + // Act + var response = await server.CreateClient().GetAsync("/path"); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + + await requestDurationCollector.WaitForMeasurementsAsync(minCount: 1).DefaultTimeout(); + + // Assert + Assert.Collection( + requestDurationCollector.GetMeasurementSnapshot(), + m => + { + Assert.True(m.Value > 0); + Assert.Equal(500, (int)m.Tags["http.response.status_code"]); + Assert.Equal("System.Exception", (string)m.Tags["error.type"]); + Assert.Equal("/new", (string)m.Tags["http.route"]); + }); + Assert.Collection(requestExceptionCollector.GetMeasurementSnapshot(), + m => AssertRequestException(m, "System.Exception", "handled")); + } + [Fact] public async Task Metrics_ExceptionThrown_Unhandled_Reported() { diff --git a/src/Shared/HttpExtensions.cs b/src/Shared/HttpExtensions.cs index 166bb53bcfaf..4a0d50de25fd 100644 --- a/src/Shared/HttpExtensions.cs +++ b/src/Shared/HttpExtensions.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. + using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; internal static class HttpExtensions { @@ -10,6 +12,9 @@ internal static class HttpExtensions internal static bool IsValidHttpMethodForForm(string method) => HttpMethods.IsPost(method) || HttpMethods.IsPut(method) || HttpMethods.IsPatch(method); + // Key is a string so shared code works across different assemblies (hosting, error handling middleware, etc). + internal const string OriginalEndpointKey = "__OriginalEndpoint"; + internal static bool IsValidContentTypeForForm(string? contentType) { if (contentType == null) @@ -27,4 +32,36 @@ internal static bool IsValidContentTypeForForm(string? contentType) return contentType.Equals(UrlEncodedFormContentType, StringComparison.OrdinalIgnoreCase) || contentType.StartsWith(MultipartFormContentType, StringComparison.OrdinalIgnoreCase); } + + internal static Endpoint? GetOriginalEndpoint(HttpContext context) + { + var endpoint = context.GetEndpoint(); + + // Some middleware re-execute the middleware pipeline with the HttpContext. Before they do this, they clear state from context, such as the previously matched endpoint. + // The original endpoint is stashed with a known key in HttpContext.Items. Use it as a fallback. + if (endpoint == null && context.Items.TryGetValue(OriginalEndpointKey, out var e) && e is Endpoint originalEndpoint) + { + endpoint = originalEndpoint; + } + return endpoint; + } + + internal static void ClearEndpoint(HttpContext context) + { + var endpoint = context.GetEndpoint(); + if (endpoint != null) + { + context.Items[OriginalEndpointKey] = endpoint; + + // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset + // the endpoint and route values to ensure things are re-calculated. + context.SetEndpoint(endpoint: null); + } + + var routeValuesFeature = context.Features.Get(); + if (routeValuesFeature != null) + { + routeValuesFeature.RouteValues = null!; + } + } }