diff --git a/src/Volo.Abp.AspNetCore.Mvc/Microsoft/AspNetCore/Builder/AbpAspNetCoreMvcApplicationBuilderExtensions.cs b/src/Volo.Abp.AspNetCore.Mvc/Microsoft/AspNetCore/Builder/AbpAspNetCoreMvcApplicationBuilderExtensions.cs index 34c669da52..91d0df29bb 100644 --- a/src/Volo.Abp.AspNetCore.Mvc/Microsoft/AspNetCore/Builder/AbpAspNetCoreMvcApplicationBuilderExtensions.cs +++ b/src/Volo.Abp.AspNetCore.Mvc/Microsoft/AspNetCore/Builder/AbpAspNetCoreMvcApplicationBuilderExtensions.cs @@ -1,4 +1,5 @@ -using Volo.Abp.AspNetCore.Mvc.Uow; +using Volo.Abp.AspNetCore.Mvc.ExceptionHandling; +using Volo.Abp.AspNetCore.Mvc.Uow; namespace Microsoft.AspNetCore.Builder { @@ -6,7 +7,21 @@ namespace Microsoft.AspNetCore.Builder { public static IApplicationBuilder UseUnitOfWork(this IApplicationBuilder app) { - return app.UseMiddleware(); + return app + .UseAbpExceptionHandling() + .UseMiddleware(); + } + + public static IApplicationBuilder UseAbpExceptionHandling(this IApplicationBuilder app) + { + //Prevent multiple add + if (app.Properties.ContainsKey("_AbpExceptionHandlingMiddleware_Added")) //TODO: Constant + { + return app; + } + + app.Properties["_AbpExceptionHandlingMiddleware_Added"] = true; + return app.UseMiddleware(); } } } diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpActionInfoInHttpContext.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpActionInfoInHttpContext.cs new file mode 100644 index 0000000000..31620cbf5d --- /dev/null +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpActionInfoInHttpContext.cs @@ -0,0 +1,7 @@ +namespace Volo.Abp.AspNetCore.Mvc +{ + public class AbpActionInfoInHttpContext + { + public bool IsObjectResult { get; set; } + } +} \ No newline at end of file diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs index 3400d6af5d..df1e1c7aea 100644 --- a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs @@ -1,16 +1,12 @@ using System.Collections.Generic; -using System.Net; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Primitives; -using Volo.Abp.Authorization; using Volo.Abp.DependencyInjection; -using Volo.Abp.Domain.Entities; using Volo.Abp.Http; -using Volo.Abp.Validation; namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling { @@ -21,10 +17,12 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling //TODO: Use EventBus to trigger error handled event like in previous ABP private readonly IExceptionToErrorInfoConverter _errorInfoConverter; + private readonly HttpExceptionStatusCodeFinder _statusCodeFinder; - public AbpExceptionFilter(IExceptionToErrorInfoConverter errorInfoConverter) + public AbpExceptionFilter(IExceptionToErrorInfoConverter errorInfoConverter, HttpExceptionStatusCodeFinder statusCodeFinder) { _errorInfoConverter = errorInfoConverter; + _statusCodeFinder = statusCodeFinder; Logger = NullLogger.Instance; } @@ -49,7 +47,7 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling return; } - context.HttpContext.Response.StatusCode = GetStatusCode(context); + context.HttpContext.Response.StatusCode = _statusCodeFinder.GetStatusCode(context.HttpContext, context.Exception); context.HttpContext.Response.Headers.Add(new KeyValuePair("_AbpErrorFormat", "true")); context.Result = new ObjectResult( @@ -62,27 +60,5 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling context.Exception = null; //Handled! } - - private static int GetStatusCode(ExceptionContext context) - { - if (context.Exception is AbpAuthorizationException) - { - return context.HttpContext.User.Identity.IsAuthenticated - ? (int)HttpStatusCode.Forbidden - : (int)HttpStatusCode.Unauthorized; - } - - if (context.Exception is AbpValidationException) - { - return (int)HttpStatusCode.BadRequest; - } - - if (context.Exception is EntityNotFoundException) - { - return (int)HttpStatusCode.NotFound; - } - - return (int)HttpStatusCode.InternalServerError; - } } } diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionHandlingMiddleware.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionHandlingMiddleware.cs new file mode 100644 index 0000000000..b769861cf7 --- /dev/null +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionHandlingMiddleware.cs @@ -0,0 +1,92 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; +using Microsoft.Net.Http.Headers; +using Volo.Abp.AspNetCore.Mvc.Uow; +using Volo.Abp.Http; +using Volo.Abp.Json; + +namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling +{ + public class AbpExceptionHandlingMiddleware + { + private readonly RequestDelegate _next; + private readonly ILogger _logger; + + private readonly Func _clearCacheHeadersDelegate; + + public AbpExceptionHandlingMiddleware(RequestDelegate next, ILogger logger) + { + _next = next; + _logger = logger; + + _clearCacheHeadersDelegate = ClearCacheHeaders; + } + + public async Task Invoke(HttpContext httpContext) + { + try + { + await _next(httpContext); + } + catch (Exception ex) + { + // We can't do anything if the response has already started, just abort. + if (httpContext.Response.HasStarted) + { + _logger.LogWarning("An exception occured, but response has already started!"); + throw; + } + + if (httpContext.Items["_AbpActionInfo"] is AbpActionInfoInHttpContext actionInfo) + { + if (actionInfo.IsObjectResult) + { + await HandleAndWrapException(httpContext, ex); + return; + } + } + + throw; + } + } + + private async Task HandleAndWrapException(HttpContext httpContext, Exception exception) + { + _logger.LogException(exception); + + var errorInfoConverter = httpContext.RequestServices.GetRequiredService(); + var statusCodeFinder = httpContext.RequestServices.GetRequiredService(); + var jsonSerializer = httpContext.RequestServices.GetRequiredService(); + + httpContext.Response.Clear(); + httpContext.Response.StatusCode = statusCodeFinder.GetStatusCode(httpContext, exception); + httpContext.Response.OnStarting(_clearCacheHeadersDelegate, httpContext.Response); + httpContext.Response.Headers.Add(new KeyValuePair("_AbpErrorFormat", "true")); //TODO: Constant + + await httpContext.Response.WriteAsync( + jsonSerializer.Serialize( + new RemoteServiceErrorResponse( + errorInfoConverter.Convert(exception) + ) + ) + ); + } + + private Task ClearCacheHeaders(object state) + { + var response = (HttpResponse)state; + + response.Headers[HeaderNames.CacheControl] = "no-cache"; + response.Headers[HeaderNames.Pragma] = "no-cache"; + response.Headers[HeaderNames.Expires] = "-1"; + response.Headers.Remove(HeaderNames.ETag); + + return Task.CompletedTask; + } + } +} diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/HttpExceptionStatusCodeFinder.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/HttpExceptionStatusCodeFinder.cs new file mode 100644 index 0000000000..c4789a75e2 --- /dev/null +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/HttpExceptionStatusCodeFinder.cs @@ -0,0 +1,35 @@ +using System; +using System.Net; +using Microsoft.AspNetCore.Http; +using Volo.Abp.Authorization; +using Volo.Abp.DependencyInjection; +using Volo.Abp.Domain.Entities; +using Volo.Abp.Validation; + +namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling +{ + public class HttpExceptionStatusCodeFinder : ITransientDependency + { + public virtual int GetStatusCode(HttpContext httpContext, Exception exception) + { + if (exception is AbpAuthorizationException) + { + return httpContext.User.Identity.IsAuthenticated + ? (int)HttpStatusCode.Forbidden + : (int)HttpStatusCode.Unauthorized; + } + + if (exception is AbpValidationException) + { + return (int)HttpStatusCode.BadRequest; + } + + if (exception is EntityNotFoundException) + { + return (int)HttpStatusCode.NotFound; + } + + return (int)HttpStatusCode.InternalServerError; + } + } +} \ No newline at end of file diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs index 8967f45aae..3f63bf70e1 100644 --- a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs @@ -1,5 +1,4 @@ -using System; -using System.Threading.Tasks; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Volo.Abp.Uow; @@ -8,15 +7,17 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow public class AbpUnitOfWorkMiddleware { private readonly RequestDelegate _next; + private readonly IUnitOfWorkManager _unitOfWorkManager; - public AbpUnitOfWorkMiddleware(RequestDelegate next) + public AbpUnitOfWorkMiddleware(RequestDelegate next, IUnitOfWorkManager unitOfWorkManager) { _next = next; + _unitOfWorkManager = unitOfWorkManager; } - public async Task Invoke(HttpContext httpContext, IUnitOfWorkManager unitOfWorkManager) + public async Task Invoke(HttpContext httpContext) { - using (var uow = unitOfWorkManager.Reserve(AbpUowActionFilter.UnitOfWorkReservationName)) + using (var uow = _unitOfWorkManager.Reserve(AbpUowActionFilter.UnitOfWorkReservationName)) { await _next(httpContext); await uow.CompleteAsync(httpContext.RequestAborted); diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs index 3016df398c..2bb1ade246 100644 --- a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs @@ -1,6 +1,8 @@ using System; using System.Net.Http; +using System.Reflection; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.Options; @@ -33,6 +35,8 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow var methodInfo = context.ActionDescriptor.GetMethodInfo(); var unitOfWorkAttr = UnitOfWorkHelper.GetUnitOfWorkAttributeOrNull(methodInfo); + SetAbpActionInfoToHttpContext(context.HttpContext, methodInfo); + if (unitOfWorkAttr?.IsDisabled == true) { await next(); @@ -62,6 +66,14 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow } } + private static void SetAbpActionInfoToHttpContext(HttpContext context, MethodInfo methodInfo) + { + context.Items["_AbpActionInfo"] = new AbpActionInfoInHttpContext + { + IsObjectResult = ActionResultHelper.IsObjectResult(methodInfo.ReturnType) + }; + } + private UnitOfWorkOptions CreateOptions(ActionExecutingContext context, UnitOfWorkAttribute unitOfWorkAttribute) { var options = new UnitOfWorkOptions(); diff --git a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs index 6bf6d992a1..b54547b5fe 100644 --- a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs +++ b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs @@ -1,7 +1,5 @@ using System; using System.Data; -using System.Threading; -using JetBrains.Annotations; namespace Volo.Abp.Uow { diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/SimpleController.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/SimpleController.cs index 213bdc175e..ffa8322592 100644 --- a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/SimpleController.cs +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/SimpleController.cs @@ -14,5 +14,10 @@ namespace Volo.Abp.AspNetCore.App { return View(); } + + public ActionResult ExceptionOnRazor() + { + return View(); + } } } diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/Views/Simple/ExceptionOnRazor.cshtml b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/Views/Simple/ExceptionOnRazor.cshtml new file mode 100644 index 0000000000..3a529fbf1f --- /dev/null +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/Views/Simple/ExceptionOnRazor.cshtml @@ -0,0 +1,5 @@ +@using Volo.Abp.Ui +

Exception test

+@{ + throw new UserFriendlyException("test exception!"); +} \ No newline at end of file diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/SimpleController_Tests.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/SimpleController_Tests.cs index c6378f9034..1c5f6288eb 100644 --- a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/SimpleController_Tests.cs +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/SimpleController_Tests.cs @@ -1,6 +1,7 @@ using System.Threading.Tasks; using Shouldly; using Volo.Abp.AspNetCore.App; +using Volo.Abp.Ui; using Xunit; namespace Volo.Abp.AspNetCore.Mvc @@ -26,5 +27,16 @@ namespace Volo.Abp.AspNetCore.Mvc result.Trim().ShouldBe("

About

"); } + + [Fact] + public async Task ActionResult_ViewResult_Exception() + { + await Assert.ThrowsAsync(async () => + { + await GetResponseAsStringAsync( + GetUrl(nameof(SimpleController.ExceptionOnRazor)) + ); + }); + } } } diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs index 1616efd77c..28eb71f624 100644 --- a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs @@ -1,7 +1,10 @@ -using System.Net; +using System.Linq; +using System.Net; using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; using Shouldly; using Volo.Abp.Http; +using Volo.Abp.Json; using Xunit; namespace Volo.Abp.AspNetCore.Mvc.Uow @@ -19,7 +22,14 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow [Fact] public async Task Should_Gracefully_Handle_Exceptions_On_Complete() { - var result = await GetResponseAsObjectAsync("/api/unitofwork-test/ExceptionOnComplete", HttpStatusCode.InternalServerError); + var response = await GetResponseAsync("/api/unitofwork-test/ExceptionOnComplete", HttpStatusCode.InternalServerError); + + response.Headers.GetValues("_AbpErrorFormat").FirstOrDefault().ShouldBe("true"); + + var resultAsString = await response.Content.ReadAsStringAsync(); + + var result = ServiceProvider.GetRequiredService().Deserialize(resultAsString); + result.Error.ShouldNotBeNull(); result.Error.Message.ShouldBe(TestUnitOfWorkConfig.ExceptionOnCompleteMessage); }