Browse Source

Razor page's exception filter ignores void and task return types.

Resolve #3969
pull/3974/head
maliming 6 years ago
parent
commit
d73281680f
  1. 11
      framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ActionResultHelper.cs
  2. 18
      framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionPageFilter.cs
  3. 6
      framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowPageFilter.cs
  4. 28
      framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage.cshtml.cs
  5. 64
      framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage_Tests.cs
  6. 6
      framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Features/FeatureTestPage.cshtml.cs
  7. 8
      framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestPage.cshtml.cs

11
framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ActionResultHelper.cs

@ -8,7 +8,7 @@ namespace Volo.Abp.AspNetCore.Mvc
{ {
public static class ActionResultHelper public static class ActionResultHelper
{ {
public static List<Type> ObjectResultTypes { get; } public static List<Type> ObjectResultTypes { get; }
static ActionResultHelper() static ActionResultHelper()
{ {
@ -20,10 +20,15 @@ namespace Volo.Abp.AspNetCore.Mvc
}; };
} }
public static bool IsObjectResult(Type returnType) public static bool IsObjectResult(Type returnType, params Type[] excludeTypes)
{ {
returnType = AsyncHelper.UnwrapTask(returnType); returnType = AsyncHelper.UnwrapTask(returnType);
if (!excludeTypes.IsNullOrEmpty() && excludeTypes.Any(t => t.IsAssignableFrom(returnType)))
{
return false;
}
if (!typeof(IActionResult).IsAssignableFrom(returnType)) if (!typeof(IActionResult).IsAssignableFrom(returnType))
{ {
return true; return true;
@ -32,4 +37,4 @@ namespace Volo.Abp.AspNetCore.Mvc
return ObjectResultTypes.Any(t => t.IsAssignableFrom(returnType)); return ObjectResultTypes.Any(t => t.IsAssignableFrom(returnType));
} }
} }
} }

18
framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionPageFilter.cs

@ -1,4 +1,4 @@
using System; using System;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc;
@ -25,7 +25,7 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
public AbpExceptionPageFilter( public AbpExceptionPageFilter(
IExceptionToErrorInfoConverter errorInfoConverter, IExceptionToErrorInfoConverter errorInfoConverter,
IHttpExceptionStatusCodeFinder statusCodeFinder, IHttpExceptionStatusCodeFinder statusCodeFinder,
IJsonSerializer jsonSerializer) IJsonSerializer jsonSerializer)
{ {
_errorInfoConverter = errorInfoConverter; _errorInfoConverter = errorInfoConverter;
@ -34,13 +34,12 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
Logger = NullLogger<AbpExceptionPageFilter>.Instance; Logger = NullLogger<AbpExceptionPageFilter>.Instance;
} }
public Task OnPageHandlerSelectionAsync(PageHandlerSelectedContext context) public Task OnPageHandlerSelectionAsync(PageHandlerSelectedContext context)
{ {
return Task.CompletedTask; return Task.CompletedTask;
} }
public async Task OnPageHandlerExecutionAsync(PageHandlerExecutingContext context, PageHandlerExecutionDelegate next) public async Task OnPageHandlerExecutionAsync(PageHandlerExecutingContext context, PageHandlerExecutionDelegate next)
{ {
if (context.HandlerMethod == null || !ShouldHandleException(context)) if (context.HandlerMethod == null || !ShouldHandleException(context))
@ -54,20 +53,20 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
{ {
return;; return;;
} }
await HandleAndWrapException(pageHandlerExecutedContext); await HandleAndWrapException(pageHandlerExecutedContext);
} }
protected virtual bool ShouldHandleException(PageHandlerExecutingContext context) protected virtual bool ShouldHandleException(PageHandlerExecutingContext context)
{ {
//TODO: Create DontWrap attribute to control wrapping..? //TODO: Create DontWrap attribute to control wrapping..?
if (context.ActionDescriptor.IsPageAction() && if (context.ActionDescriptor.IsPageAction() &&
ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType)) ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType, typeof(void)))
{ {
return true; return true;
} }
if (context.HttpContext.Request.CanAccept(MimeTypes.Application.Json)) if (context.HttpContext.Request.CanAccept(MimeTypes.Application.Json))
{ {
return true; return true;
@ -107,6 +106,5 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
context.Exception = null; //Handled! context.Exception = null; //Handled!
} }
} }
} }

