Fix for SF2649: Aggregate exceptions lost from async step definitions (…

…#2667)

* BindingInvoker changed to pass AggregateExceptions through. Use of 'PreserveStackTrace' replaced with call to ExceptionDispatchInfo.Capture.
BindingInvoker unit tests added to validate handling of various combinations of exceptions thrown from sync and async StepDefinition methods.

* BindingInvoker now passes AggregateExceptions along up the call chain. BindingInvoker changed to pass AggregateExceptions through. Use of 'PreserveStackTrace' replaced with call to ExceptionDispatchInfo.Capture.
BindingInvoker unit tests added to validate handling of various combinations of exceptions thrown from sync and async StepDefinition methods.
TestExecutionEngine updated to also use ExceptionDispatchInfo.Capture().Throw() in order to preserve the stack trace of user-hook's exception.

TechTalk.SpecFlow/Bindings/BindingInvoker.cs

@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using System.Runtime.ExceptionServices;
 using System.Threading.Tasks;
 using TechTalk.SpecFlow.Bindings.Reflection;
 using TechTalk.SpecFlow.Compatibility;
@@ -75,17 +76,10 @@ public virtual async Task<object> InvokeBindingAsync(IBinding binding, IContextM
             catch (TargetInvocationException invEx)
             {
                 var ex = invEx.InnerException;
-                ex = ex.PreserveStackTrace(errorProvider.GetMethodText(binding.Method));
-                stopwatch.Stop();
-                durationHolder.Duration = stopwatch.Elapsed;
-                throw ex;
-            }
-            catch (AggregateException aggregateEx)
-            {
-                var ex = aggregateEx.InnerExceptions.First();
-                ex = ex.PreserveStackTrace(errorProvider.GetMethodText(binding.Method));
                 stopwatch.Stop();
                 durationHolder.Duration = stopwatch.Elapsed;
+                ExceptionDispatchInfo.Capture(ex).Throw();
+                //hack,hack,hack - the compiler doesn't recognize that ExceptionDispatchInfo.Throw() exits the method; the next line will never be executed
                 throw ex;
             }
             catch (Exception)

TechTalk.SpecFlow/Infrastructure/TestExecutionEngine.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Diagnostics;
 using System.Linq;
+using System.Runtime.ExceptionServices;
 using System.Threading.Tasks;
 using BoDi;
 using TechTalk.SpecFlow.Analytics;
@@ -350,7 +351,7 @@ private async Task FireEventsAsync(HookType hookType)
             _testThreadExecutionEventPublisher.PublishEvent(new HookFinishedEvent(hookType, FeatureContext, ScenarioContext, _contextManager.StepContext, hookException));
 
             //Note: the (user-)hook exception (if any) will be thrown after the plugin hooks executed to fail the test with the right error
-            if (hookException != null) throw hookException;
+            if (hookException != null) ExceptionDispatchInfo.Capture(hookException).Throw();
         }
 
         private void FireRuntimePluginTestExecutionLifecycleEvents(HookType hookType)

Tests/TechTalk.SpecFlow.RuntimeTests/Bindings/BindingInvokerTests.cs

@@ -1,4 +1,6 @@
 using System;
+using System.CodeDom.Compiler;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using BoDi;
 using FluentAssertions;
@@ -163,4 +165,117 @@ await FluentActions.Awaiting(() => InvokeBindingAsync(sut, contextManager, typeo
     }
 
     #endregion
+
+    #region Exception Handling related tests - regression tests for SF2649
+    public enum ExceptionKind
+    {
+        Normal,
+        Aggregate
+    }
+
+    public enum InnerExceptionContentKind
+    {
+        WithInnerException,
+        WithoutInnerException
+    }
+
+    public enum StepDefInvocationStyle
+    {
+        Sync,
+        Async
+    }
+
+    class StepDefClassThatThrowsExceptions
+    {
+        public async Task AsyncThrow(ExceptionKind kindOfExceptionToThrow, InnerExceptionContentKind innerExceptionKind)
+        {
+            await Task.Run(() => ConstructAndThrowSync(kindOfExceptionToThrow, innerExceptionKind));
+        }
+
+        public void SyncThrow(ExceptionKind kindOfExceptionToThrow, InnerExceptionContentKind innerExceptionKind)
+        {
+            ConstructAndThrowSync(kindOfExceptionToThrow, innerExceptionKind);
+        }
+
+        private void ConstructAndThrowSync(ExceptionKind typeOfExceptionToThrow, InnerExceptionContentKind innerExceptionContentKind)
+        {
+            switch (typeOfExceptionToThrow, innerExceptionContentKind)
+            {
+                case (ExceptionKind.Normal, InnerExceptionContentKind.WithoutInnerException):
+                    throw new Exception("Normal Exception message (No InnerException expected).");
+
+                case (ExceptionKind.Aggregate, InnerExceptionContentKind.WithoutInnerException):
+                    throw new AggregateException("AggregateEx (without Inners)");
+
+                case (ExceptionKind.Normal, InnerExceptionContentKind.WithInnerException):
+                    try
+                    {
+                        throw new Exception("This is the message from the Inner Exception");
+                    }
+                    catch (Exception e)
+                    {
+                        throw new Exception("Normal Exception (with InnerException)", e);
+                    }
+
+                case (ExceptionKind.Aggregate, InnerExceptionContentKind.WithInnerException):
+                    {
+                        var tasks = new List<Task>
+                        {
+                            Task.Run(async () => throw new Exception("This is the first Exception embedded in the AggregateException")),
+                            Task.Run(async () => throw new Exception("This is the second Exception embedded in the AggregateException"))
+                        };
+                        var continuation = Task.WhenAll(tasks);
+
+                        // This will throw an AggregateException with two Inner Exceptions
+                        continuation.Wait();
+                        return;
+                    }
+             }
+
+        }
+    }
+
+    [Theory]
+    [InlineData(StepDefInvocationStyle.Sync, InnerExceptionContentKind.WithoutInnerException)]
+    [InlineData(StepDefInvocationStyle.Sync, InnerExceptionContentKind.WithInnerException)]
+    [InlineData(StepDefInvocationStyle.Async, InnerExceptionContentKind.WithoutInnerException)]
+    [InlineData(StepDefInvocationStyle.Async, InnerExceptionContentKind.WithInnerException)]
+    public async Task InvokeBindingAsync_WhenStepDefThrowsExceptions_ProperlyPreservesExceptionContext(StepDefInvocationStyle style, InnerExceptionContentKind inner)
+    {
+        _testOutputHelper.WriteLine($"starting Exception Handling test: {style}, {inner}");
+        var sut = CreateSut();
+        var contextManager = CreateContextManagerWith();
+
+        string methodToInvoke;
+        ExceptionKind kindOfExceptionToThrow;
+        Exception thrown;
+
+        // call step definition methods
+        if (style == StepDefInvocationStyle.Sync)
+        {
+            methodToInvoke = nameof(StepDefClassThatThrowsExceptions.SyncThrow);
+            kindOfExceptionToThrow = ExceptionKind.Normal;
+            thrown = await Assert.ThrowsAsync<Exception>(async () => await InvokeBindingAsync(sut, contextManager, typeof(StepDefClassThatThrowsExceptions), methodToInvoke, kindOfExceptionToThrow, inner));
+        }
+        else // if (style == StepDefInvocationStyle.Async)
+        {
+            methodToInvoke = nameof(StepDefClassThatThrowsExceptions.AsyncThrow);
+            kindOfExceptionToThrow = ExceptionKind.Aggregate;
+            thrown = await Assert.ThrowsAsync<AggregateException>(async () => await InvokeBindingAsync(sut, contextManager, typeof(StepDefClassThatThrowsExceptions), methodToInvoke, kindOfExceptionToThrow, inner));
+        }
+
+        _testOutputHelper.WriteLine($"Exception detail: {thrown}");
+
+        // Assert that the InnerException detail is preserved
+        if (inner == InnerExceptionContentKind.WithInnerException)
+        {
+            if (thrown is AggregateException) (thrown as AggregateException).InnerExceptions.Count.Should().BeGreaterThan(1);
+            else thrown.InnerException.Should().NotBeNull();
+        }
+
+        // Assert that the stack trace properly shows that the exception came from the throwing method (and not hidden by the SpecFlow infrastructure)
+        thrown.StackTrace.Should().Contain(methodToInvoke);
+    }
+    #endregion
+
 }

changelog.txt

@@ -18,6 +18,7 @@ Changes:
 + Default step definition skeletons are generating cucumber expressions.
 + 'ScenarioInfo.ScenarioAndFeatureTags' has been deprecated in favor of 'ScenarioInfo.CombinedTags'. Now both contain rule tags as well.
 + The interface ISpecFlowOutputHelper has been moved to the TechTalk.SpecFlow namespace (from TechTalk.SpecFlow.Infrastructure).
++ BugFix: SF2649 - AggregateExceptions thrown by async StepDefinition methods are no longer consumed; but passed along to the test host.
 
 
 3.10