cli: show some love for error reporting

[Syndace]

Checklist

• [X] I have checked the issue page for duplicates
• [X] I am running the latest version (download here)

Write your feature request here

I feel like the error reporting in the cli could use a little love. Just because cli users are often more tech-savvy than gui users doesn't mean that they want to guess from stack traces whether there was a network error or they pasted the wrong url. For example, videodownload with the id of a video that is no longer available on Twitch gives the following output:

[STATUS] - Fetching Video Info [1/4]
Unhandled exception. System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.)
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at TwitchDownloaderCore.VideoDownloader.GetPlaylistUrl()
   at TwitchDownloaderCore.VideoDownloader.DownloadAsync(IProgress`1 progress, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean )
   at System.Threading.Tasks.Task.Wait(Int32 , CancellationToken )
   at System.Threading.Tasks.Task.Wait()
   at TwitchDownloaderCLI.Modes.DownloadVideo.Download(VideoDownloadArgs inputOptions)
   at CommandLine.ParserResultExtensions.WithParsed[T](ParserResult`1 result, Action`1 action)
   at TwitchDownloaderCLI.Program.Main(String[] args)
Aborted

If I didn't know that the video no longer exists, I'd have no idea from this stack trace alone. This is neither user nor scripting friendly. (I've tried both the newest release and the newest commit on Linux btw.)

It would be great if the cli clearly differentiated between the following three major error causes:

• not found (in case the video/clip doesn't exist)
• network I/O error
• disk I/O error (e.g. if the disk is full)

This would help both users and scripts decide how to react:

• not found? whoops my bad, double-check the video/clip id
• network I/O error? check internet connection, maybe wait a bit and try again
• disk I/O error? check e.g. if the USB drive is connected correctly, make sure there is enough space available before trying again etc.

Bonus points if the kind of error would also be represented via the process' exit code and not just from the stderr output.

[ScrubN]

Didn't realize videodownload and clipdownload lacked id verification. chatdownload and chatupdate already do so I guess I just assumed the others did too.

As for exit codes that would be really neat but would probably also demand some refactoring.

[Syndace]

Oh, did I accidentally try the few error conditions that don't have nice output? I also tried videodownload without a network connection (you can use unshare -n to quickly emulate the lack of an internet connection) and it gives me the following output:

[STATUS] - Fetching Video Info [1/4]
Unhandled exception. System.AggregateException: One or more errors occurred. (Name or service not known (gql.twitch.tv:443))
 ---> System.Net.Http.HttpRequestException: Name or service not known (gql.twitch.tv:443)
 ---> System.Net.Sockets.SocketException (0xFFFDFFFF): Name or service not known
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError , CancellationToken )
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource.GetResult(Int16 )
   at System.Net.Sockets.Socket.<ConnectAsync>g__WaitForConnectWithCancellation|277_0(AwaitableSocketAsyncEventArgs , ValueTask , CancellationToken )
   at System.Net.Http.HttpConnectionPool.ConnectToTcpHostAsync(String , Int32 , HttpRequestMessage , Boolean , CancellationToken )
   --- End of inner exception stack trace ---
   at System.Net.Http.HttpConnectionPool.ConnectToTcpHostAsync(String , Int32 , HttpRequestMessage , Boolean , CancellationToken )
   at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage , Boolean , CancellationToken )
   at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage , Boolean , CancellationToken )
   at System.Net.Http.HttpConnectionPool.AddHttp11ConnectionAsync(HttpRequestMessage )
   at System.Threading.Tasks.TaskCompletionSourceWithCancellation`1.WaitWithCancellationAsync(CancellationToken )
   at System.Net.Http.HttpConnectionPool.GetHttp11ConnectionAsync(HttpRequestMessage , Boolean , CancellationToken )
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage , Boolean , Boolean , CancellationToken )
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage , Boolean , CancellationToken )
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage , HttpCompletionOption , CancellationTokenSource , Boolean , CancellationTokenSource , CancellationToken )
   at TwitchDownloaderCore.TwitchHelper.GetVideoToken(Int32 videoId, String authToken)
   at TwitchDownloaderCore.VideoDownloader.GetPlaylistUrl()
   at TwitchDownloaderCore.VideoDownloader.DownloadAsync(IProgress`1 progress, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean )
   at System.Threading.Tasks.Task.Wait(Int32 , CancellationToken )
   at System.Threading.Tasks.Task.Wait()
   at TwitchDownloaderCLI.Modes.DownloadVideo.Download(VideoDownloadArgs inputOptions)
   at CommandLine.ParserResultExtensions.WithParsed[T](ParserResult`1 result, Action`1 action)
   at TwitchDownloaderCLI.Program.Main(String[] args)
Aborted

This one is a little better but still far from user friendly.

[Syndace]

Also I'm not only talking about a malformed ids, but about valid ids of videos that for example no longer exists on Twitch because they are too old and were deleted. An example is 1659138963, that video was available until a few days ago but was deleted by Twitch since it's older than two months.

[ScrubN]

I'm not sure what we can really do about no network connection. Adding a try-catch around every httpclient request isn't very friendly and packages like polly only support 1 option for handling.

Also I'm not only talking about a malformed ids, but about valid ids of videos that for example no longer exists

Don't worry I've accounted for both. For VODs both cases return the same invalid data along with invalid clip ids, but deleted clips return the invalid data in a different form.

[Syndace]

Adding a try-catch around every httpclient request

Don't they bubble up to some central point like your main function?

Don't worry I've accounted for both

Awesome, thanks!

[ScrubN]

Adding a try-catch around every httpclient request

Don't they bubble up to some central point like your main function?

The program was not initially designed with IHttpClientFactory in mind so there is no central HttpCient handling in place. I would like to eventually do a partial rewrite to support it but as it stands thats not a huge priority

[Templayer]

Seconded.