Closing transports with error if hubs cannot be created

- Preventing from closing long polling transport with 204 in case of
error
- Reporting an error to the client if WebSocket was not closed normally

Note in case of ServerSentEvents it is not possible on the client to
tell the difference between when the server closed event stream due to
an exception or because the client left OnConnectedAsync. In both cases
the client sees only that the stream was closed.

Part of: #163

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

@@ -125,7 +125,7 @@ export class ServerSentEventsTransport implements ITransport {
 
                     // don't report an error if the transport did not start successfully
                     if (this.eventSource && this.onClosed) {
-                        this.onClosed(new Error(e.message));
+                        this.onClosed(new Error(e.message || "Error occurred"));
                     }
                 }
 

client-ts/Microsoft.AspNetCore.SignalR.Test.Server/Startup.cs

@@ -25,10 +25,8 @@ namespace Microsoft.AspNetCore.SignalR.Test.Server
 
             app.UseFileServer();
             app.UseSockets(options => options.MapEndPoint<EchoEndPoint>("echo"));
-            app.UseSignalR(routes =>
-            {
-                routes.MapHub<TestHub>("testhub");
-            });
+            app.UseSignalR(options => options.MapHub<TestHub>("testhub"));
+            app.UseSignalR(options => options.MapHub<UncreatableHub>("uncreatable"));
         }
     }
 }

client-ts/Microsoft.AspNetCore.SignalR.Test.Server/UncreatableHub.cs

@@ -0,0 +1,14 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+
+namespace Microsoft.AspNetCore.SignalR.Test.Server
+{
+    public class UncreatableHub: Hub
+    {
+        public UncreatableHub(object obj)
+        {
+        }
+    }
+}

client-ts/Microsoft.AspNetCore.SignalR.Test.Server/wwwroot/js/hubConnectionTests.js

@@ -142,5 +142,21 @@ describe('hubConnection', () => {
                     });
             });
         });
+
+        it(`over ${signalR.TransportType[transportType]} closed with error if hub cannot be created`, done =>{
+            let errorRegex = {
+                WebSockets: "1011", // Message is browser specific (e.g. 'Websocket closed with status code: 1011')
+                LongPolling: "Status: 500",
+                ServerSentEvents: "Error occurred"
+            };
+
+            let hubConnection = new signalR.HubConnection(`http://${document.location.host}/uncreatable`, 'formatType=json&format=text');
+
+            hubConnection.onClosed = error => {
+                expect(error).toMatch(errorRegex[signalR.TransportType[transportType]]);
+                done();
+            }
+            hubConnection.start(transportType)
+        });
     });
 });

src/Microsoft.AspNetCore.Sockets.Client/ServerSentEventsTransport.cs

@@ -114,6 +114,7 @@ namespace Microsoft.AspNetCore.Sockets.Client
             finally
             {
                 _transportCts.Cancel();
+                stream.Dispose();
             }
         }
 

src/Microsoft.AspNetCore.Sockets.Client/WebSocketsTransport.cs

@@ -84,7 +84,11 @@ namespace Microsoft.AspNetCore.Sockets.Client
                         {
                             _logger.LogInformation("Websocket closed by the server. Close status {0}", receiveResult.CloseStatus);
 
-                            _application.Output.Complete();
+                            _application.Output.Complete(
+                                receiveResult.CloseStatus == WebSocketCloseStatus.NormalClosure
+                                ? null
+                                : new InvalidOperationException(
+                                    $"Websocket closed with error: {receiveResult.CloseStatus}."));
                             return;
                         }
 

src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs

@@ -202,7 +202,7 @@ namespace Microsoft.AspNetCore.Sockets
 
                 var pollAgain = true;
 
-                // If the application ended before the transport task then we need to potentially need to end the 
+                // If the application ended before the transport task then we need to potentially need to end the
                 // connection
                 if (resultTask == state.ApplicationTask)
                 {

src/Microsoft.AspNetCore.Sockets.Http/Transports/LongPollingTransport.cs

@@ -32,6 +32,7 @@ namespace Microsoft.AspNetCore.Sockets.Transports
             {
                 if (!await _application.WaitToReadAsync(token))
                 {
+                    await _application.Completion;
                     _logger.LogInformation("Terminating Long Polling connection by sending 204 response.");
                     context.Response.StatusCode = StatusCodes.Status204NoContent;
                     return;
@@ -82,6 +83,11 @@ namespace Microsoft.AspNetCore.Sockets.Transports
                 // The background thread will eventually dispose this connection if it's inactive
                 _logger.LogDebug("Client disconnected from Long Polling endpoint.");
             }
+            catch (Exception ex)
+            {
+                _logger.LogError(ex, "Long Polling transport was terminated due to an error");
+                throw;
+            }
         }
     }
 }

src/Microsoft.AspNetCore.Sockets.Http/Transports/ServerSentEventsTransport.cs

@@ -65,6 +65,8 @@ namespace Microsoft.AspNetCore.Sockets.Transports
                         await output.FlushAsync();
                     }
                 }
