[browser][websockets][tests] Fix WebSocket Abort tests multiple failures #45827
Conversation
This file contains hidden or 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 ```
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsMultiple fixes for Abort. There are multiple failures with the Abort tests causing some tests to:
Error codes that are not correct tend to be when websockets tests are looking for specific error codes and messages when the browser returns 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 Associate PR's: #44666 and #45470 Fix #45674
|
…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
stephentoub
reviewed
Jan 4, 2021
...System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Outdated
Show resolved
Hide resolved
…browser-websocket-abort-fixes # Conflicts: # src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Member
|
/azp run runtime-libraries-mono outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…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.
- 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
stephentoub
reviewed
Jan 15, 2021
...System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Outdated
Show resolved
Hide resolved
…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
karelz
reviewed
Feb 4, 2021
Member
|
@kjpou1 any plans for this PR? There doesn't seem to be update in last 17 days ... |
Contributor
Author
|
@karelz Waiting for approval. We can ping some others @marek-safar @lewing @radical @stephentoub |
Member
|
/azp run runtime-libraries-mono outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
The errors in the outerloop are for the following: |
Contributor
Author
|
Closed by #47906 which was branched off of this PR. |
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.
5 participants
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.
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