From f4c61de4901720f4b3eaa0e5c6faef60d7daa035 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 2 Apr 2019 10:02:34 -0700 Subject: [PATCH] Initial design for exception page filters (#8958) - This change introduces the concept of an IDeveloperPageException filter that runs whenever the developer exception page has encountered an error. It follows the middleware pattern (chain of resposibility) which allows short circuiting or decorating the default logic. - Added tests --- ...AspNetCore.Diagnostics.Abstractions.csproj | 2 +- ....Diagnostics.Abstractions.netcoreapp3.0.cs | 10 ++ .../src/ErrorContext.cs | 35 +++++ .../src/IDeveloperPageExceptionFilter.cs | 24 ++++ ...AspNetCore.Diagnostics.Abstractions.csproj | 6 +- ...ft.AspNetCore.Diagnostics.netcoreapp3.0.cs | 2 +- .../DeveloperExceptionPageMiddleware.cs | 29 +++- .../DeveloperExceptionPageMiddlewareTest.cs | 125 +++++++++++++++++- 8 files changed, 221 insertions(+), 12 deletions(-) create mode 100644 src/Middleware/Diagnostics.Abstractions/src/ErrorContext.cs create mode 100644 src/Middleware/Diagnostics.Abstractions/src/IDeveloperPageExceptionFilter.cs diff --git a/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj b/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj index e95eda050f..eeb85ae239 100644 --- a/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj +++ b/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj @@ -5,6 +5,6 @@ - + diff --git a/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.netcoreapp3.0.cs b/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.netcoreapp3.0.cs index 8ec0333ac7..9d8c6215ca 100644 --- a/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.netcoreapp3.0.cs +++ b/src/Middleware/Diagnostics.Abstractions/ref/Microsoft.AspNetCore.Diagnostics.Abstractions.netcoreapp3.0.cs @@ -24,10 +24,20 @@ namespace Microsoft.AspNetCore.Diagnostics public int StartColumn { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public int StartLine { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } } + public partial class ErrorContext + { + public ErrorContext(Microsoft.AspNetCore.Http.HttpContext httpContext, System.Exception exception) { } + public System.Exception Exception { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public Microsoft.AspNetCore.Http.HttpContext HttpContext { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + } public partial interface ICompilationException { System.Collections.Generic.IEnumerable CompilationFailures { get; } } + public partial interface IDeveloperPageExceptionFilter + { + System.Threading.Tasks.Task HandleExceptionAsync(Microsoft.AspNetCore.Diagnostics.ErrorContext errorContext, System.Func next); + } public partial interface IExceptionHandlerFeature { System.Exception Error { get; } diff --git a/src/Middleware/Diagnostics.Abstractions/src/ErrorContext.cs b/src/Middleware/Diagnostics.Abstractions/src/ErrorContext.cs new file mode 100644 index 0000000000..5c4f973f5d --- /dev/null +++ b/src/Middleware/Diagnostics.Abstractions/src/ErrorContext.cs @@ -0,0 +1,35 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Diagnostics +{ + /// + /// Provides context about the error currently being handled bt the DeveloperExceptionPageMiddleware. + /// + public class ErrorContext + { + /// + /// Initializes the ErrorContext with the specified and . + /// + /// + /// + public ErrorContext(HttpContext httpContext, Exception exception) + { + HttpContext = httpContext ?? throw new ArgumentNullException(nameof(httpContext)); + Exception = exception ?? throw new ArgumentNullException(nameof(exception)); + } + + /// + /// The . + /// + public HttpContext HttpContext { get; } + + /// + /// The thrown during request processing. + /// + public Exception Exception { get; } + } +} diff --git a/src/Middleware/Diagnostics.Abstractions/src/IDeveloperPageExceptionFilter.cs b/src/Middleware/Diagnostics.Abstractions/src/IDeveloperPageExceptionFilter.cs new file mode 100644 index 0000000000..e908f48006 --- /dev/null +++ b/src/Middleware/Diagnostics.Abstractions/src/IDeveloperPageExceptionFilter.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Diagnostics +{ + /// + /// Provides an extensiblity point for changing the behavior of the DeveloperExceptionPageMiddleware. + /// + public interface IDeveloperPageExceptionFilter + { + /// + /// An exception handling method that is used to either format the exception or delegate to the next handler in the chain. + /// + /// The error context. + /// The next filter in the pipeline. + /// A task the completes when the handler is done executing. + Task HandleExceptionAsync(ErrorContext errorContext, Func next); + } +} diff --git a/src/Middleware/Diagnostics.Abstractions/src/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj b/src/Middleware/Diagnostics.Abstractions/src/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj index 94a61d84d0..d7f67d031c 100644 --- a/src/Middleware/Diagnostics.Abstractions/src/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj +++ b/src/Middleware/Diagnostics.Abstractions/src/Microsoft.AspNetCore.Diagnostics.Abstractions.csproj @@ -1,4 +1,4 @@ - + ASP.NET Core diagnostics middleware abstractions and feature interface definitions. @@ -9,4 +9,8 @@ aspnetcore;diagnostics + + + + diff --git a/src/Middleware/Diagnostics/ref/Microsoft.AspNetCore.Diagnostics.netcoreapp3.0.cs b/src/Middleware/Diagnostics/ref/Microsoft.AspNetCore.Diagnostics.netcoreapp3.0.cs index d0b332177b..3f6402c5d8 100644 --- a/src/Middleware/Diagnostics/ref/Microsoft.AspNetCore.Diagnostics.netcoreapp3.0.cs +++ b/src/Middleware/Diagnostics/ref/Microsoft.AspNetCore.Diagnostics.netcoreapp3.0.cs @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Diagnostics { public partial class DeveloperExceptionPageMiddleware { - public DeveloperExceptionPageMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, Microsoft.Extensions.Options.IOptions options, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, Microsoft.AspNetCore.Hosting.IWebHostEnvironment hostingEnvironment, System.Diagnostics.DiagnosticSource diagnosticSource) { } + public DeveloperExceptionPageMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, Microsoft.Extensions.Options.IOptions options, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, Microsoft.AspNetCore.Hosting.IWebHostEnvironment hostingEnvironment, System.Diagnostics.DiagnosticSource diagnosticSource, System.Collections.Generic.IEnumerable filters) { } [System.Diagnostics.DebuggerStepThroughAttribute] public System.Threading.Tasks.Task Invoke(Microsoft.AspNetCore.Http.HttpContext context) { throw null; } } diff --git a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs index ea568010ca..22c7c39b47 100644 --- a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs +++ b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs @@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Diagnostics private readonly IFileProvider _fileProvider; private readonly DiagnosticSource _diagnosticSource; private readonly ExceptionDetailsProvider _exceptionDetailsProvider; + private readonly Func _exceptionHandler; /// /// Initializes a new instance of the class @@ -40,12 +41,14 @@ namespace Microsoft.AspNetCore.Diagnostics /// /// /// + /// public DeveloperExceptionPageMiddleware( RequestDelegate next, IOptions options, ILoggerFactory loggerFactory, IWebHostEnvironment hostingEnvironment, - DiagnosticSource diagnosticSource) + DiagnosticSource diagnosticSource, + IEnumerable filters) { if (next == null) { @@ -57,12 +60,24 @@ namespace Microsoft.AspNetCore.Diagnostics throw new ArgumentNullException(nameof(options)); } + if (filters == null) + { + throw new ArgumentNullException(nameof(filters)); + } + _next = next; _options = options.Value; _logger = loggerFactory.CreateLogger(); _fileProvider = _options.FileProvider ?? hostingEnvironment.ContentRootFileProvider; _diagnosticSource = diagnosticSource; _exceptionDetailsProvider = new ExceptionDetailsProvider(_fileProvider, _options.SourceCodeLineCount); + _exceptionHandler = DisplayException; + + foreach (var filter in filters.Reverse()) + { + var nextFilter = _exceptionHandler; + _exceptionHandler = errorContext => filter.HandleExceptionAsync(errorContext, nextFilter); + } } /// @@ -91,7 +106,7 @@ namespace Microsoft.AspNetCore.Diagnostics context.Response.Clear(); context.Response.StatusCode = 500; - await DisplayException(context, ex); + await _exceptionHandler(new ErrorContext(context, ex)); if (_diagnosticSource.IsEnabled("Microsoft.AspNetCore.Diagnostics.UnhandledException")) { @@ -110,15 +125,15 @@ namespace Microsoft.AspNetCore.Diagnostics } // Assumes the response headers have not been sent. If they have, still attempt to write to the body. - private Task DisplayException(HttpContext context, Exception ex) + private Task DisplayException(ErrorContext errorContext) { - var compilationException = ex as ICompilationException; + var compilationException = errorContext.Exception as ICompilationException; if (compilationException != null) { - return DisplayCompilationException(context, compilationException); + return DisplayCompilationException(errorContext.HttpContext, compilationException); } - return DisplayRuntimeException(context, ex); + return DisplayRuntimeException(errorContext.HttpContext, errorContext.Exception); } private Task DisplayCompilationException( @@ -215,7 +230,7 @@ namespace Microsoft.AspNetCore.Diagnostics } } } - + var request = context.Request; var model = new ErrorPageModel diff --git a/src/Middleware/Diagnostics/test/UnitTests/DeveloperExceptionPageMiddlewareTest.cs b/src/Middleware/Diagnostics/test/UnitTests/DeveloperExceptionPageMiddlewareTest.cs index 2d0f6b42db..8552a2da8e 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/DeveloperExceptionPageMiddlewareTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/DeveloperExceptionPageMiddlewareTest.cs @@ -3,12 +3,11 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Diagnostics; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -46,6 +45,88 @@ namespace Microsoft.AspNetCore.Diagnostics Assert.Null(listener.DiagnosticHandledException?.Exception); } + [Fact] + public async Task ExceptionPageFiltersAreApplied() + { + // Arrange + var builder = new WebHostBuilder() + .ConfigureServices(services => + { + services.AddSingleton(); + }) + .Configure(app => + { + app.UseDeveloperExceptionPage(); + app.Run(context => + { + throw new Exception("Test exception"); + }); + }); + var server = new TestServer(builder); + + // Act + var response = await server.CreateClient().GetAsync("/path"); + + // Assert + Assert.Equal("Test exception", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task ExceptionFilterCallingNextWorks() + { + // Arrange + var builder = new WebHostBuilder() + .ConfigureServices(services => + { + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + }) + .Configure(app => + { + app.UseDeveloperExceptionPage(); + app.Run(context => + { + throw new Exception("Test exception"); + }); + }); + var server = new TestServer(builder); + + // Act + var response = await server.CreateClient().GetAsync("/path"); + + // Assert + Assert.Equal("Bad format exception!", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task ExceptionPageFiltersAreAppliedInOrder() + { + // Arrange + var builder = new WebHostBuilder() + .ConfigureServices(services => + { + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + }) + .Configure(app => + { + app.UseDeveloperExceptionPage(); + app.Run(context => + { + throw new Exception("Test exception"); + }); + }); + var server = new TestServer(builder); + + // Act + var response = await server.CreateClient().GetAsync("/path"); + + // Assert + Assert.Equal("An error occurred", await response.Content.ReadAsStringAsync()); + } + public static TheoryData CompilationExceptionData { get @@ -140,5 +221,45 @@ namespace Microsoft.AspNetCore.Diagnostics public IEnumerable CompilationFailures { get; } } + + public class ExceptionMessageFilter : IDeveloperPageExceptionFilter + { + public Task HandleExceptionAsync(ErrorContext context, Func next) + { + return context.HttpContext.Response.WriteAsync(context.Exception.Message); + } + } + + public class ExceptionToStringFilter : IDeveloperPageExceptionFilter + { + public Task HandleExceptionAsync(ErrorContext context, Func next) + { + return context.HttpContext.Response.WriteAsync(context.Exception.ToString()); + } + } + + public class AlwaysThrowSameMessageFilter : IDeveloperPageExceptionFilter + { + public Task HandleExceptionAsync(ErrorContext context, Func next) + { + return context.HttpContext.Response.WriteAsync("An error occurred"); + } + } + + public class AlwaysBadFormatExceptionFilter : IDeveloperPageExceptionFilter + { + public Task HandleExceptionAsync(ErrorContext context, Func next) + { + return next(new ErrorContext(context.HttpContext, new FormatException("Bad format exception!"))); + } + } + + public class PassThroughExceptionFilter : IDeveloperPageExceptionFilter + { + public Task HandleExceptionAsync(ErrorContext context, Func next) + { + return next(context); + } + } } }