-
Notifications
You must be signed in to change notification settings - Fork 1
Async void addition #1
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
base: master
Are you sure you want to change the base?
Conversation
ariedov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this case! I, honestly, didn't know that async void and async Task are so different in behavior.
There are a few notes left in the test.
AwaitExpception.Test/Tests.cs
Outdated
| Console.WriteLine("The exception is actually caught locally"); | ||
|
|
||
| // wait for the test to finish | ||
| await Task.Delay(10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't we going to wait only after the exception is thrown, which is never happening?
And should we really catch the exception? I would want the test to fail if the exception happened, and when it is failing - wrap it with Assert.ThrowsAsync.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reworked to the form you asked for, but the problem here may be a bit wider. Assert.Throw will not catch anything and actually nothing can catch so this case is a guarantee of crash because it shows an uncontrollable flow of exception in this pattern and actually that was a key part of the test - it just can't be run as a normal test (at least I could not yet find a way how to do so).
| { | ||
| // Unfortunately I haven't yet found the way to not fail here. | ||
| // Looks like the exception is caught but the process fails despite of that fact | ||
| Console.WriteLine("The exception is actually caught globally"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we add this code after we know how to deal with it correctly? Right now it is just dead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No this is the key code here. But it doesn't work as expected for some reason. This is a generic mechanism for catching uncaught exceptions. So we know we expect an uncaught exception, but this method doesn't handle the exception it catches. The exception just propagates further for some reason. The console log here is important that in the console you will see a log that the exception was actually caught with this mechanism before it crashes the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, looks like UnhandledException is not intended to recover from the exception it just notifies that your application will fail in a moment so you for example can log something. And we are logging that. Seems like there is nothing we can do to prevent the crash. Looks like that is an example of a 100% crash if exception is raised.
No description provided.