Skip to content

Commit 1e2abd5

Browse files
EvangelinkEvangelink
authored
Prevent swallowing inner exception in async error (#1712)
1 parent 6485a10 commit 1e2abd5

23 files changed

+181
-136
lines changed

‎src/Adapter/MSTest.TestAdapter/Execution/StackTraceHelper.cs renamed to ‎src/Adapter/MSTest.TestAdapter/Execution/ExceptionHelper.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
1515
/// <summary>
1616
/// Provides helper methods to parse stack trace.
1717
/// </summary>
18-
internal static class StackTraceHelper
18+
internal static class ExceptionHelper
1919
{
2020
/// <summary>
2121
/// Gets the types whose methods should be ignored in the reported call stacks.
@@ -33,7 +33,7 @@ internal static class StackTraceHelper
3333
/// <returns>
3434
/// The <see cref="StackTraceInformation"/> for the provided exception.
3535
/// </returns>
36-
internal static StackTraceInformation? GetStackTraceInformation(Exception ex)
36+
internal static StackTraceInformation? GetStackTraceInformation(this Exception ex)
3737
{
3838
DebugEx.Assert(ex != null, "exception should not be null.");
3939

@@ -138,7 +138,7 @@ internal static string TrimStackTrace(string stackTrace)
138138
/// The aggregated exception message that considers inner exceptions.
139139
/// </returns>
140140
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")]
141-
internal static string GetExceptionMessage(Exception ex)
141+
internal static string GetFormattedExceptionMessage(this Exception ex)
142142
{
143143
DebugEx.Assert(ex != null, "exception should not be null.");
144144

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ public void RunAssemblyInitialize(TestContext testContext)
167167

168168
var outcome = realException is AssertInconclusiveException ? UnitTestOutcome.Inconclusive : UnitTestOutcome.Failed;
169169

170-
// Do not use StackTraceHelper.GetExceptionMessage(realException) as it prefixes the message with the exception type name.
170+
// Do not use StackTraceHelper.GetFormattedExceptionMessage(realException) as it prefixes the message with the exception type name.
171171
var exceptionMessage = realException.TryGetMessage();
172172
DebugEx.Assert(AssemblyInitializeMethod.DeclaringType?.FullName is not null, "AssemblyInitializeMethod.DeclaringType.FullName is null");
173173
var errorMessage = string.Format(
@@ -177,7 +177,7 @@ public void RunAssemblyInitialize(TestContext testContext)
177177
AssemblyInitializeMethod.Name,
178178
realException.GetType().ToString(),
179179
exceptionMessage);
180-
var exceptionStackTraceInfo = StackTraceHelper.GetStackTraceInformation(realException);
180+
var exceptionStackTraceInfo = realException.GetStackTraceInformation();
181181

182182
var testFailedException = new TestFailedException(outcome, errorMessage, exceptionStackTraceInfo, realException);
183183
AssemblyInitializationException = testFailedException;
@@ -220,7 +220,7 @@ public void RunAssemblyInitialize(TestContext testContext)
220220
}
221221
else
222222
{
223-
errorMessage = StackTraceHelper.GetExceptionMessage(realException);
223+
errorMessage = realException.GetFormattedExceptionMessage();
224224
}
225225

226226
DebugEx.Assert(AssemblyCleanupMethod.DeclaringType?.Name is not null, "AssemblyCleanupMethod.DeclaringType.Name is null");
@@ -230,7 +230,7 @@ public void RunAssemblyInitialize(TestContext testContext)
230230
AssemblyCleanupMethod.DeclaringType.Name,
231231
AssemblyCleanupMethod.Name,
232232
errorMessage,
233-
StackTraceHelper.GetStackTraceInformation(realException)?.ErrorStackTrace);
233+
realException.GetStackTraceInformation()?.ErrorStackTrace);
234234
}
235235
}
236236
}
@@ -268,10 +268,10 @@ internal void ExecuteAssemblyCleanup()
268268
}
269269
else
270270
{
271-
errorMessage = StackTraceHelper.GetExceptionMessage(realException);
271+
errorMessage = realException.GetFormattedExceptionMessage();
272272
}
273273

274-
var exceptionStackTraceInfo = StackTraceHelper.GetStackTraceInformation(realException);
274+
var exceptionStackTraceInfo = realException.GetStackTraceInformation();
275275
DebugEx.Assert(AssemblyCleanupMethod.DeclaringType?.Name is not null, "AssemblyCleanupMethod.DeclaringType.Name is null");
276276

