From d237d2b9065e0d3e998b30495515756c422b4066 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 30 Mar 2023 10:18:04 -0700 Subject: [PATCH] Connect module exception handling to HttpContext HttpContext/HttpServerUtility had methods related to error handling. When these were added to the adapter, there was no obvious way to implement them, so they were added as no-ops. However, with module support, their utility can be implemented as module events was the main way people interacted with these errors. --- .../HttpApplicationMiddleware.cs | 2 + .../RequestHttpApplicationFeature.cs | 11 ++- .../Adapters/IRequestExceptionFeature.cs | 18 ++++ .../Generated/Ref.Standard.cs | 11 +++ .../HttpContext.cs | 16 ++-- .../HttpContextBase.cs | 10 ++ .../HttpContextWrapper.cs | 8 ++ .../HttpServerUtility.cs | 8 +- .../HttpContextTests.cs | 93 +++++++++++++++++++ 9 files changed, 164 insertions(+), 13 deletions(-) create mode 100644 src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IRequestExceptionFeature.cs diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationMiddleware.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationMiddleware.cs index ff51752ec5..fb5dbc8d09 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationMiddleware.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationMiddleware.cs @@ -28,6 +28,7 @@ public async Task InvokeAsync(HttpContextCore context) context.Features.Set(httpApplicationFeature); context.Features.Set(httpApplicationFeature); + context.Features.Set(httpApplicationFeature); try { @@ -48,6 +49,7 @@ public async Task InvokeAsync(HttpContextCore context) { context.Features.Set(endFeature); context.Features.Set(null); + context.Features.Set(null); _pool.Return(app); } diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/RequestHttpApplicationFeature.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/RequestHttpApplicationFeature.cs index 7e8924b56b..3619b90ad1 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/RequestHttpApplicationFeature.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/RequestHttpApplicationFeature.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.SystemWebAdapters; -internal sealed class RequestHttpApplicationFeature : IHttpApplicationFeature, IHttpResponseEndFeature +internal sealed class RequestHttpApplicationFeature : IHttpApplicationFeature, IHttpResponseEndFeature, IRequestExceptionFeature { private readonly IHttpResponseEndFeature _previous; private List? _exceptions; @@ -33,7 +33,7 @@ ValueTask IHttpApplicationFeature.RaiseEventAsync(ApplicationEvent @event) return ValueTask.CompletedTask; } - [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Need to handle all exceptions")] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Must handle all exceptions here")] private void RaiseEvent(ApplicationEvent appEvent, bool suppressThrow) { (CurrentNotification, IsPostNotification) = appEvent switch @@ -111,4 +111,11 @@ async Task IHttpResponseEndFeature.EndAsync() } private void AddError(Exception ex) => (_exceptions ??= new()).Add(ex); + + void IRequestExceptionFeature.Add(Exception exception) => AddError(exception); + + IReadOnlyList IRequestExceptionFeature.Exceptions + => ((IReadOnlyList?)_exceptions) ?? Array.Empty(); + + void IRequestExceptionFeature.Clear() => _exceptions = null; } diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IRequestExceptionFeature.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IRequestExceptionFeature.cs new file mode 100644 index 0000000000..330df10401 --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IRequestExceptionFeature.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NET6_0_OR_GREATER +using System; +using System.Collections.Generic; + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +internal interface IRequestExceptionFeature +{ + IReadOnlyList Exceptions { get; } + + void Add(Exception exception); + + void Clear(); +} +#endif diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs index 94c11b434a..dc7e28ff81 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs @@ -105,11 +105,13 @@ public partial class HttpBrowserCapabilitiesWrapper : System.Web.HttpBrowserCapa public partial class HttpContext : System.IServiceProvider { internal HttpContext() { } + public System.Exception[] AllErrors { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public System.Web.HttpApplicationState Application { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public System.Web.HttpApplication ApplicationInstance { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public System.Web.Caching.Cache Cache { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public static System.Web.HttpContext Current { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public System.Web.RequestNotification CurrentNotification { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public System.Exception Error { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public bool IsDebuggingEnabled { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public bool IsPostNotification { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public System.Collections.IDictionary Items { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } @@ -119,6 +121,7 @@ internal HttpContext() { } public System.Web.SessionState.HttpSessionState Session { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public System.DateTime Timestamp { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public System.Security.Principal.IPrincipal User { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} set { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public void AddError(System.Exception ex) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} public void ClearError() { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} public System.Web.ISubscriptionToken DisposeOnPipelineCompleted(System.IDisposable target) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} public void RewritePath(string path) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} @@ -127,10 +130,12 @@ internal HttpContext() { } public partial class HttpContextBase : System.IServiceProvider { protected HttpContextBase() { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} + public virtual System.Exception[] AllErrors { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual System.Web.HttpApplicationState Application { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual System.Web.HttpApplication ApplicationInstance { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual System.Web.Caching.Cache Cache { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual System.Web.RequestNotification CurrentNotification { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public virtual System.Exception Error { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual bool IsDebuggingEnabled { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual bool IsPostNotification { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual System.Collections.IDictionary Items { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } @@ -140,15 +145,19 @@ public partial class HttpContextBase : System.IServiceProvider public virtual System.Web.HttpSessionStateBase Session { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual System.DateTime Timestamp { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public virtual System.Security.Principal.IPrincipal User { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} set { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public virtual void AddError(System.Exception ex) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} + public virtual void ClearError() { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} public virtual object GetService(System.Type serviceType) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public partial class HttpContextWrapper : System.Web.HttpContextBase { public HttpContextWrapper(System.Web.HttpContext httpContext) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} + public override System.Exception[] AllErrors { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override System.Web.HttpApplicationState Application { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override System.Web.HttpApplication ApplicationInstance { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override System.Web.Caching.Cache Cache { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override System.Web.RequestNotification CurrentNotification { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public override System.Exception Error { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override bool IsDebuggingEnabled { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override bool IsPostNotification { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override System.Collections.IDictionary Items { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } @@ -158,6 +167,8 @@ public partial class HttpContextWrapper : System.Web.HttpContextBase public override System.Web.HttpSessionStateBase Session { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override System.DateTime Timestamp { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public override System.Security.Principal.IPrincipal User { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} set { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public override void AddError(System.Exception ex) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} + public override void ClearError() { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public sealed partial class HttpCookie { diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContext.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContext.cs index 860df49faa..9441d25d93 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContext.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContext.cs @@ -3,6 +3,7 @@ using System.Collections; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Security.Claims; using System.Security.Principal; using System.Web.Caching; @@ -41,6 +42,15 @@ internal HttpContext(HttpContextCore context) public HttpServerUtility Server => _server ??= new(_context); + public Exception? Error => _context.Features.Get()?.Exceptions is [{ } error, ..] ? error : null; + + [SuppressMessage("Performance", "CA1819:Properties should not return arrays", Justification = Constants.ApiFromAspNet)] + public Exception[] AllErrors => _context.Features.Get()?.Exceptions.ToArray() ?? Array.Empty(); + + public void ClearError() => _context.Features.Get()?.Clear(); + + public void AddError(Exception ex) => _context.Features.Get()?.Add(ex); + public RequestNotification CurrentNotification => _context.Features.GetRequired().CurrentNotification; public bool IsPostNotification => _context.Features.GetRequired().IsPostNotification; @@ -64,12 +74,6 @@ public IPrincipal User public HttpSessionState? Session => _context.Features.Get(); - [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = Constants.ApiFromAspNet)] - public void ClearError() - { - // Intentionally implemented without any behavior as there's nothing equivalent in ASP.NET Core - } - public DateTime Timestamp { get; } = DateTime.UtcNow.ToLocalTime(); public void RewritePath(string path) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextBase.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextBase.cs index 066735a0d0..943539a43a 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextBase.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextBase.cs @@ -19,6 +19,16 @@ protected HttpContextBase() public virtual HttpResponseBase Response => throw new NotImplementedException(); + [SuppressMessage("Naming", "CA1716:Identifiers should not match keywords", Justification = Constants.ApiFromAspNet)] + public virtual Exception? Error => throw new NotImplementedException(); + + [SuppressMessage("Performance", "CA1819:Properties should not return arrays", Justification = Constants.ApiFromAspNet)] + public virtual Exception[] AllErrors => throw new NotImplementedException(); + + public virtual void ClearError() => throw new NotImplementedException(); + + public virtual void AddError(Exception ex) => throw new NotImplementedException(); + public virtual HttpApplication ApplicationInstance => throw new NotImplementedException(); public virtual HttpApplicationState Application => throw new NotImplementedException(); diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextWrapper.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextWrapper.cs index fe4aac6552..ac2c78ff5d 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextWrapper.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpContextWrapper.cs @@ -29,6 +29,14 @@ public HttpContextWrapper(HttpContext httpContext) public override bool IsDebuggingEnabled => _context.IsDebuggingEnabled; + public override void AddError(Exception ex) => _context.AddError(ex); + + public override Exception[] AllErrors => _context.AllErrors; + + public override void ClearError() => _context.ClearError(); + + public override Exception? Error => _context.Error; + public override HttpRequestBase Request => _request ??= new HttpRequestWrapper(_context.Request); public override HttpResponseBase Response => _response ??= new HttpResponseWrapper(_context.Response); diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpServerUtility.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpServerUtility.cs index 6f53db2813..c9adbae707 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpServerUtility.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpServerUtility.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; +using System.Linq; using Microsoft.AspNetCore.SystemWebAdapters; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; @@ -42,12 +43,9 @@ public string MapPath(string? path) .TrimEnd(Path.DirectorySeparatorChar); } - [Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1024:Use properties where appropriate", Justification = Constants.ApiFromAspNet)] - public Exception? GetLastError() => null; + public Exception? GetLastError() => _context.GetAdapter().Error; - public void ClearError() - { - } + public void ClearError() => _context.GetAdapter().ClearError(); /// /// This method is similar to but handles the trailing character that diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/HttpContextTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/HttpContextTests.cs index 5d44c7dc42..68c17e6d73 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/HttpContextTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/HttpContextTests.cs @@ -243,6 +243,99 @@ public void DisposeOnPipelineCompletedDisposed() disposable.Verify(d => d.Dispose(), Times.Once); } + [Fact] + public void ErrorNoFeature() + { + // Arrange + var context = new HttpContext(new DefaultHttpContext()); + + // Assert + var error = context.Error; + var allErrors = context.AllErrors; + + // No ops + context.ClearError(); + context.AddError(new InvalidOperationException()); + + // Assert + Assert.Null(error); + Assert.Empty(allErrors); + } + + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [Theory] + public void ErrorWithFeatureGet(int length) + { + // Arrange + var coreContext = new DefaultHttpContext(); + var context = new HttpContext(coreContext); + + var exceptionFeature = new Mock(); + var errors = new List(); + + for (var i = 0; i < length; i++) + { + errors.Add(new Mock().Object); + } + + exceptionFeature.Setup(f => f.Exceptions).Returns(errors); + coreContext.Features.Set(exceptionFeature.Object); + + // Act + var error = context.Error; + var allErrors = context.AllErrors; + + // Assert + Assert.Equal(allErrors, errors); + Assert.NotSame(allErrors, errors); + + if (length > 0) + { + Assert.Same(error, errors[0]); + } + else + { + Assert.Null(error); + } + } + + [Fact] + public void ErrorWithFeatureClear() + { + // Arrange + var coreContext = new DefaultHttpContext(); + var context = new HttpContext(coreContext); + + var exceptionFeature = new Mock(); + coreContext.Features.Set(exceptionFeature.Object); + + // Act + context.ClearError(); + + // Assert + exceptionFeature.Verify(f => f.Clear(), Times.Once); + } + + [Fact] + public void ErrorWithFeatureAdd() + { + // Arrange + var coreContext = new DefaultHttpContext(); + var context = new HttpContext(coreContext); + + var error = new InvalidOperationException(); + var exceptionFeature = new Mock(); + coreContext.Features.Set(exceptionFeature.Object); + + // Act + context.AddError(error); + + // Assert + exceptionFeature.Verify(f => f.Add(error), Times.Once); + } + [InlineData("path1", "/path1", "", 0)] [InlineData("/path1", "/path1", "", 0)] [InlineData("path1?", "/path1", "", 0)]