Runtime: ClientWebSocket does not provide upgrade request error details

[amrmahdi]

Related to dotnet/runtime#19405

ClientWebSocket on .NET Core does not provide the upgrade request errors in the exception details as it does on the .NET Framework.

Repro code

var client = new ClientWebSocket();

client.ConnectAsync(new Uri("wss://speech.platform.bing.com/speech/recognition/interactive/cognitiveservices/v1"), CancellationToken.None).GetAwaiter().GetResult();

Behavior on .NET 462

Unhandled Exception: System.Net.WebSockets.WebSocketException: Unable to connect to the remote server ---> System.Net.WebException: The remote server returned an error: (403) Forbidden.
   at System.Net.HttpWebRequest.EndGetResponse(IAsyncResult asyncResult)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Net.WebSockets.ClientWebSocket.<ConnectAsyncCore>d__21.MoveNext()
   --- End of inner exception stack trace ---
   at System.Net.WebSockets.ClientWebSocket.<ConnectAsyncCore>d__21.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at ConsoleApp1.Program.Main(String[] args)

Behavior on .NET Core 2.1 preview 2

   Unhandled Exception: System.Net.WebSockets.WebSocketException: Unable to connect to the remote server
   at System.Net.WebSockets.WebSocketHandle.ConnectAsyncCore(Uri uri, CancellationToken cancellationToken, ClientWebSocketOptions options)
   at System.Net.WebSockets.ClientWebSocket.ConnectAsyncCore(Uri uri, CancellationToken cancellationToken)
   at ConsoleApp1.Program.Main(String[] args)

As you can see on .NET 462, the inner exception is a WebException with the error details.

Proposed Fix

Create an inner exception of type WebException in a similar fashion to
https://github.com/dotnet/corefx/blob/6acd74dda7bc4f585d2c4006da4a8b2deb0261ad/src/System.Net.Requests/src/System/Net/HttpWebRequest.cs#L1211
and throw if the response is not 200.

The original WebException in .NET framework was thrown fromHttpWebRequest.GetResponseAsync(), so I think the exception needs to be bubbled up in a similar way.

[davidsh]

This current PR dotnet/corefx#29159 should address most of this.

[amrmahdi]

Can we expose the WebException ? It has useful information like the response headers that the current PR is not addressing.

[davidsh]

Can we expose the WebException ? It has useful information like the response headers that the current PR is not addressing.

The .NET Core WebSockets.Client APIs use the new HTTP stack, SocketsHttpHandler. It does not use WebException. That exception is considered part of the legacy set of APIs including HttpWebRequest etc.

But we will look into how it might be possible to expose the HTTP response itself (HttpResponseMessage if available) which you could then use to examine the HTTP response headers.

cc: @geoffkizer @stephentoub @karelz

[amrmahdi]

Exposing HttpResponseMessage would be ideal.

[RaviPidaparthi]

It would be great if there is built in way we can read the websocket upgrade success and failure response headers.

[davidfowl]

As an added request since we're going down this rabbit hole. It would be great if we could also specify HttpMessageHandlers that run as part of the upgrade request so we don't need to special case WebSocketOptions.

[karelz]

@davidfowl can you please be more specific? How do you special case WebSocketOptions today? How do you plan to use HttpMessageHandlers?
How is it related to error details for upgrade request?

[davidfowl]

@davidfowl can you please be more specific? How do you special case WebSocketOptions today? How do you plan to use HttpMessageHandlers?

SignalR supports long polling, server sent events and websockets. We use HttpClient for the first 2 scenarios and ClientWebSocket for WebSockets. We lets users provide an HttpMessageHandler to do run arbitrary code before outgoing requests are sent with the HttpClient but that doesn't apply when using ClientWebSocket. The net effect is that we have to split out logic between configuring the HttpClientHandler, HttpMessageHandler and WebSocketOptions.

We have our own HttpConnectionOptions:

https://github.com/aspnet/SignalR/blob/679225c241b32c8e2af8c84fa693d3f8232d1085/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnectionOptions.cs#L14

We have to manually apply them in both cases:

For HttpClient:

https://github.com/aspnet/SignalR/blob/679225c241b32c8e2af8c84fa693d3f8232d1085/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs#L363

For ClientWebSocket:

https://github.com/aspnet/SignalR/blob/679225c241b32c8e2af8c84fa693d3f8232d1085/src/Microsoft.AspNetCore.Http.Connections.Client/Internal/WebSocketsTransport.cs#L47

How is it related to error details for upgrade request?

The upgrade request for websockets is the only time it's a regular HTTP request. As such, all existing things that work on http should apply. Right now we have to special case websockets we invented a whole new API that doesn't expose any of our existing HTTP stack (ClientWebSocket).

[qmfrederik]

Another use case for this: the Kubernetes API server has a bunch of WebSocket endpoints which take parameters via the query string.

If you specify a wrong value for any of those parameters, the server will respond with a 400 Bad Request and a JSON-formatted status object in the body which contains additional information.

The current design doesn't allow us to extract that information.

[dlstucki]

It's unfortunate the original report of this problem, from 2016, was closed because it happened to show a call stack from WinHttp. There were a number of "me toos" on that thread. Now it has the appearance of half as many people are asking for it and only being 17 days old.

[dlstucki]

PR dotnet/corefx#29159 just formats the status code in the exception.Message string. Are users supposed to parse that string in order to find the status code? I need to convert these into different exception types: e.g. 401->AuthorizationFailedException, 404->EndpointNotFoundException, and so on. Parsing an error message string feels like a work-around.

Could we put an HttpResponseMessage with status code, reason phrase, and response headers on the ClientWebSocket or on its ClientWebSocketOptions?

[dlstucki]

It would be great to expose the HttpResponseMessage, if present, in the resulting WebSocketException. I'd be interested in helping create a PR for this but the API would need to be determined.

Given this code which already has an HttpResponseMessage and throws the WebSocketException how should the response details be propagated in the Exception? A new property, HttpResponseMessage WebSocketException.HttpResponse { get; }? WebSocketContext WebSocketException.Context { get; }? (Putting the HttpResponse in the Exception.Data dictionary seems like a hack.)

    if (response.StatusCode != HttpStatusCode.SwitchingProtocols)
    {
        // **** TODO: put response in the Exception somewhere ****
        throw new WebSocketException(
            WebSocketError.NotAWebSocket,
            SR.Format(SR.net_WebSockets_Connect101Expected, (int)response.StatusCode));
    }

@karelz I would gladly create the PR if the API/mechanism of passing the HttpResponseMessage (or StatusCode, StatusDescription, and HTTP Response Headers) is determined.

[karelz]

Triage: Would be really nice to get it done in 5.0 for diagnostics + react to the upvotes count here.

[karelz]

Looks like this is API addition -- per https://github.com/dotnet/runtime/issues/25918#issuecomment-382158285 we should add property on exception.
Sadly, it is too late for 5.0, moving to 6.0 (and fixing labels to capture API addition).