277277
throw new TestFailedException(

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ public void RunClassInitialize(TestContext testContext)
306306

307307
var outcome = realException is AssertInconclusiveException ? ObjectModelUnitTestOutcome.Inconclusive : ObjectModelUnitTestOutcome.Failed;
308308

309-
// Do not use StackTraceHelper.GetExceptionMessage(realException) as it prefixes the message with the exception type name.
309+
// Do not use StackTraceHelper.GetFormattedExceptionMessage(realException) as it prefixes the message with the exception type name.
310310
var exceptionMessage = realException.TryGetMessage();
311311
var errorMessage = string.Format(
312312
CultureInfo.CurrentCulture,
@@ -315,7 +315,7 @@ public void RunClassInitialize(TestContext testContext)
315315
failedClassInitializeMethodName,
316316
realException.GetType().ToString(),
317317
exceptionMessage);
318-
var exceptionStackTraceInfo = StackTraceHelper.GetStackTraceInformation(realException);
318+
var exceptionStackTraceInfo = realException.GetStackTraceInformation();
319319

320320
var testFailedException = new TestFailedException(outcome, errorMessage, exceptionStackTraceInfo, realException);
321321
ClassInitializationException = testFailedException;
@@ -373,17 +373,10 @@ public void RunClassInitialize(TestContext testContext)
373373
var realException = exception.InnerException ?? exception;
374374
ClassCleanupException = realException;
375375

376-
string errorMessage;
377-
378376
// special case AssertFailedException to trim off part of the stack trace
379-
if (realException is AssertFailedException or AssertInconclusiveException)
380-
{
381-
errorMessage = realException.Message;
382-
}
383-
else
384-
{
385-
errorMessage = StackTraceHelper.GetExceptionMessage(realException);
386-
}
377+
string errorMessage = realException is AssertFailedException or AssertInconclusiveException
378+
? realException.Message
379+
: realException.GetFormattedExceptionMessage();
387380

388381
var exceptionStackTraceInfo = realException.TryGetStackTraceInformation();
389382

@@ -458,7 +451,7 @@ internal void ExecuteClassCleanup()
458451
// special case AssertFailedException to trim off part of the stack trace
459452
string errorMessage = realException is AssertFailedException or AssertInconclusiveException
460453
? realException.Message
461-
: StackTraceHelper.GetExceptionMessage(realException);
454+
: realException.GetFormattedExceptionMessage();
462455

463456
var exceptionStackTraceInfo = realException.TryGetStackTraceInformation();
464457

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

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ private static Exception HandleMethodException(Exception ex, string className, s
430430
Resource.UTA_TestMethodThrows,
431431
className,
432432
methodName,
433-
StackTraceHelper.GetExceptionMessage(realException));
433+
realException.GetFormattedExceptionMessage());
434434

435435
// Handle special case of UI objects in TestMethod to suggest UITestMethod
436436
if (realException.HResult == -2147417842)
@@ -446,7 +446,7 @@ private static Exception HandleMethodException(Exception ex, string className, s
446446
// which has no meaningful info for the user. Thus, we do not show call stack for ThreadAbortException.
447447
if (realException.GetType().Name != "ThreadAbortException")
448448
{
449-
stackTrace = StackTraceHelper.GetStackTraceInformation(realException);
449+
stackTrace = realException.GetStackTraceInformation();
450450
}
451451

452452
return new TestFailedException(ObjectModelUnitTestOutcome.Failed, errorMessage, stackTrace, realException);
@@ -508,27 +508,25 @@ private void RunTestCleanupMethod(object classInstance, TestResult result)
508508
}
509509
}
510510

511-
Exception realException = ex.GetInnerExceptionOrDefault();
511+
Exception realException = ex.GetRealException();
512+
string formattedExceptionMessage = realException.GetFormattedExceptionMessage();
512513

513514
if (testCleanupMethod != null)
514515
{
515-
// Do not use StackTraceHelper.GetExceptionMessage(realException) as it prefixes the message with the exception type name.
516516
cleanupError.AppendFormat(
517517
CultureInfo.CurrentCulture,
518518
Resource.UTA_CleanupMethodThrows,
519519
TestClassName,
520520
testCleanupMethod.Name,
521-
realException.GetType().ToString(),
522-
realException.TryGetMessage());
521+
formattedExceptionMessage);
523522
}
524523
else
525524
{
526-
// Use StackTraceHelper.GetExceptionMessage(realException) to get the message prefixed with the exception type name.
527525
cleanupError.AppendFormat(
528526
CultureInfo.CurrentCulture,
529527
Resource.UTA_CleanupMethodThrowsGeneralError,
530528
TestClassName,
531-
StackTraceHelper.GetExceptionMessage(realException));
529+
formattedExceptionMessage);
532530
}
533531

