Prevent swallowing inner exception in async error (#1712)

src/Adapter/MSTest.TestAdapter/Execution/StackTraceHelper.cs → src/Adapter/MSTest.TestAdapter/Execution/ExceptionHelper.cs

@@ -15,7 +15,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// <summary>
 /// Provides helper methods to parse stack trace.
 /// </summary>
-internal static class StackTraceHelper
+internal static class ExceptionHelper
 {
     /// <summary>
     /// Gets the types whose methods should be ignored in the reported call stacks.
@@ -33,7 +33,7 @@ internal static class StackTraceHelper
     /// <returns>
     /// The <see cref="StackTraceInformation"/> for the provided exception.
     /// </returns>
-    internal static StackTraceInformation? GetStackTraceInformation(Exception ex)
+    internal static StackTraceInformation? GetStackTraceInformation(this Exception ex)
     {
         DebugEx.Assert(ex != null, "exception should not be null.");
 
@@ -138,7 +138,7 @@ internal static string TrimStackTrace(string stackTrace)
     /// The aggregated exception message that considers inner exceptions.
     /// </returns>
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")]
-    internal static string GetExceptionMessage(Exception ex)
+    internal static string GetFormattedExceptionMessage(this Exception ex)
     {
         DebugEx.Assert(ex != null, "exception should not be null.");
 

src/Adapter/MSTest.TestAdapter/Execution/TestAssemblyInfo.cs

@@ -167,7 +167,7 @@ public void RunAssemblyInitialize(TestContext testContext)
 
         var outcome = realException is AssertInconclusiveException ? UnitTestOutcome.Inconclusive : UnitTestOutcome.Failed;
 
-        // Do not use StackTraceHelper.GetExceptionMessage(realException) as it prefixes the message with the exception type name.
+        // Do not use StackTraceHelper.GetFormattedExceptionMessage(realException) as it prefixes the message with the exception type name.
         var exceptionMessage = realException.TryGetMessage();
         DebugEx.Assert(AssemblyInitializeMethod.DeclaringType?.FullName is not null, "AssemblyInitializeMethod.DeclaringType.FullName is null");
         var errorMessage = string.Format(
@@ -177,7 +177,7 @@ public void RunAssemblyInitialize(TestContext testContext)
             AssemblyInitializeMethod.Name,
             realException.GetType().ToString(),
             exceptionMessage);
-        var exceptionStackTraceInfo = StackTraceHelper.GetStackTraceInformation(realException);
+        var exceptionStackTraceInfo = realException.GetStackTraceInformation();
 
         var testFailedException = new TestFailedException(outcome, errorMessage, exceptionStackTraceInfo, realException);
         AssemblyInitializationException = testFailedException;
@@ -220,7 +220,7 @@ public void RunAssemblyInitialize(TestContext testContext)
                 }
                 else
                 {
-                    errorMessage = StackTraceHelper.GetExceptionMessage(realException);
+                    errorMessage = realException.GetFormattedExceptionMessage();
                 }
 
                 DebugEx.Assert(AssemblyCleanupMethod.DeclaringType?.Name is not null, "AssemblyCleanupMethod.DeclaringType.Name is null");
@@ -230,7 +230,7 @@ public void RunAssemblyInitialize(TestContext testContext)
                     AssemblyCleanupMethod.DeclaringType.Name,
                     AssemblyCleanupMethod.Name,
                     errorMessage,
-                    StackTraceHelper.GetStackTraceInformation(realException)?.ErrorStackTrace);
+                    realException.GetStackTraceInformation()?.ErrorStackTrace);
             }
         }
     }
@@ -268,10 +268,10 @@ internal void ExecuteAssemblyCleanup()
                 }
                 else
                 {
-                    errorMessage = StackTraceHelper.GetExceptionMessage(realException);
+                    errorMessage = realException.GetFormattedExceptionMessage();
                 }
 
