Use `ExceptionDispatchInfo` in `MiddlewareFilterBuilder` if available

- #6596
- better-align this code with `ResourceInvoker.Rethrow()`

nits:
- take VS suggestions in `MiddlewareFilterBuilderTest`
- clean up names like `httpCtxt`
- remove unused `Pipeline2` class
This commit is contained in:
Doug Bunting 2018-01-01 16:41:16 -08:00
parent 51820e3e25
commit 5778f44bf7
2 changed files with 199 additions and 41 deletions

View File

@ -71,13 +71,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal
var resourceExecutionDelegate = feature.ResourceExecutionDelegate; var resourceExecutionDelegate = feature.ResourceExecutionDelegate;
var resourceExecutedContext = await resourceExecutionDelegate(); var resourceExecutedContext = await resourceExecutionDelegate();
if (resourceExecutedContext.ExceptionHandled)
// Ideally we want the experience of a middleware pipeline to behave the same as if it was registered,
// in Startup. In this scenario an exception thrown in a middelware later in the pipeline gets propagated
// back to earlier middleware.
// So check if a later resource filter threw an exception and propagate that back to the middleware pipeline.
if (!resourceExecutedContext.ExceptionHandled && resourceExecutedContext.Exception != null)
{ {
return;
}
// Ideally we want the experience of a middleware pipeline to behave the same as if it was registered
// in Startup. In this scenario, an Exception thrown in a middelware later in the pipeline gets
// propagated back to earlier middleware. So, check if a later resource filter threw an Exception and
// propagate that back to the middleware pipeline.
resourceExecutedContext.ExceptionDispatchInfo?.Throw();
if (resourceExecutedContext.Exception != null)
{
// This line is rarely reachable because ResourceInvoker captures thrown Exceptions using
// ExceptionDispatchInfo. That said, filters could set only resourceExecutedContext.Exception.
throw resourceExecutedContext.Exception; throw resourceExecutedContext.Exception;
} }
}); });

View File

