-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
Exceptions in Fire&Forget tasks can be missed by our tests #46808
Comments
Tagging subscribers to this area: @safern, @ViktorHofer Issue Details#46774 has shown, that there are some Fire&Forget tasks that can throw an unnoticed exception when we introduce a bug in BCL (see #46807 (comment) for details) This is of course very rare as we try to avoid Fire&Forget tasks, but anyway it is possible (at least for the I wonder whether there is anything that we have done in the past to detect such issues? If not, should we do something about it? Like for example extending xunit test runner to report a failure on [Fact]
public void ThrowingFireAndForgetThatPasses()
{
Schedule(new object());
static void Schedule(object obj)
{
Task.Factory.StartNew(state =>
{
string text = (string)state; // fails
Console.WriteLine(text);
}, obj, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
}
}
|
Not really; it's up to the test to not eat exceptions. Even a TaskScheduler.UnobservedTaskException wouldn't necessarily help, as it would only trigger upon finalization happening; there are also some places where product code has done it, in some cases intentionally allowing exceptions to be eaten (for better or worse), which would make such an enabled feature problematic. Every now and then I have done a search to ensure we don't have async void tests, but that's obviously just one manifestation of this. At the end of the day, it's not all that different from a try/catch{} that any test code could have and similarly eat all exceptions, including assert failures. |
....note that this is one of the big reasons TDD mandates writing a failing test first - if you don't know what failure looks like, you can't be sure of your success. |
Moving out of Infrastructure as this isn't specific to our test infrastructure (MSBuild, xunit aquisition, etc.) |
+1. We have an Azure App Service webapp occasionally crashing due to an unhandled exception thrown from an async void method. Easy to fix, but while attempting to follow TDD and write a failing test for that situation I've had no luck. Neither TaskScheduler.UnobservedTaskException nor AppDomain.CurrentDomain.UnhandledException catch the exception, and AppDomain.CurrentDomain.FirstChanceException does but catches all exceptions and not just unhandled. The creator of Xunit says it's due to the VSTest engine swallowing the exception (xunit/xunit#157). |
#46774 has shown, that there are some Fire&Forget tasks that can throw an unnoticed exception when we introduce a bug in BCL (see #46807 (comment) for details)
This is of course very rare as we try to avoid Fire&Forget tasks, but anyway it is possible (at least for the
MemoryCache
tests).I wonder whether there is anything that we have done in the past to detect such issues?
If not, should we do something about it? Like for example extending xunit test runner to report a failure on
TaskScheduler.UnobservedTaskException
event? (I don't know if it's currently possible, it's just a hypothetical example to start a discussion)@stephentoub @eerhardt
The text was updated successfully, but these errors were encountered: