Inconsistent behavior of ClientWebSockets on cancellation

[matthid]

In addition to the documentation issue (see @JohBa comment) I'd like to report this as API-bug as well as all current implementations seems to behave differently.

Interesting is the response from @karelz :

Failed connection will throw exception. If the source was cancallation, then it will be TaskCancelledException, otherwise different exception.
Does it help?

Because no implementation actually has this (imho expected) behavior.

For completeness:

class Program
{
    static async Task Main(string[] args)
    {
        Console.WriteLine("Hello World!");
        var websocket = new ClientWebSocket();
        try
        {
           await websocket.ConnectAsync(new Uri("wss://localhost:443/ws"), new CancellationTokenSource(TimeSpan.FromMilliseconds(10)).Token);
        }
        catch (WebSocketException e)
        {
            Console.WriteLine(e);
        }
        Console.ReadLine();
    }
}

• netcore2.2

  System.Net.WebSockets.WebSocketException (0x80004005): Unable to connect to the remote server 
  ---> System.Threading.Tasks.TaskCanceledException: A task was canceled.
  

• net461

  System.Net.WebSockets.WebSocketException (0x80004005): Unable to connect to the remote server
  ---> System.Net.WebException: The request was aborted: The request was canceled.
  

Interestingly, there not even a nice way to "detect" the request was canceled in the full framework (besides checking the token in the catch block, which only works directly on the call-site).

[matthid]

Related https://github.com/dotnet/corefx/issues/27066

[marklio]

Marking reliability as we've seen unexpected cancellation behaviors lead to reliability issues.

[stephentoub]

Marking reliability as we've seen unexpected cancellation behaviors lead to reliability issues.

This is different than the ones you've seen though, Mark, no? This case is promptly responding to a cancellation request, and an OperationCanceledException is being thrown, it's just a question of whether that exception is wrapped in something else or not.

That said, I don't really care what labels are used. If there's value to you in it being tagged as reliability, ok :)

[marklio]

This is different than the ones you've seen though, Mark, no?

I definitely agree. I wanted to mark this because it is "related" to the class of issues we've seen. Like exception handling in general, we see that code dealing with cancellation has poor test coverage across the ecosystem, and because it is "stress-like" in its difficultly to achieve meaningfully good coveragein unit tests (you need to cancel at every possible code location with a unique behavior), something like this could easily cause a lurking reliability issue in customer code.

This one is tricky as it is entangled with compat, so I wouldn't make a call on what to do based on reliability alone.

[stephentoub]

Ok :)