-                var exceptionStackTraceInfo = StackTraceHelper.GetStackTraceInformation(realException);
+                var exceptionStackTraceInfo = realException.GetStackTraceInformation();
                 DebugEx.Assert(AssemblyCleanupMethod.DeclaringType?.Name is not null, "AssemblyCleanupMethod.DeclaringType.Name is null");
 
                 throw new TestFailedException(

src/Adapter/MSTest.TestAdapter/Execution/TestClassInfo.cs

@@ -306,7 +306,7 @@ public void RunClassInitialize(TestContext testContext)
 
         var outcome = realException is AssertInconclusiveException ? ObjectModelUnitTestOutcome.Inconclusive : ObjectModelUnitTestOutcome.Failed;
 
-        // Do not use StackTraceHelper.GetExceptionMessage(realException) as it prefixes the message with the exception type name.
+        // Do not use StackTraceHelper.GetFormattedExceptionMessage(realException) as it prefixes the message with the exception type name.
         var exceptionMessage = realException.TryGetMessage();
         var errorMessage = string.Format(
             CultureInfo.CurrentCulture,
@@ -315,7 +315,7 @@ public void RunClassInitialize(TestContext testContext)
             failedClassInitializeMethodName,
             realException.GetType().ToString(),
             exceptionMessage);
-        var exceptionStackTraceInfo = StackTraceHelper.GetStackTraceInformation(realException);
+        var exceptionStackTraceInfo = realException.GetStackTraceInformation();
 
         var testFailedException = new TestFailedException(outcome, errorMessage, exceptionStackTraceInfo, realException);
         ClassInitializationException = testFailedException;
@@ -373,17 +373,10 @@ public void RunClassInitialize(TestContext testContext)
                     var realException = exception.InnerException ?? exception;
                     ClassCleanupException = realException;
 
-                    string errorMessage;
-
                     // special case AssertFailedException to trim off part of the stack trace
-                    if (realException is AssertFailedException or AssertInconclusiveException)
-                    {
-                        errorMessage = realException.Message;
-                    }
-                    else
-                    {
-                        errorMessage = StackTraceHelper.GetExceptionMessage(realException);
-                    }
+                    string errorMessage = realException is AssertFailedException or AssertInconclusiveException
+                        ? realException.Message
+                        : realException.GetFormattedExceptionMessage();
 
                     var exceptionStackTraceInfo = realException.TryGetStackTraceInformation();
 
@@ -458,7 +451,7 @@ internal void ExecuteClassCleanup()
                 // special case AssertFailedException to trim off part of the stack trace
                 string errorMessage = realException is AssertFailedException or AssertInconclusiveException
                     ? realException.Message
-                    : StackTraceHelper.GetExceptionMessage(realException);
+                    : realException.GetFormattedExceptionMessage();
 
                 var exceptionStackTraceInfo = realException.TryGetStackTraceInformation();
 

src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs

@@ -430,7 +430,7 @@ private static Exception HandleMethodException(Exception ex, string className, s
             Resource.UTA_TestMethodThrows,
             className,
             methodName,
-            StackTraceHelper.GetExceptionMessage(realException));
+            realException.GetFormattedExceptionMessage());
 
         // Handle special case of UI objects in TestMethod to suggest UITestMethod
         if (realException.HResult == -2147417842)
@@ -446,7 +446,7 @@ private static Exception HandleMethodException(Exception ex, string className, s
         // which has no meaningful info for the user. Thus, we do not show call stack for ThreadAbortException.
         if (realException.GetType().Name != "ThreadAbortException")
         {
-            stackTrace = StackTraceHelper.GetStackTraceInformation(realException);
+            stackTrace = realException.GetStackTraceInformation();
         }
 
         return new TestFailedException(ObjectModelUnitTestOutcome.Failed, errorMessage, stackTrace, realException);
@@ -508,27 +508,25 @@ private void RunTestCleanupMethod(object classInstance, TestResult result)
                 }
             }
 
-            Exception realException = ex.GetInnerExceptionOrDefault();
+            Exception realException = ex.GetRealException();
+            string formattedExceptionMessage = realException.GetFormattedExceptionMessage();
 
             if (testCleanupMethod != null)
             {
-                // Do not use StackTraceHelper.GetExceptionMessage(realException) as it prefixes the message with the exception type name.
                 cleanupError.AppendFormat(
                     CultureInfo.CurrentCulture,
                     Resource.UTA_CleanupMethodThrows,
                     TestClassName,
                     testCleanupMethod.Name,
-                    realException.GetType().ToString(),
-                    realException.TryGetMessage());
+                    formattedExceptionMessage);
             }
             else
             {
-                // Use StackTraceHelper.GetExceptionMessage(realException) to get the message prefixed with the exception type name.
                 cleanupError.AppendFormat(
                     CultureInfo.CurrentCulture,
                     Resource.UTA_CleanupMethodThrowsGeneralError,
                     TestClassName,
-                    StackTraceHelper.GetExceptionMessage(realException));
+                    formattedExceptionMessage);
             }
 
             StackTraceInformation? cleanupStackTraceInfo = null;
