diff --git a/client-ts/Microsoft.AspNetCore.SignalR.Client.TS/Transports.ts b/client-ts/Microsoft.AspNetCore.SignalR.Client.TS/Transports.ts index fca859aec8..ab3a428d86 100644 --- a/client-ts/Microsoft.AspNetCore.SignalR.Client.TS/Transports.ts +++ b/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")); } } diff --git a/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/Startup.cs b/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/Startup.cs index cb5ca7f04d..6137826907 100644 --- a/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/Startup.cs +++ b/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("echo")); - app.UseSignalR(routes => - { - routes.MapHub("testhub"); - }); + app.UseSignalR(options => options.MapHub("testhub")); + app.UseSignalR(options => options.MapHub("uncreatable")); } } } diff --git a/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/UncreatableHub.cs b/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/UncreatableHub.cs new file mode 100644 index 0000000000..17bb96c82a --- /dev/null +++ b/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) + { + } + } +} diff --git a/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/wwwroot/js/hubConnectionTests.js b/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/wwwroot/js/hubConnectionTests.js index 0d90a2d066..eea1907676 100644 --- a/client-ts/Microsoft.AspNetCore.SignalR.Test.Server/wwwroot/js/hubConnectionTests.js +++ b/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) + }); }); }); diff --git a/src/Microsoft.AspNetCore.Sockets.Client/ServerSentEventsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client/ServerSentEventsTransport.cs index cd4e1f2ce9..3b5ac78a8a 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client/ServerSentEventsTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client/ServerSentEventsTransport.cs @@ -114,6 +114,7 @@ namespace Microsoft.AspNetCore.Sockets.Client finally { _transportCts.Cancel(); + stream.Dispose(); } } diff --git a/src/Microsoft.AspNetCore.Sockets.Client/WebSocketsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client/WebSocketsTransport.cs index 16cf6b93d3..c52caa1a91 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client/WebSocketsTransport.cs +++ b/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; } diff --git a/src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs b/src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs index 55ce652fb7..9a08ea6984 100644 --- a/src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs +++ b/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) { diff --git a/src/Microsoft.AspNetCore.Sockets.Http/Transports/LongPollingTransport.cs b/src/Microsoft.AspNetCore.Sockets.Http/Transports/LongPollingTransport.cs index b1b0efab9f..a70db311a2 100644 --- a/src/Microsoft.AspNetCore.Sockets.Http/Transports/LongPollingTransport.cs +++ b/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; + } } } } diff --git a/src/Microsoft.AspNetCore.Sockets.Http/Transports/ServerSentEventsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Http/Transports/ServerSentEventsTransport.cs index cdd4b020f7..1cc8e0ff44 100644 --- a/src/Microsoft.AspNetCore.Sockets.Http/Transports/ServerSentEventsTransport.cs +++ b/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) { diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs index e652360531..73b2bb912d 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs +++ b/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(); 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(); - 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( + 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( + 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(); + + var url = _serverFixture.BaseUrl + "/uncreatable"; + var connection = new HubConnection(new Uri(url), loggerFactory); + try + { + var closeTcs = new TaskCompletionSource(); + + 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 TransportTypes() => new[] { diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/ServerFixture.cs b/test/Microsoft.AspNetCore.SignalR.Tests/ServerFixture.cs index 5fa3d2d629..59d86aa51f 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/ServerFixture.cs +++ b/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("echo")); + app.UseSignalR(options => options.MapHub("uncreatable")); } } diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/UncreatableHub.cs b/test/Microsoft.AspNetCore.SignalR.Tests/UncreatableHub.cs new file mode 100644 index 0000000000..97142cf271 --- /dev/null +++ b/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) + { + } + } +} diff --git a/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs b/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs index 65a82071a3..d77c8b70fd 100644 --- a/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs +++ b/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(); - var builder = new SocketBuilder(services.BuildServiceProvider()); - builder.UseEndPoint(); - 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() {