Skip to content

Commit

Permalink
[browser][websocket] Throw OperationCanceledException on connect (#44722
Browse files Browse the repository at this point in the history
)

* [browser][websocket] Throw OperationCanceledException on connect if cancel was requested before.

* try to handle cancellation in connect stage

* Add new test for inflight connect

- Add new supported property for skipping particular tests when Browser is detected and DOM is detected.

* first pass at throwing pnse when websocket is missing

* Address review comment

* Make the platform check explicit

* Revert CreateDefaultOptions change

* Address review comment

Co-authored-by: Larry Ewing <lewing@microsoft.com>
  • Loading branch information
kjpou1 and lewing authored Dec 9, 2020
1 parent 358b049 commit 76a443d
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 10 deletions.
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

0 comments on commit 76a443d

Please sign in to comment.