Skip to content

Avoid swallowing hook exceptions #2743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 4, 2025
Merged

Avoid swallowing hook exceptions #2743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion TUnit.Engine/Scheduling/HookOrchestratingTestExecutorAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ await _messageBus.PublishAsync(
test.EndTime = DateTimeOffset.UtcNow;

// Execute cleanup hooks
Exception? cleanupException = null;
try
{
// Use a separate cancellation token for cleanup to ensure cleanup hooks execute
Expand All @@ -139,8 +140,51 @@ await _messageBus.PublishAsync(
}
catch (Exception ex)
{
// Log but don't throw - after hooks shouldn't prevent test completion reporting
await _logger.LogErrorAsync($"Error in cleanup hooks for test {test.TestId}: {ex}");
cleanupException = ex;

// If test passed but after hooks failed, update the test result
if (test.State == TestState.Passed)
{
test.State = TestState.Failed;
test.Result = new TestResult
{
State = TestState.Failed,
Start = test.StartTime,
End = test.EndTime,
Duration = test.EndTime.GetValueOrDefault() - test.StartTime.GetValueOrDefault(),
Exception = ex,
ComputerName = Environment.MachineName
};

// Report the failure
await _messageBus.PublishAsync(
this,
new TestNodeUpdateMessage(
_sessionUid,
test.Context.ToTestNode().WithProperty(new FailedTestNodeStateProperty(ex))));
}
// If test already failed and after hooks also failed, we need to report both failures
else if (test.State == TestState.Failed && test.Result?.Exception != null)
{
var aggregateException = new AggregateException("Test and after hooks both failed", test.Result.Exception, ex);
test.Result = new TestResult
{
State = TestState.Failed,
Start = test.Result.Start,
End = test.EndTime,
Duration = test.Result.Duration,
Exception = aggregateException,
ComputerName = test.Result.ComputerName
};

// Update the failure message
await _messageBus.PublishAsync(
this,
new TestNodeUpdateMessage(
_sessionUid,
test.Context.ToTestNode().WithProperty(new FailedTestNodeStateProperty(aggregateException))));
}
}
}
}
Expand Down
51 changes: 48 additions & 3 deletions TUnit.Engine/Services/HookOrchestrator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using TUnit.Core;
using TUnit.Core.Data;
using TUnit.Core.Services;
using TUnit.Engine.Exceptions;
using TUnit.Engine.Framework;
using TUnit.Engine.Interfaces;
using TUnit.Engine.Logging;
Expand Down Expand Up @@ -93,6 +94,7 @@ private Task<ExecutionContext> GetOrCreateBeforeClassTask(
public async Task<ExecutionContext?> ExecuteAfterTestSessionHooksAsync(CancellationToken cancellationToken)
{
var hooks = await _hookCollectionService.CollectAfterTestSessionHooksAsync();
var exceptions = new List<Exception>();

foreach (var hook in hooks)
{
Expand All @@ -104,9 +106,16 @@ private Task<ExecutionContext> GetOrCreateBeforeClassTask(
catch (Exception ex)
{
await _logger.LogErrorAsync($"AfterTestSession hook failed: {ex.Message}");
// After hooks failures are logged but don't stop execution
exceptions.Add(ex);
}
}

if (exceptions.Count > 0)
{
throw exceptions.Count == 1
? new HookFailedException(exceptions[0])
: new HookFailedException("Multiple AfterTestSession hooks failed", new AggregateException(exceptions));
}

#if NET
return ExecutionContext.Capture();
Expand Down Expand Up @@ -143,6 +152,7 @@ private Task<ExecutionContext> GetOrCreateBeforeClassTask(
public async Task<ExecutionContext?> ExecuteAfterTestDiscoveryHooksAsync(CancellationToken cancellationToken)
{
var hooks = await _hookCollectionService.CollectAfterTestDiscoveryHooksAsync();
var exceptions = new List<Exception>();

foreach (var hook in hooks)
{
Expand All @@ -154,8 +164,16 @@ private Task<ExecutionContext> GetOrCreateBeforeClassTask(
catch (Exception ex)
{
await _logger.LogErrorAsync($"AfterTestDiscovery hook failed: {ex.Message}");
exceptions.Add(ex);
}
}

if (exceptions.Count > 0)
{
throw exceptions.Count == 1
? new HookFailedException(exceptions[0])
: new HookFailedException("Multiple AfterTestDiscovery hooks failed", new AggregateException(exceptions));
}

#if NET
return ExecutionContext.Capture();
Expand Down Expand Up @@ -259,8 +277,8 @@ private async Task<ExecutionContext> ExecuteBeforeAssemblyHooksAsync(Assembly as
private async Task<ExecutionContext> ExecuteAfterAssemblyHooksAsync(Assembly assembly, CancellationToken cancellationToken)
{
var hooks = await _hookCollectionService.CollectAfterAssemblyHooksAsync(assembly);

var assemblyContext = _contextProvider.GetOrCreateAssemblyContext(assembly);
var exceptions = new List<Exception>();

// Execute global AfterEveryAssembly hooks first
var everyHooks = await _hookCollectionService.CollectAfterEveryAssemblyHooksAsync();
Expand All @@ -274,6 +292,7 @@ private async Task<ExecutionContext> ExecuteAfterAssemblyHooksAsync(Assembly ass
catch (Exception ex)
{
await _logger.LogErrorAsync($"AfterEveryAssembly hook failed for {assembly.GetName().Name}: {ex.Message}");
exceptions.Add(ex);
}
}

Expand All @@ -287,8 +306,16 @@ private async Task<ExecutionContext> ExecuteAfterAssemblyHooksAsync(Assembly ass
catch (Exception ex)
{
await _logger.LogErrorAsync($"AfterAssembly hook failed for {assembly.GetName().Name}: {ex.Message}");
exceptions.Add(ex);
}
}

if (exceptions.Count > 0)
{
throw exceptions.Count == 1
? new HookFailedException(exceptions[0])
: new HookFailedException("Multiple AfterAssembly hooks failed", new AggregateException(exceptions));
}

return ExecutionContext.Capture()!;
}
Expand Down Expand Up @@ -339,8 +366,8 @@ private async Task<ExecutionContext> ExecuteAfterClassHooksAsync(
Type testClassType, CancellationToken cancellationToken)
{
var hooks = await _hookCollectionService.CollectAfterClassHooksAsync(testClassType);

var classContext = _contextProvider.GetOrCreateClassContext(testClassType);
var exceptions = new List<Exception>();

// Execute global AfterEveryClass hooks first
var everyHooks = await _hookCollectionService.CollectAfterEveryClassHooksAsync();
Expand All @@ -354,6 +381,7 @@ private async Task<ExecutionContext> ExecuteAfterClassHooksAsync(
catch (Exception ex)
{
await _logger.LogErrorAsync($"AfterEveryClass hook failed for {testClassType.Name}: {ex.Message}");
exceptions.Add(ex);
}
}

Expand All @@ -367,8 +395,16 @@ private async Task<ExecutionContext> ExecuteAfterClassHooksAsync(
catch (Exception ex)
{
await _logger.LogErrorAsync($"AfterClass hook failed for {testClassType.Name}: {ex.Message}");
exceptions.Add(ex);
}
}

if (exceptions.Count > 0)
{
throw exceptions.Count == 1
? new HookFailedException(exceptions[0])
: new HookFailedException("Multiple AfterClass hooks failed", new AggregateException(exceptions));
}

return ExecutionContext.Capture()!;
}
Expand All @@ -395,6 +431,7 @@ private async Task ExecuteBeforeEveryTestHooksAsync(Type testClassType, TestCont
private async Task ExecuteAfterEveryTestHooksAsync(Type testClassType, TestContext testContext, CancellationToken cancellationToken)
{
var hooks = await _hookCollectionService.CollectAfterEveryTestHooksAsync(testClassType);
var exceptions = new List<Exception>();

foreach (var hook in hooks)
{
Expand All @@ -406,7 +443,15 @@ private async Task ExecuteAfterEveryTestHooksAsync(Type testClassType, TestConte
catch (Exception ex)
{
await _logger.LogErrorAsync($"AfterEveryTest hook failed: {ex.Message}");
exceptions.Add(ex);
}
}

if (exceptions.Count > 0)
{
throw exceptions.Count == 1
? new HookFailedException(exceptions[0])
: new HookFailedException("Multiple AfterEveryTest hooks failed", new AggregateException(exceptions));
}
}
}
41 changes: 40 additions & 1 deletion TUnit.Engine/Services/SingleTestExecutor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using TUnit.Core.Logging;
using TUnit.Core.ReferenceTracking;
using TUnit.Core.Tracking;
using TUnit.Engine.Exceptions;
using TUnit.Engine.Extensions;
using TUnit.Engine.Interfaces;
using TUnit.Engine.Logging;
Expand Down Expand Up @@ -165,6 +166,7 @@ private async Task ExecuteTestWithHooksAsync(AbstractExecutableTest test, object
var beforeTestHooks = await _hookCollectionService.CollectBeforeTestHooksAsync(testClassType);
var afterTestHooks = await _hookCollectionService.CollectAfterTestHooksAsync(testClassType);

Exception? testException = null;
try
{
await ExecuteBeforeTestHooksAsync(beforeTestHooks, test.Context, cancellationToken);
Expand All @@ -179,13 +181,35 @@ private async Task ExecuteTestWithHooksAsync(AbstractExecutableTest test, object
catch (Exception ex)
{
HandleTestFailure(test, ex);
testException = ex;
}

try
{
await ExecuteAfterTestHooksAsync(afterTestHooks, test.Context, cancellationToken);
}
catch (Exception afterHookEx)
{
// If test already failed, aggregate the exceptions
if (testException != null)
{
throw new AggregateException("Test and after hook both failed", testException, afterHookEx);
}

// Otherwise, fail the test due to after hook failure
HandleTestFailure(test, afterHookEx);
throw;
}
finally
{
await ExecuteAfterTestHooksAsync(afterTestHooks, test.Context, cancellationToken);
await DecrementAndDisposeTrackedObjectsAsync(test);
}

// Re-throw original test exception if after hooks succeeded
if (testException != null)
{
throw testException;
}
}


Expand All @@ -212,6 +236,8 @@ private async Task ExecuteBeforeTestHooksAsync(IReadOnlyList<Func<TestContext, C

private async Task ExecuteAfterTestHooksAsync(IReadOnlyList<Func<TestContext, CancellationToken, Task>> hooks, TestContext context, CancellationToken cancellationToken)
{
var exceptions = new List<Exception>();

foreach (var hook in hooks)
{
try
Expand All @@ -223,6 +249,19 @@ private async Task ExecuteAfterTestHooksAsync(IReadOnlyList<Func<TestContext, Ca
catch (Exception ex)
{
await _logger.LogErrorAsync($"Error in after test hook: {ex.Message}");
exceptions.Add(ex);
}
}

if (exceptions.Count > 0)
{
if (exceptions.Count == 1)
{
throw new HookFailedException(exceptions[0]);
}
else
{
throw new HookFailedException("Multiple after test hooks failed", new AggregateException(exceptions));
}
}
}
Expand Down
Loading
Loading