6
framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowPageFilter.cs

@ -38,7 +38,7 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
context.HttpContext.Items["_AbpActionInfo"] = new AbpActionInfoInHttpContext context.HttpContext.Items["_AbpActionInfo"] = new AbpActionInfoInHttpContext
{ {
IsObjectResult = ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType) IsObjectResult = ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType, typeof(void))
}; };
if (unitOfWorkAttr?.IsDisabled == true) if (unitOfWorkAttr?.IsDisabled == true)
@ -71,7 +71,7 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
} }
} }
} }
private AbpUnitOfWorkOptions CreateOptions(PageHandlerExecutingContext context, UnitOfWorkAttribute unitOfWorkAttribute) private AbpUnitOfWorkOptions CreateOptions(PageHandlerExecutingContext context, UnitOfWorkAttribute unitOfWorkAttribute)
{ {
var options = new AbpUnitOfWorkOptions(); var options = new AbpUnitOfWorkOptions();
@ -102,4 +102,4 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
return result.Exception == null || result.ExceptionHandled; return result.Exception == null || result.ExceptionHandled;
} }
} }
} }

28
framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage.cshtml.cs

@ -1,18 +1,36 @@
using Microsoft.AspNetCore.Mvc; using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Volo.Abp.AspNetCore.Mvc.UI.RazorPages; using Volo.Abp.AspNetCore.Mvc.UI.RazorPages;
namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
{ {
public class ExceptionTestPage : AbpPageModel public class ExceptionTestPage : AbpPageModel
{ {
public void OnGetUserFriendlyException1() public void OnGetUserFriendlyException_void()
{ {
throw new UserFriendlyException("This is a sample exception!"); throw new UserFriendlyException("This is a sample exception!");
} }
public IActionResult OnGetUserFriendlyException2() public Task OnGetUserFriendlyException_Task()
{
throw new UserFriendlyException("This is a sample exception!");
}
public IActionResult OnGetUserFriendlyException_ActionResult()
{ {
throw new UserFriendlyException("This is a sample exception!"); throw new UserFriendlyException("This is a sample exception!");
} }
public JsonResult OnGetUserFriendlyException_JsonResult()
{
throw new UserFriendlyException("This is a sample exception!");
}
public Task<JsonResult> OnGetUserFriendlyException_Task_JsonResult()
{
throw new UserFriendlyException("This is a sample exception!");
}
} }
} }

64
framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage_Tests.cs