534532
StackTraceInformation? cleanupStackTraceInfo = null;
@@ -583,19 +581,19 @@ private bool RunTestInitializeMethod(object classInstance, TestResult result)
583581
}
584582
catch (Exception ex)
585583
{
586-
var realException = ex.GetInnerExceptionOrDefault();
584+
var realException = ex.GetRealException();
587585

588586
// Prefix the exception message with the exception type name as prefix when exception is not assert exception.
589587
var exceptionMessage = realException is UnitTestAssertException
590588
? realException.TryGetMessage()
591-
: StackTraceHelper.GetExceptionMessage(realException);
589+
: ExceptionHelper.GetFormattedExceptionMessage(realException);
592590
var errorMessage = string.Format(
593591
CultureInfo.CurrentCulture,
594592
Resource.UTA_InitMethodThrows,
595593
TestClassName,
596594
testInitializeMethod?.Name,
597595
exceptionMessage);
598-
var stackTrace = StackTraceHelper.GetStackTraceInformation(realException);
596+
var stackTrace = realException.GetStackTraceInformation();
599597

600598
result.Outcome = realException is AssertInconclusiveException ? UTF.UnitTestOutcome.Inconclusive : UTF.UnitTestOutcome.Failed;
601599
result.TestFailureException = new TestFailedException(
@@ -637,14 +635,15 @@ private bool SetTestContext(object classInstance, TestResult result)
637635
}
638636
catch (Exception ex)
639637
{
640-
var stackTraceInfo = StackTraceHelper.GetStackTraceInformation(ex.GetInnerExceptionOrDefault());
638+
var realException = ex.GetRealException();
641639
var errorMessage = string.Format(
642640
CultureInfo.CurrentCulture,
643641
Resource.UTA_TestContextSetError,
644642
TestClassName,
645-
StackTraceHelper.GetExceptionMessage(ex.GetInnerExceptionOrDefault()));
643+
realException.GetFormattedExceptionMessage());
646644

647645
result.Outcome = UTF.UnitTestOutcome.Failed;
646+
var stackTraceInfo = realException.GetStackTraceInformation();
648647
result.TestFailureException = new TestFailedException(ObjectModelUnitTestOutcome.Failed, errorMessage, stackTraceInfo);
649648
}
650649

@@ -689,8 +688,8 @@ private bool SetTestContext(object classInstance, TestResult result)
689688
// in the InnerException; or user code throws an exception.
690689
// It also seems that in rare cases the ex can be null.
691690
var actualException = ex.InnerException ?? ex;
692-
var exceptionMessage = StackTraceHelper.GetExceptionMessage(actualException);
693-
var stackTraceInfo = StackTraceHelper.GetStackTraceInformation(actualException);
691+
var exceptionMessage = actualException.GetFormattedExceptionMessage();
692+
var stackTraceInfo = actualException.GetStackTraceInformation();
694693

