Removing sending OPTIONS request if WebSocket transport requested (#1036)

Removing sending OPTIONS request if WebSocket transport requested

This removes session stickiness requirement for WebSockets

Fixes: #1035

client-ts/Microsoft.AspNetCore.SignalR.Client.TS.Tests/Connection.spec.ts

@@ -192,6 +192,10 @@ describe("Connection", () => {
     });
 
     eachTransport((requestedTransport: TransportType) => {
+        // OPTIONS is not sent when WebSockets transport is explicitly requested
+        if (requestedTransport === TransportType.WebSockets) {
+            return;
+        }
         it(`cannot be started if requested ${TransportType[requestedTransport]} transport not available on server`, async done => {
             let options: IHttpConnectionOptions = {
                 httpClient: <IHttpClient>{
@@ -244,6 +248,34 @@ describe("Connection", () => {
         }
     });
 
+    it('does not send OPTIONS request if WebSockets transport requested explicitly', async done => {
+        let options: IHttpConnectionOptions = {
+            httpClient: <IHttpClient>{
+                options(url: string): Promise<string> {
+                    return Promise.reject("Should not be called");
+                },
+                get(url: string): Promise<string> {
+                    return Promise.reject("Should not be called");
+                }
+            },
+            transport: TransportType.WebSockets,
+            logging: null
+        } as IHttpConnectionOptions;
+
+        let connection = new HttpConnection("http://tempuri.org", options);
+        try {
+            await connection.start();
+            fail();
+            done();
+        }
+        catch (e) {
+            // WebSocket is created when the transport is connecting which happens after
+            // OPTIONS request would be sent. No better/easier way to test this.
+            expect(e.message).toBe("WebSocket is not defined");
+            done();
+        }
+    });
+
     [
         [TransferMode.Text, TransferMode.Text],
         [TransferMode.Text, TransferMode.Binary],

client-ts/Microsoft.AspNetCore.SignalR.Client.TS/HttpConnection.ts

@@ -55,18 +55,25 @@ export class HttpConnection implements IConnection {
 
     private async startInternal(): Promise<void> {
         try {
-            let negotiatePayload = await this.httpClient.options(this.url);
+            if (this.options.transport === TransportType.WebSockets) {
-            let negotiateResponse: INegotiateResponse = JSON.parse(negotiatePayload);
+                this.transport = this.createTransport(this.options.transport, [TransportType[TransportType.WebSockets]]);
-            this.connectionId = negotiateResponse.connectionId;
+            }
+            else {
+                let negotiatePayload = await this.httpClient.options(this.url);
+                let negotiateResponse: INegotiateResponse = JSON.parse(negotiatePayload);
+                this.connectionId = negotiateResponse.connectionId;
 
-            // the user tries to stop the the connection when it is being started
+                // the user tries to stop the the connection when it is being started
-            if (this.connectionState == ConnectionState.Disconnected) {
+                if (this.connectionState == ConnectionState.Disconnected) {
-                return;
+                    return;
+                }
+
+                if (this.connectionId) {
+                    this.url += (this.url.indexOf("?") == -1 ? "?" : "&") + `id=${this.connectionId}`;
+                    this.transport = this.createTransport(this.options.transport, negotiateResponse.availableTransports);
+                }
             }
 
-            this.url += (this.url.indexOf("?") == -1 ? "?" : "&") + `id=${this.connectionId}`;
-
-            this.transport = this.createTransport(this.options.transport, negotiateResponse.availableTransports);
             this.transport.onreceive = this.onreceive;
             this.transport.onclose = e => this.stopConnection(true, e);
 
@@ -90,7 +97,7 @@ export class HttpConnection implements IConnection {
     }
 
     private createTransport(transport: TransportType | ITransport, availableTransports: string[]): ITransport {
-        if (!transport && availableTransports.length > 0) {
+        if ((transport === null || transport === undefined) && availableTransports.length > 0) {
             transport = TransportType[availableTransports[0]];
         }
         if (transport === TransportType.WebSockets && availableTransports.indexOf(TransportType[transport]) >= 0) {

src/Microsoft.AspNetCore.Sockets.Client.Http/DefaultTransportFactory.cs

@@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Sockets.Client
                 throw new ArgumentOutOfRangeException(nameof(requestedTransportType));
             }
 
-            if (httpClient == null)
+            if (httpClient == null && requestedTransportType != TransportType.WebSockets)
             {
                 throw new ArgumentNullException(nameof(httpClient));
             }

src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
@@ -42,6 +41,7 @@ namespace Microsoft.AspNetCore.Sockets.Client
         private ReadableChannel<byte[]> Input => _transportChannel.In;
         private WritableChannel<SendMessage> Output => _transportChannel.Out;
         private readonly List<ReceiveCallback> _callbacks = new List<ReceiveCallback>();
+        private readonly TransportType _requestedTransportType = TransportType.All;
 
         public Uri Url { get; }
 
@@ -78,8 +78,14 @@ namespace Microsoft.AspNetCore.Sockets.Client
 
             _loggerFactory = loggerFactory ?? NullLoggerFactory.Instance;
             _logger = _loggerFactory.CreateLogger<HttpConnection>();
-            _httpClient = httpMessageHandler == null ? new HttpClient() : new HttpClient(httpMessageHandler);
+
-            _httpClient.Timeout = HttpClientTimeout;
+            _requestedTransportType = transportType;
+            if (_requestedTransportType != TransportType.WebSockets)
+            {
+                _httpClient = httpMessageHandler == null ? new HttpClient() : new HttpClient(httpMessageHandler);
+                _httpClient.Timeout = HttpClientTimeout;
+            }
+
             _transportFactory = new DefaultTransportFactory(transportType, _loggerFactory, _httpClient);
         }
 
@@ -130,19 +136,27 @@ namespace Microsoft.AspNetCore.Sockets.Client
 
             try
             {
-                var negotiationResponse = await Negotiate(Url, _httpClient, _logger);
+                var connectUrl = Url;
-                _connectionId = negotiationResponse.ConnectionId;
+                if (_requestedTransportType == TransportType.WebSockets)
-
-                // Connection is being stopped while start was in progress
-                if (_connectionState == ConnectionState.Disconnected)
                 {
-                    _logger.HttpConnectionClosed(_connectionId);
+                    _transport = _transportFactory.CreateTransport(TransportType.WebSockets);
-                    return;
+                }
+                else
+                {
+                    var negotiationResponse = await Negotiate(Url, _httpClient, _logger);
+                    _connectionId = negotiationResponse.ConnectionId;
+
+                    // Connection is being stopped while start was in progress
+                    if (_connectionState == ConnectionState.Disconnected)
+                    {
+                        _logger.HttpConnectionClosed(_connectionId);
+                        return;
+                    }
+
+                    _transport = _transportFactory.CreateTransport(GetAvailableServerTransports(negotiationResponse));
+                    connectUrl = CreateConnectUrl(Url, negotiationResponse);
                 }
 
-                _transport = _transportFactory.CreateTransport(GetAvailableServerTransports(negotiationResponse));
-
-                var connectUrl = CreateConnectUrl(Url, negotiationResponse);
                 _logger.StartingTransport(_connectionId, _transport.GetType().Name, connectUrl);
                 await StartTransport(connectUrl);
             }
@@ -174,7 +188,7 @@ namespace Microsoft.AspNetCore.Sockets.Client
                     await _eventQueue.Drain();
 
                     await Task.WhenAny(_eventQueue.Drain().NoThrow(), Task.Delay(_eventQueueDrainTimeout));
-                    _httpClient.Dispose();
+                    _httpClient?.Dispose();
 
                     _logger.RaiseClosed(_connectionId);
 
@@ -449,7 +463,7 @@ namespace Microsoft.AspNetCore.Sockets.Client
                 await _receiveLoopTask;
             }
 
-            _httpClient.Dispose();
+            _httpClient?.Dispose();
         }
 
         public IDisposable OnReceived(Func<byte[], object, Task> callback, object state)

test/Microsoft.AspNetCore.SignalR.Tests/DefaultTransportFactoryTests.cs

@@ -23,15 +23,26 @@ namespace Microsoft.AspNetCore.SignalR.Tests
                 () => new DefaultTransportFactory(transportType, new LoggerFactory(), new HttpClient()));
         }
 
-        [Fact]
+        [Theory]
-        public void DefaultTransportFactoryCannotBeCreatedWithoutHttpClient()
+        [InlineData(TransportType.All)]
+        [InlineData(TransportType.LongPolling)]
+        [InlineData(TransportType.ServerSentEvents)]
+        [InlineData(TransportType.LongPolling | TransportType.WebSockets)]
+        [InlineData(TransportType.ServerSentEvents | TransportType.WebSockets)]
+        public void DefaultTransportFactoryCannotBeCreatedWithoutHttpClient(TransportType transportType)
         {
             var exception = Assert.Throws<ArgumentNullException>(
-                () => new DefaultTransportFactory(TransportType.All, new LoggerFactory(), httpClient: null));
+                () => new DefaultTransportFactory(transportType, new LoggerFactory(), httpClient: null));
 
             Assert.Equal("httpClient", exception.ParamName);
         }
 
+        [Fact]
+        public void DefaultTransportFactoryCanBeCreatedWithoutHttpClientIfWebSocketsTransportRequestedExplicitly()
+        {
+            new DefaultTransportFactory(TransportType.WebSockets, new LoggerFactory(), httpClient: null);
+        }
+
         [ConditionalTheory]
         [InlineData(TransportType.WebSockets, typeof(WebSocketsTransport))]
         [InlineData(TransportType.ServerSentEvents, typeof(ServerSentEventsTransport))]

test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs

@@ -11,11 +11,13 @@ using System.Threading.Tasks;
 using Microsoft.AspNetCore.SignalR.Client;
 using Microsoft.AspNetCore.SignalR.Tests.Common;
 using Microsoft.AspNetCore.Sockets;
-using Microsoft.AspNetCore.Sockets.Features;
 using Microsoft.AspNetCore.Sockets.Client;
+using Microsoft.AspNetCore.Sockets.Features;
 using Microsoft.AspNetCore.Testing.xunit;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Testing;
+using Moq;
+using Moq.Protected;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -97,6 +99,54 @@ namespace Microsoft.AspNetCore.SignalR.Tests
             }
         }
 
+        [ConditionalFact]
+        [OSSkipCondition(OperatingSystems.Windows, WindowsVersions.Win7, WindowsVersions.Win2008R2, SkipReason = "No WebSockets Client for this platform")]
+        public async Task HTTPRequestsNotSentWhenWebSocketsTransportRequested()
+        {
+            using (StartLog(out var loggerFactory, testName: nameof(HTTPRequestsNotSentWhenWebSocketsTransportRequested)))
+            {
+                var logger = loggerFactory.CreateLogger<EndToEndTests>();
+                var url = _serverFixture.BaseUrl + "/echo";
+
+                var mockHttpHandler = new Mock<HttpMessageHandler>();
+                mockHttpHandler.Protected()
+                    .Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
+                    .Returns<HttpRequestMessage, CancellationToken>(
+                        (request, cancellationToken) => Task.FromException<HttpResponseMessage>(new InvalidOperationException("HTTP requests should not be sent.")));
+
+                var connection = new HttpConnection(new Uri(url), TransportType.WebSockets, loggerFactory, mockHttpHandler.Object);
+
+                try
+                {
+                    var receiveTcs = new TaskCompletionSource<byte[]>();
+                    connection.OnReceived((data, state) =>
+                    {
+                        var tcs = (TaskCompletionSource<byte[]>)state;
+                        tcs.TrySetResult(data);
+                        return Task.CompletedTask;
+                    }, receiveTcs);
+
+                    var message = new byte[] { 42 };
+                    await connection.StartAsync().OrTimeout();
+                    await connection.SendAsync(message).OrTimeout();
+
+                    var receivedData = await receiveTcs.Task.OrTimeout();
+                    Assert.Equal(message, receivedData);
+                }
+                catch (Exception ex)
+                {
+                    logger.LogInformation(ex, "Test threw exception");
+                    throw;
+                }
+                finally
+                {
+                    logger.LogInformation("Disposing Connection");
+                    await connection.DisposeAsync().OrTimeout();
+                    logger.LogInformation("Disposed Connection");
+                }
+            }
+        }
+
         [Theory]
         [MemberData(nameof(TransportTypesAndTransferModes))]
         public async Task ConnectionCanSendAndReceiveMessages(TransportType transportType, TransferMode requestedTransferMode)