dotnet / command-line-api

Command line parsing, invocation, and rendering of terminal output.
https://github.com/dotnet/command-line-api/wiki
MIT License
3.38k stars 380 forks source link

Async command handler exists execution unexpectedly. #1642

Open epopov opened 2 years ago

epopov commented 2 years ago

We have declared async command handler this way:

public static async Task ImportTwilioMessages(ImportTwilioOptions options, IHost host) { }

and set it up this way:

loadTwilioSmsStatuses.Handler = CommandHandler.Create<ImportTwilioOptions, IHost>(ImportTwilioMessages);

and when this method is callled: var fetchedResult = await MessageResource.ReadAsync(dateSentAfter: from, dateSentBefore: to); the whole process just exits without any exceptions.

Just switching to not-async command handler solved the issue.

elgonzo commented 2 years ago

What exactly do you mean when saying "Just switching to not-async command handler [...]"? Is your non-async command handler still calling async Twilio functions from your non-async command handler, or is your non-async command handler only calling non-async Twilio functions?

(My gut feeling tells me the issue is somewhere in the async code your async command handler is calling, because System.CommandLine does not do any crazy magic with the Task object returned by the handler -- it's just awaiting it, thus i would not expect any surprising behavior with respect to System.CommandLine itself. And since System.CommandLine is just awaiting the handler's Task object, exceptions will also be captured just fine)

EDIT: I updated the link to my dotnetfiddle example. The original fiddle used a Task.Delay of 1s, which sometimes pushes the program execution time over the program execution time limit allowed by dotnetfiddle, which in turn would cause dotnetfiddle to silently kill the program before it got a chance of throwing the exception, thus producing a misleading output. The updated dotnetfiddle example has a reduced Task.Delay of only 50ms, which should hopefully prevent dotnetfiddle from prematurely aborting the program and allow the exception to be thrown and printed.)

gpetrounrt commented 2 years ago

If I run a few times the example above, it gives a different output than expected. I changed the delay to 5000, to make it happen more often. image

elgonzo commented 2 years ago

If I run a few times the example above, it gives a different output than expected. I changed the delay to 5000, to make it happen more often.

Oh, i thought a Task.Delay of 1s was safe for dotnetfiddle. But when i run my dotnetfiddle example multiple times (dozens of times), i can indeed sporadically observe program abortions, with the exception not being thrown before the forced abort of the program by dotnetfiddle¹. I thought a delay of "just" 1s was low enough to not cause a program abort, but i apparently guessed wrong. I updated my dotnetfiddle sample to use a delay of only 50ms, thus hopefully reliably avoiding dotnetfiddle aborting the program and thus more reliably allowing the exception to be thrown and output.

So, thanks for alerting me to this issue with my dotnetfiddle example!


¹To protect the servers running the dotnetfiddle code snippets from being throttled/overloaded or practically being ground to a halt, a "long running" program execution (anything longer than a couple of seconds) is aborted by dotnetfiddle. I don't know the exact alloted time for a program run, but it seems to be somewhat variable to me (i would suspect due to varying server loads running multiple different dotnetfiddles from different users at the same time). Unfortunately, dotnetfiddle does a poor job in indicating when it did abort program execution, with the only indicator of this happening i am aware of being the execution time being displayed as zero seconds (Execute: 0s).

Therefore, if the Task.Delay time span is changed to large-ish values (such as 5000ms or larger) in my dotnetfiddle example, it will substantially increase the chance of dotnetfiddle simply killing the program before the throw new InvalidOperationException... line is being executed, thus the exception not being printed in the output window simply because the program was aborted before it could throw the exception.