@@ -583,19 +581,19 @@ private bool RunTestInitializeMethod(object classInstance, TestResult result)
         }
         catch (Exception ex)
         {
-            var realException = ex.GetInnerExceptionOrDefault();
+            var realException = ex.GetRealException();
 
             // Prefix the exception message with the exception type name as prefix when exception is not assert exception.
             var exceptionMessage = realException is UnitTestAssertException
                 ? realException.TryGetMessage()
-                : StackTraceHelper.GetExceptionMessage(realException);
+                : ExceptionHelper.GetFormattedExceptionMessage(realException);
             var errorMessage = string.Format(
                 CultureInfo.CurrentCulture,
                 Resource.UTA_InitMethodThrows,
                 TestClassName,
                 testInitializeMethod?.Name,
                 exceptionMessage);
-            var stackTrace = StackTraceHelper.GetStackTraceInformation(realException);
+            var stackTrace = realException.GetStackTraceInformation();
 
             result.Outcome = realException is AssertInconclusiveException ? UTF.UnitTestOutcome.Inconclusive : UTF.UnitTestOutcome.Failed;
             result.TestFailureException = new TestFailedException(
@@ -637,14 +635,15 @@ private bool SetTestContext(object classInstance, TestResult result)
         }
         catch (Exception ex)
         {
-            var stackTraceInfo = StackTraceHelper.GetStackTraceInformation(ex.GetInnerExceptionOrDefault());
+            var realException = ex.GetRealException();
             var errorMessage = string.Format(
                 CultureInfo.CurrentCulture,
                 Resource.UTA_TestContextSetError,
                 TestClassName,
-                StackTraceHelper.GetExceptionMessage(ex.GetInnerExceptionOrDefault()));
+                realException.GetFormattedExceptionMessage());
 
             result.Outcome = UTF.UnitTestOutcome.Failed;
+            var stackTraceInfo = realException.GetStackTraceInformation();
             result.TestFailureException = new TestFailedException(ObjectModelUnitTestOutcome.Failed, errorMessage, stackTraceInfo);
         }
 
@@ -689,8 +688,8 @@ private bool SetTestContext(object classInstance, TestResult result)
             // in the InnerException; or user code throws an exception.
             // It also seems that in rare cases the ex can be null.
             var actualException = ex.InnerException ?? ex;
