Handle possible null ref in error handler (microsoft#819)

nohwnd · web-flow · commit 179c864e4de5 · 2021-04-16T08:44:44.000Z
I have a dump that throws access violation exception in native code and the test run fails with: 

`An exception occurred while invoking executor 'executor://mstestadapter/v2': Object reference not set to an instance of an object.`

I looked through the code and the only way I can get this result is when ex would be null in the handler. Otherwise it fails with different error, either from the asserts in the `StackTraceHelper` methods or in upstream error handlers.
diff --git a/src/Adapter/MSTest.CoreAdapter/Execution/TestMethodInfo.cs b/src/Adapter/MSTest.CoreAdapter/Execution/TestMethodInfo.cs
@@ -717,11 +717,27 @@ private object CreateTestClassInstance(TestResult result)
             }
             catch (Exception ex)
             {
+                if (ex == null)
+                {
+                    // It seems that ex can be null in some rare cases when initialization fails in native code.
+                    // Get our own exception with a stack trace to satisfy GetStackTraceInformation.
+                    try
+                    {
+                        throw new InvalidOperationException(Resource.UTA_UserCodeThrewNullValueException);
+                    }
+                    catch (Exception exception)
+                    {
+                        ex = exception;
+                    }
+                }
+
                 // In most cases, exception will be TargetInvocationException with real exception wrapped
-                // in the InnerException; or user code throws an exception
+                // 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 errorMessage = string.Format(
                     CultureInfo.CurrentCulture,
                     Resource.UTA_InstanceCreationError,
diff --git a/src/Adapter/MSTest.CoreAdapter/Execution/TestMethodRunner.cs b/src/Adapter/MSTest.CoreAdapter/Execution/TestMethodRunner.cs
@@ -270,7 +270,7 @@ internal UnitTestResult[] RunTestMethod()
                                     {
                                         testResults = new[]
                                         {
-                                            new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex.Message), ex) }
+                                            new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex?.Message, ex?.StackTrace), ex) }
                                         };
                                     }
 
@@ -324,7 +324,7 @@ internal UnitTestResult[] RunTestMethod()
                                 {
                                     testResults = new[]
                                     {
-                                        new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex.Message), ex) }
+                                        new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex?.Message, ex?.StackTrace), ex) }
                                     };
                                 }
 
@@ -346,7 +346,7 @@ internal UnitTestResult[] RunTestMethod()
                         }
                         catch (Exception ex)
                         {
-                            results.Add(new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex.Message), ex) });
+                            results.Add(new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex?.Message, ex?.StackTrace), ex) });
                         }
                     }
                 }
diff --git a/src/Adapter/MSTest.CoreAdapter/Resources/Resource.Designer.cs b/src/Adapter/MSTest.CoreAdapter/Resources/Resource.Designer.cs
diff --git a/src/Adapter/MSTest.CoreAdapter/Resources/Resource.resx b/src/Adapter/MSTest.CoreAdapter/Resources/Resource.resx
@@ -209,7 +209,7 @@ Error: {1}</value>
     <value>Class Initialization method {0}.{1} threw exception. {2}: {3}.</value>
   </data>
   <data name="UTA_ExecuteThrewException" xml:space="preserve">
-    <value>Exception thrown while executing test. If using extension of TestMethodAttribute then please contact vendor. Error message: {0}</value>
+    <value>Exception thrown while executing test. If using extension of TestMethodAttribute then please contact vendor. Error message: {0}, Stack trace: {1}</value>
   </data>
   <data name="UTA_NoTestResult" xml:space="preserve">
     <value>Error in executing test. No result returned by extension. If using extension of TestMethodAttribute then please contact vendor.</value>
@@ -320,4 +320,7 @@ Error: {1}</value>
   <data name="Execution_Test_Cancelled" xml:space="preserve">
     <value>Test '{0}' execution has been aborted.</value>
   </data>
+  <data name="UTA_UserCodeThrewNullValueException" xml:space="preserve">
+    <value>The called code threw an exception that was caught, but the exception value was null</value>
+  </data>
 </root>

Original file line number	Diff line number	Diff line change
`@@ -270,7 +270,7 @@ internal UnitTestResult[] RunTestMethod()`
`270`	`270`	`{`
`271`	`271`	`testResults = new[]`
`272`	`272`	`{`
`273`		`- new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex.Message), ex) }`
	`273`	`+ new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex?.Message, ex?.StackTrace), ex) }`
`274`	`274`	`};`
`275`	`275`	`}`
`276`	`276`
`@@ -324,7 +324,7 @@ internal UnitTestResult[] RunTestMethod()`
`324`	`324`	`{`
`325`	`325`	`testResults = new[]`
`326`	`326`	`{`
`327`		`- new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex.Message), ex) }`
	`327`	`+ new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex?.Message, ex?.StackTrace), ex) }`
`328`	`328`	`};`
`329`	`329`	`}`
`330`	`330`
`@@ -346,7 +346,7 @@ internal UnitTestResult[] RunTestMethod()`
`346`	`346`	`}`
`347`	`347`	`catch (Exception ex)`
`348`	`348`	`{`
`349`		`- results.Add(new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex.Message), ex) });`
	`349`	`+ results.Add(new UTF.TestResult() { TestFailureException = new Exception(string.Format(CultureInfo.CurrentCulture, Resource.UTA_ExecuteThrewException, ex?.Message, ex?.StackTrace), ex) });`
`350`	`350`	`}`
`351`	`351`	`}`
`352`	`352`	`}`