From 42009d1f33b52e40e5471d5c0d3ff4a8d5b2b83a Mon Sep 17 00:00:00 2001 From: Kenneth Pouncey Date: Thu, 11 Feb 2021 12:48:23 +0100 Subject: [PATCH] [browser][websockets] Support for CloseOutputAsync (#47906) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * Set the WebSocketException to Faulted to fix tests * Fix the exceptions that are being thrown to coincide with test expectations * Fix Dispose we are not processing it multiple times. * Fix Abort code so the messages align correctly with the tests. example from tests: ``` Assert.Equal() Failure info: Expected: Aborted info: Actual: Closed ``` * Set the WebSocketException to Faulted to fix test expectations * Close the connections correctly. * Fix Abort test Abort_CloseAndAbort_Success - This was causing a never ending Task when aborted after a Close already executed. - Never ended which was a cause of memory errors after left running. - See also https://github.com/dotnet/runtime/issues/45586 * - Fixes for ReceiveAsyncTest - Exception text not sent correctly. This test was expecting 'Aborted'. - Mismatched expected exception messages - Make sure the connection is torn down. - Multiple GC calls that end in running out of memory fixed. https://github.com/dotnet/runtime/issues/45586 ``` // [FAIL] System.Net.WebSockets.Client.Tests.CancelTest.ReceiveAsync_AfterCancellationDoReceiveAsync_ThrowsWebSocketException(server: ws://corefx-net-http11.azurewebsites.net/WebSocket/EchoWebSocket.ashx) // info: Assert.Equal() Failure // info: ↓ (pos 39) // info: Expected: ··· an invalid state ('Aborted') for this operation. Valid state··· // info: Actual: ··· an invalid state ('Open') for this operation. Valid states a··· // info: ↑ (pos 39) ``` * Remove ActiveIssue attributes that should be fixed * Add back code after merge conflict - Update tests that are resolved. * Fix tests that were failing when expecting CloseSent as a valid state. ``` // fail: [FAIL] System.Net.WebSockets.Client.Tests(server: ws://corefx-net-http11.azurewebsites.net/WebSocket/EchoWebSocket.ashx) // info: Assert.Throws() Failure // info: Expected: typeof(System.OperationCanceledException) // info: Actual: typeof(System.Net.WebSockets.WebSocketException): The WebSocket is in an invalid state ('Aborted') for this operation. Valid states are: 'Open, CloseSent' ``` * Fix the Abort and Cancel never ending tests. - Fix the clean up of the task and set or cancel the task cleanly. * Add ActiveIssue to websocket client SendRecieve tests * Add ActiveIssue to websocket client SendRecieve tests - intermittently failing with System.OperationCanceledException : The operation was canceled. * Add extra time to timeout for a couple of tests that were intermittently failing with System.OperationCanceledException : The operation was canceled. - This was an ActiveIssue https://github.com/dotnet/runtime/issues/46909 but extending the time seems to do the job. * Add ActiveIssue to websocket client SendRecieve tests - [browser][websocket] Hang with responses without ever signaling "end of message" - [ActiveIssue("https://github.com/dotnet/runtime/issues/46983", TestPlatforms.Browser)] // JS Fetch does not support see issue * Remove `Interlocked` code as per review comment. * Fix comment * Fix Abort tests on non chrome browsers. - Fix for those browsers that do not set Close and send an onClose event in certain instances i.e. firefox and safari. - Chrome will send an onClose event and we tear down the websocket there. Other browsers need to be handled differently. * We should not throw here. - Closing an already-closed ClientWebSocket should be a no-op. * Add `CloseOutputAsync` implementation - The browser websocket implementation does not support this so we fake it the best we can. * Remove `ActiveIssue` and add more tests - Remove `[ActiveIssue("https://github.com/dotnet/runtime/issues/45468", TestPlatforms.Browser)]` - Add more tests * Address timeout implementation review * Add code as per SignalR comments to prevent long wait times in some cases As per comments - We clear all events on the websocket (including onClose), - call websocket.close() - then call the user provided onClose method manually. * Update src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs Co-authored-by: Larry Ewing * Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs Co-authored-by: Stephen Toub * Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs Co-authored-by: Stephen Toub * Address review comments for using `TrySet*` variants * Address review about using PascalCasing * Change test to be a platform check and not an ActiveIssue as per review Co-authored-by: Larry Ewing Co-authored-by: Stephen Toub --- .../BrowserWebSockets/BrowserWebSocket.cs | 265 +++++++++++++++------ .../Net/WebSockets/WebSocketHandle.Browser.cs | 3 +- .../tests/AbortTest.cs | 5 - .../tests/CancelTest.cs | 3 - .../tests/ClientWebSocketTestBase.cs | 2 +- .../tests/CloseTest.cs | 50 +++- .../tests/SendReceiveTest.cs | 6 +- 7 files changed, 244 insertions(+), 90 deletions(-) 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 4288444..ac05989 100644 --- 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 @@ -34,6 +34,7 @@ namespace System.Net.WebSockets }); private TaskCompletionSource? _tcsClose; + private TaskCompletionSource? _tcsConnect; private WebSocketCloseStatus? _innerWebSocketCloseStatus; private string? _innerWebSocketCloseStatusDescription; @@ -41,12 +42,13 @@ namespace System.Net.WebSockets private Action? _onOpen; private Action? _onError; - private Action? _onClose; + private Action? _onClose; private Action? _onMessage; private MemoryStream? _writeBuffer; private ReceivePayload? _bufferedPayload; private readonly CancellationTokenSource _cts; + private int _closeStatus; // variable to track the close status after a close is sent. // Stages of this class. private int _state; @@ -56,9 +58,12 @@ namespace System.Net.WebSockets Created = 0, Connecting = 1, Connected = 2, - Disposed = 3 + CloseSent = 3, + Disposed = 4, + Aborted = 5, } + private bool _disposed; /// /// Initializes a new instance of the class. @@ -78,7 +83,7 @@ namespace System.Net.WebSockets { get { - if (_innerWebSocket != null && !_innerWebSocket.IsDisposed) + if (_innerWebSocket != null && !_innerWebSocket.IsDisposed && _state != (int)InternalState.Aborted) { return ReadyStateToDotNetState((int)_innerWebSocket.GetObjectProperty("readyState")); } @@ -86,6 +91,9 @@ namespace System.Net.WebSockets { InternalState.Created => WebSocketState.None, InternalState.Connecting => WebSocketState.Connecting, + InternalState.Aborted => WebSocketState.Aborted, + InternalState.Disposed => WebSocketState.Closed, + InternalState.CloseSent => WebSocketState.CloseSent, _ => WebSocketState.Closed }; } @@ -112,19 +120,27 @@ namespace System.Net.WebSockets internal async Task ConnectAsyncJavaScript(Uri uri, CancellationToken cancellationToken, List? requestedSubProtocols) { - // Check that we have not started already - int priorState = Interlocked.CompareExchange(ref _state, (int)InternalState.Connecting, (int)InternalState.Created); - if (priorState == (int)InternalState.Disposed) + // Check that we have not started already. + int prevState = _state; + if (prevState == (int)InternalState.Created) { - throw new ObjectDisposedException(GetType().FullName); + _state = (int)InternalState.Connecting; } - else if (priorState != (int)InternalState.Created) + + switch ((InternalState)prevState) { - throw new InvalidOperationException(SR.net_WebSockets_AlreadyStarted); + case InternalState.Disposed: + throw new ObjectDisposedException(GetType().FullName); + + case InternalState.Created: + break; + + default: + throw new InvalidOperationException(SR.net_WebSockets_AlreadyStarted); } CancellationTokenRegistration connectRegistration = cancellationToken.Register(cts => ((CancellationTokenSource)cts!).Cancel(), _cts); - TaskCompletionSource tcsConnect = new TaskCompletionSource(); + _tcsConnect = new TaskCompletionSource(); // For Abort/Dispose. Calling Abort on the request at any point will close the connection. _cts.Token.Register(s => ((BrowserWebSocket)s!).AbortRequest(), this); @@ -155,31 +171,7 @@ namespace System.Net.WebSockets _innerWebSocket.SetObjectProperty("onerror", _onError); // Setup the onClose callback - _onClose = (closeEvt) => - { - using (closeEvt) - { - _innerWebSocketCloseStatus = (WebSocketCloseStatus)closeEvt.GetObjectProperty("code"); - _innerWebSocketCloseStatusDescription = closeEvt.GetObjectProperty("reason")?.ToString(); - _receiveMessageQueue.Writer.TryWrite(new ReceivePayload(Array.Empty(), WebSocketMessageType.Close)); - NativeCleanup(); - if ((InternalState)_state == InternalState.Connecting) - { - if (cancellationToken.IsCancellationRequested) - { - tcsConnect.TrySetCanceled(cancellationToken); - } - else - { - tcsConnect.TrySetException(new WebSocketException(WebSocketError.NativeError)); - } - } - else - { - _tcsClose?.SetResult(); - } - } - }; + _onClose = (closeEvent) => OnCloseCallback(closeEvent, cancellationToken); // Attach the onClose callback _innerWebSocket.SetObjectProperty("onclose", _onClose); @@ -192,19 +184,21 @@ namespace System.Net.WebSockets if (!cancellationToken.IsCancellationRequested) { // Change internal _state to 'Connected' to enable the other methods - if (Interlocked.CompareExchange(ref _state, (int)InternalState.Connected, (int)InternalState.Connecting) != (int)InternalState.Connecting) + int prevState = _state; + _state = _state == (int)InternalState.Connecting ? (int)InternalState.Connected : _state; + if (prevState != (int)InternalState.Connecting) { // Aborted/Disposed during connect. - tcsConnect.TrySetException(new ObjectDisposedException(GetType().FullName)); + _tcsConnect.TrySetException(new ObjectDisposedException(GetType().FullName)); } else { - tcsConnect.SetResult(); + _tcsConnect.TrySetResult(); } } else { - tcsConnect.SetCanceled(cancellationToken); + _tcsConnect.TrySetCanceled(cancellationToken); } } }; @@ -213,11 +207,11 @@ namespace System.Net.WebSockets _innerWebSocket.SetObjectProperty("onopen", _onOpen); // Setup the onMessage callback - _onMessage = (messageEvent) => onMessageCallback(messageEvent); + _onMessage = (messageEvent) => OnMessageCallback(messageEvent); // Attach the onMessage callaback _innerWebSocket.SetObjectProperty("onmessage", _onMessage); - await tcsConnect.Task.ConfigureAwait(continueOnCapturedContext: true); + await _tcsConnect.Task.ConfigureAwait(continueOnCapturedContext: true); } catch (Exception wse) { @@ -227,7 +221,7 @@ namespace System.Net.WebSockets case OperationCanceledException: throw; default: - throw new WebSocketException(SR.net_webstatus_ConnectFailure, wse); + throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure, wse); } } finally @@ -236,7 +230,38 @@ namespace System.Net.WebSockets } } - private void onMessageCallback(JSObject messageEvent) + + private void OnCloseCallback(JSObject? closeEvt, CancellationToken cancellationToken) + { + if (closeEvt != null) + { + using (closeEvt) + { + _innerWebSocketCloseStatus = (WebSocketCloseStatus)closeEvt.GetObjectProperty("code"); + _innerWebSocketCloseStatusDescription = closeEvt.GetObjectProperty("reason")?.ToString(); + } + } + _receiveMessageQueue.Writer.TryWrite(new ReceivePayload(Array.Empty(), WebSocketMessageType.Close)); + NativeCleanup(); + if ((InternalState)_state == InternalState.Connecting || (InternalState)_state == InternalState.Aborted) + { + _state = (int)InternalState.Disposed; + if (cancellationToken.IsCancellationRequested) + { + _tcsConnect?.TrySetCanceled(cancellationToken); + } + else + { + _tcsConnect?.TrySetException(new WebSocketException(WebSocketError.NativeError)); + } + } + else + { + _tcsClose?.TrySetResult(); + } + } + + private void OnMessageCallback(JSObject messageEvent) { // get the events "data" using (messageEvent) @@ -318,32 +343,51 @@ namespace System.Net.WebSockets public override void Dispose() { - int priorState = Interlocked.Exchange(ref _state, (int)InternalState.Disposed); - if (priorState == (int)InternalState.Disposed) + if (!_disposed) { - // No cleanup required. - return; - } + if (_state < (int)InternalState.Aborted) { + _state = (int)InternalState.Disposed; + } + _disposed = true; - // registered by the CancellationTokenSource cts in the connect method - _cts.Cancel(false); - _cts.Dispose(); + if (!_cts.IsCancellationRequested) + { + // registered by the CancellationTokenSource cts in the connect method + _cts.Cancel(); + _cts.Dispose(); + } - _writeBuffer?.Dispose(); - _receiveMessageQueue.Writer.Complete(); + _writeBuffer?.Dispose(); + _receiveMessageQueue.Writer.TryComplete(); - NativeCleanup(); + NativeCleanup(); - _innerWebSocket?.Dispose(); + _innerWebSocket?.Dispose(); + } } // This method is registered by the CancellationTokenSource cts in the connect method // and called by Dispose or Abort so that any open websocket connection can be closed. private async void AbortRequest() { - if (State == WebSocketState.Open || State == WebSocketState.Connecting) + switch (State) { - await CloseAsyncCore(WebSocketCloseStatus.NormalClosure, SR.net_WebSockets_Connection_Aborted, CancellationToken.None).ConfigureAwait(continueOnCapturedContext: true); + case WebSocketState.Open: + case WebSocketState.Connecting: + { + await CloseAsyncCore(WebSocketCloseStatus.NormalClosure, SR.net_WebSockets_Connection_Aborted, CancellationToken.None).ConfigureAwait(continueOnCapturedContext: true); + // The following code is for those browsers that do not set Close and send an onClose event in certain instances i.e. firefox and safari. + // chrome will send an onClose event and we tear down the websocket there. + if (ReadyStateToDotNetState(_closeStatus) == WebSocketState.CloseSent) + { + _writeBuffer?.Dispose(); + _receiveMessageQueue.Writer.TryWrite(new ReceivePayload(Array.Empty(), WebSocketMessageType.Close)); + _receiveMessageQueue.Writer.TryComplete(); + NativeCleanup(); + _tcsConnect?.TrySetCanceled(); + } + } + break; } } @@ -423,6 +467,20 @@ namespace System.Net.WebSockets return Task.CompletedTask; } + // This method is registered by the CancellationTokenSource in the receive async method + private async void CancelRequest() + { + int prevState = _state; + _state = (int)InternalState.Aborted; + _receiveMessageQueue.Writer.TryComplete(); + if (prevState == (int)InternalState.Connected || prevState == (int)InternalState.Connecting) + { + if (prevState == (int)InternalState.Connecting) + _state = (int)InternalState.CloseSent; + await CloseAsyncCore(WebSocketCloseStatus.NormalClosure, SR.net_WebSockets_Connection_Aborted, CancellationToken.None).ConfigureAwait(continueOnCapturedContext: true); + } + } + /// /// Receives data on as an asynchronous operation. /// @@ -431,22 +489,43 @@ namespace System.Net.WebSockets /// Cancellation token. public override async Task ReceiveAsync(ArraySegment buffer, CancellationToken cancellationToken) { - WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); - ThrowIfDisposed(); - ThrowOnInvalidState(State, WebSocketState.Open, WebSocketState.CloseSent); - _bufferedPayload ??= await _receiveMessageQueue.Reader.ReadAsync(cancellationToken).ConfigureAwait(continueOnCapturedContext: true); + if (cancellationToken.IsCancellationRequested) + { + return await Task.FromException(new OperationCanceledException()).ConfigureAwait(continueOnCapturedContext: true); + } + + CancellationTokenSource _receiveCTS = new CancellationTokenSource(); + CancellationTokenRegistration receiveRegistration = cancellationToken.Register(cts => ((CancellationTokenSource)cts!).Cancel(), _receiveCTS); + _receiveCTS.Token.Register(s => ((BrowserWebSocket)s!).CancelRequest(), this); try { - bool endOfMessage = _bufferedPayload.BufferPayload(buffer, out WebSocketReceiveResult receiveResult); + WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); + + ThrowIfDisposed(); + ThrowOnInvalidState(State, WebSocketState.Open, WebSocketState.CloseSent); + _bufferedPayload ??= await _receiveMessageQueue.Reader.ReadAsync(cancellationToken).ConfigureAwait(continueOnCapturedContext: true); + bool endOfMessage = _bufferedPayload!.BufferPayload(buffer, out WebSocketReceiveResult receiveResult); if (endOfMessage) _bufferedPayload = null; return receiveResult; } catch (Exception exc) { - throw new WebSocketException(WebSocketError.NativeError, exc); + switch (exc) + { + case OperationCanceledException: + return await Task.FromException(exc).ConfigureAwait(continueOnCapturedContext: true); + case ChannelClosedException: + return await Task.FromException(new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, State, "Open, CloseSent"))).ConfigureAwait(continueOnCapturedContext: true); + default: + return await Task.FromException(new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, State, "Open, CloseSent"))).ConfigureAwait(continueOnCapturedContext: true); + } + } + finally + { + receiveRegistration.Unregister(); } } @@ -455,18 +534,25 @@ namespace System.Net.WebSockets /// public override void Abort() { - if (_state == (int)InternalState.Disposed) + if (_state != (int)InternalState.Disposed) { - return; + int prevState = _state; + if (prevState != (int)InternalState.Connecting) + { + _state = (int)InternalState.Aborted; + } + + if (prevState < (int)InternalState.Aborted) + { + _cts.Cancel(true); + _tcsClose?.TrySetResult(); + } } - _state = (int)WebSocketState.Aborted; - Dispose(); } public override Task CloseAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { _writeBuffer = null; - ThrowIfNotConnected(); WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription); @@ -478,8 +564,7 @@ namespace System.Net.WebSockets { return Task.FromException(exc); } - - return CloseAsyncCore(closeStatus, statusDescription, cancellationToken); + return State == WebSocketState.CloseSent ? Task.CompletedTask : CloseAsyncCore(closeStatus, statusDescription, cancellationToken); } private Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) @@ -490,6 +575,7 @@ namespace System.Net.WebSockets _innerWebSocketCloseStatus = closeStatus; _innerWebSocketCloseStatusDescription = statusDescription; _innerWebSocket!.Invoke("close", (int)closeStatus, statusDescription); + _closeStatus = (int)_innerWebSocket.GetObjectProperty("readyState"); return _tcsClose.Task; } catch (Exception exc) @@ -498,7 +584,44 @@ namespace System.Net.WebSockets } } - public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) => throw new PlatformNotSupportedException(); + public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) + { + _writeBuffer = null; + + WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription); + + try + { + ThrowOnInvalidState(State, WebSocketState.Connecting, WebSocketState.Open, WebSocketState.CloseReceived, WebSocketState.CloseSent); + } + catch (Exception exc) + { + return Task.FromException(exc); + } + return CloseOutputAsyncCore(closeStatus, statusDescription, cancellationToken); + } + + private Task CloseOutputAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) + { + try + { + // as per comments + // - We clear all events on the websocket (including onClose), + // - call websocket.close() + // - then call the user provided onClose method manually. + NativeCleanup(); + _innerWebSocketCloseStatus = closeStatus; + _innerWebSocketCloseStatusDescription = statusDescription; + _innerWebSocket!.Invoke("close", (int)closeStatus, statusDescription); + _closeStatus = (int)_innerWebSocket.GetObjectProperty("readyState"); + OnCloseCallback(null, cancellationToken); + return Task.CompletedTask; + } + catch (Exception exc) + { + return Task.FromException(exc); + } + } private void ThrowIfNotConnected() { 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 cf91e68..1dde389 100644 --- 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 @@ -24,6 +24,7 @@ namespace System.Net.WebSockets public void Abort() { + _abortSource.Cancel(); WebSocket?.Abort(); } @@ -67,7 +68,7 @@ namespace System.Net.WebSockets case OperationCanceledException _ when cancellationToken.IsCancellationRequested: throw; default: - throw new WebSocketException(SR.net_webstatus_ConnectFailure, exc); + throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure, exc); } } } diff --git a/src/libraries/System.Net.WebSockets.Client/tests/AbortTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/AbortTest.cs index 6b48fd2..6e9a60f 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/AbortTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/AbortTest.cs @@ -17,7 +17,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45674", TestPlatforms.Browser)] public async Task Abort_ConnectAndAbort_ThrowsWebSocketExceptionWithmessage(Uri server) { using (var cws = new ClientWebSocket()) @@ -43,7 +42,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45674", TestPlatforms.Browser)] public async Task Abort_SendAndAbort_Success(Uri server) { await TestCancellation(async (cws) => @@ -64,7 +62,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45674", TestPlatforms.Browser)] public async Task Abort_ReceiveAndAbort_Success(Uri server) { await TestCancellation(async (cws) => @@ -89,7 +86,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45674", TestPlatforms.Browser)] public async Task Abort_CloseAndAbort_Success(Uri server) { await TestCancellation(async (cws) => @@ -114,7 +110,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45468", TestPlatforms.Browser)] public async Task ClientWebSocket_Abort_CloseOutputAsync(Uri server) { await TestCancellation(async (cws) => diff --git a/src/libraries/System.Net.WebSockets.Client/tests/CancelTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/CancelTest.cs index 3278030..8c74513 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/CancelTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/CancelTest.cs @@ -91,7 +91,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45468", TestPlatforms.Browser)] public async Task CloseOutputAsync_Cancel_Success(Uri server) { await TestCancellation(async (cws) => @@ -131,7 +130,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45674", TestPlatforms.Browser)] public async Task ReceiveAsync_ReceiveThenCancel_ThrowsOperationCanceledException(Uri server) { using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) @@ -148,7 +146,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45674", TestPlatforms.Browser)] public async Task ReceiveAsync_AfterCancellationDoReceiveAsync_ThrowsWebSocketException(Uri server) { using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) diff --git a/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs b/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs index ca24740..ab8ebe2 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs @@ -25,7 +25,7 @@ namespace System.Net.WebSockets.Client.Tests new object[] { o[0], true } }).ToArray(); - public const int TimeOutMilliseconds = 20000; + public const int TimeOutMilliseconds = 30000; public const int CloseDescriptionMaxLength = 123; public readonly ITestOutputHelper _output; diff --git a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs index 0e50e78..f4dc4bb 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs @@ -158,13 +158,55 @@ namespace System.Net.WebSockets.Client.Tests await cws.CloseAsync(closeStatus, closeDescription, cts.Token); Assert.Equal(closeStatus, cws.CloseStatus); + } + } + + [OuterLoop("Uses external server")] + [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] + public async Task CloseOutputAsync_ExpectedStates(Uri server) + { + using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) + { + var cts = new CancellationTokenSource(TimeOutMilliseconds); + + var closeStatus = WebSocketCloseStatus.NormalClosure; + string closeDescription = null; + + await cws.CloseOutputAsync(closeStatus, closeDescription, cts.Token); + Assert.True( + cws.State == WebSocketState.CloseSent || cws.State == WebSocketState.Closed, + $"Expected CloseSent or Closed, got {cws.State}"); + Assert.True(string.IsNullOrEmpty(cws.CloseStatusDescription)); + } + } + + [OuterLoop("Uses external server")] + [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] + public async Task CloseAsync_CloseOutputAsync_Throws(Uri server) + { + using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) + { + var cts = new CancellationTokenSource(TimeOutMilliseconds); + + var closeStatus = WebSocketCloseStatus.NormalClosure; + string closeDescription = null; + + await cws.CloseAsync(closeStatus, closeDescription, cts.Token); + Assert.True( + cws.State == WebSocketState.CloseSent || cws.State == WebSocketState.Closed, + $"Expected CloseSent or Closed, got {cws.State}"); + Assert.True(string.IsNullOrEmpty(cws.CloseStatusDescription)); + await Assert.ThrowsAnyAsync(async () => + { await cws.CloseOutputAsync(closeStatus, closeDescription, cts.Token); }); + Assert.True( + cws.State == WebSocketState.CloseSent || cws.State == WebSocketState.Closed, + $"Expected CloseSent or Closed, got {cws.State}"); Assert.True(string.IsNullOrEmpty(cws.CloseStatusDescription)); } } [OuterLoop("Uses external server")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45468", TestPlatforms.Browser)] public async Task CloseOutputAsync_ClientInitiated_CanReceive_CanClose(Uri server) { string message = "Hello WebSockets!"; @@ -173,7 +215,8 @@ namespace System.Net.WebSockets.Client.Tests { var cts = new CancellationTokenSource(TimeOutMilliseconds); - var closeStatus = WebSocketCloseStatus.InvalidPayloadData; + // See issue for Browser websocket differences https://github.com/dotnet/runtime/issues/45538 + var closeStatus = PlatformDetection.IsBrowser ? WebSocketCloseStatus.NormalClosure : WebSocketCloseStatus.InvalidPayloadData; string closeDescription = "CloseOutputAsync_Client_InvalidPayloadData"; await cws.SendAsync(WebSocketData.GetBufferFromText(message), WebSocketMessageType.Text, true, cts.Token); @@ -181,7 +224,7 @@ namespace System.Net.WebSockets.Client.Tests // data fragments after a close has been sent. The delay allows the received data fragment to be // available before calling close. The WinRT MessageWebSocket implementation doesn't allow receiving // after a call to Close. - await Task.Delay(100); + await Task.Delay(500); await cws.CloseOutputAsync(closeStatus, closeDescription, cts.Token); // Should be able to receive the message echoed by the server. @@ -251,7 +294,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external server")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45468", TestPlatforms.Browser)] public async Task CloseOutputAsync_CloseDescriptionIsNull_Success(Uri server) { using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) diff --git a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs index 7999e4d..5e8a1ad 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs @@ -88,7 +88,7 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45586", TestPlatforms.Browser)] + [PlatformSpecific(~TestPlatforms.Browser)] // JS Websocket does not support see issue https://github.com/dotnet/runtime/issues/46983 public async Task SendReceive_PartialMessageBeforeCompleteMessageArrives_Success(Uri server) { var rand = new Random(); @@ -131,7 +131,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45586", TestPlatforms.Browser)] public async Task SendAsync_SendCloseMessageType_ThrowsArgumentExceptionWithMessage(Uri server) { using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) @@ -219,7 +218,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45586", TestPlatforms.Browser)] public async Task ReceiveAsync_MultipleOutstandingReceiveOperations_Throws(Uri server) { using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) @@ -284,7 +282,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45586", TestPlatforms.Browser)] public async Task SendAsync_SendZeroLengthPayloadAsEndOfMessage_Success(Uri server) { using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) @@ -463,7 +460,6 @@ namespace System.Net.WebSockets.Client.Tests [OuterLoop("Uses external servers")] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/45586", TestPlatforms.Browser)] public async Task ZeroByteReceive_CompletesWhenDataAvailable(Uri server) { using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) -- 2.7.4