Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[browser][websockets] Support for CloseOutputAsync #47906

Merged
merged 47 commits into from
Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
e66ff59
Set the WebSocketException to Faulted to fix tests
kjpou1 Dec 9, 2020
1bd6fa6
Fix the exceptions that are being thrown to coincide with test expect…
kjpou1 Dec 9, 2020
5bd616a
Fix Dispose we are not processing it multiple times.
kjpou1 Dec 9, 2020
334a002
Fix Abort code so the messages align correctly with the tests.
kjpou1 Dec 9, 2020
4bc9bc7
Set the WebSocketException to Faulted to fix test expectations
kjpou1 Dec 9, 2020
c13cb26
Close the connections correctly.
kjpou1 Dec 9, 2020
84315f4
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Dec 10, 2020
00c6ef7
Fix Abort test Abort_CloseAndAbort_Success
kjpou1 Dec 10, 2020
2d13833
- Fixes for ReceiveAsyncTest
kjpou1 Dec 10, 2020
616c039
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Dec 11, 2020
2ce6b21
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 5, 2021
b1c21c0
Remove ActiveIssue attributes that should be fixed
lewing Jan 5, 2021
390bbae
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 6, 2021
4f21b1d
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 11, 2021
fc15701
Add back code after merge conflict
kjpou1 Jan 11, 2021
e571896
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 12, 2021
db403cd
Fix tests that were failing when expecting CloseSent as a valid state.
kjpou1 Jan 12, 2021
a321d6e
Fix the Abort and Cancel never ending tests.
kjpou1 Jan 12, 2021
41692d5
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 13, 2021
8754b33
Add ActiveIssue to websocket client SendRecieve tests
kjpou1 Jan 13, 2021
e254ac0
Add ActiveIssue to websocket client SendRecieve tests
kjpou1 Jan 13, 2021
6586e25
Add extra time to timeout for a couple of tests that were intermitten…
kjpou1 Jan 13, 2021
77fe753
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 14, 2021
11fdc7d
Add ActiveIssue to websocket client SendRecieve tests
kjpou1 Jan 14, 2021
03b67b1
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 18, 2021
90e7b79
Remove `Interlocked` code as per review comment.
kjpou1 Jan 18, 2021
188bdc1
Fix comment
kjpou1 Jan 18, 2021
15906e6
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 25, 2021
0118322
Fix Abort tests on non chrome browsers.
kjpou1 Jan 25, 2021
be55fa0
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Jan 26, 2021
8920beb
Merge branch 'master' of https://github.com/kjpou1/runtime into wasm-…
kjpou1 Feb 4, 2021
1cede3d
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Feb 4, 2021
2fd9381
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Feb 5, 2021
4cf2b1d
We should not throw here.
kjpou1 Feb 5, 2021
a93d76e
Add `CloseOutputAsync` implementation
kjpou1 Feb 5, 2021
1ccb6ea
Remove `ActiveIssue` and add more tests
kjpou1 Feb 5, 2021
4d36bf2
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Feb 8, 2021
1fb4e1b
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Feb 9, 2021
1a9970e
Merge branch 'master' of https://github.com/dotnet/runtime into wasm-…
kjpou1 Feb 9, 2021
fb8596a
Address timeout implementation review
kjpou1 Feb 9, 2021
59d0383
Add code as per SignalR comments to prevent long wait times in some c…
kjpou1 Feb 9, 2021
0fb4017
Update src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTe…
kjpou1 Feb 10, 2021
ea5a787
Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebS…
kjpou1 Feb 11, 2021
543b31a
Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebS…
kjpou1 Feb 11, 2021
a50f11a
Address review comments for using `TrySet*` variants
kjpou1 Feb 11, 2021
baddb82
Address review about using PascalCasing
kjpou1 Feb 11, 2021
a6622b4
Change test to be a platform check and not an ActiveIssue as per review
kjpou1 Feb 11, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public void Dispose()

public void Abort()
{
_abortSource.Cancel();
WebSocket?.Abort();
}

Expand Down Expand Up @@ -67,7 +68,7 @@ public async Task ConnectAsync(Uri uri, CancellationToken cancellationToken, Cli
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);
}
}
}
Expand Down
5 changes: 0 additions & 5 deletions src/libraries/System.Net.WebSockets.Client/tests/AbortTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public AbortTest(ITestOutputHelper output) : base(output) { }