-            var exceptionMessage = StackTraceHelper.GetExceptionMessage(actualException);
-            var stackTraceInfo = StackTraceHelper.GetStackTraceInformation(actualException);
+            var exceptionMessage = actualException.GetFormattedExceptionMessage();
+            var stackTraceInfo = actualException.GetStackTraceInformation();
 
             var errorMessage = string.Format(
                 CultureInfo.CurrentCulture,

src/Adapter/MSTest.TestAdapter/Extensions/ExceptionExtensions.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
+using System.Reflection;
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
@@ -19,17 +20,15 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
 internal static class ExceptionExtensions
 {
     /// <summary>
-    /// Get the InnerException if available, else return the current Exception.
+    /// In .NET framework, the exception thrown by the test method invocation is wrapped in a TargetInvocationException, so we
+    /// need to unwrap it to get the real exception.
     /// </summary>
-    /// <param name="exception">The exception.</param>
-    /// <returns>
-    /// An <see cref="Exception"/> instance.
-    /// </returns>
-    [return: NotNullIfNotNull(nameof(exception))]
-    internal static Exception? GetInnerExceptionOrDefault(this Exception exception)
-    {
-        return exception?.InnerException ?? exception;
-    }
+    internal static Exception GetRealException(this Exception exception)
+        => exception.GetType() == typeof(TargetInvocationException)
+            && exception.Source is "mscorlib" or "System.Private.CoreLib"
+            && exception.InnerException is not null
+                ? exception.InnerException
+                : exception;
 
     /// <summary>
     /// Get the exception message if available, empty otherwise.
@@ -56,7 +55,7 @@ internal static string TryGetMessage(this Exception exception)
     {
         if (!StringEx.IsNullOrEmpty(exception?.StackTrace))
         {
-            return StackTraceHelper.CreateStackTraceInformation(exception, false, exception.StackTrace);
+            return ExceptionHelper.CreateStackTraceInformation(exception, false, exception.StackTrace);
         }
 
         return null;

src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs

@@ -153,7 +153,7 @@ public bool HasAttributeDerivedFrom<TAttribute>(MemberInfo memberInfo, bool inhe
                 Resource.UTA_ExpectedExceptionAttributeConstructionException,
                 testMethod.FullClassName,
                 testMethod.Name,
-                StackTraceHelper.GetExceptionMessage(ex));
+                ex.GetFormattedExceptionMessage());
             throw new TypeInspectionException(errorMessage);
         }
 

src/Adapter/MSTest.TestAdapter/Resources/Resource.resx

@@ -124,7 +124,7 @@
     <value>Running tests in any of the provided sources is not supported for the selected platform</value>
   </data>
   <data name="UTA_CleanupMethodThrows" xml:space="preserve">
-    <value>TestCleanup method {0}.{1} threw exception. {2}: {3}.</value>
+    <value>TestCleanup method {0}.{1} threw exception. {2}.</value>
   </data>
   <data name="UTA_EndOfInnerExceptionTrace" xml:space="preserve">
     <value>--- End of inner exception stack trace ---</value>

src/Adapter/MSTest.TestAdapter/Resources/xlf/Resource.cs.xlf

@@ -13,9 +13,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="UTA_CleanupMethodThrows">
-        <source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
-        <target state="translated">Metoda TestCleanup {0}.{1} způsobila výjimku. {2}: {3}.</target>
-        <note></note>
+        <source>TestCleanup method {0}.{1} threw exception. {2}.</source>
+        <target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
+        <note />
       </trans-unit>
       <trans-unit id="UTA_EndOfInnerExceptionTrace">
         <source>--- End of inner exception stack trace ---</source>

src/Adapter/MSTest.TestAdapter/Resources/xlf/Resource.de.xlf

@@ -13,9 +13,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="UTA_CleanupMethodThrows">
-        <source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
-        <target state="translated">Die TestCleanup-Methode "{0}.{1}" hat eine Ausnahme ausgelöst. {2}: {3}.</target>
-        <note></note>
+        <source>TestCleanup method {0}.{1} threw exception. {2}.</source>
+        <target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
+        <note />
       </trans-unit>
       <trans-unit id="UTA_EndOfInnerExceptionTrace">
         <source>--- End of inner exception stack trace ---</source>

src/Adapter/MSTest.TestAdapter/Resources/xlf/Resource.es.xlf

@@ -13,9 +13,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="UTA_CleanupMethodThrows">
-        <source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
-        <target state="translated">El método TestCleanup {0}.{1} devolvió una excepción. {2}: {3}.</target>
-        <note></note>
+        <source>TestCleanup method {0}.{1} threw exception. {2}.</source>
+        <target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
+        <note />
       </trans-unit>
       <trans-unit id="UTA_EndOfInnerExceptionTrace">
         <source>--- End of inner exception stack trace ---</source>

src/Adapter/MSTest.TestAdapter/Resources/xlf/Resource.fr.xlf

@@ -13,9 +13,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="UTA_CleanupMethodThrows">
-        <source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
-        <target state="translated">La méthode TestCleanup {0}.{1} a levé une exception. {2}: {3}.</target>
-        <note></note>
+        <source>TestCleanup method {0}.{1} threw exception. {2}.</source>
+        <target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
+        <note />
       </trans-unit>
       <trans-unit id="UTA_EndOfInnerExceptionTrace">
         <source>--- End of inner exception stack trace ---</source>

src/Adapter/MSTest.TestAdapter/Resources/xlf/Resource.it.xlf

@@ -13,9 +13,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="UTA_CleanupMethodThrows">
-        <source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
-        <target state="translated">Il metodo TestCleanup {0}.{1} ha generato un'eccezione. {2}: {3}.</target>
-        <note></note>
+        <source>TestCleanup method {0}.{1} threw exception. {2}.</source>
+        <target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
+        <note />
       </trans-unit>
       <trans-unit id="UTA_EndOfInnerExceptionTrace">
         <source>--- End of inner exception stack trace ---</source>

src/Adapter/MSTest.TestAdapter/Resources/xlf/Resource.ja.xlf

@@ -13,9 +13,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="UTA_CleanupMethodThrows">
-        <source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
-        <target state="translated">TestCleanup メソッド {0}.{1} は例外をスローしました。{2}: {3}。</target>
-        <note></note>
+        <source>TestCleanup method {0}.{1} threw exception. {2}.</source>
+        <target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
+        <note />
       </trans-unit>
       <trans-unit id="UTA_EndOfInnerExceptionTrace">
         <source>--- End of inner exception stack trace ---</source>

src/Adapter/MSTest.TestAdapter/Resources/xlf/Resource.ko.xlf

@@ -13,9 +13,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="UTA_CleanupMethodThrows">
-        <source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
-        <target state="translated">TestCleanup 메서드 {0}.{1}에서 예외를 throw했습니다. {2}: {3}.</target>
-        <note></note>
+        <source>TestCleanup method {0}.{1} threw exception. {2}.</source>
+        <target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
+        <note />
       </trans-unit>
       <trans-unit id="UTA_EndOfInnerExceptionTrace">
         <source>--- End of inner exception stack trace ---</source>