Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Add ExceptionHandled to ExceptionContext #4825

Merged
merged 1 commit into from
Jun 9, 2016
+67 −9
Merged

Add ExceptionHandled to ExceptionContext #4825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/ExceptionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ public virtual ExceptionDispatchInfo ExceptionDispatchInfo
}
}

/// <summary>
/// Gets or sets an indication that the <see cref="Exception"/> has been handled.
/// </summary>
public virtual bool ExceptionHandled { get; set; } = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why virtual? I don't think there's an easy way to replace the exception context with your own, and being get set I don't see how making it virtual would improve unit testing which is the other scenario I would consider. @rynowak am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the properties on all of these classes are virtual for mocking support


/// <summary>
/// Gets or sets the <see cref="IActionResult"/>.
/// </summary>
Expand Down
11 changes: 5 additions & 6 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.Internal;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNetCore.Mvc.Internal
Expand Down Expand Up @@ -354,7 +353,7 @@ private async Task InvokeResourceFilterAsync()
Result = _exceptionContext.Result,
};
}
else if (_exceptionContext.Exception != null)
else if (_exceptionContext.Exception != null && !_exceptionContext.ExceptionHandled)
{
// If we get here, this means that we have an unhandled exception.
// Exception filted didn't handle this, so send it on to resource filters.
Expand Down Expand Up @@ -412,7 +411,7 @@ private async Task InvokeExceptionFilterAsync()
await InvokeExceptionFilterAsync();

Debug.Assert(_exceptionContext != null);
if (_exceptionContext.Exception != null)
if (_exceptionContext.Exception != null && !_exceptionContext.ExceptionHandled)
{
_diagnosticSource.BeforeOnExceptionAsync(_exceptionContext, current.FilterAsync);

Expand All @@ -422,7 +421,7 @@ private async Task InvokeExceptionFilterAsync()

_diagnosticSource.AfterOnExceptionAsync(_exceptionContext, current.FilterAsync);

if (_exceptionContext.Exception == null)
if (_exceptionContext.Exception == null || _exceptionContext.ExceptionHandled)
{
_logger.ExceptionFilterShortCircuited(current.FilterAsync);
}
Expand All @@ -435,7 +434,7 @@ private async Task InvokeExceptionFilterAsync()
await InvokeExceptionFilterAsync();

Debug.Assert(_exceptionContext != null);
if (_exceptionContext.Exception != null)
if (_exceptionContext.Exception != null && !_exceptionContext.ExceptionHandled)
{
_diagnosticSource.BeforeOnException(_exceptionContext, current.Filter);

Expand All @@ -445,7 +444,7 @@ private async Task InvokeExceptionFilterAsync()

_diagnosticSource.AfterOnException(_exceptionContext, current.Filter);

if (_exceptionContext.Exception == null)
if (_exceptionContext.Exception == null || _exceptionContext.ExceptionHandled)
{
_logger.ExceptionFilterShortCircuited(current.Filter);
}
Expand Down
58 changes: 56 additions & 2 deletions test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public async Task InvokeAction_InvokesAsyncExceptionFilter_WhenActionThrows()
}

[Fact]
public async Task InvokeAction_InvokesExceptionFilter_ShortCircuit()
public async Task InvokeAction_InvokesExceptionFilter_ShortCircuit_ExceptionNull()
{
// Arrange
var filter1 = new Mock<IExceptionFilter>(MockBehavior.Strict);
Expand All @@ -171,12 +171,38 @@ public async Task InvokeAction_InvokesExceptionFilter_ShortCircuit()
}

[Fact]
public async Task InvokeAction_InvokesAsyncExceptionFilter_ShortCircuit()
public async Task InvokeAction_InvokesExceptionFilter_ShortCircuit_ExceptionHandled()
{
// Arrange
var filter1 = new Mock<IExceptionFilter>(MockBehavior.Strict);

var filter2 = new Mock<IExceptionFilter>(MockBehavior.Strict);
filter2
.Setup(f => f.OnException(It.IsAny<ExceptionContext>()))
.Callback<ExceptionContext>(context =>
{
context.ExceptionHandled = true;
})
.Verifiable();

var invoker = CreateInvoker(new[] { filter1.Object, filter2.Object }, actionThrows: true);

// Act
await invoker.InvokeAsync();

// Assert
filter2.Verify(
f => f.OnException(It.IsAny<ExceptionContext>()),
Times.Once());
}

[Fact]
public async Task InvokeAction_InvokesAsyncExceptionFilter_ShortCircuit_ExceptionNull()
{
// Arrange
var filter1 = new Mock<IExceptionFilter>(MockBehavior.Strict);
var filter2 = new Mock<IAsyncExceptionFilter>(MockBehavior.Strict);

filter2
.Setup(f => f.OnExceptionAsync(It.IsAny<ExceptionContext>()))
.Callback<ExceptionContext>(context =>
Expand All @@ -187,6 +213,34 @@ public async Task InvokeAction_InvokesAsyncExceptionFilter_ShortCircuit()
.Returns<ExceptionContext>((context) => Task.FromResult(true))
.Verifiable();

var filterMetadata = new IFilterMetadata[] { filter1.Object, filter2.Object };
var invoker = CreateInvoker(filterMetadata, actionThrows: true);

// Act
await invoker.InvokeAsync();

// Assert
filter2.Verify(
f => f.OnExceptionAsync(It.IsAny<ExceptionContext>()),
Times.Once());
}

[Fact]
public async Task InvokeAction_InvokesAsyncExceptionFilter_ShortCircuit_ExceptionHandled()
{
// Arrange
var filter1 = new Mock<IExceptionFilter>(MockBehavior.Strict);

var filter2 = new Mock<IAsyncExceptionFilter>(MockBehavior.Strict);
filter2
.Setup(f => f.OnExceptionAsync(It.IsAny<ExceptionContext>()))
.Callback<ExceptionContext>(context =>
{
context.ExceptionHandled = true;
})
.Returns<ExceptionContext>((context) => Task.FromResult(true))
.Verifiable();

var invoker = CreateInvoker(new IFilterMetadata[] { filter1.Object, filter2.Object }, actionThrows: true);

// Act
Expand Down
2 changes: 1 addition & 1 deletion test/WebSites/FiltersWebSite/Filters/ShortCircuitExceptionFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public class ShortCircuitExceptionFilter : ExceptionFilterAttribute
{
public override void OnException(ExceptionContext context)
{
context.Exception = null;
context.ExceptionHandled = true;
}
}
}