From 8022afd3a2644237d597b6664f553871c69a1666 Mon Sep 17 00:00:00 2001 From: moozzyk Date: Tue, 20 Dec 2016 14:20:00 -0800 Subject: [PATCH] Handling exceptions thrown on the server side Addresses: #62 --- samples/ChatSample/Hubs/Chat.cs | 2 +- samples/SocketsSample/Hubs/Chat.cs | 2 +- .../Transports.ts | 2 +- .../HubConnection.cs | 2 +- src/Microsoft.AspNetCore.SignalR/Hub.cs | 2 +- .../HubEndPoint.cs | 20 ++++++--- .../LongPollingTransport.cs | 1 + .../HubConnectionTests.cs | 22 ++++++++++ .../HubEndpointTests.cs | 42 ++++++++++++++++++- .../project.json | 1 + 10 files changed, 85 insertions(+), 11 deletions(-) diff --git a/samples/ChatSample/Hubs/Chat.cs b/samples/ChatSample/Hubs/Chat.cs index 200e7df8f3..be4786839c 100644 --- a/samples/ChatSample/Hubs/Chat.cs +++ b/samples/ChatSample/Hubs/Chat.cs @@ -24,7 +24,7 @@ namespace ChatSample.Hubs return Task.CompletedTask; } - public override Task OnDisconnectedAsync() + public override Task OnDisconnectedAsync(Exception ex) { return Task.CompletedTask; } diff --git a/samples/SocketsSample/Hubs/Chat.cs b/samples/SocketsSample/Hubs/Chat.cs index 16fe4ad31d..5fe72faeae 100644 --- a/samples/SocketsSample/Hubs/Chat.cs +++ b/samples/SocketsSample/Hubs/Chat.cs @@ -14,7 +14,7 @@ namespace SocketsSample.Hubs await Clients.All.InvokeAsync("Send", $"{Context.Connection.ConnectionId} joined"); } - public override async Task OnDisconnectedAsync() + public override async Task OnDisconnectedAsync(Exception ex) { await Clients.All.InvokeAsync("Send", $"{Context.Connection.ConnectionId} left"); } diff --git a/src/Microsoft.AspNetCore.SignalR.Client.TS/Transports.ts b/src/Microsoft.AspNetCore.SignalR.Client.TS/Transports.ts index f5082b363a..16893a8ed1 100644 --- a/src/Microsoft.AspNetCore.SignalR.Client.TS/Transports.ts +++ b/src/Microsoft.AspNetCore.SignalR.Client.TS/Transports.ts @@ -38,7 +38,7 @@ export class WebSocketTransport implements ITransport { webSocket.onclose = (event: CloseEvent) => { // webSocket will be null if the transport did not start successfully - if (thisWebSocketTransport.webSocket && event.wasClean === false) { + if (thisWebSocketTransport.webSocket && (event.wasClean === false || event.code !== 1000)) { if (thisWebSocketTransport.onError) { thisWebSocketTransport.onError(event); } diff --git a/src/Microsoft.AspNetCore.SignalR.Client/HubConnection.cs b/src/Microsoft.AspNetCore.SignalR.Client/HubConnection.cs index f9ffbe06cf..0251514ff6 100644 --- a/src/Microsoft.AspNetCore.SignalR.Client/HubConnection.cs +++ b/src/Microsoft.AspNetCore.SignalR.Client/HubConnection.cs @@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.SignalR.Client _reader = ReceiveMessages(_readerCts.Token); _connection.Output.Writing.ContinueWith( - t => CompletePendingCalls(t.IsFaulted ? t.Exception : null)); + t => CompletePendingCalls(t.IsFaulted ? t.Exception.InnerException : null)); } // TODO: Client return values/tasks? diff --git a/src/Microsoft.AspNetCore.SignalR/Hub.cs b/src/Microsoft.AspNetCore.SignalR/Hub.cs index 20566502c4..733ff49ff3 100644 --- a/src/Microsoft.AspNetCore.SignalR/Hub.cs +++ b/src/Microsoft.AspNetCore.SignalR/Hub.cs @@ -66,7 +66,7 @@ namespace Microsoft.AspNetCore.SignalR return TaskCache.CompletedTask; } - public virtual Task OnDisconnectedAsync() + public virtual Task OnDisconnectedAsync(Exception exception) { return TaskCache.CompletedTask; } diff --git a/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs b/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs index 5af668ba62..0d49bc5e2f 100644 --- a/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs +++ b/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs @@ -56,7 +56,7 @@ namespace Microsoft.AspNetCore.SignalR { // TODO: Dispatch from the caller await Task.Yield(); - + Exception exception = null; try { await _lifetimeManager.OnConnectedAsync(connection); @@ -76,6 +76,13 @@ namespace Microsoft.AspNetCore.SignalR await DispatchMessagesAsync(connection); } + catch (Exception ex) + { + _logger.LogError(0, ex, "Error when processing requests."); + exception = ex; + connection.Channel.Input.Complete(exception); + connection.Channel.Output.Complete(exception); + } finally { using (var scope = _serviceScopeFactory.CreateScope()) @@ -83,7 +90,7 @@ namespace Microsoft.AspNetCore.SignalR bool created; var hub = CreateHub(scope.ServiceProvider, connection, out created); - await hub.OnDisconnectedAsync(); + await hub.OnDisconnectedAsync(exception); if (created) { @@ -232,7 +239,7 @@ namespace Microsoft.AspNetCore.SignalR private static bool IsHubMethod(MethodInfo m) { // TODO: Add more checks - return m.IsPublic; + return m.IsPublic && !m.IsSpecialName; } Type IInvocationBinder.GetReturnType(string invocationId) @@ -243,8 +250,11 @@ namespace Microsoft.AspNetCore.SignalR Type[] IInvocationBinder.GetParameterTypes(string methodName) { Type[] types; - // TODO: null or throw? - return _paramTypes.TryGetValue(methodName, out types) ? types : null; + if (!_paramTypes.TryGetValue(methodName, out types)) + { + throw new InvalidOperationException($"The hub method '{methodName}' could not be resolved."); + } + return types; } } } diff --git a/src/Microsoft.AspNetCore.Sockets.Client/LongPollingTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client/LongPollingTransport.cs index 0ba6ca6499..eb7e98a361 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client/LongPollingTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client/LongPollingTransport.cs @@ -96,6 +96,7 @@ namespace Microsoft.AspNetCore.Sockets.Client // Shut down the output pipeline and log _logger.LogError("Error while polling '{0}': {1}", pollUrl, ex); _pipeline.Output.Complete(ex); + _pipeline.Input.Complete(ex); } } diff --git a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs index 52d1748c42..4777ec96e9 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs @@ -104,6 +104,28 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests } } + [Fact] + public async Task ServerClosesConnectionIfHubMethodCannotBeResolved() + { + var loggerFactory = new LoggerFactory(); + + using (var httpClient = _testServer.CreateClient()) + using (var pipelineFactory = new PipelineFactory()) + { + var transport = new LongPollingTransport(httpClient, loggerFactory); + using (var connection = await HubConnection.ConnectAsync(new Uri("http://test/hubs"), new JsonNetInvocationAdapter(), transport, httpClient, pipelineFactory, loggerFactory)) + { + //TODO: Get rid of this. This is to prevent "No channel" failures due to sends occuring before the first poll. + await Task.Delay(500); + + var ex = await Assert.ThrowsAnyAsync( + async () => await connection.Invoke("!@#$%")); + + Assert.Equal(ex.Message, "The hub method '!@#$%' could not be resolved."); + } + } + } + public void Dispose() { _testServer.Dispose(); diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs index 6740d1dc1f..9d47059246 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs @@ -4,9 +4,12 @@ using System; using System.IO.Pipelines; using System.Security.Claims; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Sockets; using Microsoft.Extensions.DependencyInjection; +using Moq; +using Moq.Protected; using Xunit; namespace Microsoft.AspNetCore.SignalR.Tests @@ -18,7 +21,6 @@ namespace Microsoft.AspNetCore.SignalR.Tests { var trackDispose = new TrackDispose(); var serviceProvider = CreateServiceProvider(s => s.AddSingleton(trackDispose)); - var endPoint = serviceProvider.GetService>(); using (var connectionWrapper = new ConnectionWrapper()) @@ -36,6 +38,44 @@ namespace Microsoft.AspNetCore.SignalR.Tests } } + [Fact] + public async Task OnDisconnectedCalledWithExceptionIfHubMethodNotFound() + { + var hub = Mock.Of(); + + var endPointType = GetEndPointType(hub.GetType()); + var serviceProvider = CreateServiceProvider(s => + { + s.AddSingleton(endPointType); + s.AddTransient(hub.GetType(), sp => hub); + }); + + dynamic endPoint = serviceProvider.GetService(endPointType); + + using (var connectionWrapper = new ConnectionWrapper()) + { + var endPointTask = endPoint.OnConnectedAsync(connectionWrapper.Connection); + + await connectionWrapper.HttpConnection.Input.ReadingStarted; + + var buffer = connectionWrapper.HttpConnection.Input.Alloc(); + buffer.Write(Encoding.UTF8.GetBytes("0xdeadbeef")); + await buffer.FlushAsync(); + + connectionWrapper.Connection.Channel.Dispose(); + + await endPointTask; + + Mock.Get(hub).Verify(h => h.OnDisconnectedAsync(It.IsNotNull()), Times.Once()); + } + } + + private static Type GetEndPointType(Type hubType) + { + var endPointType = typeof(HubEndPoint<>); + return endPointType.MakeGenericType(hubType); + } + private class TestHub : Hub { private TrackDispose _trackDispose; diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/project.json b/test/Microsoft.AspNetCore.SignalR.Tests/project.json index b52ff71053..812bbd9091 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/project.json +++ b/test/Microsoft.AspNetCore.SignalR.Tests/project.json @@ -10,6 +10,7 @@ "Microsoft.AspNetCore.SignalR": "1.0.0-*", "Microsoft.Extensions.DependencyInjection": "1.2.0-*", "Microsoft.Extensions.Logging": "1.2.0-*", + "Moq": "4.6.36-*", "xunit": "2.2.0-*" }, "frameworks": {