@ -3,6 +3,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Runtime.ExceptionServices;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Builder.Internal; using Microsoft.AspNetCore.Builder.Internal;
@ -25,10 +26,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// Arrange // Arrange
var services = new ServiceCollection(); var services = new ServiceCollection();
var appBuilder = new ApplicationBuilder(services.BuildServiceProvider()); var appBuilder = new ApplicationBuilder(services.BuildServiceProvider());
var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider()); var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider())
pipelineBuilderService.ApplicationBuilder = appBuilder; {
ApplicationBuilder = appBuilder,
};
var configureCount = 0; var configureCount = 0;
Pipeline1.ConfigurePipeline = (ab) => Pipeline1.ConfigurePipeline = _ =>
{ {
configureCount++; configureCount++;
}; };
@ -47,10 +51,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// Arrange // Arrange
var services = new ServiceCollection(); var services = new ServiceCollection();
var appBuilder = new ApplicationBuilder(services.BuildServiceProvider()); var appBuilder = new ApplicationBuilder(services.BuildServiceProvider());
var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider()); var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider())
pipelineBuilderService.ApplicationBuilder = appBuilder; {
ApplicationBuilder = appBuilder,
};
var configureCount = 0; var configureCount = 0;
Pipeline1.ConfigurePipeline = (ab) => Pipeline1.ConfigurePipeline = _ =>
{ {
configureCount++; configureCount++;
}; };
@ -77,11 +84,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// Arrange // Arrange
var services = new ServiceCollection(); var services = new ServiceCollection();
var appBuilder = new ApplicationBuilder(services.BuildServiceProvider()); var appBuilder = new ApplicationBuilder(services.BuildServiceProvider());
var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider()); var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider())
pipelineBuilderService.ApplicationBuilder = appBuilder;
Pipeline1.ConfigurePipeline = (ab) =>
{ {
ab.Use((httpContext, next) => ApplicationBuilder = appBuilder,
};
var httpContext = new DefaultHttpContext();
Pipeline1.ConfigurePipeline = ab =>
{
ab.Use((_, next) =>
{ {
return next(); return next();
}); });
@ -92,36 +103,48 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// Assert // Assert
Assert.NotNull(pipeline); Assert.NotNull(pipeline);
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => pipeline(new DefaultHttpContext())); var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => pipeline(httpContext));
Assert.Equal( Assert.Equal("Could not find 'IMiddlewareFilterFeature' in the feature list.", exception.Message);
"Could not find 'IMiddlewareFilterFeature' in the feature list.",
exception.Message);
} }
[Fact] [Fact]
public async Task EndMiddleware_PropagatesBackException_ToEarlierMiddleware() public async Task EndMiddleware_DoesNotThrow_IfExceptionHandled()
{ {
// Arrange // Arrange
var services = new ServiceCollection(); var services = new ServiceCollection();
var appBuilder = new ApplicationBuilder(services.BuildServiceProvider()); var appBuilder = new ApplicationBuilder(services.BuildServiceProvider());
var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider()); var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider())
pipelineBuilderService.ApplicationBuilder = appBuilder;
Pipeline1.ConfigurePipeline = (ab) =>
{ {
ab.Use((httpCtxt, next) => ApplicationBuilder = appBuilder,
};
Pipeline1.ConfigurePipeline = ab =>
{
ab.Use((_, next) =>
{ {
return next(); return next();
}); });
}; };
var middlewareFilterFeature = new MiddlewareFilterFeature();
middlewareFilterFeature.ResourceExecutionDelegate = () => var middlewareFilterFeature = new MiddlewareFilterFeature
{ {
var context = new ResourceExecutedContext( ResourceExecutionDelegate = () =>
new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor(), new ModelStateDictionary()), {
new List<IFilterMetadata>()); var actionContext = new ActionContext(
context.Exception = new InvalidOperationException("Error!!!"); new DefaultHttpContext(),
return Task.FromResult(context); new RouteData(),
new ActionDescriptor(),
new ModelStateDictionary());
var context = new ResourceExecutedContext(actionContext, new List<IFilterMetadata>())
{
Exception = new InvalidOperationException("Error!!!"),
ExceptionHandled = true,
};
return Task.FromResult(context);
},
}; };
var features = new FeatureCollection(); var features = new FeatureCollection();
features.Set<IMiddlewareFilterFeature>(middlewareFilterFeature); features.Set<IMiddlewareFilterFeature>(middlewareFilterFeature);
var httpContext = new DefaultHttpContext(features); var httpContext = new DefaultHttpContext(features);
@ -131,20 +154,148 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// Assert // Assert
Assert.NotNull(pipeline); Assert.NotNull(pipeline);
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => pipeline(httpContext));
Assert.Equal("Error!!!", exception.Message); // Does not throw.
await pipeline(httpContext);
} }
private class Pipeline1
[Fact]
public async Task EndMiddleware_PropagatesBackException_ToEarlierMiddleware()
{ {
public static Action<IApplicationBuilder> ConfigurePipeline { get; set; } // Arrange
var services = new ServiceCollection();
public void Configure(IApplicationBuilder appBuilder) var appBuilder = new ApplicationBuilder(services.BuildServiceProvider());
var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider())
{ {
ConfigurePipeline(appBuilder); ApplicationBuilder = appBuilder,
} };
Pipeline1.ConfigurePipeline = ab =>
{
ab.Use((_, next) =>
{
return next();
});
};
var middlewareFilterFeature = new MiddlewareFilterFeature
{
ResourceExecutionDelegate = () =>
{
Exception thrownException;
try
{
// Create a small stack trace.
throw new InvalidOperationException("Error!!!");
}
catch (Exception ex)
{
thrownException = ex;
}
var actionContext = new ActionContext(
new DefaultHttpContext(),
new RouteData(),
new ActionDescriptor(),
new ModelStateDictionary());
var context = new ResourceExecutedContext(actionContext, new List<IFilterMetadata>())
{
Exception = thrownException,
};
return Task.FromResult(context);
},
};
var features = new FeatureCollection();
features.Set<IMiddlewareFilterFeature>(middlewareFilterFeature);
var httpContext = new DefaultHttpContext(features);
// Act
var pipeline = pipelineBuilderService.GetPipeline(typeof(Pipeline1));
// Assert
Assert.NotNull(pipeline);
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => pipeline(httpContext));
Assert.Null(exception.InnerException);
Assert.Equal("Error!!!", exception.Message);
var stack = exception.StackTrace;
Assert.Contains(typeof(MiddlewareFilterBuilder).FullName, stack);
Assert.DoesNotContain(typeof(MiddlewareFilterBuilderTest).FullName, stack);
Assert.DoesNotContain(nameof(EndMiddleware_PropagatesBackException_ToEarlierMiddleware), stack);
} }
private class Pipeline2 [Fact]
public async Task EndMiddleware_PropagatesFullExceptionInfo_ToEarlierMiddleware()
{
// Arrange
var services = new ServiceCollection();
var appBuilder = new ApplicationBuilder(services.BuildServiceProvider());
var pipelineBuilderService = new MiddlewareFilterBuilder(new MiddlewareFilterConfigurationProvider())
{
ApplicationBuilder = appBuilder,
};
Pipeline1.ConfigurePipeline = ab =>
{
ab.Use((_, next) =>
{
return next();
});
};
var middlewareFilterFeature = new MiddlewareFilterFeature
{
ResourceExecutionDelegate = () =>
{
ExceptionDispatchInfo exceptionInfo;
try
{
// Create a small stack trace.
throw new InvalidOperationException("Error!!!");
}
catch (Exception ex)
{
exceptionInfo = ExceptionDispatchInfo.Capture(ex);
}
var actionContext = new ActionContext(
new DefaultHttpContext(),
new RouteData(),
new ActionDescriptor(),
new ModelStateDictionary());
var context = new ResourceExecutedContext(actionContext, new List<IFilterMetadata>())
{
ExceptionDispatchInfo = exceptionInfo,
};
return Task.FromResult(context);
},
};
var features = new FeatureCollection();
features.Set<IMiddlewareFilterFeature>(middlewareFilterFeature);
var httpContext = new DefaultHttpContext(features);
// Act
var pipeline = pipelineBuilderService.GetPipeline(typeof(Pipeline1));
// Assert
Assert.NotNull(pipeline);
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => pipeline(httpContext));
Assert.Null(exception.InnerException);
Assert.Equal("Error!!!", exception.Message);
var stack = exception.StackTrace;
Assert.Contains(typeof(MiddlewareFilterBuilder).FullName, stack);
Assert.Contains(typeof(MiddlewareFilterBuilderTest).FullName, stack);
Assert.Contains(nameof(EndMiddleware_PropagatesFullExceptionInfo_ToEarlierMiddleware), stack);
}
private class Pipeline1
{ {
public static Action<IApplicationBuilder> ConfigurePipeline { get; set; } public static Action<IApplicationBuilder> ConfigurePipeline { get; set; }