Skip to content
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

[browser][websocket] Throw OperationCanceledException on connect #44722

Merged
merged 9 commits into from
Dec 9, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ internal async Task ConnectAsyncJavaScript(Uri uri, CancellationToken cancellati
throw new InvalidOperationException(SR.net_WebSockets_AlreadyStarted);
}

CancellationTokenRegistration connectRegistration = cancellationToken.Register(cts => ((CancellationTokenSource)cts!).Cancel(), _cts);
TaskCompletionSource tcsConnect = new TaskCompletionSource();

// For Abort/Dispose. Calling Abort on the request at any point will close the connection.
Expand Down Expand Up @@ -164,7 +165,14 @@ internal async Task ConnectAsyncJavaScript(Uri uri, CancellationToken cancellati
NativeCleanup();
if ((InternalState)_state == InternalState.Connecting)
{
tcsConnect.TrySetException(new WebSocketException(WebSocketError.NativeError));
if (cancellationToken.IsCancellationRequested)
{
tcsConnect.TrySetCanceled(cancellationToken);
}
else
{
tcsConnect.TrySetException(new WebSocketException(WebSocketError.NativeError));
}
}
else
{
Expand Down Expand Up @@ -214,8 +222,17 @@ internal async Task ConnectAsyncJavaScript(Uri uri, CancellationToken cancellati
catch (Exception wse)
{
Dispose();
WebSocketException wex = new WebSocketException(SR.net_webstatus_ConnectFailure, wse);
throw wex;
switch (wse)
{
case OperationCanceledException:
throw;
default:
throw new WebSocketException(SR.net_webstatus_ConnectFailure, wse);
}
}
finally
{
connectRegistration.Unregister();
}
}

Expand Down Expand Up @@ -324,7 +341,7 @@ public override void Dispose()
// and called by Dispose or Abort so that any open websocket connection can be closed.
private async void AbortRequest()
{
if (State == WebSocketState.Open)
if (State == WebSocketState.Open || State == WebSocketState.Connecting)
{
await CloseAsyncCore(WebSocketCloseStatus.NormalClosure, SR.net_WebSockets_Connection_Aborted, CancellationToken.None).ConfigureAwait(continueOnCapturedContext: true);
}
Expand Down Expand Up @@ -455,7 +472,7 @@ public override async Task CloseAsync(WebSocketCloseStatus closeStatus, string?

private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken)
{
ThrowOnInvalidState(State, WebSocketState.Open, WebSocketState.CloseReceived, WebSocketState.CloseSent);
ThrowOnInvalidState(State, WebSocketState.Connecting, WebSocketState.Open, WebSocketState.CloseReceived, WebSocketState.CloseSent);

WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public async Task ConnectAsync(Uri uri, CancellationToken cancellationToken, Cli
{
try
{
cancellationToken.ThrowIfCancellationRequested(); // avoid allocating a WebSocket object if cancellation was requested before connect
CancellationTokenSource? linkedCancellation;
CancellationTokenSource externalAndAbortCancellation;
if (cancellationToken.CanBeCanceled) // avoid allocating linked source if external token is not cancelable
Expand Down Expand Up @@ -61,11 +62,13 @@ public async Task ConnectAsync(Uri uri, CancellationToken cancellationToken, Cli

Abort();

if (exc is WebSocketException)
{
throw;
switch (exc) {
case WebSocketException:
case OperationCanceledException _ when cancellationToken.IsCancellationRequested:
throw;
default:
throw new WebSocketException(SR.net_webstatus_ConnectFailure, exc);
}
throw new WebSocketException(SR.net_webstatus_ConnectFailure, exc);
}
}
}
Expand Down
13 changes: 12 additions & 1 deletion src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ public async Task ConnectAndCloseAsync_UseProxyServer_ExpectedClosedState(Uri se
}

[ConditionalFact(nameof(WebSocketsSupported))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/44720", TestPlatforms.Browser)]
public async Task ConnectAsync_CancellationRequestedBeforeConnect_ThrowsOperationCanceledException()
{
using (var clientSocket = new ClientWebSocket())
Expand All @@ -260,6 +259,18 @@ public async Task ConnectAsync_CancellationRequestedBeforeConnect_ThrowsOperatio
}
}

[ConditionalFact(nameof(WebSocketsSupported))]
public async Task ConnectAsync_CancellationRequestedInflightConnect_ThrowsOperationCanceledException()
{
using (var clientSocket = new ClientWebSocket())
{
var cts = new CancellationTokenSource();
Task t = clientSocket.ConnectAsync(new Uri("ws://" + Guid.NewGuid().ToString("N")), cts.Token);
cts.Cancel();
await Assert.ThrowsAnyAsync<OperationCanceledException>(() => t);
}
}

[ConditionalFact(nameof(WebSocketsSupported))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/34690", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/42852", TestPlatforms.Browser)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net.Test.Common;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -124,6 +125,10 @@ public static async Task<T> Retry<T>(ITestOutputHelper output, Func<Task<T>> fun
private static bool InitWebSocketSupported()
{
ClientWebSocket cws = null;
if (PlatformDetection.IsBrowser && !PlatformDetection.IsBrowserDomSupported)
{
return false;
}

try
{
Expand Down