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

* [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>

diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
index b767c31..25cecaf 100644 (file)
--- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
+++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
@@ -123,6 +123,7 @@ namespace System.Net.WebSockets
                 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.
@@ -164,7 +165,14 @@ namespace System.Net.WebSockets
                         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
                         {
@@ -214,8 +222,17 @@ namespace System.Net.WebSockets
             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();
             }
         }
 
@@ -324,7 +341,7 @@ namespace System.Net.WebSockets
         // 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);
             }
@@ -455,7 +472,7 @@ namespace System.Net.WebSockets
 
         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);
 

diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs
index 97bdfdf..cf91e68 100644 (file)
--- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs
+++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs
@@ -31,6 +31,7 @@ namespace System.Net.WebSockets
         {
             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
@@ -61,11 +62,13 @@ namespace System.Net.WebSockets
 
                 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);
             }
         }
     }

diff --git a/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.cs
index 45c94e4..24bfbc2 100644 (file)
--- a/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.cs
+++ b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.cs
@@ -248,7 +248,6 @@ namespace System.Net.WebSockets.Client.Tests
         }
 
         [ConditionalFact(nameof(WebSocketsSupported))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/44720", TestPlatforms.Browser)]
         public async Task ConnectAsync_CancellationRequestedBeforeConnect_ThrowsOperationCanceledException()
         {
             using (var clientSocket = new ClientWebSocket())
@@ -261,6 +260,18 @@ namespace System.Net.WebSockets.Client.Tests
         }
 
         [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)]
         public async Task ConnectAsync_CancellationRequestedAfterConnect_ThrowsOperationCanceledException()

diff --git a/src/libraries/System.Net.WebSockets.Client/tests/WebSocketHelper.cs b/src/libraries/System.Net.WebSockets.Client/tests/WebSocketHelper.cs
index 110f737..1f0ccd5 100644 (file)
--- a/src/libraries/System.Net.WebSockets.Client/tests/WebSocketHelper.cs
+++ b/src/libraries/System.Net.WebSockets.Client/tests/WebSocketHelper.cs
@@ -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;
 
@@ -124,6 +125,10 @@ namespace System.Net.WebSockets.Client.Tests
         private static bool InitWebSocketSupported()
         {
             ClientWebSocket cws = null;
+            if (PlatformDetection.IsBrowser && !PlatformDetection.IsBrowserDomSupported)
+            {
+                return false;
+            }
 
             try
             {