695694
var errorMessage = string.Format(
696695
CultureInfo.CurrentCulture,

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Diagnostics.CodeAnalysis;
66
using System.Globalization;
7+
using System.Reflection;
78

89
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
910
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
@@ -19,17 +20,15 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
1920
internal static class ExceptionExtensions
2021
{
2122
/// <summary>
22-
/// Get the InnerException if available, else return the current Exception.
23+
/// In .NET framework, the exception thrown by the test method invocation is wrapped in a TargetInvocationException, so we
24+
/// need to unwrap it to get the real exception.
2325
/// </summary>
24-
/// <param name="exception">The exception.</param>
25-
/// <returns>
26-
/// An <see cref="Exception"/> instance.
27-
/// </returns>
28-
[return: NotNullIfNotNull(nameof(exception))]
29-
internal static Exception? GetInnerExceptionOrDefault(this Exception exception)
30-
{
31-
return exception?.InnerException ?? exception;
32-
}
26+
internal static Exception GetRealException(this Exception exception)
27+
=> exception.GetType() == typeof(TargetInvocationException)
28+
&& exception.Source is "mscorlib" or "System.Private.CoreLib"
29+
&& exception.InnerException is not null
30+
? exception.InnerException
31+
: exception;
3332

3433
/// <summary>
3534
/// Get the exception message if available, empty otherwise.
@@ -56,7 +55,7 @@ internal static string TryGetMessage(this Exception exception)
5655
{
5756
if (!StringEx.IsNullOrEmpty(exception?.StackTrace))
5857
{
59-
return StackTraceHelper.CreateStackTraceInformation(exception, false, exception.StackTrace);
58+
return ExceptionHelper.CreateStackTraceInformation(exception, false, exception.StackTrace);
6059
}
6160

6261
return null;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public bool HasAttributeDerivedFrom<TAttribute>(MemberInfo memberInfo, bool inhe
153153
Resource.UTA_ExpectedExceptionAttributeConstructionException,
154154
testMethod.FullClassName,
155155
testMethod.Name,
156-
StackTraceHelper.GetExceptionMessage(ex));
156+
ex.GetFormattedExceptionMessage());
157157
throw new TypeInspectionException(errorMessage);
158158
}
159159

‎src/Adapter/MSTest.TestAdapter/Resources/Resource.Designer.cs

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@
124124
<value>Running tests in any of the provided sources is not supported for the selected platform</value>
125125
</data>
126126
<data name="UTA_CleanupMethodThrows" xml:space="preserve">
127-
<value>TestCleanup method {0}.{1} threw exception. {2}: {3}.</value>
127+
<value>TestCleanup method {0}.{1} threw exception. {2}.</value>
128128
</data>
129129
<data name="UTA_EndOfInnerExceptionTrace" xml:space="preserve">
130130
<value>--- End of inner exception stack trace ---</value>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
<note></note>
1414
</trans-unit>
1515
<trans-unit id="UTA_CleanupMethodThrows">
16-
<source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
17-
<target state="translated">Metoda TestCleanup {0}.{1} způsobila výjimku. {2}: {3}.</target>
18-
<note></note>
16+
<source>TestCleanup method {0}.{1} threw exception. {2}.</source>
17+
<target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
18+
<note />
1919
</trans-unit>
2020
<trans-unit id="UTA_EndOfInnerExceptionTrace">
2121
<source>--- End of inner exception stack trace ---</source>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
<note></note>
1414
</trans-unit>
1515
<trans-unit id="UTA_CleanupMethodThrows">
16-
<source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
17-
<target state="translated">Die TestCleanup-Methode "{0}.{1}" hat eine Ausnahme ausgelöst. {2}: {3}.</target>
18-
<note></note>
16+
<source>TestCleanup method {0}.{1} threw exception. {2}.</source>
17+
<target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
18+
<note />
1919
</trans-unit>
2020
<trans-unit id="UTA_EndOfInnerExceptionTrace">
2121
<source>--- End of inner exception stack trace ---</source>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
<note></note>
1414
</trans-unit>
1515
<trans-unit id="UTA_CleanupMethodThrows">
16-
<source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
17-
<target state="translated">El método TestCleanup {0}.{1} devolvió una excepción. {2}: {3}.</target>
18-
<note></note>
16+
<source>TestCleanup method {0}.{1} threw exception. {2}.</source>
17+
<target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
18+
<note />
1919
</trans-unit>
2020
<trans-unit id="UTA_EndOfInnerExceptionTrace">
2121
<source>--- End of inner exception stack trace ---</source>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
<note></note>
1414
</trans-unit>
1515
<trans-unit id="UTA_CleanupMethodThrows">
16-
<source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
17-
<target state="translated">La méthode TestCleanup {0}.{1} a levé une exception. {2}: {3}.</target>
18-
<note></note>
16+
<source>TestCleanup method {0}.{1} threw exception. {2}.</source>
17+
<target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
18+
<note />
1919
</trans-unit>
2020
<trans-unit id="UTA_EndOfInnerExceptionTrace">
2121
<source>--- End of inner exception stack trace ---</source>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
<note></note>
1414
</trans-unit>
1515
<trans-unit id="UTA_CleanupMethodThrows">
16-
<source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
17-
<target state="translated">Il metodo TestCleanup {0}.{1} ha generato un'eccezione. {2}: {3}.</target>
18-
<note></note>
16+
<source>TestCleanup method {0}.{1} threw exception. {2}.</source>
17+
<target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
18+
<note />
1919
</trans-unit>
2020
<trans-unit id="UTA_EndOfInnerExceptionTrace">
2121
<source>--- End of inner exception stack trace ---</source>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
<note></note>
1414
</trans-unit>
1515
<trans-unit id="UTA_CleanupMethodThrows">
16-
<source>TestCleanup method {0}.{1} threw exception. {2}: {3}.</source>
17-
<target state="translated">TestCleanup メソッド {0}.{1} は例外をスローしました。{2}: {3}。</target>
18-
<note></note>
16+
<source>TestCleanup method {0}.{1} threw exception. {2}.</source>
17+
<target state="new">TestCleanup method {0}.{1} threw exception. {2}.</target>
18+
<note />
1919
</trans-unit>
2020
<trans-unit id="UTA_EndOfInnerExceptionTrace">
2121
<source>--- End of inner exception stack trace ---</source>

0 commit comments

Comments
 (0)