+
+                await _application.Completion;
             }
             catch (OperationCanceledException)
             {

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

@@ -3,20 +3,21 @@
 
 using System;
 using System.Collections.Generic;
+using System.Net.Http;
 using System.Net.WebSockets;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.SignalR.Client;
+using Microsoft.AspNetCore.SignalR.Tests.Common;
 using Microsoft.AspNetCore.Sockets;
 using Microsoft.AspNetCore.Sockets.Client;
 using Microsoft.AspNetCore.Testing.xunit;
 using Microsoft.Extensions.Logging;
-using Xunit;
-
-using ClientConnection = Microsoft.AspNetCore.Sockets.Client.Connection;
-using Microsoft.AspNetCore.SignalR.Tests.Common;
-using Xunit.Abstractions;
 using Microsoft.Extensions.Logging.Testing;
+using Xunit;
+using Xunit.Abstractions;
+using ClientConnection = Microsoft.AspNetCore.Sockets.Client.Connection;
 
 namespace Microsoft.AspNetCore.SignalR.Tests
 {
@@ -85,9 +86,8 @@ namespace Microsoft.AspNetCore.SignalR.Tests
                 var logger = loggerFactory.CreateLogger<EndToEndTests>();
 
                 const string message = "Major Key";
-                var baseUrl = _serverFixture.BaseUrl;
 
-                string url = baseUrl + "/echo";
+                var url = _serverFixture.BaseUrl + "/echo";
                 var connection = new ClientConnection(new Uri(url), loggerFactory);
                 try
                 {
@@ -157,9 +157,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests
             {
                 var logger = loggerFactory.CreateLogger<EndToEndTests>();
 
-                var baseUrl = _serverFixture.BaseUrl;
-
-                string url = baseUrl + "/echo";
+                var url = _serverFixture.BaseUrl + "/echo";
                 var connection = new ClientConnection(new Uri(url), loggerFactory);
                 try
                 {
@@ -195,6 +193,62 @@ namespace Microsoft.AspNetCore.SignalR.Tests
             }
         }
 
+        [ConditionalFact]
+        [OSSkipCondition(OperatingSystems.Windows, WindowsVersions.Win7, WindowsVersions.Win2008R2, SkipReason = "No WebSockets Client for this platform")]
+        public async Task ServerClosesConnectionWithErrorIfHubCannotBeCreated_WebSocket()
+        {
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                async () => await ServerClosesConnectionWithErrorIfHubCannotBeCreated(TransportType.WebSockets));
+            Assert.Equal("Websocket closed with error: InternalServerError.", exception.Message);
+        }
+
+        [Fact]
+        public async Task ServerClosesConnectionWithErrorIfHubCannotBeCreated_LongPolling()
+        {
+            var exception = await Assert.ThrowsAsync<HttpRequestException>(
+                async () => await ServerClosesConnectionWithErrorIfHubCannotBeCreated(TransportType.LongPolling));
+            Assert.Equal("Response status code does not indicate success: 500 (Internal Server Error).", exception.Message);
+        }
+
+        private async Task ServerClosesConnectionWithErrorIfHubCannotBeCreated(TransportType transportType)
+        {
+            using (StartLog(out var loggerFactory, testName: $"ConnectionCanSendAndReceiveMessages_{transportType.ToString()}"))
+            {
+                var logger = loggerFactory.CreateLogger<EndToEndTests>();
+
+                var url = _serverFixture.BaseUrl + "/uncreatable";
+                var connection = new HubConnection(new Uri(url), loggerFactory);
+                try
+                {
+                    var closeTcs = new TaskCompletionSource<object>();
+
+                    connection.Closed += e =>
+                    {
+                        logger.LogInformation("Connection closed");
+                        if (e != null)
+                        {
+                            closeTcs.TrySetException(e);
+                        }
+                        else
+                        {
+                            closeTcs.TrySetResult(null);
+                        }
+                    };
+
+                    logger.LogInformation("Starting connection to {url}", url);
+                    await connection.StartAsync(transportType).OrTimeout();
+
+                    await closeTcs.Task.OrTimeout();
+                }
+                finally
+                {
+                    logger.LogInformation("Disposing Connection");
+                    await connection.DisposeAsync().OrTimeout();
+                    logger.LogInformation("Disposed Connection");
+                }
+            }
+        }
+
         public static IEnumerable<object[]> TransportTypes() =>
             new[]
             {

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

@@ -47,6 +47,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests
             public void Configure(IApplicationBuilder app, IHostingEnvironment env)
             {
                 app.UseSockets(options => options.MapEndPoint<EchoEndPoint>("echo"));
+                app.UseSignalR(options => options.MapHub<UncreatableHub>("uncreatable"));
             }
         }
 

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

@@ -0,0 +1,12 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+namespace Microsoft.AspNetCore.SignalR.Tests
+{
+    public class UncreatableHub: Hub
+    {
+        public UncreatableHub(object obj)
+        {
+        }
+    }
+}

test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs

@@ -290,29 +290,6 @@ namespace Microsoft.AspNetCore.Sockets.Tests
             Assert.False(exists);
         }
 
-        [Fact]
-        public async Task SynchronusExceptionEndsLongPollingConnection()
-        {
-            var manager = CreateConnectionManager();
-            var state = manager.CreateConnection();
-
-            var dispatcher = new HttpConnectionDispatcher(manager, new LoggerFactory());
-            var context = MakeRequest("/foo", state);
-
-            var services = new ServiceCollection();
-            services.AddEndPoint<SynchronusExceptionEndPoint>();
-            var builder = new SocketBuilder(services.BuildServiceProvider());
-            builder.UseEndPoint<SynchronusExceptionEndPoint>();
-            var app = builder.Build();
-            await dispatcher.ExecuteAsync(context, new HttpSocketOptions(), app);
-
-            Assert.Equal(StatusCodes.Status204NoContent, context.Response.StatusCode);
-
-            ConnectionState removed;
-            bool exists = manager.TryGetConnection(state.Connection.ConnectionId, out removed);
-            Assert.False(exists);
-        }
-
         [Fact]
         public async Task CompletedEndPointEndsLongPollingConnection()
         {