@ -1,4 +1,4 @@
using System.Net; using System.Net;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Hosting;
@ -24,15 +24,33 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
} }
[Fact] [Fact]
public async Task Should_Return_RemoteServiceErrorResponse_For_UserFriendlyException_For_Void_Return_Value() public async Task Should_Not_Handle_Exceptions_For_Void_Return_Values()
{ {
var result = await GetResponseAsObjectAsync<RemoteServiceErrorResponse>("/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException1", HttpStatusCode.Forbidden); await Assert.ThrowsAsync<UserFriendlyException>(
result.Error.ShouldNotBeNull(); async () => await GetResponseAsStringAsync(
result.Error.Message.ShouldBe("This is a sample exception!"); "/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_Void"
)
);
#pragma warning disable 4014 #pragma warning disable 4014
_fakeExceptionSubscriber _fakeExceptionSubscriber
.Received() .DidNotReceive()
.HandleAsync(Arg.Any<ExceptionNotificationContext>());
#pragma warning restore 4014
}
[Fact]
public async Task Should_Not_Handle_Exceptions_For_Task_Return_Values()
{
await Assert.ThrowsAsync<UserFriendlyException>(
async () => await GetResponseAsStringAsync(
"/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_Task"
)
);
#pragma warning disable 4014
_fakeExceptionSubscriber
.DidNotReceive()
.HandleAsync(Arg.Any<ExceptionNotificationContext>()); .HandleAsync(Arg.Any<ExceptionNotificationContext>());
#pragma warning restore 4014 #pragma warning restore 4014
} }
@ -41,8 +59,8 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
public async Task Should_Not_Handle_Exceptions_For_ActionResult_Return_Values() public async Task Should_Not_Handle_Exceptions_For_ActionResult_Return_Values()
{ {
await Assert.ThrowsAsync<UserFriendlyException>( await Assert.ThrowsAsync<UserFriendlyException>(
async () => await GetResponseAsObjectAsync<RemoteServiceErrorResponse>( async () => await GetResponseAsStringAsync(
"/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException2" "/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_ActionResult"
) )
); );
@ -50,7 +68,35 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
_fakeExceptionSubscriber _fakeExceptionSubscriber
.DidNotReceive() .DidNotReceive()
.HandleAsync(Arg.Any<ExceptionNotificationContext>()); .HandleAsync(Arg.Any<ExceptionNotificationContext>());
#pragma warning restore 4014
}
[Fact]
public async Task Should_Return_RemoteServiceErrorResponse_For_UserFriendlyException_For_Object_Return_Value()
{
var result = await GetResponseAsObjectAsync<RemoteServiceErrorResponse>("/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_JsonResult", HttpStatusCode.Forbidden);
result.Error.ShouldNotBeNull();
result.Error.Message.ShouldBe("This is a sample exception!");
#pragma warning disable 4014
_fakeExceptionSubscriber
.Received()
.HandleAsync(Arg.Any<ExceptionNotificationContext>());
#pragma warning restore 4014
}
[Fact]
public async Task Should_Return_RemoteServiceErrorResponse_For_UserFriendlyException_For_Task_Object_Return_Value()
{
var result = await GetResponseAsObjectAsync<RemoteServiceErrorResponse>("/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_Task_JsonResult", HttpStatusCode.Forbidden);
result.Error.ShouldNotBeNull();
result.Error.Message.ShouldBe("This is a sample exception!");
#pragma warning disable 4014
_fakeExceptionSubscriber
.Received()
.HandleAsync(Arg.Any<ExceptionNotificationContext>());
#pragma warning restore 4014 #pragma warning restore 4014
} }
} }
} }

6
framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Features/FeatureTestPage.cshtml.cs

@ -14,9 +14,9 @@ namespace Volo.Abp.AspNetCore.Mvc.Features
} }
[RequiresFeature("NotAllowedFeature")] [RequiresFeature("NotAllowedFeature")]
public void OnGetNotAllowedFeature() public ObjectResult OnGetNotAllowedFeature()
{ {
return new ObjectResult(42);
} }
public ObjectResult OnGetNoFeature() public ObjectResult OnGetNoFeature()
@ -24,4 +24,4 @@ namespace Volo.Abp.AspNetCore.Mvc.Features
return new ObjectResult(42); return new ObjectResult(42);
} }
} }
} }

8
framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestPage.cshtml.cs

@ -32,23 +32,23 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
} }
[UnitOfWork(isTransactional: true)] [UnitOfWork(isTransactional: true)]
public void OnGetHandledException() public ObjectResult OnGetHandledException()
{ {
CurrentUnitOfWork.ShouldNotBeNull(); CurrentUnitOfWork.ShouldNotBeNull();
CurrentUnitOfWork.Options.IsTransactional.ShouldBeTrue(); CurrentUnitOfWork.Options.IsTransactional.ShouldBeTrue();
throw new UserFriendlyException("This is a sample exception!"); throw new UserFriendlyException("This is a sample exception!");
} }
public ObjectResult OnGetExceptionOnComplete() public ObjectResult OnGetExceptionOnComplete()
{ {
CurrentUnitOfWork.ShouldNotBeNull(); CurrentUnitOfWork.ShouldNotBeNull();
CurrentUnitOfWork.Options.IsTransactional.ShouldBeFalse(); CurrentUnitOfWork.Options.IsTransactional.ShouldBeFalse();
_testUnitOfWorkConfig.ThrowExceptionOnComplete = true; _testUnitOfWorkConfig.ThrowExceptionOnComplete = true;
//Prevent rendering of pages. //Prevent rendering of pages.
return new ObjectResult(""); return new ObjectResult("");
} }
} }
} }

Loading…
Cancel
Save