[browser][websockets] Support for CloseOutputAsync #47906
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
example from tests: ``` Assert.Equal() Failure info: Expected: Aborted info: Actual: Closed ```
…browser-websocket-abort-fixes
- 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 #45586
- 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. #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) ```
…browser-websocket-abort-fixes
…browser-websocket-abort-fixes # Conflicts: # src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
…browser-websocket-abort-fixes
…browser-websocket-abort-fixes
- Update tests that are resolved.
…browser-websocket-abort-fixes
```
// 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 clean up of the task and set or cancel the task cleanly.
…browser-websocket-abort-fixes
- intermittently failing with System.OperationCanceledException : The operation was canceled.
…tly failing with System.OperationCanceledException : The operation was canceled. - This was an ActiveIssue #46909 but extending the time seems to do the job.
…browser-websocket-abort-fixes
- [browser][websocket] Hang with responses without ever signaling "end of message"
- [ActiveIssue("#46983", TestPlatforms.Browser)] // JS Fetch does not support see issue
…browser-websocket-abort-fixes
…browser-websocket-abort-fixes
- 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.
…browser-websocket-abort-fixes
This was referenced Feb 8, 2021
…browser-websocket-CloseOutputAsync
wfurt
reviewed
Feb 9, 2021
src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs
Outdated
Show resolved
Hide resolved
wfurt
reviewed
Feb 9, 2021
…browser-websocket-CloseOutputAsync
…ases As per comments - We clear all events on the websocket (including onClose), - call websocket.close() - then call the user provided onClose method manually.
|
/azp run runtime-libraries-mono outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
lewing
approved these changes
Feb 10, 2021
src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs
Outdated
Show resolved
Hide resolved
…st.cs Co-authored-by: Larry Ewing <lewing@microsoft.com>
stephentoub
reviewed
Feb 10, 2021
...System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Feb 10, 2021
...System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Feb 10, 2021
...System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Outdated
Show resolved
Hide resolved
stephentoub
reviewed
Feb 10, 2021
...System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Outdated
Show resolved
Hide resolved
…ockets/BrowserWebSockets/BrowserWebSocket.cs Co-authored-by: Stephen Toub <stoub@microsoft.com>
…ockets/BrowserWebSockets/BrowserWebSocket.cs Co-authored-by: Stephen Toub <stoub@microsoft.com>
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note this relies on PR : #45827
[ActiveIssue("https://github.com/dotnet/runtime/issues/45468", TestPlatforms.Browser)]Fix #46909
Fix #45468
Fix #45674
Fix #45586
Close #46983
Comments from parent PR:
Multiple fixes for Abort.
There are multiple failures with the Abort tests causing some tests to:
Never end
Multiple GC calls that end in running out of memory.
Some failures are expected to fail with specific WebSocketError codes:
Error codes that are not correct tend to be when websockets tests are looking for specific error codes and messages when the browser returns
NativeErrorand the websocket native error code instead.These need to be marked correctly and or caught and wrapped correctly.
Fixes for these should also fix some, note not all, of the memory runtime errors as per issue: #45586
Fix tests that were failing when expecting CloseSent as a valid state.
Tests are intermittently failing with System.OperationCanceledException : The operation was canceled.
Added ActiveIssue for these two tests. #46909Further addressed in #47906
SendReceive_PartialMessageBeforeCompleteMessageArrives_Success which describes the test as Ask the remote server to echo back received messages without ever signaling "end of message". This test hangs as javascript fetch does not support this functionality.
See issue #46983 for more detailed description and native javascript tests.
Associate PR's: #44666 and #45470
Close #45586
Fix #45674