[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())
Expand All @@ -43,7 +42,6 @@ public async Task Abort_ConnectAndAbort_ThrowsWebSocketExceptionWithmessage(Uri

[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) =>
Expand All @@ -64,7 +62,6 @@ await TestCancellation(async (cws) =>

[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) =>
Expand All @@ -89,7 +86,6 @@ await cws.SendAsync(

[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) =>
Expand All @@ -114,7 +110,6 @@ await cws.SendAsync(

[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) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ await cws.SendAsync(

[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) =>
Expand Down Expand Up @@ -131,7 +130,6 @@ public async Task ReceiveAsync_CancelThenReceive_ThrowsOperationCanceledExceptio

[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))
Expand All @@ -148,7 +146,6 @@ public async Task ReceiveAsync_ReceiveThenCancel_ThrowsOperationCanceledExceptio

[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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class ClientWebSocketTestBase
}).ToArray();

public const int TimeOutMilliseconds = 20000;
public const int BrowserTimeOutMilliseconds = 30000;
kjpou1 marked this conversation as resolved.
Show resolved Hide resolved
public const int CloseDescriptionMaxLength = 123;
public readonly ITestOutputHelper _output;

Expand Down
50 changes: 46 additions & 4 deletions src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,55 @@ public async Task CloseAsync_CloseDescriptionIsNull_Success(Uri server)

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<WebSocketException>(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!";
Expand All @@ -173,15 +215,16 @@ public async Task CloseOutputAsync_ClientInitiated_CanReceive_CanClose(Uri serve
{
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);
// Need a short delay as per WebSocket rfc6455 section 5.5.1 there isn't a requirement to receive any
// 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(PlatformDetection.IsBrowser ? 500 : 100); // default timeout is not enough for browser sometimes so we will extend it
kjpou1 marked this conversation as resolved.
Show resolved Hide resolved
await cws.CloseOutputAsync(closeStatus, closeDescription, cts.Token);

// Should be able to receive the message echoed by the server.
Expand Down Expand Up @@ -251,7 +294,6 @@ await cws.SendAsync(

[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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public async Task SendReceive_PartialMessageDueToSmallReceiveBuffer_Success(Uri

[OuterLoop("Uses external servers")]
[ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/45586", TestPlatforms.Browser)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/46983", TestPlatforms.Browser)] // JS Websocket does not support see issue
kjpou1 marked this conversation as resolved.
Show resolved Hide resolved
public async Task SendReceive_PartialMessageBeforeCompleteMessageArrives_Success(Uri server)
{
var rand = new Random();
Expand Down Expand Up @@ -131,7 +131,6 @@ public async Task SendReceive_PartialMessageBeforeCompleteMessageArrives_Success

[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))
Expand Down Expand Up @@ -219,12 +218,13 @@ public async Task SendAsync_MultipleOutstandingSendOperations_Throws(Uri server)

[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))
{
var cts = new CancellationTokenSource(TimeOutMilliseconds);
// It seems that sometimes the default timeout is not enough for browser so we will extend it
// See issue https://github.com/dotnet/runtime/issues/46909
var cts = PlatformDetection.IsBrowser ? new CancellationTokenSource(BrowserTimeOutMilliseconds) : new CancellationTokenSource(TimeOutMilliseconds);

Task[] tasks = new Task[2];

Expand Down Expand Up @@ -284,7 +284,6 @@ await SendAsync(

[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))
Expand Down Expand Up @@ -329,7 +328,10 @@ public async Task SendReceive_VaryingLengthBuffers_Success(Uri server)
using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output))
{
var rand = new Random();
var ctsDefault = new CancellationTokenSource(TimeOutMilliseconds);

kjpou1 marked this conversation as resolved.
Show resolved Hide resolved
// It seems that sometimes the default timeout is not enough for browser so we will extend it
// See issue https://github.com/dotnet/runtime/issues/46909
var ctsDefault = PlatformDetection.IsBrowser ? new CancellationTokenSource(BrowserTimeOutMilliseconds) : new CancellationTokenSource(TimeOutMilliseconds);

// Values chosen close to boundaries in websockets message length handling as well
// as in vectors used in mask application.
Expand Down Expand Up @@ -463,7 +465,6 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>

[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))
Expand Down