[browser][websockets] Support for CloseOutputAsync (#47906)
authorKenneth Pouncey <kjpou@pt.lu>
Thu, 11 Feb 2021 11:48:23 +0000 (12:48 +0100)
committerGitHub <noreply@github.com>
Thu, 11 Feb 2021 11:48:23 +0000 (12:48 +0100)
commit42009d1f33b52e40e5471d5c0d3ff4a8d5b2b83a
treedcdbf6d8baa8d1f57b5d6f169f2f39f158b0fce9
parentaa660d505260642ed04fec8a0224236cb038b69e
[browser][websockets] Support for CloseOutputAsync (#47906)

* 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 <lewing@microsoft.com>
* Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>
* Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>
* 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 <lewing@microsoft.com>
Co-authored-by: Stephen Toub <stoub@microsoft.com>
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs
src/libraries/System.Net.WebSockets.Client/tests/AbortTest.cs
src/libraries/System.Net.WebSockets.Client/tests/CancelTest.cs
src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs
src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs
src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs