Catch exceptions thrown by 'async void' methods.#5068
Conversation
Also set exception handlers of last resort
There was a problem hiding this comment.
Pull request overview
This PR implements exception handling for async void methods and other unobserved exceptions during test execution. The changes ensure that exceptions thrown asynchronously that would otherwise crash the test runner are captured and reported as test failures.
Key changes:
- Added a
SafeSynchronizationContextwrapper to catch exceptions posted fromasync voidmethods - Registered global exception handlers (
AppDomain.UnhandledExceptionandTaskScheduler.UnobservedTaskException) to catch unobserved exceptions - Added test coverage for the async void exception handling scenario
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/NUnitFramework/tests/AsyncVoidTests.cs | New test verifying that exceptions from async void methods are caught and reported |
| src/NUnitFramework/testdata/AsyncVoidFixture.cs | Test fixture demonstrating async void exception scenario with intentional delay |
| src/NUnitFramework/framework/Internal/ContextUtils.cs | Implements SafeSynchronizationContext to catch exceptions posted by async void methods |
| src/NUnitFramework/framework/Api/NUnitTestAssemblyRunner.cs | Registers and unregisters global exception handlers for unobserved exceptions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| context.CurrentResult.RecordException(e, FailureSite.Test); | ||
| context.CurrentResult.RecordTestCompletion(); |
There was a problem hiding this comment.
RecordException and RecordTestCompletion are called without synchronization. If multiple async void methods throw exceptions concurrently, this could result in race conditions when updating the test result state.
|
Currently the specific Windows SynchronizationContext tests are failing because of the extra unexpected context. I will look in them when I get time. |
|
Is this WIP ? Or can this be merged in "as is" ? I have no comments on it. |
|
No, I fixed the code so the test all pass again. |
Fixes #3930
Fixes #3740
Also set exception handlers of last resort